Skip to contentSkip to conversation

app/test: fix stack overflow in lpm6_perf_autotest

PATCH Superseded Andre Muezerie 5 participants 12 messages Dec 13, 2024 – Mar 19, 2025

lore
Share & tools
lore

Andre Muezerie

author
#0 lore raw

PATCH 1/1 app/test: fix stack overflow in lpm6_perf_autotest

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
 app/test/test_lpm6_perf.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +9 -2
index 1860a99ed6..8231ad825d 100644
@@ -117,8 +117,12 @@ test_lpm6_perf(void)
 	total_time = 0;
 	count = 0;
 
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_t next_hops[NUM_IPS_ENTRIES];
+	struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = (int32_t *)malloc(sizeof(int32_t) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(next_hops != NULL);
 
 	for (i = 0; i < NUM_IPS_ENTRIES; i++)
 		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +157,9 @@ test_lpm6_perf(void)
 	printf("Average LPM Delete: %g cycles\n",
 			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	free(next_hops);
+	free(ip_batch);
+
 	rte_lpm6_delete_all(lpm);
 	rte_lpm6_free(lpm);
Signature · 1 line
2.47.0.vfs.0.3
#1 lore raw

Hi Andre,

Quoted 15 lines · Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
  app/test/test_lpm6_perf.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +0 -0
index 1860a99ed6..8231ad825d 100644

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 120

Quoted 6 lines
app/test/test_lpm6_perf.c +1 -2
@@ -117,8 +117,12 @@ test_lpm6_perf(void)
  	total_time = 0;
  	count = 0;
 
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_t next_hops[NUM_IPS_ENTRIES];
+	struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(

why not rte_malloc?

Quoted 18 lines
app/test/test_lpm6_perf.c +8 -0
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = (int32_t *)malloc(sizeof(int32_t) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(next_hops != NULL);
 
  	for (i = 0; i < NUM_IPS_ENTRIES; i++)
  		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +157,9 @@ test_lpm6_perf(void)
  	printf("Average LPM Delete: %g cycles\n",
  			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	free(next_hops);
+	free(ip_batch);
+
  	rte_lpm6_delete_all(lpm);
  	rte_lpm6_free(lpm);
Signature · 2 lines
Regards,
Vladimir
author
#2 lore raw
Quoted 13 lines · Vladimir

Hi Andre,

Quoted 10 lines · Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
 app/test/test_lpm6_perf.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
Quoted 11 lines

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 119

Quoted 10 lines
app/test/test_lpm6_perf.c +1 -2
index 1860a99ed6..8231ad825d 100644
@@ -117,8 +117,12 @@ test_lpm6_perf(void)
 	total_time = 0;
 	count = 0;
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_t next_hops[NUM_IPS_ENTRIES];
+	struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(

why not rte_malloc?

For no good reason :-)
I'll update the patch.
Thanks for the suggestion.

Andre Muezerie

Quoted 19 lines
Quoted 15 lines
app/test/test_lpm6_perf.c +8 -0
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = (int32_t *)malloc(sizeof(int32_t) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(next_hops != NULL);
 	for (i = 0; i < NUM_IPS_ENTRIES; i++)
 		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +157,9 @@ test_lpm6_perf(void)
 	printf("Average LPM Delete: %g cycles\n",
 			(double)total_time / NUM_ROUTE_ENTRIES);
+	free(next_hops);
+	free(ip_batch);
+
 	rte_lpm6_delete_all(lpm);
 	rte_lpm6_free(lpm);
Signature · 2 lines
Regards,
Vladimir
#3 lore raw
In reply to Medvedkin, Vladimir
Quoted 21 lines · Vladimir

Hi Andre,

Quoted 18 lines · Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
  app/test/test_lpm6_perf.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +0 -0
index 1860a99ed6..8231ad825d 100644
@@ -117,8 +117,12 @@ test_lpm6_perf(void)
  	total_time = 0;
  	count = 0;
Quoted 6 lines

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 120

Quoted 3 lines
app/test/test_lpm6_perf.c +1 -2
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_t next_hops[NUM_IPS_ENTRIES];
+	struct rte_ipv6_addr *ip_batch = (struct rte_ipv6_addr *)malloc(

why not rte_malloc?

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 122

Quoted 2 lines
app/test/test_lpm6_perf.c +2 -0
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES);
+	TEST_LPM_ASSERT(ip_batch != NULL);

There is no need for rte_malloc() here. The data doesn't need to come from hugepages and regular malloc() has more checking.

But the cast is unnecessary in C.

author
#4 lore raw

PATCH v2 1/1 app/test: fix stack overflow in lpm6_perf_autotest

In reply to Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
 app/test/test_lpm6_perf.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +12 -2
index 1860a99ed6..a09e8611ce 100644
@@ -10,6 +10,7 @@
 
 #include <rte_cycles.h>
 #include <rte_random.h>
+#include <rte_malloc.h>
 #include <rte_memory.h>
 #include <rte_lpm6.h>
 

@@ -117,8 +118,14 @@ test_lpm6_perf(void)
 	total_time = 0;
 	count = 0;
 
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_t next_hops[NUM_IPS_ENTRIES];
+	struct rte_ipv6_addr *ip_batch =
+			(struct rte_ipv6_addr *)rte_malloc("ip_batch",
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES, 0);
+	TEST_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = (int32_t *)rte_malloc("next_hops",
+			sizeof(int32_t) * NUM_IPS_ENTRIES, 0);
+	TEST_LPM_ASSERT(next_hops != NULL);
 
 	for (i = 0; i < NUM_IPS_ENTRIES; i++)
 		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +160,9 @@ test_lpm6_perf(void)
 	printf("Average LPM Delete: %g cycles\n",
 			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	rte_free(next_hops);
+	rte_free(ip_batch);
+
 	rte_lpm6_delete_all(lpm);
 	rte_lpm6_free(lpm);
Signature · 1 line
2.47.0.vfs.0.3
#5 lore raw

app/test/test_lpm6_perf.c test_lpm6_perf(void)

Quoted 3 lines · On Fri, 13 Dec 2024 09:08:22 -0800 Andre Muezerie…
app/test/test_lpm6_perf.c +3 -0
+	struct rte_ipv6_addr *ip_batch =
+			(struct rte_ipv6_addr *)rte_malloc("ip_batch",
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES, 0

Cast is not needed here.
If you are going to allocate an array, use calloc() or rte_calloc()

author
#6 lore raw
Quoted 9 lines · Stephen Hemminger

app/test/test_lpm6_perf.c test_lpm6_perf(void)

Quoted 3 lines · On Fri, 13 Dec 2024 09:08:22 -0800 Andre Muezerie…
app/test/test_lpm6_perf.c +3 -0
+	struct rte_ipv6_addr *ip_batch =
+			(struct rte_ipv6_addr *)rte_malloc("ip_batch",
+			sizeof(struct rte_ipv6_addr) * NUM_IPS_ENTRIES, 0

Cast is not needed here.
If you are going to allocate an array, use calloc() or rte_calloc()

Thanks for the comments. I'll update the patch.

author
#7 lore raw

PATCH v3 1/1 app/test: fix stack overflow in lpm6_perf_autotest

In reply to Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
 app/test/test_lpm6_perf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +11 -2
index 1860a99ed6..59df0a958a 100644
@@ -10,6 +10,7 @@
 
 #include <rte_cycles.h>
 #include <rte_random.h>
+#include <rte_malloc.h>
 #include <rte_memory.h>
 #include <rte_lpm6.h>
 

@@ -117,8 +118,13 @@ test_lpm6_perf(void)
 	total_time = 0;
 	count = 0;
 
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_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_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = rte_calloc("next_hops",
+			NUM_IPS_ENTRIES, sizeof(int32_t), 0);
+	TEST_LPM_ASSERT(next_hops != NULL);
 
 	for (i = 0; i < NUM_IPS_ENTRIES; i++)
 		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +159,9 @@ test_lpm6_perf(void)
 	printf("Average LPM Delete: %g cycles\n",
 			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	rte_free(next_hops);
+	rte_free(ip_batch);
+
 	rte_lpm6_delete_all(lpm);
 	rte_lpm6_free(lpm);
Signature · 1 line
2.47.0.vfs.0.3
Acked-by on v3 1/1
#8 lore raw
Quoted 26 lines · Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
 app/test/test_lpm6_perf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +1 -0
index 1860a99ed6..59df0a958a 100644
@@ -10,6 +10,7 @@
 
 #include <rte_cycles.h>
 #include <rte_random.h>
+#include <rte_malloc.h>
 #include <rte_memory.h>
 #include <rte_lpm6.h>
 

@@ -117,8 +118,13 @@ test_lpm6_perf(void)
 	total_time = 0;
 	count = 0;

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 128

Quoted 10 lines
app/test/test_lpm6_perf.c +7 -2
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_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_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = rte_calloc("next_hops",
+			NUM_IPS_ENTRIES, sizeof(int32_t), 0);
+	TEST_LPM_ASSERT(next_hops != NULL);

While I don't think we need to use the "rte_" versions of allocation, this is still an ok fix - and I see that in v1 regular malloc was used.

With either calloc or rte_calloc used.

Acked-by Bruce Richardson <[email protected]>
Quoted 15 lines
app/test/test_lpm6_perf.c +3 -0
 	for (i = 0; i < NUM_IPS_ENTRIES; i++)
 		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +159,9 @@ test_lpm6_perf(void)
 	printf("Average LPM Delete: %g cycles\n",
 			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	rte_free(next_hops);
+	rte_free(ip_batch);
+
 	rte_lpm6_delete_all(lpm);
 	rte_lpm6_free(lpm);
Signature · 1 line
2.47.0.vfs.0.3
author
#9 lore raw
Quoted 27 lines · Bruce Richardson
Quoted 26 lines · Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie <[email protected]>
---
 app/test/test_lpm6_perf.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +1 -0
index 1860a99ed6..59df0a958a 100644
@@ -10,6 +10,7 @@
 
 #include <rte_cycles.h>
 #include <rte_random.h>
+#include <rte_malloc.h>
 #include <rte_memory.h>
 #include <rte_lpm6.h>
 

@@ -117,8 +118,13 @@ test_lpm6_perf(void)
 	total_time = 0;
 	count = 0;
Quoted 13 lines

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 128

Quoted 10 lines
app/test/test_lpm6_perf.c +7 -2
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_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_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = rte_calloc("next_hops",
+			NUM_IPS_ENTRIES, sizeof(int32_t), 0);
+	TEST_LPM_ASSERT(next_hops != NULL);

While I don't think we need to use the "rte_" versions of allocation, this is still an ok fix - and I see that in v1 regular malloc was used.

The reviewers had different opinions on which allocator function should be used, that's why it was changed. Thanks for being flexible on this.

Quoted 21 lines

With either calloc or rte_calloc used.

Acked-by Bruce Richardson <[email protected]>
Quoted 15 lines
app/test/test_lpm6_perf.c +3 -0
 	for (i = 0; i < NUM_IPS_ENTRIES; i++)
 		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +159,9 @@ test_lpm6_perf(void)
 	printf("Average LPM Delete: %g cycles\n",
 			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	rte_free(next_hops);
+	rte_free(ip_batch);
+
 	rte_lpm6_delete_all(lpm);
 	rte_lpm6_free(lpm);
Signature · 1 line
2.47.0.vfs.0.3
Acked-by
#10 lore raw

Hi Andre,

Quoted 40 lines · Andre Muezerie
Quoted 27 lines · Bruce Richardson
Quoted 26 lines · Andre Muezerie

Test lpm6_perf_autotest was hitting a stack overflow on Windows with both MSVC and Clang.

The fix is to move some of the data from the stack to the heap.

Signed-off-by Andre Muezerie<[email protected]>
---
  app/test/test_lpm6_perf.c | 13 +++++++++++--
  1 file changed, 11 insertions(+), 2 deletions(-)
app/test/test_lpm6_perf.c +1 -0
index 1860a99ed6..59df0a958a 100644
@@ -10,6 +10,7 @@
 
  #include <rte_cycles.h>
  #include <rte_random.h>
+#include <rte_malloc.h>
  #include <rte_memory.h>
  #include <rte_lpm6.h>
 

@@ -117,8 +118,13 @@ test_lpm6_perf(void)
  	total_time = 0;
  	count = 0;
Quoted 12 lines

app/test/test_lpm6_perf.c test_lpm6_perf(void) · line 128

Quoted 10 lines
app/test/test_lpm6_perf.c +7 -2
-	struct rte_ipv6_addr ip_batch[NUM_IPS_ENTRIES];
-	int32_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_LPM_ASSERT(ip_batch != NULL);
+
+	int32_t *next_hops = rte_calloc("next_hops",
+			NUM_IPS_ENTRIES, sizeof(int32_t), 0);
+	TEST_LPM_ASSERT(next_hops != NULL);

While I don't think we need to use the "rte_" versions of allocation, this is still an ok fix - and I see that in v1 regular malloc was used.

Quoted 2 lines

The reviewers had different opinions on which allocator function should be used, that's why it was changed. Thanks for being flexible on this.

As I mentioned earlier, I think it's better to use the memory allocated from hugepages here since we are measuring performance.

Acked-by Vladimir Medvedkin <[email protected]>
Quoted 21 lines
Quoted 21 lines

With either calloc or rte_calloc used.

Acked-by Bruce Richardson<[email protected]>
Quoted 15 lines
app/test/test_lpm6_perf.c +3 -0
  	for (i = 0; i < NUM_IPS_ENTRIES; i++)
  		ip_batch[i] = large_ips_table[i].ip;

@@ -153,6 +159,9 @@ test_lpm6_perf(void)
  	printf("Average LPM Delete: %g cycles\n",
  			(double)total_time / NUM_ROUTE_ENTRIES);
 
+	rte_free(next_hops);
+	rte_free(ip_batch);
+
  	rte_lpm6_delete_all(lpm);
  	rte_lpm6_free(lpm);
Signature · 1 line
2.47.0.vfs.0.3
Signature · 2 lines
Regards,
Vladimir