Re: [PATCH v4] eventdev: ensure 16-byte alignment for events

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Mon, Nov 13, 2023 at 08:58:19AM +0100, Morten Brørup wrote:
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Monday, 13 November 2023 00.32
> > 
> > On Sun, 12 Nov 2023 09:30:24 +0100
> > Morten Brørup <mb@smartsharesystems.com> wrote:
> > 
> > > > > +static_assert(sizeof(struct rte_event) == 16, "Event structure
> > size
> > > > is not 16-bytes in size");
> > > > > +
> > > > >  static struct rte_eventdev
> > rte_event_devices[RTE_EVENT_MAX_DEVS];
> > > >
> > > > Please don't reinvent RTE_BUILD_BUG_ON().
> > > > Instead fix that to be a static_assert()
> > >
> > > I would say the opposite:
> > > With our upgrade to the C11 standard, let's get rid of the
> > RTE_BUILD_BUG_ON() workaround for the lack of static_assert() in older
> > C standards.
> > >
> > > Unfortunately, the static_assert(expression) variant without the
> > "message" parameter, which would make our RTE_BUILD_BUG_ON() macro
> > completely obsolete, requires C23. And I don't see how we can make this
> > variant available with C11. So we probably have to wait until DPDK
> > requires C23.
> > >
> > > Until then, let's gradually phase out the DPDK-specific
> > RTE_BUILD_BUG_ON() in favor of standard C's static_assert(), and live
> > with the inconvenience of having to provide a message parameter for it.
> > >
> > > Please also note that static_assert() can be used outside code
> > blocks, which makes it handy for use in header files.
> > 
> > If you look at my RFC, the message is just as good as the one in this
> > code.
> > It ends up being stringified version of the expression. Which is more
> > exact than the wording used in some other places.
> 
> I agree that the output of your RFC is better than that of static_assert(expr, msg).
> I'm arguing that RTE_BUILD_BUG_ON() would be considered reinventing the wheel if introduced today, because the C11 standard already offers something similar. So we should prefer that over RTE_BUILD_BUG_ON(), not the other way around.
> 
> It's a difference in opinion. Both RTE_BUILD_BUG_ON() and static_assert() are viable; you seem to prefer the first, I prefer the latter.
> 
> Both occur in existing DPDK code, and Bruce happened to chose static_assert() for this patch.
> 
> The overall question is a choice between three options:
> 1. prefer using RTE_BUILD_BUG_ON() (using your RFC implementation),
> 2. phase out RTE_BUILD_BUG_ON() in favor of the C11 standard's static_assert(), or
> 3. continue using both?
> 
> I'm not strongly opposed to either of the three, as long as the community agrees.
> 

not a strong preference, but i see no reason to maintain macros when the
standard offers nearly equivalent.

* keep RTE_BUILD_BUG_ON
* discourage new additions using RTE_BUILD_BUG_ON with checkpatches
* convert to static_assert gradually over time

glad to see the conversion picked up real problems though, thank you
stephen for doing this.