trace: allow enabling packet trace at runtime

If the -x, --trace-packets flag is used, enable packet tracing instead
of relying on an undocumented TRACE_PACKETS compilation flag.

Limited testing shows that the performance seems unaffected when this
flag is not used.

Signed-off-by: Robin Jarry <rjarry@redhat.com>

.github/workflows/ci.yml

@@ -15,7 +15,7 @@ jobs:
     env:
       SANITIZE: none
       BUILDTYPE: debugoptimized
-      MESON_EXTRA_OPTS: "-Dc_args=-DTRACE_PACKETS -Ddpdk:platform=generic"
+      MESON_EXTRA_OPTS: "-Ddpdk:platform=generic"
       DEBIAN_FRONTEND: noninteractive
       NEEDRESTART_MODE: l
     steps:

docs/grout.1.scd

@@ -14,7 +14,7 @@ Grout is a software router based on DPDK rte_graph.
 
 ; Please keep flags/options in alphabetical order.
 
-*grout* [*-h*] [*-p*] [*-s* _PATH_] [*-t*] [*-v*]
+*grout* [*-h*] [*-p*] [*-s* _PATH_] [*-t*] [*-v*] [*-x*]
 
 # OPTIONS
 
@@ -30,6 +30,8 @@ Grout is a software router based on DPDK rte_graph.
 	Run in test mode (no huge pages).
 *-v*, *--verbose*
 	Increase verbosity. Can be specified multiple times.
+*-x*, *--trace-packets*
+	Print all ingress/egress packets (for debugging purposes).
 
 # AUTHORS
 

main/gr_log.h

@@ -52,4 +52,6 @@ static inline void *errno_set_null(int errnum) {
 	return NULL;
 }
 
+extern bool packet_trace_enabled;
+
 #endif

main/main.c

@@ -32,7 +32,7 @@
 // Please keep options/flags in alphabetical order.
 
 static void usage(const char *prog) {
-	printf("Usage: %s [-h] [-p] [-s PATH] [-t] [-v]\n", prog);
+	printf("Usage: %s [-h] [-p] [-s PATH] [-t] [-v] [-x]\n", prog);
 	puts("");
 	printf("  Graph router version %s.\n", GROUT_VERSION);
 	puts("");
@@ -44,9 +44,11 @@ static void usage(const char *prog) {
 	printf("                             %s).\n", GR_DEFAULT_SOCK_PATH);
 	puts("  -t, --test-mode            Run in test mode (no hugepages).");
 	puts("  -v, --verbose              Increase verbosity.");
+	puts("  -x, --trace-packets        Print all ingress/egress packets.");
 }
 
 static struct gr_args args;
+bool packet_trace_enabled;
 
 const struct gr_args *gr_args(void) {
 	return &args;
@@ -55,13 +57,14 @@ const struct gr_args *gr_args(void) {
 static int parse_args(int argc, char **argv) {
 	int c;
 
-#define FLAGS ":hps:tv"
+#define FLAGS ":hps:tvx"
 	static struct option long_options[] = {
 		{"help", no_argument, NULL, 'h'},
 		{"poll-mode", no_argument, NULL, 'p'},
 		{"socket", required_argument, NULL, 's'},
 		{"test-mode", no_argument, NULL, 't'},
 		{"verbose", no_argument, NULL, 'v'},
+		{"trace-packets", no_argument, NULL, 'x'},
 		{0},
 	};
 
@@ -87,6 +90,9 @@ static int parse_args(int argc, char **argv) {
 		case 'v':
 			args.log_level++;
 			break;
+		case 'x':
+			packet_trace_enabled = true;
+			break;
 		case ':':
 			usage(argv[0]);
 			fprintf(stderr, "error: -%c requires a value", optopt);

modules/infra/datapath/drop.c

@@ -12,9 +12,9 @@ drop_packets(struct rte_graph *graph, struct rte_node *node, void **objs, uint16
 	(void)node;
 	(void)graph;
 
-#ifdef TRACE_PACKETS
-	LOG(NOTICE, "[%s] %u packets", node->name, nb_objs);
-#endif
+	if (unlikely(packet_trace_enabled)) {
+		LOG(NOTICE, "[%s] %u packets", node->name, nb_objs);
+	}
 	rte_pktmbuf_free_bulk((struct rte_mbuf **)objs, nb_objs);
 
 	return nb_objs;

modules/infra/datapath/eth_output.c

@@ -6,6 +6,7 @@
 
 #include <gr_graph.h>
 #include <gr_iface.h>
+#include <gr_log.h>
 #include <gr_port.h>
 #include <gr_vlan.h>
 
@@ -68,7 +69,8 @@ eth_output_process(struct rte_graph *graph, struct rte_node *node, void **objs,
 		rte_ether_addr_copy(src_mac, &eth->src_addr);
 		eth->ether_type = priv->ether_type;
 		mbuf->port = port->port_id;
-		trace_packet("tx", priv->iface->name, mbuf);
+		if (unlikely(packet_trace_enabled))
+			trace_packet("tx", priv->iface->name, mbuf);
 		rte_node_enqueue_x1(graph, node, TX, mbuf);
 	}
 

modules/infra/datapath/gr_datapath.h

@@ -4,155 +4,10 @@
 #ifndef _GR_INFRA_DATAPATH
 #define _GR_INFRA_DATAPATH
 
-#include <gr_log.h>
-#include <gr_net_types.h>
-
-#include <rte_arp.h>
-#include <rte_byteorder.h>
-#include <rte_errno.h>
-#include <rte_ether.h>
-#include <rte_graph_worker.h>
-#include <rte_icmp.h>
-#include <rte_ip.h>
 #include <rte_mbuf.h>
 
 void *gr_datapath_loop(void *priv);
 
-#ifdef TRACE_PACKETS
-static inline void trace_packet(const char *node, const char *iface, const struct rte_mbuf *m) {
-	char buf[BUFSIZ], src[64], dst[64];
-	const struct rte_ether_hdr *eth;
-	uint16_t ether_type;
-	size_t offset = 0;
-	ssize_t n = 0;
-
-	eth = rte_pktmbuf_mtod_offset(m, const struct rte_ether_hdr *, offset);
-	offset += sizeof(*eth);
-	ether_type = rte_be_to_cpu_16(eth->ether_type);
-
-	n += snprintf(
-		buf + n,
-		sizeof(buf) - n,
-		ETH_ADDR_FMT " > " ETH_ADDR_FMT,
-		ETH_BYTES_SPLIT(eth->src_addr.addr_bytes),
-		ETH_BYTES_SPLIT(eth->dst_addr.addr_bytes)
-	);
-
-	if (ether_type == RTE_ETHER_TYPE_VLAN) {
-		const struct rte_vlan_hdr *vlan;
-		uint16_t vlan_id;
-
-		vlan = rte_pktmbuf_mtod_offset(m, const struct rte_vlan_hdr *, offset);
-		offset += sizeof(*vlan);
-		vlan_id = rte_be_to_cpu_16(vlan->vlan_tci) & 0xfff;
-		ether_type = rte_be_to_cpu_16(vlan->eth_proto);
-		n += snprintf(buf + n, sizeof(buf) - n, " / VLAN id=%u", vlan_id);
-	}
-
-	switch (ether_type) {
-	case RTE_ETHER_TYPE_IPV4: {
-ipv4:
-		const struct rte_ipv4_hdr *ip;
-
-		ip = rte_pktmbuf_mtod_offset(m, const struct rte_ipv4_hdr *, offset);
-		offset += sizeof(*ip);
-		inet_ntop(AF_INET, &ip->src_addr, src, sizeof(src));
-		inet_ntop(AF_INET, &ip->dst_addr, dst, sizeof(dst));
-		n += snprintf(
-			buf + n,
-			sizeof(buf) - n,
-			" / IP %s > %s ttl=%hhu",
-			src,
-			dst,
-			ip->time_to_live
-		);
-
-		switch (ip->next_proto_id) {
-		case IPPROTO_ICMP: {
-			const struct rte_icmp_hdr *icmp;
-			icmp = rte_pktmbuf_mtod_offset(m, const struct rte_icmp_hdr *, offset);
-			n += snprintf(buf + n, sizeof(buf) - n, " / ICMP");
-
-			if (icmp->icmp_type == RTE_IP_ICMP_ECHO_REQUEST && icmp->icmp_code == 0) {
-				n += snprintf(buf + n, sizeof(buf) - n, " echo request");
-			} else if (icmp->icmp_type == RTE_IP_ICMP_ECHO_REPLY
-				   && icmp->icmp_code == 0) {
-				n += snprintf(buf + n, sizeof(buf) - n, " echo reply");
-			} else {
-				n += snprintf(
-					buf + n,
-					sizeof(buf) - n,
-					" type=%hhu code=%hhu",
-					icmp->icmp_type,
-					icmp->icmp_code
-				);
-			}
-			n += snprintf(
-				buf + n,
-				sizeof(buf) - n,
-				" id=%u seq=%u",
-				rte_be_to_cpu_16(icmp->icmp_ident),
-				rte_be_to_cpu_16(icmp->icmp_seq_nb)
-			);
-			break;
-		}
-		case IPPROTO_IPIP:
-			goto ipv4;
-		default:
-			n += snprintf(buf + n, sizeof(buf) - n, " proto=%hhu", ip->next_proto_id);
-			break;
-		}
-
-		break;
-	}
-	case RTE_ETHER_TYPE_ARP: {
-		const struct rte_arp_hdr *arp;
-
-		arp = rte_pktmbuf_mtod_offset(m, const struct rte_arp_hdr *, offset);
-
-		switch (rte_be_to_cpu_16(arp->arp_opcode)) {
-		case RTE_ARP_OP_REQUEST:
-			inet_ntop(AF_INET, &arp->arp_data.arp_sip, src, sizeof(src));
-			inet_ntop(AF_INET, &arp->arp_data.arp_tip, dst, sizeof(dst));
-			n += snprintf(
-				buf + n,
-				sizeof(buf) - n,
-				" / ARP request who has %s? tell %s",
-				dst,
-				src
-			);
-			break;
-		case RTE_ARP_OP_REPLY:
-			inet_ntop(AF_INET, &arp->arp_data.arp_sip, src, sizeof(src));
-			n += snprintf(
-				buf + n,
-				sizeof(buf) - n,
-				" / ARP reply %s is at " ETH_ADDR_FMT,
-				src,
-				ETH_BYTES_SPLIT(eth->src_addr.addr_bytes)
-			);
-			break;
-		default:
-			n += snprintf(
-				buf + n,
-				sizeof(buf) - n,
-				" / ARP opcode=%u",
-				rte_be_to_cpu_16(arp->arp_opcode)
-			);
-			break;
-		}
-		break;
-	}
-	default:
-		n += snprintf(buf + n, sizeof(buf) - n, " type=0x%04x", ether_type);
-		break;
-	}
-	n += snprintf(buf + n, sizeof(buf) - n, ", (pkt_len=%u)", m->pkt_len);
-
-	LOG(NOTICE, "[%s %s] %s", node, iface, buf);
-}
-#else
-#define trace_packet(node, iface, mbuf)
-#endif // TRACE_PACKETS
+void trace_packet(const char *node, const char *iface, const struct rte_mbuf *m);
 
 #endif

modules/infra/datapath/meson.build

@@ -8,6 +8,7 @@ src += files(
   'main_loop.c',
   'mbuf.c',
   'rx.c',
+  'trace.c',
   'tx.c',
 )
 inc += include_directories('.')

modules/infra/datapath/rx.c

@@ -38,6 +38,7 @@ rx_process(struct rte_graph *graph, struct rte_node *node, void **objs, uint16_t
 	const struct iface *iface;
 	struct rx_port_queue q;
 	uint16_t rx;
+	unsigned r;
 
 	(void)objs;
 
@@ -52,10 +53,15 @@ rx_process(struct rte_graph *graph, struct rte_node *node, void **objs, uint16_t
 			rte_node_enqueue(graph, node, NO_IFACE, &node->objs[count], rx);
 			continue;
 		}
-		for (int r = count; r < count + rx; r++) {
+		for (r = count; r < count + rx; r++) {
 			eth_input_mbuf_data(node->objs[r])->iface = iface;
-			trace_packet("rx", iface->name, node->objs[r]);
 		}
+		if (unlikely(packet_trace_enabled)) {
+			for (r = count; r < count + rx; r++) {
+				trace_packet("rx", iface->name, node->objs[r]);
+			}
+		}
+
 		count += rx;
 	}