public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
* [PATCH] examples/symmetric_mp: log/ignore promiscuous fail
@ 2025-05-20 18:37 mamcgove
  2025-07-09 14:29 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: mamcgove @ 2025-05-20 18:37 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Matthew G McGovern

From: Matthew G McGovern <mamcgove@microsoft.com>

The example apps have a few different failure modes when enabling promiscuous mode:

- testpmd will warn about the failure and continue.

- l3fwd has a flag '-P' to explicitly require promiscuous mode.

- symmetric_mp will exit with an error code

This patch changes symmetric_mp to warn and continue.

Signed-off-by: Matthew G McGovern <mamcgove@microsoft.com>
---
 examples/multi_process/symmetric_mp/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
index f7d8439cd4..974fed2cd5 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -275,7 +275,8 @@ smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
 
 	retval = rte_eth_promiscuous_enable(port);
 	if (retval != 0)
-		return retval;
+		printf("Error during enabling promiscuous mode for port %u: %s - ignore\n",
+			port, rte_strerror(-retval));
 
 	retval  = rte_eth_dev_start(port);
 	if (retval < 0)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] examples/symmetric_mp: log/ignore promiscuous fail
  2025-05-20 18:37 [PATCH] examples/symmetric_mp: log/ignore promiscuous fail mamcgove
@ 2025-07-09 14:29 ` Thomas Monjalon
  2025-09-03 15:09   ` [EXTERNAL] " Matthew McGovern (LINUX)
  2025-09-16 17:44 ` [PATCH 0/2] " mamcgove
       [not found] ` <cover.1758045692.git.mamcgove@microsoft.com>
  2 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2025-07-09 14:29 UTC (permalink / raw)
  To: Matthew G McGovern; +Cc: Anatoly Burakov, dev

20/05/2025 20:37, mamcgove@microsoft.com:
> From: Matthew G McGovern <mamcgove@microsoft.com>
> 
> The example apps have a few different failure modes when enabling promiscuous mode:
> 
> - testpmd will warn about the failure and continue.
> 
> - l3fwd has a flag '-P' to explicitly require promiscuous mode.
> 
> - symmetric_mp will exit with an error code
> 
> This patch changes symmetric_mp to warn and continue.

Why not doing the same in examples/multi_process/client_server_mp/mp_server/ ?
What about other examples?



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXTERNAL] Re: [PATCH] examples/symmetric_mp: log/ignore promiscuous fail
  2025-07-09 14:29 ` Thomas Monjalon
@ 2025-09-03 15:09   ` Matthew McGovern (LINUX)
  2025-09-03 15:24     ` Thomas Monjalon
  0 siblings, 1 reply; 9+ messages in thread
From: Matthew McGovern (LINUX) @ 2025-09-03 15:09 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Anatoly Burakov, dev@dpdk.org

No reason other than to keep the patch small. I'll add some additional patches for the other apps. 

-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net> 
Sent: Wednesday, July 9, 2025 7:30 AM
To: Matthew McGovern (LINUX) <Matthew.Mcgovern@microsoft.com>
Cc: Anatoly Burakov <anatoly.burakov@intel.com>; dev@dpdk.org
Subject: [EXTERNAL] Re: [PATCH] examples/symmetric_mp: log/ignore promiscuous fail

20/05/2025 20:37, mamcgove@microsoft.com:
> From: Matthew G McGovern <mamcgove@microsoft.com>
> 
> The example apps have a few different failure modes when enabling promiscuous mode:
> 
> - testpmd will warn about the failure and continue.
> 
> - l3fwd has a flag '-P' to explicitly require promiscuous mode.
> 
> - symmetric_mp will exit with an error code
> 
> This patch changes symmetric_mp to warn and continue.

Why not doing the same in examples/multi_process/client_server_mp/mp_server/ ?
What about other examples?



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [EXTERNAL] Re: [PATCH] examples/symmetric_mp: log/ignore promiscuous fail
  2025-09-03 15:09   ` [EXTERNAL] " Matthew McGovern (LINUX)
@ 2025-09-03 15:24     ` Thomas Monjalon
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2025-09-03 15:24 UTC (permalink / raw)
  To: Matthew McGovern (LINUX); +Cc: Anatoly Burakov, dev@dpdk.org

03/09/2025 17:09, Matthew McGovern (LINUX):
> No reason other than to keep the patch small. I'll add some additional patches for the other apps. 
> 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net> 
> Sent: Wednesday, July 9, 2025 7:30 AM
> To: Matthew McGovern (LINUX) <Matthew.Mcgovern@microsoft.com>
> Cc: Anatoly Burakov <anatoly.burakov@intel.com>; dev@dpdk.org
> Subject: [EXTERNAL] Re: [PATCH] examples/symmetric_mp: log/ignore promiscuous fail
> 
> 20/05/2025 20:37, mamcgove@microsoft.com:
> > From: Matthew G McGovern <mamcgove@microsoft.com>
> > 
> > The example apps have a few different failure modes when enabling promiscuous mode:
> > 
> > - testpmd will warn about the failure and continue.
> > 
> > - l3fwd has a flag '-P' to explicitly require promiscuous mode.
> > 
> > - symmetric_mp will exit with an error code
> > 
> > This patch changes symmetric_mp to warn and continue.
> 
> Why not doing the same in examples/multi_process/client_server_mp/mp_server/ ?
> What about other examples?

I think it's better making this consistent across examples.
Thank you for working on this.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/2] examples/symmetric_mp: log/ignore promiscuous fail
  2025-05-20 18:37 [PATCH] examples/symmetric_mp: log/ignore promiscuous fail mamcgove
  2025-07-09 14:29 ` Thomas Monjalon
@ 2025-09-16 17:44 ` mamcgove
  2025-09-16 17:44   ` [PATCH 1/2] examples/symmetric-mp: ignore promisc enable failure mamcgove
  2025-09-16 17:44   ` [PATCH 2/2] examples/mp_server: warn on " mamcgove
       [not found] ` <cover.1758045692.git.mamcgove@microsoft.com>
  2 siblings, 2 replies; 9+ messages in thread
From: mamcgove @ 2025-09-16 17:44 UTC (permalink / raw)
  To: dev; +Cc: Matthew G McGovern

From: Matthew G McGovern <mamcgove@microsoft.com>

Changes all examples/multi_process apps to warn and continue
after a failure to enable promiscuous mode.

This is to allow their use with hardware which does not allow
changes to this mode setting.

Matthew G McGovern (2):
  examples/symmetric-mp: ignore promisc enable failure
  examples/mp_server: warn on promisc enable failure
---

v2:
* wraps commits/messages to width 70
* adds second commit to apply warn/continue to other mp examples

 examples/multi_process/client_server_mp/mp_server/init.c | 4 +++-
 examples/multi_process/symmetric_mp/main.c               | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] examples/symmetric-mp: ignore promisc enable failure
  2025-09-16 17:44 ` [PATCH 0/2] " mamcgove
@ 2025-09-16 17:44   ` mamcgove
  2025-09-16 17:44   ` [PATCH 2/2] examples/mp_server: warn on " mamcgove
  1 sibling, 0 replies; 9+ messages in thread
From: mamcgove @ 2025-09-16 17:44 UTC (permalink / raw)
  To: dev; +Cc: Matthew G McGovern

From: Matthew G McGovern <mamcgove@microsoft.com>

Problem: Some devices do not allow promiscuous mode.

Testpmd handles this by warning and ignoring failures from
rte_eth_promiscuous_enable.

Changes: make symmetric-mp warn if promiscuous mode cannot be
enabled, rather than failing.

Signed-off-by: Matthew G McGovern <mamcgove@microsoft.com>
---
 examples/multi_process/symmetric_mp/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
index f7d8439cd4..22ede756fd 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -275,7 +275,9 @@ smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
 
 	retval = rte_eth_promiscuous_enable(port);
 	if (retval != 0)
-		return retval;
+		fprintf(stderr,
+			"Error during enabling promiscuous mode for port %u: %s - ignore\n",
+			retval, rte_strerror(-retval));
 
 	retval  = rte_eth_dev_start(port);
 	if (retval < 0)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] examples/mp_server: warn on promisc enable failure
  2025-09-16 17:44 ` [PATCH 0/2] " mamcgove
  2025-09-16 17:44   ` [PATCH 1/2] examples/symmetric-mp: ignore promisc enable failure mamcgove
@ 2025-09-16 17:44   ` mamcgove
  1 sibling, 0 replies; 9+ messages in thread
From: mamcgove @ 2025-09-16 17:44 UTC (permalink / raw)
  To: dev; +Cc: Matthew G McGovern

From: Matthew G McGovern <mamcgove@microsoft.com>

Change to treat promiscuous enablement failures as nonfatal in the
client_mp/server_mp examples.

Some devices (mana) support promiscuous mode; but require
a cloud networking configuration change to enable it.

This change will allow examples/multi_process applications
to run in these environments.

Signed-off-by: Matthew G McGovern <mamcgove@microsoft.com>
---
 examples/multi_process/client_server_mp/mp_server/init.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/examples/multi_process/client_server_mp/mp_server/init.c b/examples/multi_process/client_server_mp/mp_server/init.c
index 65713dbea8..87acc38101 100644
--- a/examples/multi_process/client_server_mp/mp_server/init.c
+++ b/examples/multi_process/client_server_mp/mp_server/init.c
@@ -134,7 +134,9 @@ init_port(uint16_t port_num)
 
 	retval = rte_eth_promiscuous_enable(port_num);
 	if (retval < 0)
-		return retval;
+		fprintf(stderr,
+			"Error during enabling promiscuous mode for port %u: %s - ignore\n",
+			retval, rte_strerror(-retval));
 
 	retval  = rte_eth_dev_start(port_num);
 	if (retval < 0) return retval;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 1/1] examples/mp_server: warn on promisc enable failure
       [not found] ` <cover.1758045692.git.mamcgove@microsoft.com>
@ 2025-09-16 18:51   ` mamcgove
  2026-01-14  6:13     ` Stephen Hemminger
  0 siblings, 1 reply; 9+ messages in thread
From: mamcgove @ 2025-09-16 18:51 UTC (permalink / raw)
  To: dev; +Cc: Matthew G McGovern

From: Matthew G McGovern <mamcgove@microsoft.com>

Problem: Some devices do not allow promiscuous mode.

Testpmd handles this by warning and ignoring failures from
rte_eth_promiscuous_enable.

Changes: make mp example apps warn if promiscuous mode cannot be
enabled, rather than failing.

Signed-off-by: Matthew G McGovern <mamcgove@microsoft.com>

---

v3:
* fix incorrect statement in v2 2/2 commit message
* squash small changes to a single commit

 examples/multi_process/client_server_mp/mp_server/init.c | 4 +++-
 examples/multi_process/symmetric_mp/main.c               | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/examples/multi_process/client_server_mp/mp_server/init.c b/examples/multi_process/client_server_mp/mp_server/init.c
index 65713dbea8..87acc38101 100644
--- a/examples/multi_process/client_server_mp/mp_server/init.c
+++ b/examples/multi_process/client_server_mp/mp_server/init.c
@@ -134,7 +134,9 @@ init_port(uint16_t port_num)
 
 	retval = rte_eth_promiscuous_enable(port_num);
 	if (retval < 0)
-		return retval;
+		fprintf(stderr,
+			"Error during enabling promiscuous mode for port %u: %s - ignore\n",
+			retval, rte_strerror(-retval));
 
 	retval  = rte_eth_dev_start(port_num);
 	if (retval < 0) return retval;
diff --git a/examples/multi_process/symmetric_mp/main.c b/examples/multi_process/symmetric_mp/main.c
index f7d8439cd4..22ede756fd 100644
--- a/examples/multi_process/symmetric_mp/main.c
+++ b/examples/multi_process/symmetric_mp/main.c
@@ -275,7 +275,9 @@ smp_port_init(uint16_t port, struct rte_mempool *mbuf_pool,
 
 	retval = rte_eth_promiscuous_enable(port);
 	if (retval != 0)
-		return retval;
+		fprintf(stderr,
+			"Error during enabling promiscuous mode for port %u: %s - ignore\n",
+			retval, rte_strerror(-retval));
 
 	retval  = rte_eth_dev_start(port);
 	if (retval < 0)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/1] examples/mp_server: warn on promisc enable failure
  2025-09-16 18:51   ` [PATCH v3 1/1] " mamcgove
@ 2026-01-14  6:13     ` Stephen Hemminger
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2026-01-14  6:13 UTC (permalink / raw)
  To: mamcgove; +Cc: dev

On Tue, 16 Sep 2025 11:51:14 -0700
mamcgove@microsoft.com wrote:

> From: Matthew G McGovern <mamcgove@microsoft.com>
> 
> Problem: Some devices do not allow promiscuous mode.
> 
> Testpmd handles this by warning and ignoring failures from
> rte_eth_promiscuous_enable.
> 
> Changes: make mp example apps warn if promiscuous mode cannot be
> enabled, rather than failing.
> 
> Signed-off-by: Matthew G McGovern <mamcgove@microsoft.com>
> 
> ---

AI review spotted a bug in this. Note, examples don't have to use RTE_LOG().
Only do so if other code in the example does.

## DPDK Patch Review: examples/mp_server: warn on promisc enable failure

### Summary

This patch changes promiscuous mode enable failure handling in two multi-process example applications from a fatal error to a warning, aligning behavior with testpmd. While the intent is good, there's a **critical bug** in the implementation.

---

### Critical Bug (Error - Must Fix)

**Wrong argument in fprintf format string** - Both changes print `retval` (the error code) where the port number should be:

```c
fprintf(stderr,
    "Error during enabling promiscuous mode for port %u: %s - ignore\n",
    retval, rte_strerror(-retval));
//  ^^^^^^ BUG: should be port_num (init.c) or port (main.c)
```

The `%u` format specifier expects the port number, but receives the negative error code. This will print nonsensical port numbers (e.g., "port 4294967287" for `-ENOTSUP`).

**Fix:**
```c
// In init.c:
fprintf(stderr,
    "Error during enabling promiscuous mode for port %u: %s - ignore\n",
    port_num, rte_strerror(-retval));

// In main.c:
fprintf(stderr,
    "Error during enabling promiscuous mode for port %u: %s - ignore\n",
    port, rte_strerror(-retval));
```

---

### Commit Message Review

| Check | Status | Notes |
|-------|--------|-------|
| Subject ≤60 chars | ✓ | 49 characters |
| Lowercase after colon | ✓ | |
| Imperative mood | ✓ | "warn" is imperative |
| No trailing period | ✓ | |
| Body wrapped at 75 chars | ✓ | |
| Body doesn't start with "It" | ✓ | |
| Signed-off-by present | ✓ | |

**Warning: Subject prefix mismatch** - The subject says `examples/mp_server:` but the patch also modifies `examples/multi_process/symmetric_mp/main.c`. Consider using `examples/multi_process:` to accurately reflect the scope of changes.

---

### Code Style Review

| Check | Status |
|-------|--------|
| Line length ≤100 chars | ✓ |
| No trailing whitespace | ✓ |
| Consistent with surrounding code | ✓ |

---

### Design Considerations (Info)

1. **Consistency with testpmd**: The approach matches how testpmd handles this, which is good for consistency across DPDK examples.

2. **Using fprintf vs RTE_LOG**: Example code commonly uses `fprintf(stderr, ...)` rather than `RTE_LOG()`, so this is acceptable style for examples.

3. **Message wording**: The "- ignore" suffix is slightly awkward. Consider: `"...port %u: %s (continuing anyway)\n"` or simply `"Warning: ..."`

---

### Verdict

**Changes Requested** - The patch has the right intent but contains a significant bug that would produce incorrect/confusing output. After fixing the format string arguments, this should be straightforward to accept.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-01-14  6:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-20 18:37 [PATCH] examples/symmetric_mp: log/ignore promiscuous fail mamcgove
2025-07-09 14:29 ` Thomas Monjalon
2025-09-03 15:09   ` [EXTERNAL] " Matthew McGovern (LINUX)
2025-09-03 15:24     ` Thomas Monjalon
2025-09-16 17:44 ` [PATCH 0/2] " mamcgove
2025-09-16 17:44   ` [PATCH 1/2] examples/symmetric-mp: ignore promisc enable failure mamcgove
2025-09-16 17:44   ` [PATCH 2/2] examples/mp_server: warn on " mamcgove
     [not found] ` <cover.1758045692.git.mamcgove@microsoft.com>
2025-09-16 18:51   ` [PATCH v3 1/1] " mamcgove
2026-01-14  6:13     ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox