New subject: [PATCH v5 01/19] flow: Clarify and enforce flow state transitions

17 May 2024

      On Fri, 17 May 2024 13:57:58 +1000
David Gibson  wrote:
...
On Thu, May 16, 2024 at 11:30:58AM +0200, Stefano Brivio wrote:
...
On Tue, 14 May 2024 11:03:19 +1000
David Gibson  wrote:
[...]
...
/**
  * struct flow_common - Common fields for packet flows
+ * @state:	State of the flow table entry
  * @type:	Type of packet flow
  */
 struct flow_common {
+	uint8_t		state;
In this case, I would typically do
(https://seitan.rocks/seitan/tree/common/gluten.h?id=5a9302bab9c9bb3d1577f046...):
#ifdef __GNUC__
  enum flow_state		state:8;
#else
  uint8_t			state;
#endif
I don't object to that, but I have two questions
- What's the advantage to using the explicit enum?  Is that for the
    benefit of static checkers and/or compiler diagnostics?
Yes: if we assign a value that's not in the enum, I expect static
checkers to complain. But also for humans: even with that ifdef, a
reader would know right away what values that might have.
...
AFAIK C
    itself doesn't really treat enums any differently to integer
    types.
Right.
...
- What's the need for GNUC?  Are enum bitfields a gnu extension?
They're rather permitted by gcc's interpretation of the standard:

  https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Structures-unions-enumerations...

  Allowable bit-field types other than _Bool, signed int, and unsigned
  int (C99 and C11 6.7.2.1).

  Other integer types, such as long int, and enumerated types are permitted
  even in strictly conforming mode.

but C11 says (6.7.2.1):

  A bit-field shall have a type that is a qualified or unqualified
  version of _Bool, signed int, unsigned int, or some other
  implementation-defined type. It is implementation-defined whether
  atomic types are permitted.

Is an enum a "version" of those? Maybe not. That was at least the
interpretation adopted by older gcc versions, up to 4.7.4:

  https://gcc.gnu.org/onlinedocs/gcc-4.7.4/gcc/Structures-unions-enumerations-...

  Allowable bit-field types other than _Bool, signed int, and unsigned
  int (C99 6.7.2.1).

  No other types are permitted in strictly conforming mode.

Did that change from C99? Not really:

  A bit-field shall have a type that is a qualified or unqualified
  version of _Bool, signed int, unsigned int, or some other
  implementation-defined type.
...
Even versus C11, which we already require?
Yes, I would say it doesn't change things. Only C23 would improve that:
  https://open-std.org/JTC1/SC22/WG14/www/docs/n3030.htm#design-constant.type

by allowing us to define the underlying type.
...
...
...and in any case we need to make sure to assign single values in the
enum above: there are no guarantees that FLOW_STATE_ACTIVE is 3
otherwise (except for that static_assert(), but that's not its purpose).
I'm not clear how this comment relates to the one before.
It's unrelated, but:
...
AFAIK
nothing in here (or the rest of the series) relies on the specific
numeric values of the flow state values (although we do rely on them
being ordered as written in some places).
while we rely on the fact that no value is bigger than 255, I realised
that the standards already guarantee that values start from 0 and every
subsequent constant is defined as one more than the previous one, all
the way from C90 to C11, so this would actually be fine.

Sorry, I don't know exactly why I thought that wouldn't be the case, I
was pretty sure of the opposite until I checked.

-- 
Stefano

Re: [PATCH v5 01/19] flow: Clarify and enforce flow state transitions

Stefano Brivio

David Gibson

tags

participants (2)