From: "Li, Xiaoyun" <xiaoyun.li@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
"Iremonger, Bernard" <bernard.iremonger@intel.com>,
"Yang, SteveX" <stevex.yang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"stable@dpdk.org" <stable@dpdk.org>,
"lance.richardson@broadcom.com" <lance.richardson@broadcom.com>,
"oulijun@huawei.com" <oulijun@huawei.com>,
"wisamm@mellanox.com" <wisamm@mellanox.com>,
"lihuisong@huawei.com" <lihuisong@huawei.com>
Subject: Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
Date: Wed, 27 Jan 2021 03:04:54 +0000 [thread overview]
Message-ID: <CY4PR11MB17508CB605B25959C34CE2E999BB9@CY4PR11MB1750.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210125181548.2713326-1-ferruh.yigit@intel.com>
Except a minor typo inline.
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, January 26, 2021 02:16
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org;
> lance.richardson@broadcom.com; oulijun@huawei.com;
> wisamm@mellanox.com; lihuisong@huawei.com
> Subject: [PATCH v5] app/testpmd: fix setting maximum packet length
>
> From: Steve Yang <stevex.yang@intel.com>
>
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
>
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the flag status
> needs to be calculated whenever 'max_rx_pkt_len' changes.
>
> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
reproduced
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it didn't.
>
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and 'cmd_config_max_pkt_len_parsed()'.
>
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it is
> zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
>
> [1]
> --------------------------------------------------------------------------
> dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
> --rxq=4 --txq=4 --disable-rss
> testpmd> set verbose 3
> testpmd> port stop all
> testpmd> port config all max-pkt-len 1518 port start all
>
> // Got fail error info without this patch Configuring Port 0 (socket 1) Ethdev
> port_id=0 rx_queue_id=0, new added offloads 0x800 must be within per-queue
> offload capabilities 0x0 in rte_eth_rx_queue_setup() Fail to configure port 0 rx
> queues //<-- Fail error info;
> --------------------------------------------------------------------------
>
> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN packets")
> Cc: stable@dpdk.org
>
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>
> v5:
> * 'update_jumbo_frame_offload()' helper updated
> * check zero 'max-pkt-len' value
> * Update how queue offload flags updated
> * Update MTU if JUMBO_FRAME flag is not set
> * Default testpmd 'max-pkt-len' value set to zero
>
> Cc: lance.richardson@broadcom.com
> Cc: oulijun@huawei.com
> Cc: wisamm@mellanox.com
> Cc: lihuisong@huawei.com
> ---
> app/test-pmd/cmdline.c | 13 ++++++
> app/test-pmd/testpmd.c | 102 +++++++++++++++++++++++++++++++++--------
> app/test-pmd/testpmd.h | 1 +
> 3 files changed, 97 insertions(+), 19 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 89034c8b7272..9ada4316c6c0 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1877,7 +1877,9 @@ cmd_config_max_pkt_len_parsed(void
> *parsed_result,
> __rte_unused void *data)
> {
> struct cmd_config_max_pkt_len_result *res = parsed_result;
> + uint32_t max_rx_pkt_len_backup = 0;
> portid_t pid;
> + int ret;
>
> if (!all_ports_stopped()) {
> printf("Please stop all ports first\n"); @@ -1896,7 +1898,18 @@
> cmd_config_max_pkt_len_parsed(void *parsed_result,
> if (res->value == port-
> >dev_conf.rxmode.max_rx_pkt_len)
> return;
>
> + ret = eth_dev_info_get_print_err(pid, &port->dev_info);
> + if (ret != 0) {
> + printf("rte_eth_dev_info_get() failed for
> port %u\n",
> + pid);
> + return;
> + }
> +
> + max_rx_pkt_len_backup = port-
> >dev_conf.rxmode.max_rx_pkt_len;
> +
> port->dev_conf.rxmode.max_rx_pkt_len = res->value;
> + if (update_jumbo_frame_offload(pid) != 0)
> + port->dev_conf.rxmode.max_rx_pkt_len =
> max_rx_pkt_len_backup;
> } else {
> printf("Unknown parameter\n");
> return;
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> c256e719aea2..b69fcd3fde72 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -443,8 +443,11 @@ lcoreid_t latencystats_lcore_id = -1;
> * Ethernet device configuration.
> */
> struct rte_eth_rxmode rx_mode = {
> - .max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> - /**< Default maximum frame length. */
> + /* Default maximum frame length.
> + * Zero is converted to "RTE_ETHER_MTU + PMD Ethernet overhead"
> + * in init_config().
> + */
> + .max_rx_pkt_len = 0,
> };
>
> struct rte_eth_txmode tx_mode = {
> @@ -1410,7 +1413,6 @@ init_config(void)
> struct rte_gro_param gro_param;
> uint32_t gso_types;
> uint16_t data_size;
> - uint16_t eth_overhead;
> bool warning = 0;
> int k;
> int ret;
> @@ -1447,22 +1449,10 @@ init_config(void)
> rte_exit(EXIT_FAILURE,
> "rte_eth_dev_info_get() failed\n");
>
> - /* Update the max_rx_pkt_len to have MTU as
> RTE_ETHER_MTU */
> - if (port->dev_info.max_mtu != UINT16_MAX &&
> - port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
> - eth_overhead = port->dev_info.max_rx_pktlen -
> - port->dev_info.max_mtu;
> - else
> - eth_overhead =
> - RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> -
> - if (port->dev_conf.rxmode.max_rx_pkt_len <=
> - (uint32_t)(RTE_ETHER_MTU + eth_overhead))
> - port->dev_conf.rxmode.max_rx_pkt_len =
> - RTE_ETHER_MTU + eth_overhead;
> - else
> - port->dev_conf.rxmode.offloads |=
> - DEV_RX_OFFLOAD_JUMBO_FRAME;
> + ret = update_jumbo_frame_offload(pid);
> + if (ret != 0)
> + printf("Updating jumbo frame offload failed for
> port %u\n",
> + pid);
>
> if (!(port->dev_info.tx_offload_capa &
> DEV_TX_OFFLOAD_MBUF_FAST_FREE)) @@ -3358,6
> +3348,80 @@ rxtx_port_config(struct rte_port *port)
> }
> }
>
> +/*
> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME
> +offload,
> + * MTU is also aligned if JUMBO_FRAME offload is not set.
> + *
> + * port->dev_info should be get before calling this function.
> + *
> + * return 0 on success, negative on error */ int
> +update_jumbo_frame_offload(portid_t portid) {
> + struct rte_port *port = &ports[portid];
> + uint32_t eth_overhead;
> + uint64_t rx_offloads;
> + int ret;
> + bool on;
> +
> + /* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
> + if (port->dev_info.max_mtu != UINT16_MAX &&
> + port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
> + eth_overhead = port->dev_info.max_rx_pktlen -
> + port->dev_info.max_mtu;
> + else
> + eth_overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> +
> + rx_offloads = port->dev_conf.rxmode.offloads;
> +
> + /* Default config value is 0 to use PMD specific overhead */
> + if (port->dev_conf.rxmode.max_rx_pkt_len == 0)
> + port->dev_conf.rxmode.max_rx_pkt_len = RTE_ETHER_MTU +
> eth_overhead;
> +
> + if (port->dev_conf.rxmode.max_rx_pkt_len <= RTE_ETHER_MTU +
> eth_overhead) {
> + rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> + on = false;
> + } else {
> + if ((port->dev_info.rx_offload_capa &
> DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> + printf("Frame size (%u) is not supported by port %u\n",
> + port->dev_conf.rxmode.max_rx_pkt_len,
> + portid);
> + return -1;
> + }
> + rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> + on = true;
> + }
> +
> + if (rx_offloads != port->dev_conf.rxmode.offloads) {
> + uint16_t qid;
> +
> + port->dev_conf.rxmode.offloads = rx_offloads;
> +
> + /* Apply JUMBO_FRAME offload configuration to Rx queue(s)
> */
> + for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> + if (on)
> + port->rx_conf[qid].offloads |=
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> + else
> + port->rx_conf[qid].offloads &=
> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> + }
> + }
> +
> + /* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
> + * if unset do it here
> + */
> + if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> + ret = rte_eth_dev_set_mtu(portid,
> + port->dev_conf.rxmode.max_rx_pkt_len -
> eth_overhead);
> + if (ret)
> + printf("Failed to set MTU to %u for port %u\n",
> + port->dev_conf.rxmode.max_rx_pkt_len -
> eth_overhead,
> + portid);
> + }
> +
> + return 0;
> +}
> +
> void
> init_port_config(void)
> {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 5f2316210726..2f8f5a92e46a 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -1005,6 +1005,7 @@ uint16_t tx_pkt_set_dynf(uint16_t port_id,
> __rte_unused uint16_t queue,
> __rte_unused void *user_param);
> void add_tx_dynf_callback(portid_t portid); void
> remove_tx_dynf_callback(portid_t portid);
> +int update_jumbo_frame_offload(portid_t portid);
>
> /*
> * Work-around of a compilation error with ICC on invocations of the
> --
> 2.29.2
next prev parent reply other threads:[~2021-01-27 3:05 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210125083202.38267-1-stevex.yang@intel.com>
2021-01-25 18:15 ` Ferruh Yigit
2021-01-25 19:41 ` Lance Richardson
2021-01-26 0:44 ` Ferruh Yigit
2021-01-26 3:22 ` Lance Richardson
2021-01-26 3:45 ` Lance Richardson
2021-01-26 7:54 ` Li, Xiaoyun
2021-01-26 11:01 ` Ferruh Yigit
2021-01-28 21:36 ` Lance Richardson
2021-01-28 22:12 ` Ferruh Yigit
2021-01-26 9:02 ` Wisam Monther
2021-01-27 3:04 ` Li, Xiaoyun [this message]
2021-01-28 1:57 ` Chen, BoX C
2021-01-28 9:18 ` Wisam Monther
2021-01-28 9:26 ` Ferruh Yigit
2021-01-28 11:08 ` Wisam Monther
2021-01-28 12:07 ` [dpdk-stable] [PATCH v6] " Ferruh Yigit
2021-01-29 9:34 ` Ferruh Yigit
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=CY4PR11MB17508CB605B25959C34CE2E999BB9@CY4PR11MB1750.namprd11.prod.outlook.com \
--to=xiaoyun.li@intel.com \
--cc=bernard.iremonger@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=lance.richardson@broadcom.com \
--cc=lihuisong@huawei.com \
--cc=oulijun@huawei.com \
--cc=stable@dpdk.org \
--cc=stevex.yang@intel.com \
--cc=wenzhuo.lu@intel.com \
--cc=wisamm@mellanox.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).