DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Ben Magistro <koncept1@gmail.com>
Cc: dev@dpdk.org, Stephen Hemminger <stephen@networkplumber.org>
Subject: [PATCH v2 4/6] app/dumpcap: support multiple interfaces
Date: Tue,  3 Jan 2023 19:38:13 -0800	[thread overview]
Message-ID: <20230104033815.35496-5-stephen@networkplumber.org> (raw)
In-Reply-To: <20230104033815.35496-1-stephen@networkplumber.org>

The code to handle multiple interfaces did not work before.
Need to initialize the list of interfaces while parsing the command
line. Each interface should the ability to have its own options.

Fixes another bug where dumpcap would always disable promiscious
on exit, even if set by the application.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/dumpcap/main.c | 259 +++++++++++++++++++++++++++++----------------
 1 file changed, 165 insertions(+), 94 deletions(-)

diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index a7aec021204a..86a4423b6de1 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -55,19 +55,15 @@ static const char *progname;
 static bool quit_signal;
 static bool group_read;
 static bool quiet;
-static bool promiscuous_mode = true;
 static bool use_pcapng = true;
 static char *output_name;
-static const char *filter_str;
 static unsigned int ring_size = 2048;
 static const char *capture_comment;
 static const char *file_prefix;
-static uint32_t snaplen = RTE_MBUF_DEFAULT_BUF_SIZE;
 static bool dump_bpf;
 static bool show_interfaces;
-static bool select_interfaces;
-const char *interface_arg;
 
+/* capture limit options */
 static struct {
 	uint64_t  duration;	/* nanoseconds */
 	unsigned long packets;  /* number of packets in file */
@@ -75,14 +71,25 @@ static struct {
 } stop;
 
 /* Running state */
-static struct rte_bpf_prm *bpf_prm;
 static uint64_t start_time, end_time;
 static uint64_t packets_received;
 static size_t file_size;
 
+/* capture options */
+struct capture_options {
+	const char *filter;
+	uint32_t snap_len;
+	bool promisc_mode;
+} capture = {
+	.snap_len = RTE_MBUF_DEFAULT_BUF_SIZE,
+	.promisc_mode = true,
+};
+
 struct interface {
 	TAILQ_ENTRY(interface) next;
 	uint16_t port;
+	struct capture_options opts;
+	struct rte_bpf_prm *bpf_prm;
 	char name[RTE_ETH_NAME_MAX_LEN];
 
 	struct rte_rxtx_callback *rx_cb[RTE_MAX_QUEUES_PER_PORT];
@@ -192,7 +199,7 @@ static void auto_stop(char *opt)
 }
 
 /* Add interface to list of interfaces to capture */
-static void add_interface(uint16_t port, const char *name)
+static struct interface *add_interface(const char *name)
 {
 	struct interface *intf;
 
@@ -204,24 +211,29 @@ static void add_interface(uint16_t port, const char *name)
 		rte_exit(EXIT_FAILURE, "no memory for interface\n");
 
 	memset(intf, 0, sizeof(*intf));
-	intf->port = port;
 	rte_strscpy(intf->name, name, sizeof(intf->name));
-
-	printf("Capturing on '%s'\n", name);
+	intf->opts = capture;
+	intf->port = -1;	/* port set later after EAL init */
 
 	TAILQ_INSERT_TAIL(&interfaces, intf, next);
+	return intf;
 }
 
-/* Select all valid DPDK interfaces */
-static void select_all_interfaces(void)
+/* Name has been set but need to lookup port after eal_init */
+static void find_interfaces(void)
 {
-	char name[RTE_ETH_NAME_MAX_LEN];
-	uint16_t p;
+	struct interface *intf;
 
-	RTE_ETH_FOREACH_DEV(p) {
-		if (rte_eth_dev_get_name_by_port(p, name) < 0)
-			continue;
-		add_interface(p, name);
+	TAILQ_FOREACH(intf, &interfaces, next) {
+		/* if name is valid then just record port */
+		if (rte_eth_dev_get_port_by_name(intf->name, &intf->port) == 0)
+ 			continue;
+
+		/* maybe got passed port number string as name */
+		intf->port = get_uint(intf->name, "port_number", UINT16_MAX);
+		if (rte_eth_dev_get_name_by_port(intf->port, intf->name) < 0)
+			rte_exit(EXIT_FAILURE, "Invalid port number %u\n",
+				 intf->port);
 	}
 }
 
@@ -231,38 +243,21 @@ static void select_all_interfaces(void)
  */
 static void set_default_interface(void)
 {
+	struct interface *intf;
 	char name[RTE_ETH_NAME_MAX_LEN];
 	uint16_t p;
 
 	RTE_ETH_FOREACH_DEV(p) {
 		if (rte_eth_dev_get_name_by_port(p, name) < 0)
 			continue;
-		add_interface(p, name);
+
+		intf = add_interface(name);
+		intf->port = p;
 		return;
 	}
 	rte_exit(EXIT_FAILURE, "No usable interfaces found\n");
 }
 
-/* Lookup interface by name or port and add it to the list */
-static void select_interface(const char *arg)
-{
-	uint16_t port;
-
-	if (strcmp(arg, "*") == 0)
-		select_all_interfaces();
-	else if (rte_eth_dev_get_port_by_name(arg, &port) == 0)
-		add_interface(port, arg);
-	else {
-		char name[RTE_ETH_NAME_MAX_LEN];
-
-		port = get_uint(arg, "port_number", UINT16_MAX);
-		if (rte_eth_dev_get_name_by_port(port, name) < 0)
-			rte_exit(EXIT_FAILURE, "Invalid port number %u\n",
-				 port);
-		add_interface(port, name);
-	}
-}
-
 /* Display list of possible interfaces that can be used. */
 static void dump_interfaces(void)
 {
@@ -278,37 +273,50 @@ static void dump_interfaces(void)
 	exit(0);
 }
 
-static void compile_filter(void)
+static void compile_filters(void)
 {
-	struct bpf_program bf;
-	pcap_t *pcap;
+	struct interface *intf;
 
-	pcap = pcap_open_dead(DLT_EN10MB, snaplen);
-	if (!pcap)
-		rte_exit(EXIT_FAILURE, "can not open pcap\n");
+	TAILQ_FOREACH(intf, &interfaces, next) {
+		struct rte_bpf_prm *bpf_prm;
+		struct bpf_program bf;
+		pcap_t *pcap;
 
-	if (pcap_compile(pcap, &bf, filter_str,
-			 1, PCAP_NETMASK_UNKNOWN) != 0)
-		rte_exit(EXIT_FAILURE, "pcap filter string not valid (%s)\n",
-			 pcap_geterr(pcap));
+		pcap = pcap_open_dead(DLT_EN10MB, intf->opts.snap_len);
+		if (!pcap)
+			rte_exit(EXIT_FAILURE, "can not open pcap\n");
 
-	bpf_prm = rte_bpf_convert(&bf);
-	if (bpf_prm == NULL)
-		rte_exit(EXIT_FAILURE,
-			 "bpf convert failed: %s(%d)\n",
-			 rte_strerror(rte_errno), rte_errno);
-
-	if (dump_bpf) {
-		printf("cBPF program (%u insns)\n", bf.bf_len);
-		bpf_dump(&bf, 1);
-		printf("\neBPF program (%u insns)\n", bpf_prm->nb_ins);
-		rte_bpf_dump(stdout, bpf_prm->ins, bpf_prm->nb_ins);
-		exit(0);
-	}
+		if (pcap_compile(pcap, &bf, intf->opts.filter,
+				 1, PCAP_NETMASK_UNKNOWN) != 0) {
+			fprintf(stderr,
+				"Invalid capture filter \"%s\": for interface '%s'\n",
+				intf->opts.filter, intf->name);
+			rte_exit(EXIT_FAILURE, "\n%s\n",
+				 pcap_geterr(pcap));
+		}
 
-	/* Don't care about original program any more */
-	pcap_freecode(&bf);
-	pcap_close(pcap);
+		bpf_prm = rte_bpf_convert(&bf);
+		if (bpf_prm == NULL)
+			rte_exit(EXIT_FAILURE,
+				 "BPF convert interface '%s'\n%s(%d)\n",
+				 intf->name,
+				 rte_strerror(rte_errno), rte_errno);
+
+		if (dump_bpf) {
+			printf("cBPF program (%u insns)\n", bf.bf_len);
+			bpf_dump(&bf, 1);
+			printf("\neBPF program (%u insns)\n",
+			       bpf_prm->nb_ins);
+			rte_bpf_dump(stdout, bpf_prm->ins, bpf_prm->nb_ins);
+			exit(0);
+		}
+
+		intf->bpf_prm = bpf_prm;
+
+		/* Don't care about original program any more */
+		pcap_freecode(&bf);
+		pcap_close(pcap);
+	}
 }
 
 /*
@@ -332,6 +340,8 @@ static void parse_opts(int argc, char **argv)
 		{ NULL },
 	};
 	int option_index, c;
+	struct interface *last_intf = NULL;
+	uint32_t len;
 
 	for (;;) {
 		c = getopt_long(argc, argv, "a:b:c:dDf:ghi:nN:pPqs:vw:",
@@ -369,7 +379,10 @@ static void parse_opts(int argc, char **argv)
 			show_interfaces = true;
 			break;
 		case 'f':
-			filter_str = optarg;
+			if (last_intf == NULL)
+				capture.filter = optarg;
+			else
+				last_intf->opts.filter = optarg;
 			break;
 		case 'g':
 			group_read = true;
@@ -379,8 +392,7 @@ static void parse_opts(int argc, char **argv)
 			usage();
 			exit(0);
 		case 'i':
-			select_interfaces = true;
-			interface_arg = optarg;
+			last_intf = add_interface(optarg);
 			break;
 		case 'n':
 			use_pcapng = true;
@@ -389,7 +401,18 @@ static void parse_opts(int argc, char **argv)
 			ring_size = get_uint(optarg, "packet_limit", 0);
 			break;
 		case 'p':
-			promiscuous_mode = false;
+			/* Like dumpcap this option can occur multiple times.
+			 *
+			 * If used before the first occurrence of the -i option,
+			 * no interface will be put into the promiscuous mode.
+			 * If used after an -i option, the interface specified
+			 * by the last -i option occurring before this option
+			 * will not be put into the promiscuous mode.
+			 */
+			if (last_intf == NULL)
+				capture.promisc_mode = false;
+			else
+				last_intf->opts.promisc_mode = false;
 			break;
 		case 'P':
 			use_pcapng = false;
@@ -398,7 +421,12 @@ static void parse_opts(int argc, char **argv)
 			quiet = true;
 			break;
 		case 's':
-			snaplen = get_uint(optarg, "snap_len", 0);
+			len = get_uint(optarg, "snap_len", 0);
+			if (last_intf == NULL)
+				capture.snap_len = len;
+			else
+				last_intf->opts.snap_len = len;
+
 			break;
 		case 'w':
 			output_name = optarg;
@@ -438,7 +466,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->opts.promisc_mode)
 			rte_eth_promiscuous_disable(intf->port);
 	}
 }
@@ -582,17 +610,27 @@ static struct rte_ring *create_ring(void)
 
 static struct rte_mempool *create_mempool(void)
 {
+	const struct interface *intf;
 	static const char pool_name[] = "capture_mbufs";
 	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;
 
+	/* Common pool so size mbuf for biggest snap length */
+	TAILQ_FOREACH(intf, &interfaces, next) {
+		uint32_t mbuf_size = rte_pcapng_mbuf_size(intf->opts.snap_len);
+
+		if (mbuf_size > data_size)
+			data_size = mbuf_size;
+	}
+
 	mp = rte_pktmbuf_pool_create_by_ops(pool_name, num_mbufs,
 					    MBUF_POOL_CACHE_SIZE, 0,
-					    rte_pcapng_mbuf_size(snaplen),
+					    data_size,
 					    rte_socket_id(), "ring_mp_sc");
 	if (mp == NULL)
 		rte_exit(EXIT_FAILURE,
@@ -673,7 +711,8 @@ static dumpcap_out_t create_output(void)
 	} else {
 		pcap_t *pcap;
 
-		pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB, snaplen,
+		pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB,
+							    capture.snap_len,
 							    PCAP_TSTAMP_PRECISION_NANO);
 		if (pcap == NULL)
 			rte_exit(EXIT_FAILURE, "pcap_open_dead failed\n");
@@ -690,6 +729,7 @@ static dumpcap_out_t create_output(void)
 static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
 {
 	struct interface *intf;
+	unsigned int count = 0;
 	uint32_t flags;
 	int ret;
 
@@ -698,22 +738,55 @@ static void enable_pdump(struct rte_ring *r, struct rte_mempool *mp)
 		flags |= RTE_PDUMP_FLAG_PCAPNG;
 
 	TAILQ_FOREACH(intf, &interfaces, next) {
-		if (promiscuous_mode) {
-			ret = rte_eth_promiscuous_enable(intf->port);
-			if (ret != 0)
-				fprintf(stderr,
-					"port %u set promiscuous enable failed: %d\n",
-					intf->port, ret);
-		}
-
 		ret = rte_pdump_enable_bpf(intf->port, RTE_PDUMP_ALL_QUEUES,
-					   flags, snaplen,
-					   r, mp, bpf_prm);
-		if (ret < 0)
+					   flags, intf->opts.snap_len,
+					   r, mp, intf->bpf_prm);
+		if (ret < 0) {
+			const struct interface *intf2;
+
+			/* unwind any previous enables */
+			TAILQ_FOREACH(intf2, &interfaces, next) {
+				if (intf == intf2)
+					break;
+				rte_pdump_disable(intf2->port,
+						  RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+				if (intf2->opts.promisc_mode)
+					rte_eth_promiscuous_disable(intf2->port);
+			}
 			rte_exit(EXIT_FAILURE,
-				 "Packet dump enable failed: %s\n",
-				 rte_strerror(-ret));
+				"Packet dump enable on %u:%s failed %s\n",
+				intf->port, intf->name,
+				rte_strerror(-ret));
+		}
+
+		if (intf->opts.promisc_mode) {
+			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);
+				intf->opts.promisc_mode = false;
+			}
+		}
+		++count;
+	}
+
+	fputs("Capturing on ", stdout);
+	TAILQ_FOREACH(intf, &interfaces, next) {
+		if (intf != TAILQ_FIRST(&interfaces)) {
+			if (count > 2)
+				putchar(',');
+			putchar(' ');
+			if (TAILQ_NEXT(intf, next) == NULL)
+				fputs("and ", stdout);
+		}
+		printf("'%s'", intf->name);
 	}
+	putchar('\n');
 }
 
 /*
@@ -736,7 +809,7 @@ static ssize_t
 pcap_write_packets(pcap_dumper_t *dumper,
 		   struct rte_mbuf *pkts[], uint16_t n)
 {
-	uint8_t temp_data[snaplen];
+	uint8_t temp_data[RTE_MBUF_DEFAULT_BUF_SIZE];
 	struct pcap_pkthdr header;
 	uint16_t i;
 	size_t total = 0;
@@ -747,7 +820,7 @@ pcap_write_packets(pcap_dumper_t *dumper,
 		struct rte_mbuf *m = pkts[i];
 
 		header.len = rte_pktmbuf_pkt_len(m);
-		header.caplen = RTE_MIN(header.len, snaplen);
+		header.caplen = RTE_MIN(header.len, sizeof(temp_data));
 
 		pcap_dump((u_char *)dumper, &header,
 			  rte_pktmbuf_read(m, 0, header.caplen, temp_data));
@@ -819,14 +892,12 @@ int main(int argc, char **argv)
 	if (rte_eth_dev_count_avail() == 0)
 		rte_exit(EXIT_FAILURE, "No Ethernet ports found\n");
 
-	if (select_interfaces)
-		select_interface(interface_arg);
-
-	if (filter_str)
-		compile_filter();
-
 	if (TAILQ_EMPTY(&interfaces))
 		set_default_interface();
+	else
+		find_interfaces();
+
+	compile_filters();
 
 	r = create_ring();
 	mp = create_mempool();
-- 
2.39.0


  parent reply	other threads:[~2023-01-04  3:38 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-02 16:24 [PATCH 1/6] app/dumpcap: add additional dump info Ben Magistro
2023-01-02 16:24 ` [PATCH 2/6] app/dumpcap: fix storing port identifier Ben Magistro
2023-01-02 16:58   ` Stephen Hemminger
2023-01-04  3:04   ` Stephen Hemminger
2023-01-02 16:24 ` [PATCH 3/6] app/dumpcap: fix preserving promiscuous mode Ben Magistro
2023-01-02 16:58   ` Stephen Hemminger
2023-01-04  3:04   ` Stephen Hemminger
2023-01-02 16:24 ` [PATCH 4/6] app/dumpcap: fix capturing on multiple interfaces Ben Magistro
2023-01-04  3:01   ` Stephen Hemminger
2023-01-02 16:24 ` [PATCH 5/6] app/dumpcap: improve per interface arg parsing Ben Magistro
2023-01-04  3:04   ` Stephen Hemminger
2023-01-02 16:24 ` [PATCH 6/6] app/dumpcap: refactor add all and default Ben Magistro
2023-01-02 16:57 ` [PATCH 1/6] app/dumpcap: add additional dump info Stephen Hemminger
2023-01-02 17:01 ` [RFT] dumpcap: fix multiple interface and promiscious handling Stephen Hemminger
2023-01-04  2:58 ` [PATCH 1/6] app/dumpcap: add additional dump info Stephen Hemminger
2023-01-04  3:38 ` [PATCH v2 0/6] dumpcap support multiple interfaces Stephen Hemminger
2023-01-04  3:38   ` [PATCH v2 1/6] app/dumpcap: fix storing port identifier Stephen Hemminger
2023-01-04  3:38   ` [PATCH v2 2/6] app/dumpcap: remove unused variable Stephen Hemminger
2023-01-04  3:38   ` [PATCH v2 3/6] app/dumpcap: check for invalid interface name Stephen Hemminger
2023-01-04  3:38   ` Stephen Hemminger [this message]
2023-01-04  3:38   ` [PATCH v2 5/6] pcapng: require per-interface information Stephen Hemminger
2023-01-04  3:38   ` [PATCH v2 6/6] app/dumpcap: support interface name and description Stephen Hemminger
2023-02-06 11:18   ` [PATCH v2 0/6] dumpcap support multiple interfaces Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230104033815.35496-5-stephen@networkplumber.org \
    --to=stephen@networkplumber.org \
    --cc=dev@dpdk.org \
    --cc=koncept1@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).