[3/6] app/dumpcap: fix preserving promiscuous mode

Message ID 20230102162441.6205-3-koncept1@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/6] app/dumpcap: add additional dump info |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Ben Magistro Jan. 2, 2023, 4:24 p.m. UTC
  When stopping dumpcap and the main application set an interface to
promiscuous mode, it would be disabled when dumpcap performed its
cleanup.  This results in a change of behavior for the main application
after running/utilizing dumpcap. The initial promiscuous mode is now
stored and compared when cleaning up allowing that to be preserved.

Fixes: d59fb4d ("app/dumpcap: add new packet capture application")
Cc: stephen@networkplumber.org
Cc: stable@dpdk.org

Signed-off-by: Ben Magistro <koncept1@gmail.com>
---
 app/dumpcap/main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
  

Comments

Stephen Hemminger Jan. 2, 2023, 4:58 p.m. UTC | #1
On Mon,  2 Jan 2023 16:24:38 +0000
Ben Magistro <koncept1@gmail.com> wrote:

> When stopping dumpcap and the main application set an interface to
> promiscuous mode, it would be disabled when dumpcap performed its
> cleanup.  This results in a change of behavior for the main application
> after running/utilizing dumpcap. The initial promiscuous mode is now
> stored and compared when cleaning up allowing that to be preserved.
> 
> Fixes: d59fb4d ("app/dumpcap: add new packet capture application")
> Cc: stephen@networkplumber.org
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ben Magistro <koncept1@gmail.com>
> ---
>  app/dumpcap/main.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
> index aaee9349b1..26c641df61 100644
> --- a/app/dumpcap/main.c
> +++ b/app/dumpcap/main.c
> @@ -84,6 +84,7 @@ struct interface {
>  	TAILQ_ENTRY(interface) next;
>  	uint16_t port;
>  	char name[RTE_ETH_NAME_MAX_LEN];
> +	int promiscuous_exit; /* 1 when promicuous is set prior to starting dumpcap */
>  
>  	struct rte_rxtx_callback *rx_cb[RTE_MAX_QUEUES_PER_PORT];
>  };
> @@ -204,6 +205,8 @@ static void add_interface(uint16_t port, const char *name)
>  	memset(intf, 0, sizeof(*intf));
>  	intf->port = port;
>  	rte_strscpy(intf->name, name, sizeof(intf->name));
> +	// not checking error here; should only error if given an invalid port id
> +	intf->promiscuous_exit = rte_eth_promiscuous_get(port);
>  
>  	printf("Capturing on '%s'\n", name);
>  
> @@ -462,7 +465,7 @@ cleanup_pdump_resources(void)
>  	TAILQ_FOREACH(intf, &interfaces, next) {
>  		rte_pdump_disable(intf->port,
>  				  RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
> -		if (promiscuous_mode)
> +		if (!intf->promiscuous_exit && promiscuous_mode)
>  			rte_eth_promiscuous_disable(intf->port);
>  	}
>  }

Thanks, could you test mine instead.
  
Stephen Hemminger Jan. 4, 2023, 3:04 a.m. UTC | #2
On Mon,  2 Jan 2023 16:24:38 +0000
Ben Magistro <koncept1@gmail.com> wrote:

> When stopping dumpcap and the main application set an interface to
> promiscuous mode, it would be disabled when dumpcap performed its
> cleanup.  This results in a change of behavior for the main application
> after running/utilizing dumpcap. The initial promiscuous mode is now
> stored and compared when cleaning up allowing that to be preserved.
> 
> Fixes: d59fb4d ("app/dumpcap: add new packet capture application")
> Cc: stephen@networkplumber.org
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ben Magistro <koncept1@gmail.com>

What I did in the fix patch set add promisc_mode flag
to existing interface array.
  

Patch

diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index aaee9349b1..26c641df61 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -84,6 +84,7 @@  struct interface {
 	TAILQ_ENTRY(interface) next;
 	uint16_t port;
 	char name[RTE_ETH_NAME_MAX_LEN];
+	int promiscuous_exit; /* 1 when promicuous is set prior to starting dumpcap */
 
 	struct rte_rxtx_callback *rx_cb[RTE_MAX_QUEUES_PER_PORT];
 };
@@ -204,6 +205,8 @@  static void add_interface(uint16_t port, const char *name)
 	memset(intf, 0, sizeof(*intf));
 	intf->port = port;
 	rte_strscpy(intf->name, name, sizeof(intf->name));
+	// not checking error here; should only error if given an invalid port id
+	intf->promiscuous_exit = rte_eth_promiscuous_get(port);
 
 	printf("Capturing on '%s'\n", name);
 
@@ -462,7 +465,7 @@  cleanup_pdump_resources(void)
 	TAILQ_FOREACH(intf, &interfaces, next) {
 		rte_pdump_disable(intf->port,
 				  RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
-		if (promiscuous_mode)
+		if (!intf->promiscuous_exit && promiscuous_mode)
 			rte_eth_promiscuous_disable(intf->port);
 	}
 }