From patchwork Mon Sep 10 05:45:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 44465 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1E4014CB5; Mon, 10 Sep 2018 07:46:02 +0200 (CEST) Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id 8444D4CB3 for ; Mon, 10 Sep 2018 07:46:00 +0200 (CEST) Received: by mail-wm0-f65.google.com with SMTP id 207-v6so19887067wme.5 for ; Sun, 09 Sep 2018 22:46:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=rXgcIL4PPWW2Qlra3wC6GonhaJ7cBMGi+7KbcbFBZJ4=; b=TUr1/a8ptl9QLH1/DVPKjC2faWTeaQf0eTqDaDr/WgCvKjPpQin7zTjniMSpGR4/uM UOeQ7Rsui73Cj40EABYe6vv2GM3LJ/WOMQpwVKxObQHnEaAN2qLmWXothaKcmtfQaAET 7aEp5lEdNU6U7aYPv5MkMLDmW03YAn4VbcRvS4n2rhK/yjS34izgujo9ESAdug25WDWR oVG+4k4CM4cpHdYRAfvfikAyWTraUCfuNy+MLkkKtubFLhZKxA8hAABbtiJzg/zk9B4E 4fw3ZR2itO9YWuO0cx0vEwX82TPP9l5bLaxAc4HCr4K7WNPHf59Sm08I9jiDNBPwygx+ 0Tlg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=rXgcIL4PPWW2Qlra3wC6GonhaJ7cBMGi+7KbcbFBZJ4=; b=WtPmMULVLI8pkHROYUCYItrxX/rHxTNM3SC3W/IWD2BFNVwKwGIOE7hseVNg1eKf9y TQRHT0nitjgb6adhAJmpxdoKSJxOml79tCKoi1takc0z3TR/2X761yzByOErvHdiBVvc 7kx5QR69Hzn8m/083zOmqcRCHNlekOs4wsXtlvm9k3CxObXAx3EZRUrMgkb/dYUezp6E 8DSlznRqzjVS4+UY3BEXFm4AnjDIByHF1V/r+qrh/bTkhcXEUzh43hVDBtCE42j9V9t5 ii3ZR0qAEo0ZsqZbXdt4ANJZkjuKfYSs35PacFA/P4KMpsFE8ojGHN3f1G+00B3Yzc7k w0MA== X-Gm-Message-State: APzg51AJB+C6kpczGbW/uymnrh0Fo7/uVEKiRcEpPk2jEma9yANy+k/L 3Wn7NFHlkp0fYX89A+BZbfdcj+mC2kg= X-Google-Smtp-Source: ANB0VdYifXA44nRsfTHR5zPZOo+fhv2yaQU1zhW3Gzu4gD6jRW9JgrxLmJP+1PP7t2iIZ8KbdoLrdw== X-Received: by 2002:a1c:9290:: with SMTP id u138-v6mr12351534wmd.52.1536558359737; Sun, 09 Sep 2018 22:45:59 -0700 (PDT) Received: from 6wind.com ([109.190.253.16]) by smtp.gmail.com with ESMTPSA id t4-v6sm14667473wrb.45.2018.09.09.22.45.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 09 Sep 2018 22:45:59 -0700 (PDT) From: David Marchand To: dev@dpdk.org Cc: olivier.matz@6wind.com, wenzhuo.lu@intel.com, jingjing.wu@intel.com, bernard.iremonger@intel.com Date: Mon, 10 Sep 2018 07:45:47 +0200 Message-Id: <20180910054547.18494-4-david.marchand@6wind.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180910054547.18494-1-david.marchand@6wind.com> References: <20180910054547.18494-1-david.marchand@6wind.com> Subject: [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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 --- 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 : 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