Skip to contentSkip to conversation

dumpcap: correctly disable promiscuous mode at exit

PATCH In review Isaac Boukris 3 participants 6 messages Feb 27, 2024 – Mar 18, 2024

lore
Share & tools
lore

Isaac Boukris

author
#0 lore raw

PATCH 1/1 dumpcap: correctly disable promiscuous mode at exit

Signed-off-by Isaac Boukris <[email protected]>
---
 app/dumpcap/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
app/dumpcap/main.c +3 -2
index d57db0589a..88cec43086 100644
@@ -830,11 +830,12 @@ static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
 				intf->opts.promisc_mode = false;
 			} else {
 				ret = rte_eth_promiscuous_enable(intf->port);
-				if (ret != 0)
+				if (ret != 0) {
 					fprintf(stderr,
 						"port %u set promiscuous enable failed: %d\n",
 						intf->port, ret);
-				intf->opts.promisc_mode = false;
+					intf->opts.promisc_mode = false;
+				}
 			}
 		}
 		++count;
Signature · 1 line
2.43.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 14 lines · On Tue, 27 Feb 2024 11:23:26 +0200 Isaac Boukris <[email protected]> wrote:
Signed-off-by Isaac Boukris <[email protected]>
---
 app/dumpcap/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
app/dumpcap/main.c +0 -1
index d57db0589a..88cec43086 100644
@@ -830,11 +830,12 @@ static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
 				intf->opts.promisc_mode = false;
 			} else {
 				ret = rte_eth_promiscuous_enable(intf->port);
-				if (ret != 0)

app/dumpcap/main.c static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp) · line 841

Quoted 10 lines
app/dumpcap/main.c +3 -1
+				if (ret != 0) {
 					fprintf(stderr,
 						"port %u set promiscuous enable failed: %d\n",
 						intf->port, ret);
-				intf->opts.promisc_mode = false;
+					intf->opts.promisc_mode = false;
+				}
 			}
 		}
 		++count;
Reviewed-by Stephen Hemminger <[email protected]>
#3 lore raw

PATCH v2 1/1 dumpcap: correctly disable promiscuous mode at exit

In reply to Isaac Boukris

From: Isaac Boukris <[email protected]>

If request to set promiscious mode failed at startup, then it is not necessary to disable it when shutting down.

This should only be issue if with a buggy driver because if driver does not support setting promiscious it would just ignore request to disable it as well.

Fixes 6026bfae07d4 ("app/dumpcap: support multiple interfaces")
Signed-off-by Isaac Boukris <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>

---
v2 - add more to commit log, and simplify the logic

 app/dumpcap/main.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
app/dumpcap/main.c +3 -6
index 76c747511444..11e470391edb 100644
@@ -828,12 +828,9 @@ static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
 			if (rte_eth_promiscuous_get(intf->port) == 1) {
 				/* promiscuous already enabled */
 				intf->opts.promisc_mode = false;
-			} else {
-				ret = rte_eth_promiscuous_enable(intf->port);
-				if (ret != 0)
-					fprintf(stderr,
-						"port %u set promiscuous enable failed: %d\n",
-						intf->port, ret);
+			} else if (rte_eth_promiscuous_enable(intf->port) < 0) {
+				fprintf(stderr, "port %u:%s set promiscuous failed\n",
+					intf->port, intf->name);
 				intf->opts.promisc_mode = false;
 			}
 		}
Signature · 1 line
2.43.0
author
#4 lore raw
Quoted 40 lines · Stephen Hemminger

From: Isaac Boukris <[email protected]>

If request to set promiscious mode failed at startup, then it is not necessary to disable it when shutting down.

This should only be issue if with a buggy driver because if driver does not support setting promiscious it would just ignore request to disable it as well.

Fixes 6026bfae07d4 ("app/dumpcap: support multiple interfaces")
Signed-off-by Isaac Boukris <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>

---
v2 - add more to commit log, and simplify the logic

 app/dumpcap/main.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
app/dumpcap/main.c +3 -7
index 76c747511444..11e470391edb 100644
@@ -828,12 +828,9 @@ static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
                        if (rte_eth_promiscuous_get(intf->port) == 1) {
                                /* promiscuous already enabled */
                                intf->opts.promisc_mode = false;
-                       } else {
-                               ret = rte_eth_promiscuous_enable(intf->port);
-                               if (ret != 0)
-                                       fprintf(stderr,
-                                               "port %u set promiscuous enable failed: %d\n",
-                                               intf->port, ret);
+                       } else if (rte_eth_promiscuous_enable(intf->port) < 0) {
+                               fprintf(stderr, "port %u:%s set promiscuous failed\n",
+                                       intf->port, intf->name);
                                intf->opts.promisc_mode = false;
                        }
                }
--
Quoted 2 lines

2.43.0

V2 looks good to me.

Thanks!

Applied
#5 lore raw

08/03/2024 08:09, Isaac Boukris:

Quoted 12 lines
Quoted 10 lines · Stephen Hemminger

From: Isaac Boukris <[email protected]>

If request to set promiscious mode failed at startup, then it is not necessary to disable it when shutting down.

This should only be issue if with a buggy driver because if driver does not support setting promiscious it would just ignore request to disable it as well.

Quoted 7 lines
Quoted 3 lines
Fixes 6026bfae07d4 ("app/dumpcap: support multiple interfaces")
Signed-off-by Isaac Boukris <[email protected]>
Signed-off-by Stephen Hemminger <[email protected]>

V2 looks good to me.

Thanks!

Applied, thanks.