[v6,02/23] mbuf: consolidate driver asserts for mbuf struct

Message ID 1709012499-12813-3-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series stop and remove RTE_MARKER typedefs |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Tyler Retzlaff Feb. 27, 2024, 5:41 a.m. UTC
  Collect duplicated RTE_BUILD_BUG_ON checks from drivers and place them
at global scope with struct rte_mbuf definition using static_assert.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/mbuf/rte_mbuf_core.h | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
  

Comments

Konstantin Ananyev Feb. 27, 2024, 10:02 a.m. UTC | #1
> Subject: [PATCH v6 02/23] mbuf: consolidate driver asserts for mbuf struct
> 
> Collect duplicated RTE_BUILD_BUG_ON checks from drivers and place them
> at global scope with struct rte_mbuf definition using static_assert.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  lib/mbuf/rte_mbuf_core.h | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index 7000c04..36551c2 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -16,8 +16,11 @@
>   * New fields and flags should fit in the "dynamic space".
>   */
> 
> +#include <assert.h>
> +#include <stddef.h>
>  #include <stdint.h>
> 
> +#include <rte_common.h>
>  #include <rte_byteorder.h>
>  #include <rte_stdatomic.h>
> 
> @@ -673,6 +676,37 @@ struct rte_mbuf {
>  	uint32_t dynfield1[9]; /**< Reserved for dynamic fields. */
>  } __rte_cache_aligned;
> 
> +static_assert(!(offsetof(struct rte_mbuf, ol_flags) !=
> +	offsetof(struct rte_mbuf, rearm_data) + 8), "ol_flags");
> +static_assert(!(offsetof(struct rte_mbuf, rearm_data) !=
> +	RTE_ALIGN(offsetof(struct rte_mbuf, rearm_data), 16)), "rearm_data");
> +static_assert(!(offsetof(struct rte_mbuf, data_off) !=
> +	offsetof(struct rte_mbuf, rearm_data)), "data_off");
> +static_assert(!(offsetof(struct rte_mbuf, data_off) <
> +	offsetof(struct rte_mbuf, rearm_data)), "data_off");
> +static_assert(!(offsetof(struct rte_mbuf, refcnt) <
> +	offsetof(struct rte_mbuf, rearm_data)), "refcnt");
> +static_assert(!(offsetof(struct rte_mbuf, nb_segs) <
> +	offsetof(struct rte_mbuf, rearm_data)), "nb_segs");
> +static_assert(!(offsetof(struct rte_mbuf, port) <
> +	offsetof(struct rte_mbuf, rearm_data)), "port");
> +static_assert(!(offsetof(struct rte_mbuf, data_off) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "data_off");
> +static_assert(!(offsetof(struct rte_mbuf, refcnt) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "refcnt");
> +static_assert(!(offsetof(struct rte_mbuf, nb_segs) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "nb_segs");
> +static_assert(!(offsetof(struct rte_mbuf, port) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "port");
> +static_assert(!(offsetof(struct rte_mbuf, pkt_len) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 4), "pkt_len");
> +static_assert(!(offsetof(struct rte_mbuf, data_len) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 8), "data_len");
> +static_assert(!(offsetof(struct rte_mbuf, vlan_tci) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 10), "vlan_tci");
> +static_assert(!(offsetof(struct rte_mbuf, hash) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 12), "hash");
> +
>  /**
>   * Function typedef of callback to free externally attached buffer.
>   */
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
 

> 1.8.3.1
  
Tyler Retzlaff March 14, 2024, 4:51 p.m. UTC | #2
We've gone around in circles a little on this series.  Let's discuss it
at the next techboard meeting, please put it on the agenda.

Summary

MSVC does not support the typedef of zero-sized typed arrays used in
struct rte_mbuf and a handful of other structs built on Windows. Better
known as ``RTE_MARKER`` fields.

There are two competing solutions we would like to know which to move
forward with.

1. Use C11 anonymous unions and anonymous struct extensions to replace
the RTE_MARKER fields which can maintain ABI and API compatibility.

2. Provide inline accessors for struct rte_mbuf for some fields and
removing others which maintains ABI but breaks API.

I'm proposing a mix of 1 & 2 to maintain ABI and some API for struct
rte_mbuf fields but remove (API breaking) cacheline{0,1} RTE_MARKER
fields in favor of existing inline functions for prefetching.

Thanks!

On Mon, Feb 26, 2024 at 09:41:18PM -0800, Tyler Retzlaff wrote:
> Collect duplicated RTE_BUILD_BUG_ON checks from drivers and place them
> at global scope with struct rte_mbuf definition using static_assert.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  lib/mbuf/rte_mbuf_core.h | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> index 7000c04..36551c2 100644
> --- a/lib/mbuf/rte_mbuf_core.h
> +++ b/lib/mbuf/rte_mbuf_core.h
> @@ -16,8 +16,11 @@
>   * New fields and flags should fit in the "dynamic space".
>   */
>  
> +#include <assert.h>
> +#include <stddef.h>
>  #include <stdint.h>
>  
> +#include <rte_common.h>
>  #include <rte_byteorder.h>
>  #include <rte_stdatomic.h>
>  
> @@ -673,6 +676,37 @@ struct rte_mbuf {
>  	uint32_t dynfield1[9]; /**< Reserved for dynamic fields. */
>  } __rte_cache_aligned;
>  
> +static_assert(!(offsetof(struct rte_mbuf, ol_flags) !=
> +	offsetof(struct rte_mbuf, rearm_data) + 8), "ol_flags");
> +static_assert(!(offsetof(struct rte_mbuf, rearm_data) !=
> +	RTE_ALIGN(offsetof(struct rte_mbuf, rearm_data), 16)), "rearm_data");
> +static_assert(!(offsetof(struct rte_mbuf, data_off) !=
> +	offsetof(struct rte_mbuf, rearm_data)), "data_off");
> +static_assert(!(offsetof(struct rte_mbuf, data_off) <
> +	offsetof(struct rte_mbuf, rearm_data)), "data_off");
> +static_assert(!(offsetof(struct rte_mbuf, refcnt) <
> +	offsetof(struct rte_mbuf, rearm_data)), "refcnt");
> +static_assert(!(offsetof(struct rte_mbuf, nb_segs) <
> +	offsetof(struct rte_mbuf, rearm_data)), "nb_segs");
> +static_assert(!(offsetof(struct rte_mbuf, port) <
> +	offsetof(struct rte_mbuf, rearm_data)), "port");
> +static_assert(!(offsetof(struct rte_mbuf, data_off) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "data_off");
> +static_assert(!(offsetof(struct rte_mbuf, refcnt) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "refcnt");
> +static_assert(!(offsetof(struct rte_mbuf, nb_segs) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "nb_segs");
> +static_assert(!(offsetof(struct rte_mbuf, port) -
> +	offsetof(struct rte_mbuf, rearm_data) > 6), "port");
> +static_assert(!(offsetof(struct rte_mbuf, pkt_len) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 4), "pkt_len");
> +static_assert(!(offsetof(struct rte_mbuf, data_len) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 8), "data_len");
> +static_assert(!(offsetof(struct rte_mbuf, vlan_tci) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 10), "vlan_tci");
> +static_assert(!(offsetof(struct rte_mbuf, hash) !=
> +	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 12), "hash");
> +
>  /**
>   * Function typedef of callback to free externally attached buffer.
>   */
> -- 
> 1.8.3.1
  

Patch

diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
index 7000c04..36551c2 100644
--- a/lib/mbuf/rte_mbuf_core.h
+++ b/lib/mbuf/rte_mbuf_core.h
@@ -16,8 +16,11 @@ 
  * New fields and flags should fit in the "dynamic space".
  */
 
+#include <assert.h>
+#include <stddef.h>
 #include <stdint.h>
 
+#include <rte_common.h>
 #include <rte_byteorder.h>
 #include <rte_stdatomic.h>
 
@@ -673,6 +676,37 @@  struct rte_mbuf {
 	uint32_t dynfield1[9]; /**< Reserved for dynamic fields. */
 } __rte_cache_aligned;
 
+static_assert(!(offsetof(struct rte_mbuf, ol_flags) !=
+	offsetof(struct rte_mbuf, rearm_data) + 8), "ol_flags");
+static_assert(!(offsetof(struct rte_mbuf, rearm_data) !=
+	RTE_ALIGN(offsetof(struct rte_mbuf, rearm_data), 16)), "rearm_data");
+static_assert(!(offsetof(struct rte_mbuf, data_off) !=
+	offsetof(struct rte_mbuf, rearm_data)), "data_off");
+static_assert(!(offsetof(struct rte_mbuf, data_off) <
+	offsetof(struct rte_mbuf, rearm_data)), "data_off");
+static_assert(!(offsetof(struct rte_mbuf, refcnt) <
+	offsetof(struct rte_mbuf, rearm_data)), "refcnt");
+static_assert(!(offsetof(struct rte_mbuf, nb_segs) <
+	offsetof(struct rte_mbuf, rearm_data)), "nb_segs");
+static_assert(!(offsetof(struct rte_mbuf, port) <
+	offsetof(struct rte_mbuf, rearm_data)), "port");
+static_assert(!(offsetof(struct rte_mbuf, data_off) -
+	offsetof(struct rte_mbuf, rearm_data) > 6), "data_off");
+static_assert(!(offsetof(struct rte_mbuf, refcnt) -
+	offsetof(struct rte_mbuf, rearm_data) > 6), "refcnt");
+static_assert(!(offsetof(struct rte_mbuf, nb_segs) -
+	offsetof(struct rte_mbuf, rearm_data) > 6), "nb_segs");
+static_assert(!(offsetof(struct rte_mbuf, port) -
+	offsetof(struct rte_mbuf, rearm_data) > 6), "port");
+static_assert(!(offsetof(struct rte_mbuf, pkt_len) !=
+	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 4), "pkt_len");
+static_assert(!(offsetof(struct rte_mbuf, data_len) !=
+	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 8), "data_len");
+static_assert(!(offsetof(struct rte_mbuf, vlan_tci) !=
+	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 10), "vlan_tci");
+static_assert(!(offsetof(struct rte_mbuf, hash) !=
+	offsetof(struct rte_mbuf, rx_descriptor_fields1) + 12), "hash");
+
 /**
  * Function typedef of callback to free externally attached buffer.
  */