diff mbox series

[v7,3/4] security: remove rte marker fields

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

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Tyler Retzlaff March 20, 2024, 10:01 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).

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 doc/guides/rel_notes/release_24_03.rst | 3 +++
 lib/security/rte_security_driver.h     | 5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Morten Brørup March 26, 2024, 10:28 a.m. UTC | #1 
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Wednesday, 20 March 2024 23.02
> 
> 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).
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  doc/guides/rel_notes/release_24_03.rst | 3 +++
>  lib/security/rte_security_driver.h     | 5 +++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_24_03.rst
> b/doc/guides/rel_notes/release_24_03.rst
> index 4f18cca..75d40d4 100644
> --- a/doc/guides/rel_notes/release_24_03.rst
> +++ b/doc/guides/rel_notes/release_24_03.rst
> @@ -219,6 +219,9 @@ Removed Items
>  * mbuf: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
>    have been removed from ``struct rte_mbuf``.
> 
> +* security: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
> +  have been removed from ``struct rte_security_session``.
> +
>  API Changes
>  -----------
> 
> diff --git a/lib/security/rte_security_driver.h
> b/lib/security/rte_security_driver.h
> index 09829ab..18a1e3c 100644
> --- a/lib/security/rte_security_driver.h
> +++ b/lib/security/rte_security_driver.h
> @@ -12,6 +12,8 @@
>   * RTE Security Common Definitions
>   */
> 
> +#include <stdalign.h>
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -24,7 +26,6 @@
>   * Security session to be used by library for internal usage
>   */
>  struct rte_security_session {
> -	RTE_MARKER cacheline0;
>  	uint64_t opaque_data;
>  	/**< Opaque user defined data */
>  	uint64_t fast_mdata;
> @@ -32,7 +33,7 @@ struct rte_security_session {
>  	rte_iova_t driver_priv_data_iova;
>  	/**< session private data IOVA address */
> 
> -	alignas(RTE_CACHE_LINE_MIN_SIZE) RTE_MARKER cacheline1;
> +	alignas(RTE_CACHE_LINE_MIN_SIZE)
>  	uint8_t driver_priv_data[];
>  	/**< Private session material, variable size (depends on driver)
> */
>  };
> --
> 1.8.3.1

No explicit alignment was ever specified for the struct rte_security_session itself. I wonder which implicit alignment applies to it.

Anyway, the changes are correct, so
Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
Tyler Retzlaff March 27, 2024, 7:59 p.m. UTC | #2 
On Tue, Mar 26, 2024 at 11:28:21AM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Wednesday, 20 March 2024 23.02
> > 
> > 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).
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> >  doc/guides/rel_notes/release_24_03.rst | 3 +++
> >  lib/security/rte_security_driver.h     | 5 +++--
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/doc/guides/rel_notes/release_24_03.rst
> > b/doc/guides/rel_notes/release_24_03.rst
> > index 4f18cca..75d40d4 100644
> > --- a/doc/guides/rel_notes/release_24_03.rst
> > +++ b/doc/guides/rel_notes/release_24_03.rst
> > @@ -219,6 +219,9 @@ Removed Items
> >  * mbuf: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
> >    have been removed from ``struct rte_mbuf``.
> > 
> > +* security: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
> > +  have been removed from ``struct rte_security_session``.
> > +
> >  API Changes
> >  -----------
> > 
> > diff --git a/lib/security/rte_security_driver.h
> > b/lib/security/rte_security_driver.h
> > index 09829ab..18a1e3c 100644
> > --- a/lib/security/rte_security_driver.h
> > +++ b/lib/security/rte_security_driver.h
> > @@ -12,6 +12,8 @@
> >   * RTE Security Common Definitions
> >   */
> > 
> > +#include <stdalign.h>
> > +
> >  #ifdef __cplusplus
> >  extern "C" {
> >  #endif
> > @@ -24,7 +26,6 @@
> >   * Security session to be used by library for internal usage
> >   */
> >  struct rte_security_session {
> > -	RTE_MARKER cacheline0;
> >  	uint64_t opaque_data;
> >  	/**< Opaque user defined data */
> >  	uint64_t fast_mdata;
> > @@ -32,7 +33,7 @@ struct rte_security_session {
> >  	rte_iova_t driver_priv_data_iova;
> >  	/**< session private data IOVA address */
> > 
> > -	alignas(RTE_CACHE_LINE_MIN_SIZE) RTE_MARKER cacheline1;
> > +	alignas(RTE_CACHE_LINE_MIN_SIZE)
> >  	uint8_t driver_priv_data[];
> >  	/**< Private session material, variable size (depends on driver)
> > */
> >  };
> > --
> > 1.8.3.1
> 
> No explicit alignment was ever specified for the struct rte_security_session itself. I wonder which implicit alignment applies to it.

it falls back to standard C. it will be over aligned to the strictest
alignment of any of the struct fields. so it will be aligned to
alignas(RTE_CACHE_LINE_MIN_SIZE).

> 
> Anyway, the changes are correct, so

thanks!

> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst
index 4f18cca..75d40d4 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -219,6 +219,9 @@  Removed Items
 * mbuf: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
   have been removed from ``struct rte_mbuf``.
 
+* security: ``RTE_MARKER`` fields ``cacheline0`` and ``cacheline1``
+  have been removed from ``struct rte_security_session``.
+
 API Changes
 -----------
 
diff --git a/lib/security/rte_security_driver.h b/lib/security/rte_security_driver.h
index 09829ab..18a1e3c 100644
--- a/lib/security/rte_security_driver.h
+++ b/lib/security/rte_security_driver.h
@@ -12,6 +12,8 @@ 
  * RTE Security Common Definitions
  */
 
+#include <stdalign.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -24,7 +26,6 @@ 
  * Security session to be used by library for internal usage
  */
 struct rte_security_session {
-	RTE_MARKER cacheline0;
 	uint64_t opaque_data;
 	/**< Opaque user defined data */
 	uint64_t fast_mdata;
@@ -32,7 +33,7 @@  struct rte_security_session {
 	rte_iova_t driver_priv_data_iova;
 	/**< session private data IOVA address */
 
-	alignas(RTE_CACHE_LINE_MIN_SIZE) RTE_MARKER cacheline1;
+	alignas(RTE_CACHE_LINE_MIN_SIZE)
 	uint8_t driver_priv_data[];
 	/**< Private session material, variable size (depends on driver) */
 };