[PATCH 00/10] enable "app" to be compiled with MSVC

[PATCH 00/10] enable "app" to be compiled with MSVC

[Andre Muezerie]

This series fixes many issues that prevent the "app" directory
from being compiled with MSVC.

Andre Muezerie (10):
  eal: add workaround for __builtin_constant_p
  test_alarm: avoid warning about different qualifiers
  test-pmd: fix printf format string mismatch
  test-pmd: do explicit 64-bit shift to avoid implicit conversion
  test-pmd: avoid undefined behavior
  test-pmd: avoid non-constant initializer
  test-pmd: don't return value from void function
  test-pmd: declare lcore_count atomic when using C11 memory model
  test: add workaround for __builtin_constant_p in test_memcpy_perf
  app: enable app directory to be compiled with MSVC

 app/meson.build                     |   4 --
 app/test-pmd/cmdline.c              |  19 +++--
 app/test-pmd/cmdline_flow.c         |  22 +++---
 app/test-pmd/csumonly.c             |  12 ++--
 app/test-pmd/meson.build            |   4 +-
 app/test-pmd/testpmd.c              |   2 +-
 app/test-pmd/util.c                 |   2 +-
 app/test/test_alarm.c               |  12 ++--
 app/test/test_memcpy_perf.c         | 106 ++++++++++++++--------------
 app/test/test_ring_perf.c           |   6 +-
 lib/eal/include/generic/rte_pause.h |   2 +-
 lib/eal/include/rte_common.h        |  12 ++++
 12 files changed, 114 insertions(+), 89 deletions(-)

--
2.47.2.vfs.0.1

[PATCH 01/10] eal: add workaround for __builtin_constant_p

[Andre Muezerie]

There's no MSVC equivalent for compiler extension __builtin_constant_p.
EAL already had __rte_constant which was used as a first attempt to
workaround __builtin_constant_p when using MSVC. However, there are
pieces of code that would benefit from being able to provide a default
value to be used instead of it being always 0 like how it was done by
__rte_constant.

A new macro is added here allowing such default to be provided by the
caller.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 lib/eal/include/generic/rte_pause.h |  2 +-
 lib/eal/include/rte_common.h        | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/generic/rte_pause.h b/lib/eal/include/generic/rte_pause.h
index 968c0886d3..57e13807ea 100644
--- a/lib/eal/include/generic/rte_pause.h
+++ b/lib/eal/include/generic/rte_pause.h
@@ -130,7 +130,7 @@ rte_wait_until_equal_64(volatile uint64_t *addr, uint64_t expected,
  *  rte_memory_order_acquire and rte_memory_order_relaxed.
  */
 #define RTE_WAIT_UNTIL_MASKED(addr, mask, cond, expected, memorder) do { \
-	RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder));               \
+	RTE_BUILD_BUG_ON(!__rte_constant_with_default(memorder, 1));     \
 	RTE_BUILD_BUG_ON((memorder) != rte_memory_order_acquire &&       \
 		(memorder) != rte_memory_order_relaxed);                 \
 	typeof(*(addr)) expected_value = (expected);                     \
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index 3f77b7624e..6bdce70551 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -50,6 +50,18 @@ extern "C" {
 #define __rte_constant(e) __extension__(__builtin_constant_p(e))
 #endif
 
+/**
+ * Determine if an expression's value is constant at compile time.
+ * All compilers except MSVC will return 1 if the first argument is a
+ * compile-time constant, and 0 otherwise. MSVC will just return the second
+ * argument as a default value.
+ */
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_constant_with_default(e, def) def
+#else
+#define __rte_constant_with_default(e, def) __extension__(__builtin_constant_p(e))
+#endif
+
 /*
  * RTE_TOOLCHAIN_GCC is defined if the target is built with GCC,
  * while a host application (like pmdinfogen) may have another compiler.
-- 
2.47.2.vfs.0.1

Re: [PATCH 01/10] eal: add workaround for __builtin_constant_p

[Stephen Hemminger]

On Tue, 11 Feb 2025 14:01:57 -0800
Andre Muezerie <andremue@linux.microsoft.com> wrote:

> There's no MSVC equivalent for compiler extension __builtin_constant_p.
> EAL already had __rte_constant which was used as a first attempt to
> workaround __builtin_constant_p when using MSVC. However, there are
> pieces of code that would benefit from being able to provide a default
> value to be used instead of it being always 0 like how it was done by
> __rte_constant.
> 
> A new macro is added here allowing such default to be provided by the
> caller.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

There is a hack way of determining if expression is constant using sizeof
tricks, maybe that would be ideal solution?

Re: [PATCH 01/10] eal: add workaround for __builtin_constant_p

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:01, Andre Muezerie wrote:
> There's no MSVC equivalent for compiler extension __builtin_constant_p.
> EAL already had __rte_constant which was used as a first attempt to
> workaround __builtin_constant_p when using MSVC. However, there are
> pieces of code that would benefit from being able to provide a default
> value to be used instead of it being always 0 like how it was done by
> __rte_constant.
> 
> A new macro is added here allowing such default to be provided by the
> caller.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 02/10] test_alarm: avoid warning about different qualifiers

[Andre Muezerie]

Compiling with MSVC results in the warning below:

app/test/test_alarm.c(54): warning C4090: 'function':
    different '_Atomic' qualifiers

The fix is to use a macro to explicitly drop the qualifier.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test/test_alarm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/test/test_alarm.c b/app/test/test_alarm.c
index 9ed8c6f72c..6445f713fe 100644
--- a/app/test/test_alarm.c
+++ b/app/test/test_alarm.c
@@ -51,12 +51,12 @@ test_alarm(void)
 			 "Expected rte_eal_alarm_cancel to fail with null callback parameter");
 
 	/* check if can set a alarm for one second */
-	TEST_ASSERT_SUCCESS(rte_eal_alarm_set(US_PER_SEC, test_alarm_callback, &triggered),
-			    "Setting one second alarm failed");
+	TEST_ASSERT_SUCCESS(rte_eal_alarm_set(US_PER_SEC, test_alarm_callback,
+			    RTE_PTR_UNQUAL(&triggered)), "Setting one second alarm failed");
 
 	/* set a longer alarm that will be canceled. */
-	TEST_ASSERT_SUCCESS(rte_eal_alarm_set(10 * US_PER_SEC, test_alarm_callback, &later),
-			    "Setting ten second alarm failed");
+	TEST_ASSERT_SUCCESS(rte_eal_alarm_set(10 * US_PER_SEC, test_alarm_callback,
+			    RTE_PTR_UNQUAL(&later)), "Setting ten second alarm failed");
 
 	/* wait for alarm to happen */
 	while (rte_atomic_load_explicit(&triggered, rte_memory_order_acquire) == false)
@@ -65,11 +65,11 @@ test_alarm(void)
 	TEST_ASSERT(!rte_atomic_load_explicit(&later, rte_memory_order_acquire),
 		    "Only one alarm should have fired.");
 
-	ret = rte_eal_alarm_cancel(test_alarm_callback, &triggered);
+	ret = rte_eal_alarm_cancel(test_alarm_callback, RTE_PTR_UNQUAL(&triggered));
 	TEST_ASSERT(ret == 0 && rte_errno == ENOENT,
 		    "Canceling alarm after run ret %d: %s", ret, rte_strerror(rte_errno));
 
-	ret = rte_eal_alarm_cancel(test_alarm_callback, &later);
+	ret = rte_eal_alarm_cancel(test_alarm_callback, RTE_PTR_UNQUAL(&later));
 	TEST_ASSERT(ret == 1, "Canceling ten second alarm failed %d: %s",
 		    ret, rte_strerror(rte_errno));
 
-- 
2.47.2.vfs.0.1

Re: [PATCH 02/10] test_alarm: avoid warning about different qualifiers

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:01, Andre Muezerie wrote:
> Compiling with MSVC results in the warning below:
> 
> app/test/test_alarm.c(54): warning C4090: 'function':
>     different '_Atomic' qualifiers
> 
> The fix is to use a macro to explicitly drop the qualifier.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 03/10] test-pmd: fix printf format string mismatch

[Andre Muezerie]

Compiling with MSVC results in warnings like the one below:

app/test-pmd/csumonly.c(1085): warning C4477: 'printf' : format string
    '%d' requires an argument of type 'int',
    but variadic argument 1 has type 'uint64_t'

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test-pmd/csumonly.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index d77a140641..6ad9e2a7e9 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -1070,7 +1070,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				info.l2_len, rte_be_to_cpu_16(info.ethertype),
 				info.l3_len, info.l4_proto, info.l4_len, buf);
 			if (rx_ol_flags & RTE_MBUF_F_RX_LRO)
-				printf("rx: m->lro_segsz=%u\n", m->tso_segsz);
+				printf("rx: m->lro_segsz=%d\n", (int)m->tso_segsz);
 			if (info.is_tunnel == 1)
 				printf("rx: outer_l2_len=%d outer_ethertype=%x "
 					"outer_l3_len=%d\n", info.outer_l2_len,
@@ -1084,7 +1084,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				info.tso_segsz != 0)
 				printf("tx: m->l2_len=%d m->l3_len=%d "
 					"m->l4_len=%d\n",
-					m->l2_len, m->l3_len, m->l4_len);
+					(int)m->l2_len, (int)m->l3_len, (int)m->l4_len);
 			if (info.is_tunnel == 1) {
 				if ((tx_offloads &
 				    RTE_ETH_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
@@ -1093,17 +1093,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				    (tx_ol_flags & RTE_MBUF_F_TX_OUTER_IPV6))
 					printf("tx: m->outer_l2_len=%d "
 						"m->outer_l3_len=%d\n",
-						m->outer_l2_len,
-						m->outer_l3_len);
+						(int)m->outer_l2_len,
+						(int)m->outer_l3_len);
 				if (info.tunnel_tso_segsz != 0 &&
 						(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
 							RTE_MBUF_F_TX_UDP_SEG)))
 					printf("tx: m->tso_segsz=%d\n",
-						m->tso_segsz);
+						(int)m->tso_segsz);
 			} else if (info.tso_segsz != 0 &&
 					(m->ol_flags & (RTE_MBUF_F_TX_TCP_SEG |
 						RTE_MBUF_F_TX_UDP_SEG)))
-				printf("tx: m->tso_segsz=%d\n", m->tso_segsz);
+				printf("tx: m->tso_segsz=%d\n", (int)m->tso_segsz);
 			rte_get_tx_ol_flag_list(m->ol_flags, buf, sizeof(buf));
 			printf("tx: flags=%s", buf);
 			printf("\n");
-- 
2.47.2.vfs.0.1

Re: [PATCH 03/10] test-pmd: fix printf format string mismatch

[Stephen Hemminger]

On Tue, 11 Feb 2025 14:01:59 -0800
Andre Muezerie <andremue@linux.microsoft.com> wrote:

> Compiling with MSVC results in warnings like the one below:
> 
> app/test-pmd/csumonly.c(1085): warning C4477: 'printf' : format string
>     '%d' requires an argument of type 'int',
>     but variadic argument 1 has type 'uint64_t'
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---

In general, DPDK code prefers to use inttypes.h which has PRIu64 macro for this.

Re: [PATCH 03/10] test-pmd: fix printf format string mismatch

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:01, Andre Muezerie wrote:
> Compiling with MSVC results in warnings like the one below:
> 
> app/test-pmd/csumonly.c(1085): warning C4477: 'printf' : format string
>     '%d' requires an argument of type 'int',
>     but variadic argument 1 has type 'uint64_t'
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 04/10] test-pmd: do explicit 64-bit shift to avoid implicit conversion

[Andre Muezerie]

Compiling with MSVC results in warnings like the one below:

app/test-pmd/util.c(201): warning C4334: '<<': result of 32-bit shift
    implicitly converted to 64 bits (was 64-bit shift intended?)

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test-pmd/cmdline.c | 4 ++--
 app/test-pmd/testpmd.c | 2 +-
 app/test-pmd/util.c    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 86d763b66a..2afcf916c0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -12632,11 +12632,11 @@ cmd_config_dynf_specific_parsed(void *parsed_result,
 	}
 	old_port_flags = ports[res->port_id].mbuf_dynf;
 	if (!strcmp(res->value, "set")) {
-		ports[res->port_id].mbuf_dynf |= 1UL << flag;
+		ports[res->port_id].mbuf_dynf |= RTE_BIT64(flag);
 		if (old_port_flags == 0)
 			add_tx_dynf_callback(res->port_id);
 	} else {
-		ports[res->port_id].mbuf_dynf &= ~(1UL << flag);
+		ports[res->port_id].mbuf_dynf &= ~RTE_BIT64(flag);
 		if (ports[res->port_id].mbuf_dynf == 0)
 			remove_tx_dynf_callback(res->port_id);
 	}
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0520aba0db..0a5b999c3a 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -4152,7 +4152,7 @@ get_eth_dcb_conf(struct rte_eth_conf *eth_conf, enum dcb_mode_enable dcb_mode,
 		for (i = 0; i < vmdq_rx_conf->nb_pool_maps; i++) {
 			vmdq_rx_conf->pool_map[i].vlan_id = vlan_tags[i];
 			vmdq_rx_conf->pool_map[i].pools =
-				1 << (i % vmdq_rx_conf->nb_queue_pools);
+				RTE_BIT64(i % vmdq_rx_conf->nb_queue_pools);
 		}
 		for (i = 0; i < RTE_ETH_DCB_NUM_USER_PRIORITIES; i++) {
 			vmdq_rx_conf->dcb_tc[i] = i % num_tcs;
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5fa05fad16..3934831b19 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -201,7 +201,7 @@ dump_pkt_burst(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
 				MKDUMPSTR(print_buf, buf_size, cur_len,
 					  " - dynf %s: %d",
 					  dynf_names[dynf_index],
-					  !!(ol_flags & (1UL << dynf_index)));
+					  !!(ol_flags & RTE_BIT64(dynf_index)));
 		}
 		if (mb->packet_type) {
 			rte_get_ptype_name(mb->packet_type, buf, sizeof(buf));
-- 
2.47.2.vfs.0.1

Re: [PATCH 04/10] test-pmd: do explicit 64-bit shift to avoid implicit conversion

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:02, Andre Muezerie wrote:
> Compiling with MSVC results in warnings like the one below:
> 
> app/test-pmd/util.c(201): warning C4334: '<<': result of 32-bit shift
>     implicitly converted to 64 bits (was 64-bit shift intended?)
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 05/10] test-pmd: avoid undefined behavior

[Andre Muezerie]

Compiling with MSVC results in warnings like below:

app/test-pmd/cmdline.c(9023): warning C5101: use of preprocessor
    directive in function-like macro argument list is undefined behavior

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test-pmd/cmdline.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2afcf916c0..4f0b0340c8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -9011,6 +9011,18 @@ static void cmd_dump_parsed(void *parsed_result,
 }
 
 static cmdline_parse_token_string_t cmd_dump_dump =
+#ifdef RTE_EXEC_ENV_WINDOWS
+	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
+		"dump_physmem#"
+		"dump_memzone#"
+		"dump_socket_mem#"
+		"dump_struct_sizes#"
+		"dump_ring#"
+		"dump_mempool#"
+		"dump_devargs#"
+		"dump_lcores#"
+		"dump_log_types");
+#else
 	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
 		"dump_physmem#"
 		"dump_memzone#"
@@ -9020,10 +9032,9 @@ static cmdline_parse_token_string_t cmd_dump_dump =
 		"dump_mempool#"
 		"dump_devargs#"
 		"dump_lcores#"
-#ifndef RTE_EXEC_ENV_WINDOWS
 		"dump_trace#"
-#endif
 		"dump_log_types");
+#endif
 
 static cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.47.2.vfs.0.1

Re: [PATCH 05/10] test-pmd: avoid undefined behavior

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:02, Andre Muezerie wrote:
> Compiling with MSVC results in warnings like below:
> 
> app/test-pmd/cmdline.c(9023): warning C5101: use of preprocessor
>     directive in function-like macro argument list is undefined behavior
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 06/10] test-pmd: avoid non-constant initializer

[Andre Muezerie]

Compiling with MSVC results in errors like the one below:

app/test-pmd/cmdline_flow.c(8819): error C2099: initializer
    is not a constant

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test-pmd/cmdline_flow.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index e1720e54d7..24323d8891 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -8812,8 +8812,6 @@ parse_vc_spec(struct context *ctx, const struct token *token,
 		return -1;
 	/* Parse parameter types. */
 	switch (ctx->curr) {
-		static const enum index prefix[] = NEXT_ENTRY(COMMON_PREFIX);
-
 	case ITEM_PARAM_IS:
 		index = 0;
 		objmask = 1;
@@ -8828,7 +8826,7 @@ parse_vc_spec(struct context *ctx, const struct token *token,
 		/* Modify next token to expect a prefix. */
 		if (ctx->next_num < 2)
 			return -1;
-		ctx->next[ctx->next_num - 2] = prefix;
+		ctx->next[ctx->next_num - 2] = NEXT_ENTRY(COMMON_PREFIX);
 		/* Fall through. */
 	case ITEM_PARAM_MASK:
 		index = 2;
@@ -9270,7 +9268,6 @@ parse_vc_action_rss_type(struct context *ctx, const struct token *token,
 			  const char *str, unsigned int len,
 			  void *buf, unsigned int size)
 {
-	static const enum index next[] = NEXT_ENTRY(ACTION_RSS_TYPE);
 	struct action_rss_data *action_rss_data;
 	unsigned int i;
 
@@ -9296,7 +9293,7 @@ parse_vc_action_rss_type(struct context *ctx, const struct token *token,
 	/* Repeat token. */
 	if (ctx->next_num == RTE_DIM(ctx->next))
 		return -1;
-	ctx->next[ctx->next_num++] = next;
+	ctx->next[ctx->next_num++] = NEXT_ENTRY(ACTION_RSS_TYPE);
 	if (!ctx->object)
 		return len;
 	action_rss_data = ctx->object;
@@ -9314,7 +9311,6 @@ parse_vc_action_rss_queue(struct context *ctx, const struct token *token,
 			  const char *str, unsigned int len,
 			  void *buf, unsigned int size)
 {
-	static const enum index next[] = NEXT_ENTRY(ACTION_RSS_QUEUE);
 	struct action_rss_data *action_rss_data;
 	const struct arg *arg;
 	int ret;
@@ -9347,7 +9343,7 @@ parse_vc_action_rss_queue(struct context *ctx, const struct token *token,
 	/* Repeat token. */
 	if (ctx->next_num == RTE_DIM(ctx->next))
 		return -1;
-	ctx->next[ctx->next_num++] = next;
+	ctx->next[ctx->next_num++] = NEXT_ENTRY(ACTION_RSS_QUEUE);
 end:
 	if (!ctx->object)
 		return len;
-- 
2.47.2.vfs.0.1

Re: [PATCH 06/10] test-pmd: avoid non-constant initializer

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:02, Andre Muezerie wrote:
> Compiling with MSVC results in errors like the one below:
> 
> app/test-pmd/cmdline_flow.c(8819): error C2099: initializer
>     is not a constant
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 07/10] test-pmd: don't return value from void function

[Andre Muezerie]

Compiling with MSVC results in the warning below:

app/test-pmd/cmdline_flow.c(13964): warning C4098: 'cmd_set_raw_parsed':
    'void' function returning a value

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test-pmd/cmdline_flow.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 24323d8891..15a18db2c7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -13952,11 +13952,15 @@ cmd_set_raw_parsed(const struct buffer *in)
 	int gtp_psc = -1; /* GTP PSC option index. */
 	const void *src_spec;
 
-	if (in->command == SET_SAMPLE_ACTIONS)
-		return cmd_set_raw_parsed_sample(in);
+	if (in->command == SET_SAMPLE_ACTIONS) {
+		cmd_set_raw_parsed_sample(in);
+		return;
+	}
 	else if (in->command == SET_IPV6_EXT_PUSH ||
-		 in->command == SET_IPV6_EXT_REMOVE)
-		return cmd_set_ipv6_ext_parsed(in);
+			 in->command == SET_IPV6_EXT_REMOVE) {
+		cmd_set_ipv6_ext_parsed(in);
+		return;
+	}
 	RTE_ASSERT(in->command == SET_RAW_ENCAP ||
 		   in->command == SET_RAW_DECAP);
 	if (in->command == SET_RAW_ENCAP) {
-- 
2.47.2.vfs.0.1

Re: [PATCH 07/10] test-pmd: don't return value from void function

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:02, Andre Muezerie wrote:
> Compiling with MSVC results in the warning below:
> 
> app/test-pmd/cmdline_flow.c(13964): warning C4098: 'cmd_set_raw_parsed':
>     'void' function returning a value
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 08/10] test-pmd: declare lcore_count atomic when using C11 memory model

[Andre Muezerie]

Compiling with MSVC results in the error below:

app/test/test_ring_perf.c(197): error C7712: address argument to atomic
    operation must be a pointer to an atomic integer,
    'volatile unsigned int *' is not valid

The fix is to mark lcore_count as atomic when using C11 memory model.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test/test_ring_perf.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index 57cd04a124..3bb7577629 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -34,7 +34,11 @@ struct lcore_pair {
 	unsigned c1, c2;
 };
 
-static volatile unsigned lcore_count = 0;
+#ifdef RTE_USE_C11_MEM_MODEL
+static RTE_ATOMIC(unsigned int) lcore_count;
+#else
+static volatile unsigned int lcore_count;
+#endif
 
 static void
 test_ring_print_test_string(unsigned int api_type, int esize,
-- 
2.47.2.vfs.0.1

Re: [PATCH 08/10] test-pmd: declare lcore_count atomic when using C11 memory model

[Stephen Hemminger]

On Tue, 11 Feb 2025 14:02:04 -0800
Andre Muezerie <andremue@linux.microsoft.com> wrote:

> Compiling with MSVC results in the error below:
> 
> app/test/test_ring_perf.c(197): error C7712: address argument to atomic
>     operation must be a pointer to an atomic integer,
>     'volatile unsigned int *' is not valid
> 
> The fix is to mark lcore_count as atomic when using C11 memory model.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

Prefer using RTE_ATOMIC() all teh time now.

Re: [PATCH 08/10] test-pmd: declare lcore_count atomic when using C11 memory model

[Andre Muezerie]

On Tue, Feb 11, 2025 at 02:12:12PM -0800, Stephen Hemminger wrote:
> On Tue, 11 Feb 2025 14:02:04 -0800
> Andre Muezerie <andremue@linux.microsoft.com> wrote:
> 
> > Compiling with MSVC results in the error below:
> > 
> > app/test/test_ring_perf.c(197): error C7712: address argument to atomic
> >     operation must be a pointer to an atomic integer,
> >     'volatile unsigned int *' is not valid
> > 
> > The fix is to mark lcore_count as atomic when using C11 memory model.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> 
> Prefer using RTE_ATOMIC() all teh time now.

You mean even when not explicitly using the C11 model, right?

Re: [PATCH 08/10] test-pmd: declare lcore_count atomic when using C11 memory model

[fengchengwen]

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

On 2025/2/12 6:02, Andre Muezerie wrote:
> Compiling with MSVC results in the error below:
> 
> app/test/test_ring_perf.c(197): error C7712: address argument to atomic
>     operation must be a pointer to an atomic integer,
>     'volatile unsigned int *' is not valid
> 
> The fix is to mark lcore_count as atomic when using C11 memory model.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

[PATCH 09/10] test: add workaround for __builtin_constant_p in test_memcpy_perf

[Andre Muezerie]

There's no MSVC equivalent for compiler extension __builtin_constant_p,
so a workaround is needed.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test/test_memcpy_perf.c | 106 ++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/app/test/test_memcpy_perf.c b/app/test/test_memcpy_perf.c
index 5c05a84619..6091b6f9dd 100644
--- a/app/test/test_memcpy_perf.c
+++ b/app/test/test_memcpy_perf.c
@@ -167,66 +167,66 @@ do_uncached_write(uint8_t *dst, int is_dst_cached,
  * Run a single memcpy performance test. This is a macro to ensure that if
  * the "size" parameter is a constant it won't be converted to a variable.
  */
-#define SINGLE_PERF_TEST(dst, is_dst_cached, dst_uoffset,                   \
-                         src, is_src_cached, src_uoffset, size)             \
-do {                                                                        \
-    unsigned int iter, t;                                                   \
-    size_t dst_addrs[TEST_BATCH_SIZE], src_addrs[TEST_BATCH_SIZE];          \
-    uint64_t start_time, total_time = 0;                                    \
-    uint64_t total_time2 = 0;                                               \
-    for (iter = 0; iter < (TEST_ITERATIONS / TEST_BATCH_SIZE); iter++) {    \
-        fill_addr_arrays(dst_addrs, is_dst_cached, dst_uoffset,             \
-                         src_addrs, is_src_cached, src_uoffset);            \
-        start_time = rte_rdtsc();                                           \
-        for (t = 0; t < TEST_BATCH_SIZE; t++)                               \
-            rte_memcpy(dst+dst_addrs[t], src+src_addrs[t], size);           \
-        total_time += rte_rdtsc() - start_time;                             \
-    }                                                                       \
-    for (iter = 0; iter < (TEST_ITERATIONS / TEST_BATCH_SIZE); iter++) {    \
-        fill_addr_arrays(dst_addrs, is_dst_cached, dst_uoffset,             \
-                         src_addrs, is_src_cached, src_uoffset);            \
-        start_time = rte_rdtsc();                                           \
-        for (t = 0; t < TEST_BATCH_SIZE; t++)                               \
-            memcpy(dst+dst_addrs[t], src+src_addrs[t], size);               \
-        total_time2 += rte_rdtsc() - start_time;                            \
-    }                                                                       \
-    printf("%3.0f -", (double)total_time  / TEST_ITERATIONS);                 \
-    printf("%3.0f",   (double)total_time2 / TEST_ITERATIONS);                 \
-    printf("(%6.2f%%) ", ((double)total_time - total_time2)*100/total_time2); \
+#define SINGLE_PERF_TEST(dst, is_dst_cached, dst_uoffset,                         \
+			 src, is_src_cached, src_uoffset, size)                   \
+do {                                                                              \
+	unsigned int iter, t;                                                     \
+	size_t dst_addrs[TEST_BATCH_SIZE], src_addrs[TEST_BATCH_SIZE];            \
+	uint64_t start_time, total_time = 0;                                      \
+	uint64_t total_time2 = 0;                                                 \
+	for (iter = 0; iter < (TEST_ITERATIONS / TEST_BATCH_SIZE); iter++) {      \
+		fill_addr_arrays(dst_addrs, is_dst_cached, dst_uoffset,           \
+				 src_addrs, is_src_cached, src_uoffset);          \
+		start_time = rte_rdtsc();                                         \
+		for (t = 0; t < TEST_BATCH_SIZE; t++)                             \
+			rte_memcpy(dst+dst_addrs[t], src+src_addrs[t], size);     \
+		total_time += rte_rdtsc() - start_time;                           \
+	}                                                                         \
+	for (iter = 0; iter < (TEST_ITERATIONS / TEST_BATCH_SIZE); iter++) {      \
+		fill_addr_arrays(dst_addrs, is_dst_cached, dst_uoffset,           \
+				 src_addrs, is_src_cached, src_uoffset);          \
+		start_time = rte_rdtsc();                                         \
+		for (t = 0; t < TEST_BATCH_SIZE; t++)                             \
+			memcpy(dst+dst_addrs[t], src+src_addrs[t], size);         \
+		total_time2 += rte_rdtsc() - start_time;                          \
+	}                                                                         \
+	printf("%3.0f -", (double)total_time  / TEST_ITERATIONS);                 \
+	printf("%3.0f",   (double)total_time2 / TEST_ITERATIONS);                 \
+	printf("(%6.2f%%) ", ((double)total_time - total_time2)*100/total_time2); \
 } while (0)
 
 /* Run aligned memcpy tests for each cached/uncached permutation */
-#define ALL_PERF_TESTS_FOR_SIZE(n)                                       \
-do {                                                                     \
-    if (__builtin_constant_p(n))                                         \
-        printf("\nC%6u", (unsigned)n);                                   \
-    else                                                                 \
-        printf("\n%7u", (unsigned)n);                                    \
-    SINGLE_PERF_TEST(small_buf_write, 1, 0, small_buf_read, 1, 0, n);    \
-    SINGLE_PERF_TEST(large_buf_write, 0, 0, small_buf_read, 1, 0, n);    \
-    SINGLE_PERF_TEST(small_buf_write, 1, 0, large_buf_read, 0, 0, n);    \
-    SINGLE_PERF_TEST(large_buf_write, 0, 0, large_buf_read, 0, 0, n);    \
+#define ALL_PERF_TESTS_FOR_SIZE(n, def)                                      \
+do {                                                                         \
+	if (__rte_constant_with_default(n, def))                             \
+		printf("\nC%6u", (unsigned int)n);                           \
+	else                                                                 \
+		printf("\n%7u", (unsigned int)n);                            \
+	SINGLE_PERF_TEST(small_buf_write, 1, 0, small_buf_read, 1, 0, n);    \
+	SINGLE_PERF_TEST(large_buf_write, 0, 0, small_buf_read, 1, 0, n);    \
+	SINGLE_PERF_TEST(small_buf_write, 1, 0, large_buf_read, 0, 0, n);    \
+	SINGLE_PERF_TEST(large_buf_write, 0, 0, large_buf_read, 0, 0, n);    \
 } while (0)
 
 /* Run unaligned memcpy tests for each cached/uncached permutation */
-#define ALL_PERF_TESTS_FOR_SIZE_UNALIGNED(n)                             \
-do {                                                                     \
-    if (__builtin_constant_p(n))                                         \
-        printf("\nC%6u", (unsigned)n);                                   \
-    else                                                                 \
-        printf("\n%7u", (unsigned)n);                                    \
-    SINGLE_PERF_TEST(small_buf_write, 1, 1, small_buf_read, 1, 5, n);    \
-    SINGLE_PERF_TEST(large_buf_write, 0, 1, small_buf_read, 1, 5, n);    \
-    SINGLE_PERF_TEST(small_buf_write, 1, 1, large_buf_read, 0, 5, n);    \
-    SINGLE_PERF_TEST(large_buf_write, 0, 1, large_buf_read, 0, 5, n);    \
+#define ALL_PERF_TESTS_FOR_SIZE_UNALIGNED(n, def)                            \
+do {                                                                         \
+	if (__rte_constant_with_default(n, def))                             \
+		printf("\nC%6u", (unsigned int)n);                           \
+	else                                                                 \
+		printf("\n%7u", (unsigned int)n);                            \
+	SINGLE_PERF_TEST(small_buf_write, 1, 1, small_buf_read, 1, 5, n);    \
+	SINGLE_PERF_TEST(large_buf_write, 0, 1, small_buf_read, 1, 5, n);    \
+	SINGLE_PERF_TEST(small_buf_write, 1, 1, large_buf_read, 0, 5, n);    \
+	SINGLE_PERF_TEST(large_buf_write, 0, 1, large_buf_read, 0, 5, n);    \
 } while (0)
 
 /* Run memcpy tests for constant length */
-#define ALL_PERF_TEST_FOR_CONSTANT                                      \
-do {                                                                    \
-    TEST_CONSTANT(6U); TEST_CONSTANT(64U); TEST_CONSTANT(128U);         \
-    TEST_CONSTANT(192U); TEST_CONSTANT(256U); TEST_CONSTANT(512U);      \
-    TEST_CONSTANT(768U); TEST_CONSTANT(1024U); TEST_CONSTANT(1536U);    \
+#define ALL_PERF_TEST_FOR_CONSTANT                                                \
+do {                                                                              \
+	TEST_CONSTANT(6U, 1); TEST_CONSTANT(64U, 1); TEST_CONSTANT(128U, 1);      \
+	TEST_CONSTANT(192U, 1); TEST_CONSTANT(256U, 1); TEST_CONSTANT(512U, 1);   \
+	TEST_CONSTANT(768U, 1); TEST_CONSTANT(1024U, 1); TEST_CONSTANT(1536U, 1); \
 } while (0)
 
 /* Run all memcpy tests for aligned constant cases */
@@ -253,7 +253,7 @@ perf_test_variable_aligned(void)
 {
 	unsigned i;
 	for (i = 0; i < RTE_DIM(buf_sizes); i++) {
-		ALL_PERF_TESTS_FOR_SIZE((size_t)buf_sizes[i]);
+		ALL_PERF_TESTS_FOR_SIZE(buf_sizes[i], 0);
 	}
 }
 
@@ -263,7 +263,7 @@ perf_test_variable_unaligned(void)
 {
 	unsigned i;
 	for (i = 0; i < RTE_DIM(buf_sizes); i++) {
-		ALL_PERF_TESTS_FOR_SIZE_UNALIGNED((size_t)buf_sizes[i]);
+		ALL_PERF_TESTS_FOR_SIZE_UNALIGNED(buf_sizes[i], 0);
 	}
 }
 
-- 
2.47.2.vfs.0.1

Re: [PATCH 09/10] test: add workaround for __builtin_constant_p in test_memcpy_perf

[Stephen Hemminger]

On Tue, 11 Feb 2025 14:02:05 -0800
Andre Muezerie <andremue@linux.microsoft.com> wrote:

> There's no MSVC equivalent for compiler extension __builtin_constant_p,
> so a workaround is needed.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

Prefer that __rte_constant worked on all platforms,
but template code is hard to maintain.

Re: [PATCH 09/10] test: add workaround for __builtin_constant_p in test_memcpy_perf

[Andre Muezerie]

On Tue, Feb 11, 2025 at 02:13:05PM -0800, Stephen Hemminger wrote:
> On Tue, 11 Feb 2025 14:02:05 -0800
> Andre Muezerie <andremue@linux.microsoft.com> wrote:
> 
> > There's no MSVC equivalent for compiler extension __builtin_constant_p,
> > so a workaround is needed.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> 
> Prefer that __rte_constant worked on all platforms,
> but template code is hard to maintain.

I'm not a huge fan of __rte_constant_with_default either. Here are some thoughts about it:

In test_memcpy_perf we could get rid of __builtin_constant_p or similar macros and just use the second argument (def) I'm passing to __rte_constant_with_default as the condition in the if() statement. I only used __rte_constant_with_default to keep the source code practically the same when using non-msvc compilers. But I don't see advantages beyond that, other than not having to pass that second parameter (def), which now we have to anyways.

__rte_constant_with_default is needed though in the first patch of this series, unless we decide to remove that extra check where it is being used.