[09/15] net/iavf: pack structures when building with MSVC

Message ID 1710968771-16435-10-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series fix packing of structs when building with MSVC |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Tyler Retzlaff March 20, 2024, 9:06 p.m. UTC
  Add __rte_msvc_pushpack(1) to all __rte_packed structs to cause packing
when building with MSVC.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 drivers/net/iavf/iavf_ipsec_crypto.h | 3 +++
 drivers/net/iavf/iavf_rxtx.c         | 1 +
 2 files changed, 4 insertions(+)
  

Comments

Bruce Richardson March 21, 2024, 4:26 p.m. UTC | #1
On Wed, Mar 20, 2024 at 02:06:05PM -0700, Tyler Retzlaff wrote:
> Add __rte_msvc_pushpack(1) to all __rte_packed structs to cause packing
> when building with MSVC.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  drivers/net/iavf/iavf_ipsec_crypto.h | 3 +++
>  drivers/net/iavf/iavf_rxtx.c         | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/iavf/iavf_ipsec_crypto.h b/drivers/net/iavf/iavf_ipsec_crypto.h
> index 49f9202..36ced50 100644
> --- a/drivers/net/iavf/iavf_ipsec_crypto.h
> +++ b/drivers/net/iavf/iavf_ipsec_crypto.h
> @@ -11,12 +11,14 @@
>  
>  
>  
> +__rte_msvc_pack
>  struct iavf_tx_ipsec_desc {
>  	union {
>  		struct {
>  			__le64 qw0;
>  			__le64 qw1;
>  		};
> +		__rte_msvc_pack
>  		struct {
>  			__le16 l4payload_length;
>  			__le32 esn;

Looking at the structure, I believe only the inner packed attribute is
necessary. However, given that we need packed attributes, for safety, let's
keep them both.

> @@ -84,6 +86,7 @@ enum iavf_ipsec_iv_len {
>   * transmit data path. Parameters set for session by calling
>   * rte_security_set_pkt_metadata() API.
>   */
> +__rte_msvc_pack
>  struct iavf_ipsec_crypto_pkt_metadata {
>  	uint32_t sa_idx;                /* SA hardware index (20b/4B) */
>  

This is exactly 16-bytes in size, with all members explicitly sized and
properly aligned. Therefore we can drop the packed atttribute.

> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
> index 0a5246d..ba0fa6f 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -50,6 +50,7 @@
>  static uint16_t vxlan_gpe_udp_port = RTE_VXLAN_GPE_DEFAULT_PORT;
>  static uint16_t geneve_udp_port = RTE_GENEVE_DEFAULT_PORT;
>  
> +__rte_msvc_pack
>  struct simple_gre_hdr {
>  	uint16_t flags;
>  	uint16_t proto;

Two 16-bit fields only, so no need to pack this struct either.

/Bruce
  

Patch

diff --git a/drivers/net/iavf/iavf_ipsec_crypto.h b/drivers/net/iavf/iavf_ipsec_crypto.h
index 49f9202..36ced50 100644
--- a/drivers/net/iavf/iavf_ipsec_crypto.h
+++ b/drivers/net/iavf/iavf_ipsec_crypto.h
@@ -11,12 +11,14 @@ 
 
 
 
+__rte_msvc_pack
 struct iavf_tx_ipsec_desc {
 	union {
 		struct {
 			__le64 qw0;
 			__le64 qw1;
 		};
+		__rte_msvc_pack
 		struct {
 			__le16 l4payload_length;
 			__le32 esn;
@@ -84,6 +86,7 @@  enum iavf_ipsec_iv_len {
  * transmit data path. Parameters set for session by calling
  * rte_security_set_pkt_metadata() API.
  */
+__rte_msvc_pack
 struct iavf_ipsec_crypto_pkt_metadata {
 	uint32_t sa_idx;                /* SA hardware index (20b/4B) */
 
diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 0a5246d..ba0fa6f 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -50,6 +50,7 @@ 
 static uint16_t vxlan_gpe_udp_port = RTE_VXLAN_GPE_DEFAULT_PORT;
 static uint16_t geneve_udp_port = RTE_GENEVE_DEFAULT_PORT;
 
+__rte_msvc_pack
 struct simple_gre_hdr {
 	uint16_t flags;
 	uint16_t proto;