On Thu, Jan 09, 2025 at 05:27:11PM +0000, Konstantin Ananyev wrote:
>
>
> > Removed VLA for compatibility with MSVC (which does not support VLAs).
> > Used alloca when a constant fixed length that can be used instead is
> > not known.
> >
> ...
>
> > Signed-off-by: Andre Muezerie <andre...@linux.microsoft.com>
> > ---
>
> ...
>
> > diff --git a/lib/ipsec/esp_inb.c b/lib/ipsec/esp_inb.c
> > index f159bf7460..305ac48dc5 100644
> > --- a/lib/ipsec/esp_inb.c
> > +++ b/lib/ipsec/esp_inb.c
> > @@ -370,8 +370,9 @@ esp_inb_pkt_prepare(const struct rte_ipsec_session *ss,
> > struct rte_mbuf *mb[],
> > struct rte_cryptodev_sym_session *cs;
> > struct replay_sqn *rsn;
> > union sym_op_data icv;
> > - uint32_t dr[num];
> > + uint32_t *dr;
>
> I understand the intention, but obviously as the lib maintainer, I am no
> very happy with mechanic replacement of VLAs with alloca() calls.
> Again, while it helps to make MSVC happy, it doesn't really solve a potential
> problem
> with using VLAs here - I we can end-up with stack overflowed if someone will
> call
> one of these functions with input arrays of huge size.
> It would be much better to re-work the internal functions to work over
> fixed-size array
> an then call them in a loop.
> The main issue here is that by API convention we need to re-arrange input
> array by
> moving bad (not processed) mbuf after the good ones.
> But even with that splitting processing into multiple iterations over small
> chunks
> might help I think.
Understood. I'll focus on the other patches being reviewed and revisit this one
at some other time.
>
> > + dr = alloca(sizeof(uint32_t) * num);
> > sa = ss->sa;
> > cs = ss->crypto.ses;
> > rsn = rsn_acquire(sa);
> > @@ -576,12 +577,16 @@ tun_process(struct rte_ipsec_sa *sa, struct rte_mbuf
> > *mb[],
> > uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t sqh_len)
> > {
> > uint32_t adj, i, k, tl, bytes;
> > - uint32_t hl[num], to[num];
> > - struct rte_esp_tail espt[num];
> > - struct rte_mbuf *ml[num];
> > + uint32_t *hl, *to;
> > + struct rte_esp_tail *espt;
> > + struct rte_mbuf **ml;
> > const void *outh;
> > void *inh;
> >
> > + hl = alloca(sizeof(uint32_t) * num);
> > + to = alloca(sizeof(uint32_t) * num);
> > + espt = alloca(sizeof(struct rte_esp_tail) * num);
> > + ml = alloca(sizeof(struct rte_mbuf *) * num);
> > /*
> > * remove icv, esp trailer and high-order
> > * 32 bits of esn from packet length
