[v4,2/5] dumpcap: allow multiple invocations

Message ID 20231109173412.108093-3-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dumpcap and pcapng fixes |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Stephen Hemminger Nov. 9, 2023, 5:34 p.m. UTC
  If dumpcap is run twice with each instance pointing a different
interface, it would fail because of overlap in ring a pool names.
Fix by putting process id in the name.

It is still not allowed to do multiple invocations on the same
interface because only one callback is allowed and only one copy
of mbuf is done. Dumpcap will fail with error in this case:

   pdump_prepare_client_request(): client request for pdump enable/disable failed
   EAL: Error - exiting with code: 1
     Cause: Packet dump enable on 0:net_null0 failed File exists

Fixes: cbb44143be74 ("app/dumpcap: add new packet capture application")
Reported-by: Isaac Boukris <iboukris@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/dumpcap/main.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
  

Comments

Morten Brørup Nov. 9, 2023, 6:30 p.m. UTC | #1
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 9 November 2023 18.34
> 
> If dumpcap is run twice with each instance pointing a different
> interface, it would fail because of overlap in ring a pool names.
> Fix by putting process id in the name.
> 
> It is still not allowed to do multiple invocations on the same
> interface because only one callback is allowed and only one copy
> of mbuf is done. Dumpcap will fail with error in this case:
> 
>    pdump_prepare_client_request(): client request for pdump
> enable/disable failed
>    EAL: Error - exiting with code: 1
>      Cause: Packet dump enable on 0:net_null0 failed File exists
> 
> Fixes: cbb44143be74 ("app/dumpcap: add new packet capture application")
> Reported-by: Isaac Boukris <iboukris@gmail.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

[...]

> +	snprintf(ring_name, sizeof(ring_name),
> +		 "dumpcap-%d", getpid());

Fixed - thank you.

[...]

> +	snprintf(pool_name, sizeof(pool_name), "capture_%u", getpid());

Should change from %u to %d here too. ;-)

Either way,

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
  

Patch

diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index 64294bbfb3e6..74c754e272c5 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -44,7 +44,6 @@ 
 #include <pcap/pcap.h>
 #include <pcap/bpf.h>
 
-#define RING_NAME "capture-ring"
 #define MONITOR_INTERVAL  (500 * 1000)
 #define MBUF_POOL_CACHE_SIZE 32
 #define BURST_SIZE 32
@@ -647,6 +646,7 @@  static void dpdk_init(void)
 static struct rte_ring *create_ring(void)
 {
 	struct rte_ring *ring;
+	char ring_name[RTE_RING_NAMESIZE];
 	size_t size, log2;
 
 	/* Find next power of 2 >= size. */
@@ -660,28 +660,28 @@  static struct rte_ring *create_ring(void)
 		ring_size = size;
 	}
 
-	ring = rte_ring_lookup(RING_NAME);
-	if (ring == NULL) {
-		ring = rte_ring_create(RING_NAME, ring_size,
-					rte_socket_id(), 0);
-		if (ring == NULL)
-			rte_exit(EXIT_FAILURE, "Could not create ring :%s\n",
-				 rte_strerror(rte_errno));
-	}
+	/* Want one ring per invocation of program */
+	snprintf(ring_name, sizeof(ring_name),
+		 "dumpcap-%d", getpid());
+
+	ring = rte_ring_create(ring_name, ring_size,
+			       rte_socket_id(), 0);
+	if (ring == NULL)
+		rte_exit(EXIT_FAILURE, "Could not create ring :%s\n",
+			 rte_strerror(rte_errno));
+
 	return ring;
 }
 
 static struct rte_mempool *create_mempool(void)
 {
 	const struct interface *intf;
-	static const char pool_name[] = "capture_mbufs";
+	char pool_name[RTE_MEMPOOL_NAMESIZE];
 	size_t num_mbufs = 2 * ring_size;
 	struct rte_mempool *mp;
 	uint32_t data_size = 128;
 
-	mp = rte_mempool_lookup(pool_name);
-	if (mp)
-		return mp;
+	snprintf(pool_name, sizeof(pool_name), "capture_%u", getpid());
 
 	/* Common pool so size mbuf for biggest snap length */
 	TAILQ_FOREACH(intf, &interfaces, next) {
@@ -826,7 +826,7 @@  static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
 			rte_exit(EXIT_FAILURE,
 				"Packet dump enable on %u:%s failed %s\n",
 				intf->port, intf->name,
-				rte_strerror(-ret));
+				rte_strerror(rte_errno));
 		}
 
 		if (intf->opts.promisc_mode) {