Re: [PATCH] net: fix IPv4 cksum simple function

[David Marchand <david.marchand@redhat.com>]

On Tue, Nov 5, 2024 at 10:09 AM Morten Brørup <mb@smartsharesystems.com> wrote:
> > diff --git a/lib/net/rte_ip4.h b/lib/net/rte_ip4.h
> > index 4dd0058cc5..f9b8333332 100644
> > --- a/lib/net/rte_ip4.h
> > +++ b/lib/net/rte_ip4.h
> > @@ -39,7 +39,7 @@ extern "C" {
> >  /**
> >   * IPv4 Header
> >   */
> > -struct rte_ipv4_hdr {
> > +struct __rte_aligned(2) rte_ipv4_hdr {
>
> <unrelated>
> I wonder why we have a convention of putting __rte_packed after the struct, and not between the "struct" tag and the name of the struct.
> It would make the code much more readable having it here, like __rte_aligned().
> </unrelated>

I agree that the previous convention was not great, as it has resulted
in some funny jokes, like getting some __rte_XXX variables (in the
absence of the right header inclusion defining __rte_XXX attribute
macro).

__rte_aligned() "conventional" location has been changed recently by Tyler.
__rte_packed is still conventionnally placed in a "legacy" position
around the dpdk tree.
It could probably be moved in the same way.

But there is still the question of packed structures with MSVC.
Tyler proposal seemed to rely on the current __rte_packed conventional position.
https://patchwork.dpdk.org/project/dpdk/patch/1713225913-20792-2-git-send-email-roretzla@linux.microsoft.com/
Note that I am not a fan of this push/pop stuff.

Maybe Andre will find a better solution.

In any case, I prefer we keep __rte_packed position as is until this
question is resolved.


-- 
David Marchand