Except a minor typo inline. Acked-by: Xiaoyun Li <xiaoyun...@intel.com>
> -----Original Message----- > From: Yigit, Ferruh <ferruh.yi...@intel.com> > Sent: Tuesday, January 26, 2021 02:16 > To: Lu, Wenzhuo <wenzhuo...@intel.com>; Li, Xiaoyun <xiaoyun...@intel.com>; > Iremonger, Bernard <bernard.iremon...@intel.com>; Yang, SteveX > <stevex.y...@intel.com> > Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; dev@dpdk.org; sta...@dpdk.org; > lance.richard...@broadcom.com; ouli...@huawei.com; > wis...@mellanox.com; lihuis...@huawei.com > Subject: [PATCH v5] app/testpmd: fix setting maximum packet length > > From: Steve Yang <stevex.y...@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: sta...@dpdk.org > > Signed-off-by: Steve Yang <stevex.y...@intel.com> > Signed-off-by: Ferruh Yigit <ferruh.yi...@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.richard...@broadcom.com > Cc: ouli...@huawei.com > Cc: wis...@mellanox.com > Cc: lihuis...@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