[RFC PATCH] net: TEST 2-byte align packed headers

[RFC PATCH] net: TEST 2-byte align packed headers

[Morten Brørup]

Testing idea discussed in:
https://inbox.dpdk.org/dev/20240821162516.610624-17-rjarry@redhat.com/T/#mc598fde6ba5d540cde5096575cc3b6e27d9640cd

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/net/rte_ether.h | 2 +-
 lib/net/rte_ip.h    | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index 403e84f50b..e759c51a65 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -301,7 +301,7 @@ struct __rte_aligned(2) rte_ether_hdr {
  * Contains the 16-bit VLAN Tag Control Identifier and the Ethernet type
  * of the encapsulated frame.
  */
-struct rte_vlan_hdr {
+struct __rte_aligned(2) rte_vlan_hdr {
 	rte_be16_t vlan_tci;  /**< Priority (3) + CFI (1) + Identifier Code (12) */
 	rte_be16_t eth_proto; /**< Ethernet type of encapsulated frame. */
 } __rte_packed;
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index 0d103d4127..88bfb6a44d 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -38,7 +38,7 @@ extern "C" {
 /**
  * IPv4 Header
  */
-struct rte_ipv4_hdr {
+struct __rte_aligned(2) rte_ipv4_hdr {
 	__extension__
 	union {
 		uint8_t version_ihl;    /**< version and header length */
@@ -523,7 +523,7 @@ rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
 /**
  * IPv6 Header
  */
-struct rte_ipv6_hdr {
+struct __rte_aligned(2) rte_ipv6_hdr {
 	rte_be32_t vtc_flow;	/**< IP version, traffic class & flow label. */
 	rte_be16_t payload_len;	/**< IP payload size, including ext. headers */
 	uint8_t  proto;		/**< Protocol, next header. */
@@ -538,7 +538,7 @@ struct rte_ipv6_hdr {
 /**
  * IPv6 Routing Extension Header
  */
-struct rte_ipv6_routing_ext {
+struct __rte_aligned(2) rte_ipv6_routing_ext {
 	uint8_t next_hdr;			/**< Protocol, next header. */
 	uint8_t hdr_len;			/**< Header length. */
 	uint8_t type;				/**< Extension header type. */
@@ -783,7 +783,7 @@ rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
 #define RTE_IPV6_SET_FRAG_DATA(fo, mf)	\
 	(((fo) & RTE_IPV6_EHDR_FO_MASK) | ((mf) & RTE_IPV6_EHDR_MF_MASK))
 
-struct rte_ipv6_fragment_ext {
+struct __rte_aligned(2) rte_ipv6_fragment_ext {
 	uint8_t next_header;	/**< Next header type */
 	uint8_t reserved;	/**< Reserved */
 	rte_be16_t frag_data;	/**< All fragmentation data */
-- 
2.43.0

[RFC PATCH v2] net: 2-byte align packed headers

[Morten Brørup]

Network headers are (at minimum) 2-byte aligned.
Mark them as such.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/net/rte_esp.h    | 10 +++++++++-
 lib/net/rte_ether.h  | 23 ++++++++++++++++++++---
 lib/net/rte_geneve.h |  2 +-
 lib/net/rte_icmp.h   | 12 ++++++------
 lib/net/rte_ip.h     | 30 ++++++++++++++++++++++++++----
 lib/net/rte_sctp.h   |  9 ++++++++-
 lib/net/rte_tcp.h    |  9 ++++++++-
 lib/net/rte_udp.h    | 11 +++++++++--
 lib/net/rte_vxlan.h  |  9 ++++++++-
 9 files changed, 95 insertions(+), 20 deletions(-)

diff --git a/lib/net/rte_esp.h b/lib/net/rte_esp.h
index 745a9847fe..5d3af9c81f 100644
--- a/lib/net/rte_esp.h
+++ b/lib/net/rte_esp.h
@@ -11,16 +11,24 @@
  * ESP-related defines
  */
 
+#include <assert.h>
+#include <stdalign.h>
+
 #include <rte_byteorder.h>
 
 /**
  * ESP Header
  */
-struct rte_esp_hdr {
+struct __rte_aligned(2) rte_esp_hdr {
 	rte_be32_t spi;  /**< Security Parameters Index */
 	rte_be32_t seq;  /**< packet sequence number */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_esp_hdr) == 8,
+		"sizeof(struct rte_esp_hdr) == 8");
+static_assert(alignof(struct rte_esp_hdr) == 2,
+		"alignof(struct rte_esp_hdr) == 2");
+
 /**
  * ESP Trailer
  */
diff --git a/lib/net/rte_ether.h b/lib/net/rte_ether.h
index 403e84f50b..64aefd1a6c 100644
--- a/lib/net/rte_ether.h
+++ b/lib/net/rte_ether.h
@@ -11,6 +11,8 @@
  * Ethernet Helpers in RTE
  */
 
+#include <assert.h>
+#include <stdalign.h>
 #include <stdint.h>
 #include <stdio.h>
 
@@ -75,6 +77,11 @@ struct __rte_aligned(2) rte_ether_addr {
 	uint8_t addr_bytes[RTE_ETHER_ADDR_LEN]; /**< Addr bytes in tx order */
 };
 
+static_assert(sizeof(struct rte_ether_addr) == 6,
+		"sizeof(struct rte_ether_addr) == 6");
+static_assert(alignof(struct rte_ether_addr) == 2,
+		"alignof(struct rte_ether_addr) == 2");
+
 #define RTE_ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */
 #define RTE_ETHER_GROUP_ADDR  0x01 /**< Multicast or broadcast Eth. address. */
 
@@ -290,21 +297,31 @@ rte_ether_unformat_addr(const char *str, struct rte_ether_addr *eth_addr);
  * Ethernet header: Contains the destination address, source address
  * and frame type.
  */
-struct __rte_aligned(2) rte_ether_hdr {
+struct /* native alignment: __rte_aligned(2) */ rte_ether_hdr {
 	struct rte_ether_addr dst_addr; /**< Destination address. */
 	struct rte_ether_addr src_addr; /**< Source address. */
 	rte_be16_t ether_type; /**< Frame type. */
 };
 
+static_assert(sizeof(struct rte_ether_hdr) == 14,
+		"sizeof(struct rte_ether_hdr) == 14");
+static_assert(alignof(struct rte_ether_hdr) == 2,
+		"alignof(struct rte_ether_hdr) == 2");
+
 /**
  * Ethernet VLAN Header.
  * Contains the 16-bit VLAN Tag Control Identifier and the Ethernet type
  * of the encapsulated frame.
  */
-struct rte_vlan_hdr {
+struct /* native alignment: __rte_aligned(2) */ rte_vlan_hdr {
 	rte_be16_t vlan_tci;  /**< Priority (3) + CFI (1) + Identifier Code (12) */
 	rte_be16_t eth_proto; /**< Ethernet type of encapsulated frame. */
-} __rte_packed;
+};
+
+static_assert(sizeof(struct rte_vlan_hdr) == 4,
+		"sizeof(struct rte_vlan_hdr) == 4");
+static_assert(alignof(struct rte_vlan_hdr) == 2,
+		"alignof(struct rte_vlan_hdr) == 2");
 
 
 
diff --git a/lib/net/rte_geneve.h b/lib/net/rte_geneve.h
index eb2c85f1e9..9a62b4dad1 100644
--- a/lib/net/rte_geneve.h
+++ b/lib/net/rte_geneve.h
@@ -34,7 +34,7 @@
  * More-bits (optional) variable length options.
  */
 __extension__
-struct rte_geneve_hdr {
+struct __rte_aligned(2) rte_geneve_hdr {
 #if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
 	uint8_t ver:2;		/**< Version. */
 	uint8_t opt_len:6;	/**< Options length. */
diff --git a/lib/net/rte_icmp.h b/lib/net/rte_icmp.h
index 7a33280aa1..a461cd339d 100644
--- a/lib/net/rte_icmp.h
+++ b/lib/net/rte_icmp.h
@@ -21,33 +21,33 @@
 /**
  * ICMP base header
  */
-struct rte_icmp_base_hdr {
+struct /* native alignment: __rte_aligned(2) */ rte_icmp_base_hdr {
 	uint8_t type;
 	uint8_t code;
 	rte_be16_t checksum;
-} __rte_packed;
+};
 
 /**
  * ICMP echo header
  */
-struct rte_icmp_echo_hdr {
+struct /* native alignment: __rte_aligned(2) */ rte_icmp_echo_hdr {
 	struct rte_icmp_base_hdr base;
 	rte_be16_t identifier;
 	rte_be16_t sequence;
-} __rte_packed;
+};
 
 /**
  * ICMP Header
  *
  * @see rte_icmp_echo_hdr which is similar.
  */
-struct rte_icmp_hdr {
+struct /* native alignment: __rte_aligned(2) */ rte_icmp_hdr {
 	uint8_t  icmp_type;     /* ICMP packet type. */
 	uint8_t  icmp_code;     /* ICMP packet code. */
 	rte_be16_t icmp_cksum;  /* ICMP packet checksum. */
 	rte_be16_t icmp_ident;  /* ICMP packet identifier. */
 	rte_be16_t icmp_seq_nb; /* ICMP packet sequence number. */
-} __rte_packed;
+};
 
 /* ICMP packet types */
 #define RTE_IP_ICMP_ECHO_REPLY   0
diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
index 0d103d4127..c8b9fe8f48 100644
--- a/lib/net/rte_ip.h
+++ b/lib/net/rte_ip.h
@@ -15,6 +15,8 @@
  * IP-related defines
  */
 
+#include <assert.h>
+#include <stdalign.h>
 #include <stdint.h>
 
 #ifdef RTE_EXEC_ENV_WINDOWS
@@ -38,7 +40,7 @@ extern "C" {
 /**
  * IPv4 Header
  */
-struct rte_ipv4_hdr {
+struct __rte_aligned(2) rte_ipv4_hdr {
 	__extension__
 	union {
 		uint8_t version_ihl;    /**< version and header length */
@@ -63,6 +65,11 @@ struct rte_ipv4_hdr {
 	rte_be32_t dst_addr;		/**< destination address */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_ipv4_hdr) == 20,
+		"sizeof(struct rte_ipv4_hdr) == 20");
+static_assert(alignof(struct rte_ipv4_hdr) == 2,
+		"alignof(struct rte_ipv4_hdr) == 2");
+
 /** Create IPv4 address */
 #define RTE_IPV4(a, b, c, d) ((uint32_t)(((a) & 0xff) << 24) | \
 					   (((b) & 0xff) << 16) | \
@@ -523,7 +530,7 @@ rte_ipv4_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
 /**
  * IPv6 Header
  */
-struct rte_ipv6_hdr {
+struct __rte_aligned(2) rte_ipv6_hdr {
 	rte_be32_t vtc_flow;	/**< IP version, traffic class & flow label. */
 	rte_be16_t payload_len;	/**< IP payload size, including ext. headers */
 	uint8_t  proto;		/**< Protocol, next header. */
@@ -532,13 +539,18 @@ struct rte_ipv6_hdr {
 	uint8_t  dst_addr[16];	/**< IP address of destination host(s). */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_ipv6_hdr) == 40,
+		"sizeof(struct rte_ipv6_hdr) == 40");
+static_assert(alignof(struct rte_ipv6_hdr) == 2,
+		"alignof(struct rte_ipv6_hdr) == 2");
+
 /* IPv6 routing extension type definition. */
 #define RTE_IPV6_SRCRT_TYPE_4 4
 
 /**
  * IPv6 Routing Extension Header
  */
-struct rte_ipv6_routing_ext {
+struct __rte_aligned(2) rte_ipv6_routing_ext {
 	uint8_t next_hdr;			/**< Protocol, next header. */
 	uint8_t hdr_len;			/**< Header length. */
 	uint8_t type;				/**< Extension header type. */
@@ -555,6 +567,11 @@ struct rte_ipv6_routing_ext {
 	/* Next are 128-bit IPv6 address fields to describe segments. */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_ipv6_routing_ext) == 8,
+		"sizeof(struct rte_ipv6_routing_ext) == 8");
+static_assert(alignof(struct rte_ipv6_routing_ext) == 2,
+		"alignof(struct rte_ipv6_routing_ext) == 2");
+
 /* IPv6 vtc_flow: IPv / TC / flow_label */
 #define RTE_IPV6_HDR_FL_SHIFT 0
 #define RTE_IPV6_HDR_TC_SHIFT 20
@@ -783,13 +800,18 @@ rte_ipv6_udptcp_cksum_mbuf_verify(const struct rte_mbuf *m,
 #define RTE_IPV6_SET_FRAG_DATA(fo, mf)	\
 	(((fo) & RTE_IPV6_EHDR_FO_MASK) | ((mf) & RTE_IPV6_EHDR_MF_MASK))
 
-struct rte_ipv6_fragment_ext {
+struct __rte_aligned(2) rte_ipv6_fragment_ext {
 	uint8_t next_header;	/**< Next header type */
 	uint8_t reserved;	/**< Reserved */
 	rte_be16_t frag_data;	/**< All fragmentation data */
 	rte_be32_t id;		/**< Packet ID */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_ipv6_fragment_ext) == 8,
+		"sizeof(struct rte_ipv6_fragment_ext) == 8");
+static_assert(alignof(struct rte_ipv6_fragment_ext) == 2,
+		"alignof(struct rte_ipv6_fragment_ext) == 2");
+
 /* IPv6 fragment extension header size */
 #define RTE_IPV6_FRAG_HDR_SIZE	sizeof(struct rte_ipv6_fragment_ext)
 
diff --git a/lib/net/rte_sctp.h b/lib/net/rte_sctp.h
index e757c57db3..3bc920a38f 100644
--- a/lib/net/rte_sctp.h
+++ b/lib/net/rte_sctp.h
@@ -14,6 +14,8 @@
 #ifndef _RTE_SCTP_H_
 #define _RTE_SCTP_H_
 
+#include <assert.h>
+#include <stdalign.h>
 #include <stdint.h>
 
 #include <rte_byteorder.h>
@@ -21,11 +23,16 @@
 /**
  * SCTP Header
  */
-struct rte_sctp_hdr {
+struct __rte_aligned(2) rte_sctp_hdr {
 	rte_be16_t src_port; /**< Source port. */
 	rte_be16_t dst_port; /**< Destin port. */
 	rte_be32_t tag;      /**< Validation tag. */
 	rte_be32_t cksum;    /**< Checksum. */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_sctp_hdr) == 12,
+		"sizeof(struct rte_sctp_hdr) == 12");
+static_assert(alignof(struct rte_sctp_hdr) == 2,
+		"alignof(struct rte_sctp_hdr) == 2");
+
 #endif /* RTE_SCTP_H_ */
diff --git a/lib/net/rte_tcp.h b/lib/net/rte_tcp.h
index 1bcacbf038..94ecdd2f9c 100644
--- a/lib/net/rte_tcp.h
+++ b/lib/net/rte_tcp.h
@@ -14,6 +14,8 @@
  * TCP-related defines
  */
 
+#include <assert.h>
+#include <stdalign.h>
 #include <stdint.h>
 
 #include <rte_byteorder.h>
@@ -21,7 +23,7 @@
 /**
  * TCP Header
  */
-struct rte_tcp_hdr {
+struct __rte_aligned(2) rte_tcp_hdr {
 	rte_be16_t src_port; /**< TCP source port. */
 	rte_be16_t dst_port; /**< TCP destination port. */
 	rte_be32_t sent_seq; /**< TX data sequence number. */
@@ -33,6 +35,11 @@ struct rte_tcp_hdr {
 	rte_be16_t tcp_urp;  /**< TCP urgent pointer, if any. */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_tcp_hdr) == 20,
+		"sizeof(struct rte_tcp_hdr) == 20");
+static_assert(alignof(struct rte_tcp_hdr) == 2,
+		"alignof(struct rte_tcp_hdr) == 2");
+
 /**
  * TCP Flags
  */
diff --git a/lib/net/rte_udp.h b/lib/net/rte_udp.h
index c01dad9c9b..3a74d69c14 100644
--- a/lib/net/rte_udp.h
+++ b/lib/net/rte_udp.h
@@ -14,6 +14,8 @@
  * UDP-related defines
  */
 
+#include <assert.h>
+#include <stdalign.h>
 #include <stdint.h>
 
 #include <rte_byteorder.h>
@@ -21,11 +23,16 @@
 /**
  * UDP Header
  */
-struct rte_udp_hdr {
+struct /* native alignment: __rte_aligned(2) */ rte_udp_hdr {
 	rte_be16_t src_port;    /**< UDP source port. */
 	rte_be16_t dst_port;    /**< UDP destination port. */
 	rte_be16_t dgram_len;   /**< UDP datagram length */
 	rte_be16_t dgram_cksum; /**< UDP datagram checksum */
-} __rte_packed;
+};
+
+static_assert(sizeof(struct rte_udp_hdr) == 8,
+		"sizeof(struct rte_udp_hdr) == 8");
+static_assert(alignof(struct rte_udp_hdr) == 2,
+		"alignof(struct rte_udp_hdr) == 2");
 
 #endif /* RTE_UDP_H_ */
diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
index bd1c89835e..ca63bb4185 100644
--- a/lib/net/rte_vxlan.h
+++ b/lib/net/rte_vxlan.h
@@ -11,6 +11,8 @@
  * VXLAN-related definitions
  */
 
+#include <assert.h>
+#include <stdalign.h>
 #include <stdint.h>
 
 #include <rte_byteorder.h>
@@ -27,7 +29,7 @@
  * Reserved fields (24 bits and 8 bits)
  */
 __extension__ /* no named member in struct */
-struct rte_vxlan_hdr {
+struct __rte_aligned(2) rte_vxlan_hdr {
 	union {
 		rte_be32_t vx_flags; /**< flags (8 bits) + extensions (24 bits). */
 		struct {
@@ -97,6 +99,11 @@ struct rte_vxlan_hdr {
 	}; /* end of 2nd 32-bit word */
 } __rte_packed;
 
+static_assert(sizeof(struct rte_vxlan_hdr) == 8,
+		"sizeof(struct rte_vxlan_hdr) == 8");
+static_assert(alignof(struct rte_vxlan_hdr) == 2,
+		"alignof(struct rte_vxlan_hdr) == 2");
+
 /** VXLAN tunnel header length. */
 #define RTE_ETHER_VXLAN_HLEN \
 	(sizeof(struct rte_udp_hdr) + sizeof(struct rte_vxlan_hdr))
-- 
2.43.0

RE: [RFC PATCH v2] net: 2-byte align packed headers

[Morten Brørup]

Recheck-request: iol-unit-amd64-testing

Re: [RFC PATCH v2] net: 2-byte align packed headers

[Bruce Richardson]

On Fri, Oct 11, 2024 at 04:06:53PM +0000, Morten Brørup wrote:
> Network headers are (at minimum) 2-byte aligned.
> Mark them as such.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/net/rte_esp.h    | 10 +++++++++-
>  lib/net/rte_ether.h  | 23 ++++++++++++++++++++---
>  lib/net/rte_geneve.h |  2 +-
>  lib/net/rte_icmp.h   | 12 ++++++------
>  lib/net/rte_ip.h     | 30 ++++++++++++++++++++++++++----
>  lib/net/rte_sctp.h   |  9 ++++++++-
>  lib/net/rte_tcp.h    |  9 ++++++++-
>  lib/net/rte_udp.h    | 11 +++++++++--
>  lib/net/rte_vxlan.h  |  9 ++++++++-
>  9 files changed, 95 insertions(+), 20 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [RFC PATCH v2] net: 2-byte align packed headers

[David Marchand]

On Wed, Oct 30, 2024 at 1:35 PM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> Recheck-request: iol-unit-amd64-testing

It will need some rebase, regardless of the retest.


-- 
David Marchand

RE: [RFC PATCH v2] net: 2-byte align packed headers

[Morten Brørup]

Recheck-request: rebase=main, iol-unit-amd64-testing

Thanks David.

Re: [RFC PATCH v2] net: 2-byte align packed headers

[David Marchand]

On Wed, Oct 30, 2024 at 1:55 PM Morten Brørup <mb@smartsharesystems.com> wrote:
>
> Recheck-request: rebase=main, iol-unit-amd64-testing

There is a conflict because of a change in rte_ether.h and the
rte_ip.h split into rte_ip4.h/rte_ip6.h (Robin series).
Could you send a new revision?

Thanks.


-- 
David Marchand

RE: [RFC PATCH v2] net: 2-byte align packed headers

[Morten Brørup]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, 30 October 2024 16.11
> 
> On Wed, Oct 30, 2024 at 1:55 PM Morten Brørup
> <mb@smartsharesystems.com> wrote:
> >
> > Recheck-request: rebase=main, iol-unit-amd64-testing
> 
> There is a conflict because of a change in rte_ether.h and the
> rte_ip.h split into rte_ip4.h/rte_ip6.h (Robin series).
> Could you send a new revision?

Will do.

-Morten