[v2] graph: fix pcapng file support

Message ID 20230531172753.65074-1-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] graph: fix pcapng file support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS

Commit Message

Stephen Hemminger May 31, 2023, 5:27 p.m. UTC
  The interface to rte_pcapng changed in last release
so that the interfaces used need to be added to the pcapng
file via the API. If this step is missing the pcapng
file will not be valid and can't be read by wireshark etc.

Fixes: d1da6d0d04c7 ("pcapng: require per-interface information")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/graph/graph_pcap.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
  

Comments

Amit Prakash Shukla June 1, 2023, 11:22 a.m. UTC | #1
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, May 31, 2023 10:58 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>
> Subject: [EXT] [PATCH v2] graph: fix pcapng file support
> 
> External Email
> 
> ----------------------------------------------------------------------
> The interface to rte_pcapng changed in last release
> so that the interfaces used need to be added to the pcapng
> file via the API. If this step is missing the pcapng
> file will not be valid and can't be read by wireshark etc.
> 
> Fixes: d1da6d0d04c7 ("pcapng: require per-interface information")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/graph/graph_pcap.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/graph/graph_pcap.c b/lib/graph/graph_pcap.c
> index 6c433300290b..db722c375fa7 100644
> --- a/lib/graph/graph_pcap.c
> +++ b/lib/graph/graph_pcap.c
> @@ -7,6 +7,7 @@
>  #include <stdlib.h>
>  #include <unistd.h>
> 
> +#include <rte_ethdev.h>
>  #include <rte_mbuf.h>
>  #include <rte_pcapng.h>
> 
> @@ -80,7 +81,8 @@ graph_pcap_default_path_get(char **dir_path)
>  int
>  graph_pcap_file_open(const char *filename)
>  {
> -	int fd;
> +	int fd, ret;
> +	uint16_t portid;
>  	char file_name[RTE_GRAPH_PCAP_FILE_SZ];
>  	char *pcap_dir;
> 
> @@ -110,12 +112,29 @@ graph_pcap_file_open(const char *filename)
>  				      NULL);
>  	if (pcapng_fd == NULL) {
>  		graph_err("Graph rte_pcapng_fdopen failed.");
> -		close(fd);
> -		return -1;
> +		goto error;
> +	}
> +
> +	/* Add the configured interfaces as possible capture ports */
> +	RTE_ETH_FOREACH_DEV(portid) {
> +		ret = rte_pcapng_add_interface(pcapng_fd, portid,
> +					       NULL, NULL, NULL);
> +		if (ret < 0) {
> +			graph_err("Graph rte_pcapng_add_interface port
> %u failed: %d",
> +				  portid, ret);
> +			goto error;
> +		}
>  	}
> 
>  done:
>  	return 0;
> +error:
> +	if (pcapng_fd != NULL) {
> +		rte_pcapng_close(pcapng_fd);
> +		pcapng_fd = NULL;
> +	}
> +	close(fd);
> +	return -1;
>  }
> 
>  int
> --
> 2.39.2

Looks good to me. Thanks.

Acked-by: Amit Prakash Shukla <amitprakashs@marvell.com>
  
Thomas Monjalon June 12, 2023, 8:25 p.m. UTC | #2
> > The interface to rte_pcapng changed in last release
> > so that the interfaces used need to be added to the pcapng
> > file via the API. If this step is missing the pcapng
> > file will not be valid and can't be read by wireshark etc.
> > 
> > Fixes: d1da6d0d04c7 ("pcapng: require per-interface information")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Looks good to me. Thanks.
> 
> Acked-by: Amit Prakash Shukla <amitprakashs@marvell.com>

Applied, thanks.
  

Patch

diff --git a/lib/graph/graph_pcap.c b/lib/graph/graph_pcap.c
index 6c433300290b..db722c375fa7 100644
--- a/lib/graph/graph_pcap.c
+++ b/lib/graph/graph_pcap.c
@@ -7,6 +7,7 @@ 
 #include <stdlib.h>
 #include <unistd.h>
 
+#include <rte_ethdev.h>
 #include <rte_mbuf.h>
 #include <rte_pcapng.h>
 
@@ -80,7 +81,8 @@  graph_pcap_default_path_get(char **dir_path)
 int
 graph_pcap_file_open(const char *filename)
 {
-	int fd;
+	int fd, ret;
+	uint16_t portid;
 	char file_name[RTE_GRAPH_PCAP_FILE_SZ];
 	char *pcap_dir;
 
@@ -110,12 +112,29 @@  graph_pcap_file_open(const char *filename)
 				      NULL);
 	if (pcapng_fd == NULL) {
 		graph_err("Graph rte_pcapng_fdopen failed.");
-		close(fd);
-		return -1;
+		goto error;
+	}
+
+	/* Add the configured interfaces as possible capture ports */
+	RTE_ETH_FOREACH_DEV(portid) {
+		ret = rte_pcapng_add_interface(pcapng_fd, portid,
+					       NULL, NULL, NULL);
+		if (ret < 0) {
+			graph_err("Graph rte_pcapng_add_interface port %u failed: %d",
+				  portid, ret);
+			goto error;
+		}
 	}
 
 done:
 	return 0;
+error:
+	if (pcapng_fd != NULL) {
+		rte_pcapng_close(pcapng_fd);
+		pcapng_fd = NULL;
+	}
+	close(fd);
+	return -1;
 }
 
 int