app/test: fix stack overflow in fib6_perf_autotest
Checks
Commit Message
Test fib6_perf_autotest was hitting a stack overflow on Windows
with MSVC.
The fix is to move some of the data from the stack to the heap.
Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
app/test/test_fib6_perf.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
Comments
On Mon, 23 Dec 2024 13:10:33 -0800
Andre Muezerie <andremue@linux.microsoft.com> wrote:
> From: Andre Muezerie <andremue@linux.microsoft.com>
> To: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> Cc: dev@dpdk.org, Andre Muezerie <andremue@linux.microsoft.com>
> Subject: [PATCH] app/test: fix stack overflow in fib6_perf_autotest
> Date: Mon, 23 Dec 2024 13:10:33 -0800
> X-Mailer: git-send-email 1.8.3.1
>
> Test fib6_perf_autotest was hitting a stack overflow on Windows
> with MSVC.
>
> The fix is to move some of the data from the stack to the heap.
>
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
Use regular malloc please.
rte_malloc comes from hugepages which are more limited and slower to manipulate.
On Mon, Dec 23, 2024 at 01:30:00PM -0800, Stephen Hemminger wrote:
> On Mon, 23 Dec 2024 13:10:33 -0800
> Andre Muezerie <andremue@linux.microsoft.com> wrote:
>
> > From: Andre Muezerie <andremue@linux.microsoft.com>
> > To: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > Cc: dev@dpdk.org, Andre Muezerie <andremue@linux.microsoft.com>
> > Subject: [PATCH] app/test: fix stack overflow in fib6_perf_autotest
> > Date: Mon, 23 Dec 2024 13:10:33 -0800
> > X-Mailer: git-send-email 1.8.3.1
> >
> > Test fib6_perf_autotest was hitting a stack overflow on Windows
> > with MSVC.
> >
> > The fix is to move some of the data from the stack to the heap.
> >
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
>
> Use regular malloc please.
> rte_malloc comes from hugepages which are more limited and slower to manipulate.
I recently submitted a patch for a test with a very similar issue and
during review one of the reviewers encouraged me to use rte_calloc to
allocate memory for the arrays, which I think makes sense (I had used
malloc initially):
https://inbox.dpdk.org/dev/20241218151206.GA25758@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net/
Even though this is a perf test, the code responsible for the memory
allocations is not in the path for which time measurements are being
taken (points between rte_rdtsc calls), so perf for the memory
allocation code is probably not so critical.
That being said, if you still feel strongly that malloc should be used
instead let me know and I can make that change.
Thanks,
Andre Muezerie
Hi,
I think using rte_malloc(rte_calloc) here would be a better choice because:
- As Andre mentioned allocation happens outside of the performance
measurement section
- Due to relatively large size of the memory allocated for
ip_batch/next_hops some CPUs may experience TLB cache pressure, which
will affect performance measurement results.
With that being said
Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
On 24/12/2024 01:47, Andre Muezerie wrote:
> On Mon, Dec 23, 2024 at 01:30:00PM -0800, Stephen Hemminger wrote:
>> On Mon, 23 Dec 2024 13:10:33 -0800
>> Andre Muezerie <andremue@linux.microsoft.com> wrote:
>>
>>> From: Andre Muezerie <andremue@linux.microsoft.com>
>>> To: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
>>> Cc: dev@dpdk.org, Andre Muezerie <andremue@linux.microsoft.com>
>>> Subject: [PATCH] app/test: fix stack overflow in fib6_perf_autotest
>>> Date: Mon, 23 Dec 2024 13:10:33 -0800
>>> X-Mailer: git-send-email 1.8.3.1
>>>
>>> Test fib6_perf_autotest was hitting a stack overflow on Windows
>>> with MSVC.
>>>
>>> The fix is to move some of the data from the stack to the heap.
>>>
>>> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
>> Use regular malloc please.
>> rte_malloc comes from hugepages which are more limited and slower to manipulate.
> I recently submitted a patch for a test with a very similar issue and
> during review one of the reviewers encouraged me to use rte_calloc to
> allocate memory for the arrays, which I think makes sense (I had used
> malloc initially):
>
> https://inbox.dpdk.org/dev/20241218151206.GA25758@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net/
>
> Even though this is a perf test, the code responsible for the memory
> allocations is not in the path for which time measurements are being
> taken (points between rte_rdtsc calls), so perf for the memory
> allocation code is probably not so critical.
>
> That being said, if you still feel strongly that malloc should be used
> instead let me know and I can make that change.
>
> Thanks,
>
> Andre Muezerie
@@ -9,6 +9,7 @@
#include <rte_cycles.h>
#include <rte_random.h>
+#include <rte_malloc.h>
#include <rte_memory.h>
#include <rte_fib6.h>
@@ -73,8 +74,14 @@ test_fib6_perf(void)
uint64_t next_hop_add;
int status = 0;
int64_t count = 0;
- struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
- uint64_t next_hops[NUM_IPS_ENTRIES];
+
+ struct rte_ipv6_addr *ip_batch = rte_calloc("ip_batch",
+ NUM_IPS_ENTRIES, sizeof(struct rte_ipv6_addr), 0);
+ TEST_FIB_ASSERT(ip_batch != NULL);
+
+ uint64_t *next_hops = rte_calloc("next_hops",
+ NUM_IPS_ENTRIES, sizeof(uint64_t), 0);
+ TEST_FIB_ASSERT(next_hops != NULL);
conf.type = RTE_FIB6_TRIE;
conf.default_nh = 0;
@@ -151,6 +158,9 @@ test_fib6_perf(void)
rte_fib6_free(fib);
+ rte_free(next_hops);
+ rte_free(ip_batch);
+
return 0;
}