From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 68FA645F83; Tue, 31 Dec 2024 19:38:27 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5119840273; Tue, 31 Dec 2024 19:38:25 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 35A634021E for ; Tue, 31 Dec 2024 19:38:23 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1213) id 34B7E2046765; Tue, 31 Dec 2024 10:38:22 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 34B7E2046765 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1735670302; bh=5JykVYS+z0ZJ3F45j8I+SOQXdDuZcBeiYVfSTMAE7Bg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=M2zbYI6IhVNDk5JaV8pgIF2pqualKE+F2x49agaSBVYRS9GaPBGjZQ0aQwJgepmfo pby96aO1MHg19A/DSM2mQ8AAyyT2avU9435bgRtDyY+bmQBlOgVR7cznOr2xR0tMjo W4drarQ8U9hwYMMHB+roLO8fhiWi3Zaf9dqxDmB0= From: Andre Muezerie To: roretzla@linux.microsoft.com Cc: aman.deep.singh@intel.com, anatoly.burakov@intel.com, bruce.richardson@intel.com, byron.marohn@intel.com, conor.walsh@intel.com, cristian.dumitrescu@intel.com, david.hunt@intel.com, dev@dpdk.org, dsosnowski@nvidia.com, gakhil@marvell.com, jerinj@marvell.com, jingjing.wu@intel.com, kirill.rybalchenko@intel.com, konstantin.v.ananyev@yandex.ru, matan@nvidia.com, mb@smartsharesystems.com, orika@nvidia.com, radu.nicolau@intel.com, ruifeng.wang@arm.com, sameh.gobriel@intel.com, sivaprasad.tummala@amd.com, skori@marvell.com, stephen@networkplumber.org, suanmingm@nvidia.com, vattunuru@marvell.com, viacheslavo@nvidia.com, vladimir.medvedkin@intel.com, yipeng1.wang@intel.com, Andre Muezerie Subject: [PATCH v8 00/29] fix packing of structs when building with MSVC Date: Tue, 31 Dec 2024 10:37:42 -0800 Message-Id: <1735670291-23224-1-git-send-email-andremue@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1710968771-16435-1-git-send-email-roretzla@linux.microsoft.com> References: <1710968771-16435-1-git-send-email-roretzla@linux.microsoft.com> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 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 (29): 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 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