Re: [PATCH] rcu: remove VLAs

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

On Fri, May 16, 2025 at 3:09 PM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> On Fri, May 16, 2025 at 11:22:05AM +0200, David Marchand wrote:
> > Andre,
> >
> > On Fri, Mar 7, 2025 at 2:40 AM Andre Muezerie
> > <andremue@linux.microsoft.com> wrote:
> > >
> > > There are two lines that were using VLAs, which are not supported by
> > > MSVC.
> > >
> > > 1)
> > > ../lib/rcu/rte_rcu_qsbr.c:326:12: warning: variable length array used [-Wvla]
> > >   326 |         char data[dq->esize];
> > >       |                   ^~~~~~~~~
> > > 2)
> > > ../lib/rcu/rte_rcu_qsbr.c:389:12: warning: variable length array used [-Wvla]
> > >   389 |         char data[dq->esize];
> > >       |                   ^~~~~~~~~
> > >
> > > The short-term fix is to use alloca, to allow progress with the msvc
> > > compatibility work.
> > > The long-term plan involves API changes and therefore can only be applied
> > > with a new release. This long-term plan consists of introducing some
> > > reasonable limitation on RCU DQ element size.
> > >
> > > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> >
> > Afaiu, enabling RCU with MSVC depends on this current patch and a fix
> > in test/ipfrag:
> > https://patchwork.dpdk.org/project/dpdk/patch/1741313581-14300-1-git-send-email-andremue@linux.microsoft.com/.
> > Is there anything else missing?
> >
> > If not, then I would be for doing this enabling as part of this
> > current patch (I can do this when applying).
> > WDYT?
> >
>
> You're correct. When enabling RCU lib, test_ipfrag is build and fails if the mentioned
> patch is not applied yet. If you apply that patch first, it would be great if you
> could enable RCU on Windows as part of this patch. Nothing else should be missing.
>
> I had not enabled it before to not break the CI build.

Ok, I did my checks and it looks fine, so I squashed the mention
change as part of this patch.

I see patches on lpm and fib library for which I have similar questions.
I must stop merging for today but could you double check those
libraries can be enabled too?

This way it will be easier for me next week.


Thanks.

-- 
David Marchand