From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 82D231B156 for ; Tue, 25 Sep 2018 15:17:16 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Sep 2018 06:17:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,302,1534834800"; d="scan'208";a="93083566" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by fmsmga001.fm.intel.com with ESMTP; 25 Sep 2018 06:17:04 -0700 Received: from irsmsx107.ger.corp.intel.com ([169.254.10.116]) by IRSMSX151.ger.corp.intel.com ([169.254.4.94]) with mapi id 14.03.0319.002; Tue, 25 Sep 2018 14:17:03 +0100 From: "Iremonger, Bernard" To: David Marchand , "dev@dpdk.org" CC: "olivier.matz@6wind.com" , "Lu, Wenzhuo" , "Wu, Jingjing" Thread-Topic: [PATCH 3/3] app/testpmd: add sanity checks on received/sent packets Thread-Index: AQHUSMsp+yQa11N4RU6igNSNivSr86UBD5tw Date: Tue, 25 Sep 2018 13:17:02 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C260CF34F5@IRSMSX107.ger.corp.intel.com> References: <20180910054547.18494-1-david.marchand@6wind.com> <20180910054547.18494-4-david.marchand@6wind.com> In-Reply-To: <20180910054547.18494-4-david.marchand@6wind.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzM5ODAwODAtMGYyNC00NzgyLWE1ZGMtMzkwNDM5ZWI1NjhjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibk1HTGhGVitCRm8rUG9jUmo5ZjhjUTNwbDJkZmZwT0xPYmg0dnRWSnAwTEsyUXMrV3ZiMWtRaWNnMWd6d204dCJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: add sanity checks on received/sent packets X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Sep 2018 13:17:17 -0000 Hi David, > -----Original Message----- > From: David Marchand [mailto:david.marchand@6wind.com] > Sent: Monday, September 10, 2018 6:46 AM > To: dev@dpdk.org > Cc: olivier.matz@6wind.com; Lu, Wenzhuo ; Wu, > Jingjing ; Iremonger, Bernard > > Subject: [PATCH 3/3] app/testpmd: add sanity checks on received/sent > packets >=20 > Make use of the newly introduced rte_mbuf_check() to (optionally) check a= ll > 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 si= de. > Setting the verbose level to some > 0 value will dump all packets in the > associated rx/tx callback to further help in the debugging. >=20 > Signed-off-by: David Marchand > --- > 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(+) There should probably be an entry in section 4.6 of the Testpmd User Guide = for the=20 "port config all sanity_check" command. >=20 > 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, >=20 > "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" > ); > } >=20 > @@ -17602,6 +17605,65 @@ cmdline_parse_inst_t > cmd_config_per_queue_tx_offload =3D { > } > }; >=20 > +/* *** 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 =3D 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 =3D sanity_checks; > + > + cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1); } > + > +cmdline_parse_token_string_t cmd_config_sanity_check_all_port =3D > + TOKEN_STRING_INITIALIZER(struct cmd_config_speed_all, port, > "port"); > +cmdline_parse_token_string_t cmd_config_sanity_check_all_keyword =3D > + TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, > keyword, > + "config"); > +cmdline_parse_token_string_t cmd_config_sanity_check_all_all =3D > + TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, all, > + "all"); > +cmdline_parse_token_string_t cmd_config_sanity_check_all_sanity_check > =3D > + TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, > + sanity_check, "sanity_check"); > +cmdline_parse_token_string_t cmd_config_sanity_check_all_mode =3D > + TOKEN_STRING_INITIALIZER(struct cmd_config_sanity_check_all, > mode, > + "none#rx#tx#rx+tx"); > + > +cmdline_parse_inst_t cmd_config_sanity_check_all =3D { > + .f =3D cmd_config_sanity_check_all_parsed, > + .data =3D NULL, > + .help_str =3D "port config all sanity_check none|rx|tx|rx+tx", > + .tokens =3D { > + (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, > + }, > +}; > + > /* > ********************************************************** > ********************** */ >=20 > /* list of instructions */ > @@ -17863,6 +17925,7 @@ cmdline_parse_ctx_t main_ctx[] =3D { > (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); >=20 > + if (ports[pid].sanity_checks & SANITY_CHECK_RX) > + printf(" RX sanity checks enabled\n"); > + > printf(" Rx offloads=3D0x%"PRIx64" Tx > offloads=3D0x%"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); > } >=20 > + 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 =3D 0; qid < 1; qid++) { > rc =3D 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) >=20 > printf("\n\n"); > } > + > +int > +set_sanity_checks(const char *arg) > +{ > + if (!strcmp(arg, "rx")) { > + sanity_checks =3D SANITY_CHECK_RX; > + return 0; > + } else if (!strcmp(arg, "tx")) { > + sanity_checks =3D SANITY_CHECK_TX; > + return 0; > + } else if (!strcmp(arg, "rx+tx")) { > + sanity_checks =3D > + 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=3DN: 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 : enable rx/tx sanity checks on > +mbuf\n"); > } >=20 > #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 }, > }; >=20 > @@ -1147,6 +1149,11 @@ launch_args_parse(int argc, char** argv) > do_mlockall =3D 1; > if (!strcmp(lgopts[opt_idx].name, "no-mlockall")) > do_mlockall =3D 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 =3D 0; queueid_t nb_rxq =3D 1; /**< > Number of RX queues per port. */ queueid_t nb_txq =3D 1; /**< Number of > TX queues per port. */ >=20 > +/* 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 =3D > port->dev_conf.txmode.offloads; >=20 > + /* Configure sanity checks with initial value from cmdline */ > + port->sanity_checks =3D sanity_checks; > + > /* set flag to initialize port/queue */ > port->need_reconfig =3D 1; > port->need_reconfig_queues =3D 1; > @@ -1632,6 +1638,59 @@ port_is_closed(portid_t port_id) > return 1; > } >=20 > +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 =3D 0; > + unsigned int i; > + > + for (i =3D 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++] =3D 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 =3D 0; > + unsigned int i; > + > + for (i =3D 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++] =3D 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; >=20 > if (port_id_is_invalid(pid, ENABLED_WARN)) > return 0; > @@ -1671,6 +1731,32 @@ start_port(portid_t pid) > } > } >=20 > + /* Free any remaining rx/tx callbacks before changing > + * rxq/txq count. > + */ > + rte_eth_dev_info_get(pi, &dev_info); > + for (qi =3D 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 =3D 1; > + return -1; > + } > + port->tx_checks_cb[qi] =3D NULL; > + } > + for (qi =3D 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 =3D 1; > + return -1; > + } > + port->rx_checks_cb[qi] =3D 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])); >=20 > + if (diag =3D=3D 0 && > + port->tx_checks_cb[qi]) { > + if (!rte_eth_remove_tx_callback(pi, > qi, > + port- > >tx_checks_cb[qi])) > + port->tx_checks_cb[qi] =3D > NULL; > + else > + diag =3D -1; > + } > + > + if (diag =3D=3D 0 && > + port->sanity_checks & SANITY_CHECK_TX) > { > + port->tx_checks_cb[qi] =3D > + rte_eth_add_tx_callback(pi, > qi, > + mbuf_tx_check, > NULL); > + if (!port->tx_checks_cb[qi]) > + diag =3D -1; > + } > + > if (diag =3D=3D 0) > continue; >=20 > @@ -1753,6 +1857,25 @@ start_port(portid_t pid) > &(port->rx_conf[qi]), > mp); > } > + > + if (diag =3D=3D 0 && > + port->rx_checks_cb[qi]) { > + if (!rte_eth_remove_rx_callback(pi, > qi, > + port- > >rx_checks_cb[qi])) > + port->rx_checks_cb[qi] =3D > NULL; > + else > + diag =3D -1; > + } > + > + if (diag =3D=3D 0 && > + port->sanity_checks & SANITY_CHECK_RX) > { > + port->rx_checks_cb[qi] =3D > + rte_eth_add_rx_callback(pi, > qi, > + mbuf_rx_check, > NULL); > + if (!port->rx_checks_cb[qi]) > + diag =3D -1; > + } > + > if (diag =3D=3D 0) > continue; >=20 > 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; >=20 > +extern uint8_t sanity_checks; > + > extern uint16_t mbuf_data_size; /**< Mbuf data space size. */ extern > uint32_t param_total_num_mbufs; >=20 > @@ -730,6 +737,8 @@ int close_file(uint8_t *buf); >=20 > void port_queue_region_info_display(portid_t port_id, void *buf); >=20 > +int set_sanity_checks(const char *arg); > + > enum print_warning { > ENABLED_WARN =3D 0, > DISABLED_WARN > -- > 2.17.1 Regards, Bernard.