[v9,2/4] mbuf: remove rte marker fields

Message ID 1712088530-24948-3-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series remove use of RTE_MARKER fields in libraries |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Tyler Retzlaff April 2, 2024, 8:08 p.m. UTC
  RTE_MARKER typedefs are a GCC extension unsupported by MSVC. Remove
RTE_MARKER fields from rte_mbuf struct.

Maintain alignment of fields after removed cacheline1 marker by placing
C11 alignas(RTE_CACHE_LINE_MIN_SIZE).

Provide new rearm_data and rx_descriptor_fields1 fields in anonymous
unions as single element arrays of with types matching the original
markers to maintain API compatibility.

This change breaks the API for cacheline{0,1} fields that have been
removed from rte_mbuf but it does not break the ABI, to address the
false positives of the removed (but 0 size fields) provide the minimum
libabigail.abignore for type = rte_mbuf.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 devtools/libabigail.abignore           |   6 +
 doc/guides/rel_notes/release_24_03.rst |   2 +
 lib/mbuf/rte_mbuf.h                    |   4 +-
 lib/mbuf/rte_mbuf_core.h               | 200 +++++++++++++++++----------------
 4 files changed, 114 insertions(+), 98 deletions(-)
  

Comments

Stephen Hemminger April 2, 2024, 8:45 p.m. UTC | #1
On Tue,  2 Apr 2024 13:08:48 -0700
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> RTE_MARKER typedefs are a GCC extension unsupported by MSVC. Remove
> RTE_MARKER fields from rte_mbuf struct.
> 
> Maintain alignment of fields after removed cacheline1 marker by placing
> C11 alignas(RTE_CACHE_LINE_MIN_SIZE).
> 
> Provide new rearm_data and rx_descriptor_fields1 fields in anonymous
> unions as single element arrays of with types matching the original
> markers to maintain API compatibility.
> 
> This change breaks the API for cacheline{0,1} fields that have been
> removed from rte_mbuf but it does not break the ABI, to address the
> false positives of the removed (but 0 size fields) provide the minimum
> libabigail.abignore for type = rte_mbuf.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Release note should be for 24.07 not 24.03.
  
Tyler Retzlaff April 2, 2024, 8:51 p.m. UTC | #2
On Tue, Apr 02, 2024 at 01:45:49PM -0700, Stephen Hemminger wrote:
> On Tue,  2 Apr 2024 13:08:48 -0700
> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
> 
> > RTE_MARKER typedefs are a GCC extension unsupported by MSVC. Remove
> > RTE_MARKER fields from rte_mbuf struct.
> > 
> > Maintain alignment of fields after removed cacheline1 marker by placing
> > C11 alignas(RTE_CACHE_LINE_MIN_SIZE).
> > 
> > Provide new rearm_data and rx_descriptor_fields1 fields in anonymous
> > unions as single element arrays of with types matching the original
> > markers to maintain API compatibility.
> > 
> > This change breaks the API for cacheline{0,1} fields that have been
> > removed from rte_mbuf but it does not break the ABI, to address the
> > false positives of the removed (but 0 size fields) provide the minimum
> > libabigail.abignore for type = rte_mbuf.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> 
> Release note should be for 24.07 not 24.03.

yeah, pressed send and noticed it seconds later. when the new empty
release notes are added i'll move the notes to 24.07.

thanks.
  

Patch

diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore
index 645d289..ad13179 100644
--- a/devtools/libabigail.abignore
+++ b/devtools/libabigail.abignore
@@ -37,3 +37,9 @@ 
 [suppress_type]
 	name = rte_eth_fp_ops
 	has_data_member_inserted_between = {offset_of(reserved2), end}
+
+[suppress_type]
+	name = rte_mbuf
+	type_kind = struct
+	has_size_change = no
+	has_data_member = {cacheline0, rearm_data, rx_descriptor_fields1, cacheline1}
diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst
index 013c12f..ffc0d62 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -161,6 +161,8 @@  Removed Items
 
 * acc101: Removed obsolete code for non productized HW variant.
 
+* mbuf: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
+  have been removed from ``struct rte_mbuf``.
 
 API Changes
 -----------
diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index 286b32b..4c4722e 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -108,7 +108,7 @@ 
 static inline void
 rte_mbuf_prefetch_part1(struct rte_mbuf *m)
 {
-	rte_prefetch0(&m->cacheline0);
+	rte_prefetch0(m);
 }
 
 /**
@@ -126,7 +126,7 @@ 
 rte_mbuf_prefetch_part2(struct rte_mbuf *m)
 {
 #if RTE_CACHE_LINE_SIZE == 64
-	rte_prefetch0(&m->cacheline1);
+	rte_prefetch0(RTE_PTR_ADD(m, RTE_CACHE_LINE_MIN_SIZE));
 #else
 	RTE_SET_USED(m);
 #endif
diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
index 9f58076..9d838b8 100644
--- a/lib/mbuf/rte_mbuf_core.h
+++ b/lib/mbuf/rte_mbuf_core.h
@@ -465,8 +465,6 @@  enum {
  * The generic rte_mbuf, containing a packet mbuf.
  */
 struct __rte_cache_aligned rte_mbuf {
-	RTE_MARKER cacheline0;
-
 	void *buf_addr;           /**< Virtual address of segment buffer. */
 #if RTE_IOVA_IN_MBUF
 	/**
@@ -488,127 +486,138 @@  struct __rte_cache_aligned rte_mbuf {
 #endif
 
 	/* next 8 bytes are initialised on RX descriptor rearm */
-	RTE_MARKER64 rearm_data;
-	uint16_t data_off;
-
-	/**
-	 * Reference counter. Its size should at least equal to the size
-	 * of port field (16 bits), to support zero-copy broadcast.
-	 * It should only be accessed using the following functions:
-	 * rte_mbuf_refcnt_update(), rte_mbuf_refcnt_read(), and
-	 * rte_mbuf_refcnt_set(). The functionality of these functions (atomic,
-	 * or non-atomic) is controlled by the RTE_MBUF_REFCNT_ATOMIC flag.
-	 */
-	RTE_ATOMIC(uint16_t) refcnt;
+	union {
+		uint64_t rearm_data[1];
+		__extension__
+		struct {
+			uint16_t data_off;
+
+			/**
+			 * Reference counter. Its size should at least equal to the size
+			 * of port field (16 bits), to support zero-copy broadcast.
+			 * It should only be accessed using the following functions:
+			 * rte_mbuf_refcnt_update(), rte_mbuf_refcnt_read(), and
+			 * rte_mbuf_refcnt_set(). The functionality of these functions (atomic,
+			 * or non-atomic) is controlled by the RTE_MBUF_REFCNT_ATOMIC flag.
+			 */
+			RTE_ATOMIC(uint16_t) refcnt;
 
-	/**
-	 * Number of segments. Only valid for the first segment of an mbuf
-	 * chain.
-	 */
-	uint16_t nb_segs;
+			/**
+			 * Number of segments. Only valid for the first segment of an mbuf
+			 * chain.
+			 */
+			uint16_t nb_segs;
 
-	/** Input port (16 bits to support more than 256 virtual ports).
-	 * The event eth Tx adapter uses this field to specify the output port.
-	 */
-	uint16_t port;
+			/** Input port (16 bits to support more than 256 virtual ports).
+			 * The event eth Tx adapter uses this field to specify the output port.
+			 */
+			uint16_t port;
+		};
+	};
 
 	uint64_t ol_flags;        /**< Offload features. */
 
-	/* remaining bytes are set on RX when pulling packet from descriptor */
-	RTE_MARKER rx_descriptor_fields1;
-
-	/*
-	 * The packet type, which is the combination of outer/inner L2, L3, L4
-	 * and tunnel types. The packet_type is about data really present in the
-	 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
-	 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
-	 * vlan is stripped from the data.
-	 */
+	/* remaining 24 bytes are set on RX when pulling packet from descriptor */
 	union {
-		uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
+		/* void * type of the array elements is retained for driver compatibility. */
+		void *rx_descriptor_fields1[24 / sizeof(void *)];
 		__extension__
 		struct {
-			uint8_t l2_type:4;   /**< (Outer) L2 type. */
-			uint8_t l3_type:4;   /**< (Outer) L3 type. */
-			uint8_t l4_type:4;   /**< (Outer) L4 type. */
-			uint8_t tun_type:4;  /**< Tunnel type. */
+			/*
+			 * The packet type, which is the combination of outer/inner L2, L3, L4
+			 * and tunnel types. The packet_type is about data really present in the
+			 * mbuf. Example: if vlan stripping is enabled, a received vlan packet
+			 * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the
+			 * vlan is stripped from the data.
+			 */
 			union {
-				uint8_t inner_esp_next_proto;
-				/**< ESP next protocol type, valid if
-				 * RTE_PTYPE_TUNNEL_ESP tunnel type is set
-				 * on both Tx and Rx.
-				 */
+				uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
 				__extension__
 				struct {
-					uint8_t inner_l2_type:4;
-					/**< Inner L2 type. */
-					uint8_t inner_l3_type:4;
-					/**< Inner L3 type. */
+					uint8_t l2_type:4;   /**< (Outer) L2 type. */
+					uint8_t l3_type:4;   /**< (Outer) L3 type. */
+					uint8_t l4_type:4;   /**< (Outer) L4 type. */
+					uint8_t tun_type:4;  /**< Tunnel type. */
+					union {
+						/**< ESP next protocol type, valid if
+						 * RTE_PTYPE_TUNNEL_ESP tunnel type is set
+						 * on both Tx and Rx.
+						 */
+						uint8_t inner_esp_next_proto;
+						__extension__
+						struct {
+							/**< Inner L2 type. */
+							uint8_t inner_l2_type:4;
+							/**< Inner L3 type. */
+							uint8_t inner_l3_type:4;
+						};
+					};
+					uint8_t inner_l4_type:4; /**< Inner L4 type. */
 				};
 			};
-			uint8_t inner_l4_type:4; /**< Inner L4 type. */
-		};
-	};
 
-	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
-	uint16_t data_len;        /**< Amount of data in segment buffer. */
-	/** VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_VLAN is set. */
-	uint16_t vlan_tci;
+			uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 
-	union {
-		union {
-			uint32_t rss;     /**< RSS hash result if RSS enabled */
-			struct {
+			uint16_t data_len;        /**< Amount of data in segment buffer. */
+			/** VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_VLAN is set. */
+			uint16_t vlan_tci;
+
+			union {
 				union {
+					uint32_t rss;     /**< RSS hash result if RSS enabled */
 					struct {
-						uint16_t hash;
-						uint16_t id;
-					};
-					uint32_t lo;
-					/**< Second 4 flexible bytes */
-				};
-				uint32_t hi;
-				/**< First 4 flexible bytes or FD ID, dependent
-				 * on RTE_MBUF_F_RX_FDIR_* flag in ol_flags.
-				 */
-			} fdir;	/**< Filter identifier if FDIR enabled */
-			struct rte_mbuf_sched sched;
-			/**< Hierarchical scheduler : 8 bytes */
-			struct {
-				uint32_t reserved1;
-				uint16_t reserved2;
-				uint16_t txq;
-				/**< The event eth Tx adapter uses this field
-				 * to store Tx queue id.
-				 * @see rte_event_eth_tx_adapter_txq_set()
-				 */
-			} txadapter; /**< Eventdev ethdev Tx adapter */
-			uint32_t usr;
-			/**< User defined tags. See rte_distributor_process() */
-		} hash;                   /**< hash information */
-	};
+						union {
+							struct {
+								uint16_t hash;
+								uint16_t id;
+							};
+							/**< Second 4 flexible bytes */
+							uint32_t lo;
+						};
+						/**< First 4 flexible bytes or FD ID, dependent
+						 * on RTE_MBUF_F_RX_FDIR_* flag in ol_flags.
+						 */
+						uint32_t hi;
+					} fdir;	/**< Filter identifier if FDIR enabled */
+					struct rte_mbuf_sched sched;
+					/**< Hierarchical scheduler : 8 bytes */
+					struct {
+						uint32_t reserved1;
+						uint16_t reserved2;
+						/**< The event eth Tx adapter uses this field
+						 * to store Tx queue id.
+						 * @see rte_event_eth_tx_adapter_txq_set()
+						 */
+						uint16_t txq;
+					} txadapter; /**< Eventdev ethdev Tx adapter */
+					/**< User defined tags. See rte_distributor_process() */
+					uint32_t usr;
+				} hash;                   /**< hash information */
+			};
 
-	/** Outer VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_QINQ is set. */
-	uint16_t vlan_tci_outer;
+			/** Outer VLAN TCI (CPU order), valid if RTE_MBUF_F_RX_QINQ is set. */
+			uint16_t vlan_tci_outer;
 
-	uint16_t buf_len;         /**< Length of segment buffer. */
+			uint16_t buf_len;         /**< Length of segment buffer. */
+		};
+	};
 
 	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
 
 	/* second cache line - fields only used in slow path or on TX */
-	alignas(RTE_CACHE_LINE_MIN_SIZE) RTE_MARKER cacheline1;
-
 #if RTE_IOVA_IN_MBUF
 	/**
 	 * Next segment of scattered packet. Must be NULL in the last
 	 * segment or in case of non-segmented packet.
 	 */
+	alignas(RTE_CACHE_LINE_MIN_SIZE)
 	struct rte_mbuf *next;
 #else
 	/**
 	 * Reserved for dynamic fields
 	 * when the next pointer is in first cache line (i.e. RTE_IOVA_IN_MBUF is 0).
 	 */
+	alignas(RTE_CACHE_LINE_MIN_SIZE)
 	uint64_t dynfield2;
 #endif
 
@@ -617,17 +626,16 @@  struct __rte_cache_aligned rte_mbuf {
 		uint64_t tx_offload;       /**< combined for easy fetch */
 		__extension__
 		struct {
-			uint64_t l2_len:RTE_MBUF_L2_LEN_BITS;
 			/**< L2 (MAC) Header Length for non-tunneling pkt.
 			 * Outer_L4_len + ... + Inner_L2_len for tunneling pkt.
 			 */
-			uint64_t l3_len:RTE_MBUF_L3_LEN_BITS;
+			uint64_t l2_len:RTE_MBUF_L2_LEN_BITS;
 			/**< L3 (IP) Header Length. */
-			uint64_t l4_len:RTE_MBUF_L4_LEN_BITS;
+			uint64_t l3_len:RTE_MBUF_L3_LEN_BITS;
 			/**< L4 (TCP/UDP) Header Length. */
-			uint64_t tso_segsz:RTE_MBUF_TSO_SEGSZ_BITS;
+			uint64_t l4_len:RTE_MBUF_L4_LEN_BITS;
 			/**< TCP TSO segment size */
-
+			uint64_t tso_segsz:RTE_MBUF_TSO_SEGSZ_BITS;
 			/*
 			 * Fields for Tx offloading of tunnels.
 			 * These are undefined for packets which don't request
@@ -640,10 +648,10 @@  struct __rte_cache_aligned rte_mbuf {
 			 * Applications are expected to set appropriate tunnel
 			 * offload flags when they fill in these fields.
 			 */
-			uint64_t outer_l3_len:RTE_MBUF_OUTL3_LEN_BITS;
 			/**< Outer L3 (IP) Hdr Length. */
-			uint64_t outer_l2_len:RTE_MBUF_OUTL2_LEN_BITS;
+			uint64_t outer_l3_len:RTE_MBUF_OUTL3_LEN_BITS;
 			/**< Outer L2 (MAC) Hdr Length. */
+			uint64_t outer_l2_len:RTE_MBUF_OUTL2_LEN_BITS;
 
 			/* uint64_t unused:RTE_MBUF_TXOFLD_UNUSED_BITS; */
 		};