From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
To: Andre Muezerie <andremue@linux.microsoft.com>,
Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [PATCH 1/2] lib/ipsec: compile ipsec on Windows
Date: Thu, 9 Jan 2025 17:27:11 +0000 [thread overview]
Message-ID: <506a31ede96645a299b31445f8574fa2@huawei.com> (raw)
In-Reply-To: <1736181948-8907-2-git-send-email-andremue@linux.microsoft.com>
> 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.
> + 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
next prev parent reply other threads:[~2025-01-09 17:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-06 16:45 [PATCH 0/2] " Andre Muezerie
2025-01-06 16:45 ` [PATCH 1/2] lib/ipsec: " Andre Muezerie
2025-01-09 15:31 ` Konstantin Ananyev
2025-01-09 17:27 ` Konstantin Ananyev [this message]
2025-01-06 16:45 ` [PATCH 2/2] app/test: enable ipsec-related tests Andre Muezerie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=506a31ede96645a299b31445f8574fa2@huawei.com \
--to=konstantin.ananyev@huawei.com \
--cc=andremue@linux.microsoft.com \
--cc=dev@dpdk.org \
--cc=konstantin.v.ananyev@yandex.ru \
--cc=vladimir.medvedkin@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).