[dpdk-dev] [PATCH 3/3] app/testpmd: add sanity checks on received/sent packets

[David Marchand <david.marchand@6wind.com>]

Make use of the newly introduced rte_mbuf_check() to (optionally) check
all packets received/sent through a port.
The idea behind this is to help to quickly identify badly formatted mbufs
coming from the pmd on the rx side, and from the application on the tx
side.
Setting the verbose level to some > 0 value will dump all packets in the
associated rx/tx callback to further help in the debugging.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 app/test-pmd/cmdline.c    |  63 +++++++++++++++++++
 app/test-pmd/config.c     |  23 +++++++
 app/test-pmd/parameters.c |   7 +++
 app/test-pmd/testpmd.c    | 123 ++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |   9 +++
 5 files changed, 225 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 589121d69..1de533999 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -912,6 +912,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"port config (port_id) udp_tunnel_port add|rm vxlan|geneve (udp_port)\n\n"
 			"    Add/remove UDP tunnel port for tunneling offload\n\n"
+
+			"port config all sanity_check (none|rx|tx|rx+tx)\n\n"
+			"    Configure sanity checks\n\n"
 		);
 	}
 
@@ -17602,6 +17605,65 @@ cmdline_parse_inst_t cmd_config_per_queue_tx_offload = {
 	}
 };
 
+/* *** configure sanity check for all ports *** */
+struct cmd_config_sanity_check_all {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t all;
+	cmdline_fixed_string_t sanity_check;
+	cmdline_fixed_string_t mode;
+};
+
+static void
+cmd_config_sanity_check_all_parsed(void *parsed_result,
+	__rte_unused struct cmdline *cl, __rte_unused void *data)
+{
+	struct cmd_config_sanity_check_all *res = parsed_result;
+	portid_t pid;
+
+	if (!all_ports_stopped()) {
+		printf("Please stop all ports first\n");
+		return;
+	}
+
+	if (set_sanity_checks(res->mode) < 0)
+		return;
+
+	RTE_ETH_FOREACH_DEV(pid)
+		ports[pid].sanity_checks = sanity_checks;
+
+	cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1);
+}
+
+cmdline_parse_token_string_t cmd_config_sanity_check_all_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_speed_all, port, "port");
+cmdline_parse_token_string_t cmd_config_sanity_check_all_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, keyword,
+		"config");
+cmdline_parse_token_string_t cmd_config_sanity_check_all_all =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, all,
+		"all");
+cmdline_parse_token_string_t cmd_config_sanity_check_all_sanity_check =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all,
+		sanity_check, "sanity_check");
+cmdline_parse_token_string_t cmd_config_sanity_check_all_mode =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, mode,
+		"none#rx#tx#rx+tx");
+
+cmdline_parse_inst_t cmd_config_sanity_check_all = {
+	.f = cmd_config_sanity_check_all_parsed,
+	.data = NULL,
+	.help_str = "port config all sanity_check none|rx|tx|rx+tx",
+	.tokens = {
+		(void *)&cmd_config_sanity_check_all_port,
+		(void *)&cmd_config_sanity_check_all_keyword,
+		(void *)&cmd_config_sanity_check_all_all,
+		(void *)&cmd_config_sanity_check_all_sanity_check,
+		(void *)&cmd_config_sanity_check_all_mode,
+		NULL,
+	},
+};
+
 /* ******************************************************************************** */
 
 /* list of instructions */
@@ -17863,6 +17925,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_tx_offload_get_configuration,
 	(cmdline_parse_inst_t *)&cmd_config_per_port_tx_offload,
 	(cmdline_parse_inst_t *)&cmd_config_per_queue_tx_offload,
+	(cmdline_parse_inst_t *)&cmd_config_sanity_check_all,
 #ifdef RTE_LIBRTE_BPF
 	(cmdline_parse_inst_t *)&cmd_operate_bpf_ld_parse,
 	(cmdline_parse_inst_t *)&cmd_operate_bpf_unld_parse,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 14ccd6864..f34327d02 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1849,6 +1849,9 @@ rxtx_config_display(void)
 		printf("  port %d: RX queue number: %d Tx queue number: %d\n",
 				(unsigned int)pid, nb_rxq, nb_txq);
 
+		if (ports[pid].sanity_checks & SANITY_CHECK_RX)
+			printf("    RX sanity checks enabled\n");
+
 		printf("    Rx offloads=0x%"PRIx64" Tx offloads=0x%"PRIx64"\n",
 				ports[pid].dev_conf.rxmode.offloads,
 				ports[pid].dev_conf.txmode.offloads);
@@ -1873,6 +1876,9 @@ rxtx_config_display(void)
 				rx_conf[qid].offloads);
 		}
 
+		if (ports[pid].sanity_checks & SANITY_CHECK_TX)
+			printf("    TX sanity checks enabled\n");
+
 		/* per tx queue config only for first queue to be less verbose */
 		for (qid = 0; qid < 1; qid++) {
 			rc = rte_eth_tx_queue_info_get(pid, qid, &tx_qinfo);
@@ -3827,3 +3833,20 @@ port_queue_region_info_display(portid_t port_id, void *buf)
 
 	printf("\n\n");
 }
+
+int
+set_sanity_checks(const char *arg)
+{
+	if (!strcmp(arg, "rx")) {
+		sanity_checks = SANITY_CHECK_RX;
+		return 0;
+	} else if (!strcmp(arg, "tx")) {
+		sanity_checks = SANITY_CHECK_TX;
+		return 0;
+	} else if (!strcmp(arg, "rx+tx")) {
+		sanity_checks =
+			SANITY_CHECK_RX|SANITY_CHECK_TX;
+		return 0;
+	} else
+		return -1;
+}
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 962fad789..5a06dc592 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -190,6 +190,7 @@ usage(char* progname)
 	printf("  --vxlan-gpe-port=N: UPD port of tunnel VXLAN-GPE\n");
 	printf("  --mlockall: lock all memory\n");
 	printf("  --no-mlockall: do not lock all memory\n");
+	printf("  --sanity-checks <rx|tx|rx+tx>: enable rx/tx sanity checks on mbuf\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -625,6 +626,7 @@ launch_args_parse(int argc, char** argv)
 		{ "vxlan-gpe-port",		1, 0, 0 },
 		{ "mlockall",			0, 0, 0 },
 		{ "no-mlockall",		0, 0, 0 },
+		{ "sanity-checks",		1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1147,6 +1149,11 @@ launch_args_parse(int argc, char** argv)
 				do_mlockall = 1;
 			if (!strcmp(lgopts[opt_idx].name, "no-mlockall"))
 				do_mlockall = 0;
+			if (!strcmp(lgopts[opt_idx].name, "sanity-checks")) {
+				if (set_sanity_checks(optarg) < 0)
+					rte_exit(EXIT_FAILURE,
+						 "invalid sanity-checks argument\n");
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index ee48db2a3..a310431eb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -210,6 +210,9 @@ uint8_t dcb_test = 0;
 queueid_t nb_rxq = 1; /**< Number of RX queues per port. */
 queueid_t nb_txq = 1; /**< Number of TX queues per port. */
 
+/* Sanity checks configuration */
+uint8_t sanity_checks;
+
 /*
  * Configurable number of RX/TX ring descriptors.
  * Defaults are supplied by drivers via ethdev.
@@ -769,6 +772,9 @@ init_config(void)
 			port->tx_conf[k].offloads =
 				port->dev_conf.txmode.offloads;
 
+		/* Configure sanity checks with initial value from cmdline */
+		port->sanity_checks = sanity_checks;
+
 		/* set flag to initialize port/queue */
 		port->need_reconfig = 1;
 		port->need_reconfig_queues = 1;
@@ -1632,6 +1638,59 @@ port_is_closed(portid_t port_id)
 	return 1;
 }
 
+static uint16_t
+mbuf_tx_check(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
+	      uint16_t nb_pkts, __rte_unused void *user_param)
+{
+	unsigned int count = 0;
+	unsigned int i;
+
+	for (i = 0; i < nb_pkts; i++) {
+		const char *reason;
+
+		if (verbose_level > 0)
+			rte_pktmbuf_dump(stdout, pkts[i], 0);
+
+		if (!rte_mbuf_check(pkts[i], 1, &reason)) {
+			pkts[count++] = pkts[i];
+			continue;
+		}
+
+		TESTPMD_LOG(ERR, "invalid tx mbuf on port %"PRIu16" queue %"
+			    PRIu16": %s\n", port_id, queue, reason);
+		rte_pktmbuf_free(pkts[i]);
+	}
+
+	return count;
+}
+
+static uint16_t
+mbuf_rx_check(uint16_t port_id, uint16_t queue, struct rte_mbuf *pkts[],
+	      uint16_t nb_pkts, __rte_unused uint16_t max_pkts,
+	      __rte_unused void *user_param)
+{
+	unsigned int count = 0;
+	unsigned int i;
+
+	for (i = 0; i < nb_pkts; i++) {
+		const char *reason;
+
+		if (verbose_level > 0)
+			rte_pktmbuf_dump(stdout, pkts[i], 0);
+
+		if (!rte_mbuf_check(pkts[i], 1, &reason)) {
+			pkts[count++] = pkts[i];
+			continue;
+		}
+
+		TESTPMD_LOG(ERR, "invalid rx mbuf on port %"PRIu16" queue %"
+			    PRIu16": %s\n", port_id, queue, reason);
+		rte_pktmbuf_free(pkts[i]);
+	}
+
+	return count;
+}
+
 int
 start_port(portid_t pid)
 {
@@ -1641,6 +1700,7 @@ start_port(portid_t pid)
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 	enum rte_eth_event_type event_type;
+	struct rte_eth_dev_info dev_info;
 
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
@@ -1671,6 +1731,32 @@ start_port(portid_t pid)
 				}
 			}
 
+			/* Free any remaining rx/tx callbacks before changing
+			 * rxq/txq count.
+			 */
+			rte_eth_dev_info_get(pi, &dev_info);
+			for (qi = 0; qi < dev_info.nb_tx_queues; qi++) {
+				if (!port->tx_checks_cb[qi])
+					continue;
+				if (rte_eth_remove_tx_callback(pi, qi,
+						port->tx_checks_cb[qi]) < 0) {
+					/* try to reconfigure port next time */
+					port->need_reconfig = 1;
+					return -1;
+				}
+				port->tx_checks_cb[qi] = NULL;
+			}
+			for (qi = 0; qi < dev_info.nb_rx_queues; qi++) {
+				if (!port->rx_checks_cb[qi])
+					continue;
+				if (rte_eth_remove_rx_callback(pi, qi,
+						port->rx_checks_cb[qi]) < 0) {
+					/* try to reconfigure port next time */
+					port->need_reconfig = 1;
+					return -1;
+				}
+				port->rx_checks_cb[qi] = NULL;
+			}
 			printf("Configuring Port %d (socket %u)\n", pi,
 					port->socket_id);
 			/* configure port */
@@ -1703,6 +1789,24 @@ start_port(portid_t pid)
 						port->socket_id,
 						&(port->tx_conf[qi]));
 
+				if (diag == 0 &&
+				    port->tx_checks_cb[qi]) {
+					if (!rte_eth_remove_tx_callback(pi, qi,
+							port->tx_checks_cb[qi]))
+						port->tx_checks_cb[qi] = NULL;
+					else
+						diag = -1;
+				}
+
+				if (diag == 0 &&
+				    port->sanity_checks & SANITY_CHECK_TX) {
+					port->tx_checks_cb[qi] =
+						rte_eth_add_tx_callback(pi, qi,
+							mbuf_tx_check, NULL);
+					if (!port->tx_checks_cb[qi])
+						diag = -1;
+				}
+
 				if (diag == 0)
 					continue;
 
@@ -1753,6 +1857,25 @@ start_port(portid_t pid)
 					     &(port->rx_conf[qi]),
 					     mp);
 				}
+
+				if (diag == 0 &&
+				    port->rx_checks_cb[qi]) {
+					if (!rte_eth_remove_rx_callback(pi, qi,
+							port->rx_checks_cb[qi]))
+						port->rx_checks_cb[qi] = NULL;
+					else
+						diag = -1;
+				}
+
+				if (diag == 0 &&
+				    port->sanity_checks & SANITY_CHECK_RX) {
+					port->rx_checks_cb[qi] =
+						rte_eth_add_rx_callback(pi, qi,
+							mbuf_rx_check, NULL);
+					if (!port->rx_checks_cb[qi])
+						diag = -1;
+				}
+
 				if (diag == 0)
 					continue;
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a1f661472..bdab372b2 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -180,6 +180,11 @@ struct rte_port {
 	uint32_t                mc_addr_nb; /**< nb. of addr. in mc_addr_pool */
 	uint8_t                 slave_flag; /**< bonding slave port */
 	struct port_flow        *flow_list; /**< Associated flows. */
+#define SANITY_CHECK_RX ((uint8_t)1 << 0)
+#define SANITY_CHECK_TX ((uint8_t)1 << 1)
+	uint8_t                 sanity_checks;
+	const struct rte_eth_rxtx_callback *rx_checks_cb[MAX_QUEUE_ID+1];
+	const struct rte_eth_rxtx_callback *tx_checks_cb[MAX_QUEUE_ID+1];
 #ifdef SOFTNIC
 	struct softnic_port     softport;  /**< softnic params */
 #endif
@@ -378,6 +383,8 @@ extern int16_t tx_rs_thresh;
 extern uint8_t dcb_config;
 extern uint8_t dcb_test;
 
+extern uint8_t sanity_checks;
+
 extern uint16_t mbuf_data_size; /**< Mbuf data space size. */
 extern uint32_t param_total_num_mbufs;
 
@@ -730,6 +737,8 @@ int close_file(uint8_t *buf);
 
 void port_queue_region_info_display(portid_t port_id, void *buf);
 
+int set_sanity_checks(const char *arg);
+
 enum print_warning {
 	ENABLED_WARN = 0,
 	DISABLED_WARN
-- 
2.17.1