Re: [PATCH 1/2] lib/ipsec: compile ipsec on Windows

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

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 <andremue@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