[PATCH v10 00/30] fix packing of structs when building with MSVC
Andre Muezerie
andremue at linux.microsoft.com
Thu Jan 9 03:45:44 CET 2025
MSVC struct packing is not compatible with GCC. Different alternatives
were considered:
1) Have a macro __RTE_PACKED(decl) to which the struct/union is passed
and the macro would define the struct/union with the appropriate
packing attribute for the compiler in use.
Advantages:
* Can be placed in front of a struct, or even in the middle. Good
for readability.
* Does not require a different macro to be placed at the end of the
structure.
However, problems can arise when compiler directives are present in the
struct, as they become arguments for __RTE_PACKED macro. This is not
portable. Two problematic situations observed in the DPDK code:
a) #defines mentioned in the struct. In this situation we could just
move the #define out of the struct.
b) #if/#ifdef/#elif mentioned in the struct.
This is a somewhat common pattern in structs where fields change
based on endianness and would require code duplication to be
handled, which makes the code hard to read and maintain.
2) Have macros __rte_msvc_pack_begin and __rte_msvc_pack_end which
would be placed at the beginning and end of the struct/union
respectively. Concerns were raised about having macros for
specific compilers, or even having compiler names mentioned in
the macros' names.
3) Instead of providing macros exclusively for MSVC and for GCC,
have a macro __rte_packed_begin and __rte_packed_end which would
be placed at the beginning and end of the struct/union respectively.
With MSVC both macros end up having a purpose. With GCC and Clang
only __rte_packed_end has a purpose, as can be seen below.
This makes the solution generic and is the approach taken in this
patchset.
#ifdef RTE_TOOLCHAIN_MSVC
#define __rte_packed_begin __pragma(pack(push, 1))
#define __rte_packed_end __pragma(pack(pop))
#else
#define __rte_packed_begin
#define __rte_packed_end __attribute__((__packed__))
#endif
Macro __rte_packed_end is deliberately utilized to trigger a
MSVC compiler warning if no existing packing has been pushed allowing
easy identification of locations where the __rte_packed_begin is
missing.
Macro __rte_packed is marked deprecated and the two new macros represent
the new way to enable packing in the DPDK code.
Script checkpatches.sh was enhanced to ensure that:
* __rte_packed_begin and __rte_packed_end show up in pairs.
* __rte_packed_begin is not used with enums.
* __rte_packed_begin is only used after struct, union,
__rte_cache_aligned, __rte_cache_min_aligned or __rte_aligned
v10:
* added a note to commit message on patch 27/30 about storage size
change for variables of type enum rte_ipv6_mc_scope.
v9:
* moved the deprecation of __rte_packed to the end of the patchset
v8:
* moved __rte_packed_begin after the struct and union keywords
* added more packing related tests to checkpatches.sh
v7:
* added __rte_packed back but marked it deprecated
v6:
* replace __rte_msvc_pack with __rte_packed_begin
* replace __rte_packed with __rte_packed_end
* update checkpatches.sh to ensure __rte_packed_begin and
__rte_packed_end are used in pairs
* remove __rte_packed
v5:
* rebase on top of latest main
v4:
* add another missing __rte_msvc_pack to crypto/mlx5 patch
* correct commit message for duplicated packing in
crypto/mlx5 patch
v3:
* add missing __rte_msvc_pack to crypto/mlx5
* fix commit messages to reference __rte_msvc_pack macro instead
of __rte_msvc_pushpack(1)
v2:
* app/testpmd, remove packing from simple_gre_hdr
* net/iavf, remove packing from iavf_ipsec_crypto_pkt_metadata,
simple_gre_hdr
* examples, remove packing from pkt_key_qinq, pkt_key_ipv4_5tuple,
pkt_key_ipv6_5tuple, pkt_key_ipv4_addr, pkt_key_ipv6_addr
* eal, remove packing from rte_config, __rte_trace_stream_header
Andre Muezerie (30):
devtools: check packed attributes
eal/include: add new packing macros
app/test-pmd: remove unnecessary packed attributes
app/test: replace packed attributes
doc/guides: replace packed attributes
drivers/baseband: replace packed attributes
drivers/bus: replace packed attributes
drivers/common: replace packed attributes
drivers/compress: replace packed attributes
drivers/crypto: replace packed attributes
drivers/dma: replace packed attributes
drivers/event: replace packed attributes
drivers/mempool: replace packed attributes
drivers/net: replace packed attributes
drivers/raw: replace packed attributes
drivers/regex: replace packed attributes
drivers/vdpa: replace packed attributes
examples/common: replace packed attributes
examples/ip-pipeline: remove packed attributes
examples/ipsec_secgw: replace packed attributes
examples/l3fwd-power: replace packed attributes
examples/l3fwd: replace packed attributes
examples/ptpclient: replace packed attributes
examples/vhost_blk: replace packed attributes
lib/eal: replace packed attributes
lib/ipsec: replace packed attributes
lib/net: replace packed attributes
lib/pipeline: replace packed attributes
lib/vhost: replace packed attributes
eal/include: deprecate macro __rte_packed
app/test-pmd/csumonly.c | 2 +-
app/test/test_efd.c | 4 +-
app/test/test_hash.c | 4 +-
app/test/test_member.c | 4 +-
devtools/checkpatches.sh | 43 +
doc/guides/nics/ark.rst | 4 +-
.../prog_guide/packet_classif_access_ctrl.rst | 4 +-
drivers/baseband/acc/acc_common.h | 52 +-
drivers/baseband/fpga_5gnr_fec/agx100_pmd.h | 16 +-
.../baseband/fpga_5gnr_fec/fpga_5gnr_fec.h | 4 +-
drivers/baseband/fpga_5gnr_fec/vc_5gnr_pmd.h | 8 +-
drivers/baseband/fpga_lte_fec/fpga_lte_fec.c | 12 +-
drivers/baseband/la12xx/bbdev_la12xx_ipc.h | 32 +-
drivers/bus/dpaa/include/fsl_bman.h | 20 +-
drivers/bus/dpaa/include/fsl_fman.h | 4 +-
drivers/bus/dpaa/include/fsl_qman.h | 160 +-
drivers/bus/ifpga/bus_ifpga_driver.h | 8 +-
drivers/bus/vmbus/rte_vmbus_reg.h | 108 +-
drivers/common/cnxk/hw/sdp.h | 4 +-
drivers/common/cnxk/roc_npc.h | 16 +-
drivers/common/cnxk/roc_npc_mcam_dump.c | 4 +-
drivers/common/cnxk/roc_platform.h | 3 +-
drivers/common/dpaax/compat.h | 3 -
drivers/common/iavf/iavf_osdep.h | 8 +-
drivers/common/iavf/virtchnl_inline_ipsec.h | 44 +-
drivers/common/idpf/base/idpf_osdep.h | 8 +-
drivers/common/mlx5/mlx5_common_mr.h | 16 +-
drivers/common/mlx5/mlx5_common_utils.h | 4 +-
drivers/common/mlx5/mlx5_prm.h | 120 +-
drivers/common/qat/qat_adf/icp_qat_fw_la.h | 8 +-
drivers/common/qat/qat_common.h | 8 +-
drivers/compress/qat/qat_comp.h | 4 +-
drivers/crypto/caam_jr/caam_jr.c | 4 +-
drivers/crypto/caam_jr/caam_jr_desc.h | 64 +-
drivers/crypto/caam_jr/caam_jr_hw_specific.h | 48 +-
drivers/crypto/dpaa_sec/dpaa_sec.h | 12 +-
drivers/crypto/ionic/ionic_crypto_if.h | 36 +-
drivers/crypto/mlx5/mlx5_crypto.h | 8 +-
drivers/crypto/mlx5/mlx5_crypto_gcm.c | 4 +-
drivers/crypto/qat/qat_sym.h | 8 +-
drivers/crypto/qat/qat_sym_session.h | 4 +-
drivers/dma/dpaa/dpaa_qdma.h | 20 +-
drivers/dma/dpaa2/dpaa2_qdma.h | 16 +-
drivers/dma/ioat/ioat_hw_defs.h | 4 +-
drivers/event/octeontx/timvf_evdev.c | 4 +-
drivers/event/octeontx/timvf_evdev.h | 12 +-
drivers/mempool/octeontx/octeontx_fpavf.c | 16 +-
drivers/net/ark/ark_ddm.h | 4 +-
drivers/net/ark/ark_pktchkr.h | 8 +-
drivers/net/ark/ark_pktdir.h | 5 +-
drivers/net/ark/ark_pktgen.h | 4 +-
drivers/net/ark/ark_udm.h | 4 +-
drivers/net/atlantic/hw_atl/hw_atl_utils.h | 120 +-
.../net/atlantic/hw_atl/hw_atl_utils_fw2x.c | 8 +-
drivers/net/avp/rte_avp_common.h | 12 +-
drivers/net/bnxt/bnxt.h | 8 +-
drivers/net/bnxt/hsi_struct_def_dpdk.h | 3344 ++++++++---------
drivers/net/bnxt/tf_core/tf_resources.h | 32 +-
drivers/net/bnxt/tf_core/v3/tfc_mpc_table.c | 20 +-
drivers/net/bonding/rte_eth_bond_8023ad.h | 32 +-
drivers/net/cnxk/cn10k_rxtx.h | 4 +-
drivers/net/cnxk/cn20k_rxtx.h | 4 +-
drivers/net/cnxk/cn9k_ethdev.h | 4 +-
drivers/net/cnxk/cnxk_rep_msg.h | 64 +-
drivers/net/dpaa/dpaa_rxtx.h | 28 +-
drivers/net/dpaa/fmlib/fm_ext.h | 4 +-
drivers/net/dpaa2/base/dpaa2_hw_dpni_annot.h | 4 +-
drivers/net/dpaa2/dpaa2_recycle.c | 16 +-
drivers/net/enic/base/vnic_devcmd.h | 40 +-
drivers/net/enic/base/vnic_flowman.h | 120 +-
drivers/net/gve/base/gve_desc.h | 16 +-
drivers/net/gve/base/gve_desc_dqo.h | 32 +-
drivers/net/gve/base/gve_osdep.h | 3 -
drivers/net/hns3/hns3_mbx.h | 8 +-
drivers/net/hns3/hns3_rxtx.h | 4 +-
drivers/net/i40e/base/i40e_osdep.h | 8 +-
drivers/net/iavf/iavf_ipsec_crypto.h | 10 +-
drivers/net/iavf/iavf_rxtx.c | 2 +-
drivers/net/ice/base/ice_osdep.h | 11 +-
drivers/net/ionic/ionic_if.h | 72 +-
drivers/net/memif/memif.h | 36 +-
drivers/net/mlx4/mlx4_mr.h | 12 +-
drivers/net/mlx5/hws/mlx5dr.h | 4 +-
drivers/net/mlx5/mlx5.h | 4 +-
drivers/net/mlx5/mlx5_flow.h | 16 +-
drivers/net/mlx5/mlx5_hws_cnt.h | 4 +-
drivers/net/mlx5/mlx5_utils.h | 16 +-
drivers/net/netvsc/hn_nvs.h | 72 +-
drivers/net/netvsc/ndis.h | 8 +-
drivers/net/nfp/flower/nfp_flower_cmsg.h | 4 +-
drivers/net/nfp/flower/nfp_flower_flow.h | 4 +-
drivers/net/nfp/nfd3/nfp_nfd3.h | 4 +-
drivers/net/nfp/nfp_rxtx.h | 8 +-
drivers/net/nfp/nfpcore/nfp_nsp.c | 4 +-
drivers/net/ntnic/dbsconfig/ntnic_dbsconfig.c | 12 +-
drivers/net/octeon_ep/otx_ep_mbox.h | 4 +-
drivers/net/octeontx/base/octeontx_pki_var.h | 4 +-
drivers/net/pfe/pfe_hif.h | 4 +-
drivers/net/virtio/virtio.h | 4 +-
drivers/net/virtio/virtio_cvq.h | 8 +-
drivers/net/virtio/virtio_user/vhost_user.c | 4 +-
drivers/net/zxdh/zxdh_common.c | 8 +-
drivers/net/zxdh/zxdh_msg.h | 16 +-
drivers/net/zxdh/zxdh_pci.h | 4 +-
drivers/net/zxdh/zxdh_queue.h | 64 +-
drivers/net/zxdh/zxdh_rxtx.h | 8 +-
drivers/raw/ifpga/afu_pmd_n3000.h | 8 +-
drivers/raw/ifpga/base/opae_hw_api.h | 4 +-
drivers/regex/cn9k/cn9k_regexdev.c | 4 +-
drivers/regex/mlx5/mlx5_rxp.h | 16 +-
drivers/vdpa/ifc/base/ifcvf.h | 4 +-
drivers/vdpa/mlx5/mlx5_vdpa.h | 4 +-
examples/common/neon/port_group.h | 4 +-
examples/ip_pipeline/cli.c | 10 +-
examples/ipsec-secgw/ipsec.h | 4 +-
examples/l3fwd-power/main.c | 8 +-
examples/l3fwd/l3fwd_route.h | 8 +-
examples/ptpclient/ptpclient.c | 32 +-
examples/vhost_blk/blk_spec.h | 4 +-
lib/eal/common/eal_private.h | 2 +-
lib/eal/include/rte_common.h | 23 +-
lib/eal/include/rte_memory.h | 4 +-
lib/eal/include/rte_memzone.h | 4 +-
lib/eal/include/rte_trace_point.h | 2 +-
lib/eal/x86/include/rte_memcpy.h | 12 +-
lib/ipsec/crypto.h | 44 +-
lib/net/rte_arp.h | 8 +-
lib/net/rte_dtls.h | 4 +-
lib/net/rte_esp.h | 8 +-
lib/net/rte_geneve.h | 4 +-
lib/net/rte_gre.h | 16 +-
lib/net/rte_gtp.h | 20 +-
lib/net/rte_ib.h | 4 +-
lib/net/rte_icmp.h | 12 +-
lib/net/rte_ip4.h | 4 +-
lib/net/rte_ip6.h | 14 +-
lib/net/rte_l2tpv2.h | 16 +-
lib/net/rte_macsec.h | 8 +-
lib/net/rte_mpls.h | 4 +-
lib/net/rte_pdcp_hdr.h | 16 +-
lib/net/rte_ppp.h | 4 +-
lib/net/rte_sctp.h | 4 +-
lib/net/rte_tcp.h | 4 +-
lib/net/rte_tls.h | 4 +-
lib/net/rte_udp.h | 4 +-
lib/net/rte_vxlan.h | 28 +-
lib/pipeline/rte_table_action.c | 64 +-
lib/vhost/vhost_user.h | 8 +-
148 files changed, 2952 insertions(+), 2897 deletions(-)
--
2.47.0.vfs.0.3
More information about the dev
mailing list