[PATCH nft 8/9] datatype: use __attribute__((packed)) instead of enum bitfields

[Thomas Haller <thaller@redhat.com>]

At some places we use bitfields of those enums, to save space inside the
structure. We can achieve that in a better way, by using GCC's
__attribute__((packed)) on the enum type.

It's better because a :8 bitfield makes the assumption that all enum
values (valid or invalid) fit into that field. With packed enums, we
don't need that assumption as the field can hold all possible numbers
that the enum type can hold. This reduces the places we need to worry
about truncating a value to casts between other types and the enum.
Those places already require us to be careful.

On the other hand, previously casting an int (or uint32_t) likely didn't
cause a truncation as the underlying type was large enough. So we could
check for invalid enum values after the cast. We might do that at
places. For example, we do

	key  = nftnl_expr_get_u32(nle, NFTNL_EXPR_META_KEY);
	expr = meta_expr_alloc(loc, key);

where we cast from an uint32_t to an enum without checking. Note that
`enum nft_meta_keys` is not packed by this patch. But this is an example
how things could be wrong. But the bug already exits before: don't make
assumption about the underlying enum type and take care of truncation
during casts.

This makes the change potentially dangerous, and it's hard to be sure
that it doesn't uncover bugs (due tow rong assumptions about enum types).

Note that we were already using the GCC-ism __attribute__((packed))
previously, however on a struct and not on an enum. Anyway. It seems
unlikely that we support any other compilers than GCC/Clang. Those both
support this attribute.  We should not worry about portability towards
hypothetical compilers (the C standard), unless there is a real compiler
that we can use and test and shows a problem with this. Especially when
we support both GCC and Clang, which themselves are ubiquitous and
accessible to all users (as they also need to build the kernel in the
first place).

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 include/datatype.h   |  1 +
 include/expression.h |  8 +++++---
 include/proto.h      | 11 +++++++----
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/datatype.h b/include/datatype.h
index 202935bd322f..c8b3b77ad0c0 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -112,6 +112,7 @@ enum datatypes {
  * @BYTEORDER_HOST_ENDIAN:	host endian
  * @BYTEORDER_BIG_ENDIAN:	big endian
  */
+__attribute__((packed))
 enum byteorder {
 	BYTEORDER_INVALID,
 	BYTEORDER_HOST_ENDIAN,
diff --git a/include/expression.h b/include/expression.h
index aede223db741..11a1dbf00b8c 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -45,6 +45,7 @@
  * @EXPR_SET_ELEM_CATCHALL catchall element expression
  * @EXPR_FLAGCMP	flagcmp expression
  */
+__attribute__((packed))
 enum expr_types {
 	EXPR_INVALID,
 	EXPR_VERDICT,
@@ -80,6 +81,7 @@ enum expr_types {
 	EXPR_MAX = EXPR_FLAGCMP
 };
 
+__attribute__((packed))
 enum ops {
 	OP_INVALID,
 	OP_IMPLICIT,
@@ -247,9 +249,9 @@ struct expr {
 	unsigned int		flags;
 
 	const struct datatype	*dtype;
-	enum byteorder		byteorder:8;
-	enum expr_types		etype:8;
-	enum ops		op:8;
+	enum byteorder		byteorder;
+	enum expr_types		etype;
+	enum ops		op;
 	unsigned int		len;
 	struct cmd		*cmd;
 
diff --git a/include/proto.h b/include/proto.h
index 3a20ff8c4071..3756a4ab79a4 100644
--- a/include/proto.h
+++ b/include/proto.h
@@ -13,6 +13,7 @@
  * @PROTO_BASE_NETWORK_HDR:	network layer header
  * @PROTO_BASE_TRANSPORT_HDR:	transport layer header
  */
+__attribute__((packed))
 enum proto_bases {
 	PROTO_BASE_INVALID,
 	PROTO_BASE_LL_HDR,
@@ -26,6 +27,7 @@ enum proto_bases {
 extern const char *proto_base_names[];
 extern const char *proto_base_tokens[];
 
+__attribute__((packed))
 enum icmp_hdr_field_type {
 	PROTO_ICMP_ANY = 0,
 	PROTO_ICMP_ECHO,	/* echo and reply */
@@ -52,9 +54,9 @@ struct proto_hdr_template {
 	const struct datatype		*dtype;
 	uint16_t			offset;
 	uint16_t			len;
-	enum byteorder			byteorder:8;
+	enum byteorder			byteorder;
 	enum nft_meta_keys		meta_key:8;
-	enum icmp_hdr_field_type	icmp_dep:8;
+	enum icmp_hdr_field_type	icmp_dep;
 };
 
 #define PROTO_HDR_TEMPLATE(__token, __dtype,  __byteorder, __offset, __len)\
@@ -77,6 +79,7 @@ struct proto_hdr_template {
 #define PROTO_UPPER_MAX		16
 #define PROTO_HDRS_MAX		20
 
+__attribute__((packed))
 enum proto_desc_id {
 	PROTO_DESC_UNKNOWN	= 0,
 	PROTO_DESC_AH,
@@ -119,8 +122,8 @@ enum proto_desc_id {
  */
 struct proto_desc {
 	const char			*name;
-	enum proto_desc_id		id:8;
-	enum proto_bases		base:8;
+	enum proto_desc_id		id;
+	enum proto_bases		base;
 	enum nft_payload_csum_types	checksum_type:8;
 	uint16_t			checksum_key;
 	uint16_t			protocol_key;
-- 
2.41.0