Re: [PATCH] rcu: remove VLAs

[Andre Muezerie <andremue@linux.microsoft.com>]

On Fri, May 16, 2025 at 03:30:23PM +0200, David Marchand wrote:
> 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.
> 

I updated both patches to also enable the build when using MSVC since there's
nothing blocking that anymore.

> 
> Thanks.
> 
> -- 
> David Marchand