From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org, Akhil Goyal <gakhil@marvell.com>,
Aman Singh <aman.deep.singh@intel.com>,
Anatoly Burakov <anatoly.burakov@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>,
Byron Marohn <byron.marohn@intel.com>,
Conor Walsh <conor.walsh@intel.com>,
Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
Dariusz Sosnowski <dsosnowski@nvidia.com>,
David Hunt <david.hunt@intel.com>,
Jerin Jacob <jerinj@marvell.com>,
Jingjing Wu <jingjing.wu@intel.com>,
Kirill Rybalchenko <kirill.rybalchenko@intel.com>,
Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
Matan Azrad <matan@nvidia.com>, Ori Kam <orika@nvidia.com>,
Radu Nicolau <radu.nicolau@intel.com>,
Ruifeng Wang <ruifeng.wang@arm.com>,
Sameh Gobriel <sameh.gobriel@intel.com>,
Sivaprasad Tummala <sivaprasad.tummala@amd.com>,
Suanming Mou <suanmingm@nvidia.com>,
Sunil Kumar Kori <skori@marvell.com>,
Vamsi Attunuru <vattunuru@marvell.com>,
Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
Vladimir Medvedkin <vladimir.medvedkin@intel.com>,
Yipeng Wang <yipeng1.wang@intel.com>,
Yuying Zhang <Yuying.Zhang@intel.com>
Subject: Re: [PATCH 00/15] fix packing of structs when building with MSVC
Date: Thu, 21 Mar 2024 08:46:42 -0700 [thread overview]
Message-ID: <20240321154642.GA11257@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> (raw)
In-Reply-To: <20240321083219.5504c54f@hermes.local>
On Thu, Mar 21, 2024 at 08:32:19AM -0700, Stephen Hemminger wrote:
> On Wed, 20 Mar 2024 14:05:56 -0700
> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
>
> > MSVC struct packing is not compatible with GCC provide a macro that can
> > be used to push existing pack value and sets packing to 1-byte. The
> > existing __rte_packed macro is then used to restore the pack value
> > prior to the push.
> >
> > Instead of providing macros exclusively for MSVC and for GCC the
> > existing macro is deliberately utilized to trigger a warning if no
> > existing packing has been pushed allowing easy identification of
> > locations where the __rte_msvc_pack is missing.
> >
> > I've decided to only add the macro to packed structs that are built
> > for Windows. It seems there is little value in adding the macro tree
> > wide and if new code arrives that is built on Windows the __rte_packed
> > will flag where the preamble macro is required.
> >
>
> Why is packed used so many places in DPDK?
> Packed should be reserved for things like network protocols
> with unaligned fields.
I made the same observation. Even network frame formats can be handled
without packing so long as you do some extra work to copy small units
of data out which is often worth doing manually and once only instead
of obfuscated behind codegen and direct field access.
> Many of these structure have no holes and are naturally packed.
> Adding packed attribute unnecessarily can generate poor code on
> architectures that do not support unaligned access.
For the specific case where their fields and alignments are naturally
packed I'm not entirely certain you get different codegen but I agree
it might and we'd like to avoid it if it does.
In a lot of the cases it seems to be drivers and often appears like it
is either structs that are used to hardware registers or firmware
protocol data units. Both of which may be need packing.
I will commit to removing packing in structs in this series iff
individual maintainers identify the specific structs in reply to the
series. If there is no request I'll leave it to them to figure out.
Maintainers should review and request removal as appropriate.
Thanks!
next prev parent reply other threads:[~2024-03-21 15:46 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-20 21:05 Tyler Retzlaff
2024-03-20 21:05 ` [PATCH 01/15] eal: provide pack start macro for MSVC Tyler Retzlaff
2024-03-20 21:05 ` [PATCH 02/15] eal: pack structures when building with MSVC Tyler Retzlaff
2024-03-21 16:02 ` Bruce Richardson
2024-03-20 21:05 ` [PATCH 03/15] net: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 04/15] common/iavf: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 05/15] common/idpf: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 06/15] common/mlx5: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 07/15] dma/ioat: " Tyler Retzlaff
2024-03-21 16:13 ` Bruce Richardson
2024-03-27 22:51 ` Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 08/15] net/i40e: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 09/15] net/iavf: " Tyler Retzlaff
2024-03-21 16:26 ` Bruce Richardson
2024-03-20 21:06 ` [PATCH 10/15] net/ice: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 11/15] net/mlx5: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 12/15] net/octeon_ep: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 13/15] app/testpmd: " Tyler Retzlaff
2024-03-21 16:28 ` Bruce Richardson
2024-03-20 21:06 ` [PATCH 14/15] app/test: " Tyler Retzlaff
2024-03-20 21:06 ` [PATCH 15/15] examples: " Tyler Retzlaff
2024-03-21 16:31 ` Bruce Richardson
2024-03-21 15:32 ` [PATCH 00/15] fix packing of structs " Stephen Hemminger
2024-03-21 15:46 ` Tyler Retzlaff [this message]
2024-03-27 23:09 ` [PATCH v2 " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 01/15] eal: provide pack start macro for MSVC Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 02/15] eal: pack structures when building with MSVC Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 03/15] net: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 04/15] common/iavf: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 05/15] common/idpf: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 06/15] common/mlx5: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 07/15] dma/ioat: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 08/15] net/i40e: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 09/15] net/iavf: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 10/15] net/ice: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 11/15] net/mlx5: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 12/15] net/octeon_ep: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 13/15] app/testpmd: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 14/15] app/test: " Tyler Retzlaff
2024-03-27 23:09 ` [PATCH v2 15/15] examples: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 00/16] fix packing of structs " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 01/16] eal: provide pack start macro for MSVC Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 02/16] eal: pack structures when building with MSVC Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 03/16] net: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 04/16] common/iavf: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 05/16] common/idpf: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 06/16] common/mlx5: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 07/16] dma/ioat: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 08/16] net/i40e: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 09/16] net/iavf: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 10/16] net/ice: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 11/16] net/mlx5: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 12/16] net/octeon_ep: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 13/16] app/testpmd: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 14/16] app/test: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 15/16] examples: " Tyler Retzlaff
2024-04-15 23:51 ` [PATCH v3 16/16] crypto/mlx5: " Tyler Retzlaff
2024-04-16 0:04 ` [PATCH v4 00/16] fix packing of structs " Tyler Retzlaff
2024-04-16 0:04 ` [PATCH v4 01/16] eal: provide pack start macro for MSVC Tyler Retzlaff
2024-04-16 0:04 ` [PATCH v4 02/16] eal: pack structures when building with MSVC Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 03/16] net: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 04/16] common/iavf: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 05/16] common/idpf: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 06/16] common/mlx5: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 07/16] dma/ioat: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 08/16] net/i40e: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 09/16] net/iavf: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 10/16] net/ice: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 11/16] net/mlx5: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 12/16] net/octeon_ep: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 13/16] app/testpmd: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 14/16] app/test: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 15/16] examples: " Tyler Retzlaff
2024-04-16 0:05 ` [PATCH v4 16/16] crypto/mlx5: " Tyler Retzlaff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240321154642.GA11257@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net \
--to=roretzla@linux.microsoft.com \
--cc=Yuying.Zhang@intel.com \
--cc=aman.deep.singh@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=bruce.richardson@intel.com \
--cc=byron.marohn@intel.com \
--cc=conor.walsh@intel.com \
--cc=cristian.dumitrescu@intel.com \
--cc=david.hunt@intel.com \
--cc=dev@dpdk.org \
--cc=dsosnowski@nvidia.com \
--cc=gakhil@marvell.com \
--cc=jerinj@marvell.com \
--cc=jingjing.wu@intel.com \
--cc=kirill.rybalchenko@intel.com \
--cc=konstantin.v.ananyev@yandex.ru \
--cc=matan@nvidia.com \
--cc=orika@nvidia.com \
--cc=radu.nicolau@intel.com \
--cc=ruifeng.wang@arm.com \
--cc=sameh.gobriel@intel.com \
--cc=sivaprasad.tummala@amd.com \
--cc=skori@marvell.com \
--cc=stephen@networkplumber.org \
--cc=suanmingm@nvidia.com \
--cc=vattunuru@marvell.com \
--cc=viacheslavo@nvidia.com \
--cc=vladimir.medvedkin@intel.com \
--cc=yipeng1.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).