Re: [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix max-pkt-len option invalid

[Lance Richardson <lance.richardson@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

On Mon, Jan 25, 2021 at 3:35 AM Steve Yang <stevex.yang@intel.com> wrote:
>
> Moved the setting of 'DEV_RX_OFFLOAD_JUMBO_FRAME' from
> 'cmd_config_max_pkt_len_parsed()' to 'init_config()' caused fail the case
> where 'max_rx_pkt_len' is changed from the command line via
> "port config all max-pkt-len".
>
> The 'init_config()' function is only called when testpmd is started,
> but the DEV_RX_OFFLOAD_JUMBO_FRAME setting needs to be recomputed whenever
> 'max_rx_pkt_len' changes.
>
> Define the 'update_jumbo_frame_offload()' function for both 'init_config()'
> and 'cmd_config_max_pkt_len_parsed()', and detect if 'max_rx_pkt_len'
> should be update to 1500 + overhead as default configuration. At the same
> time, the offloads of rx queue also should update the value once port
> offloads changed (e.g.: disabled JUMBO_FRAME offload via changed
> max-pkt-len, reproduce steps [1]), otherwise, it would cause unexpected
> result.
>
> [1]
> --------------------------------------------------------------------------
> ./x86_64-native-linuxapp-gcc/app/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: 761c4d6690 ("app/testpmd: fix max Rx packet length for VLAN packets")
>
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> ---
>  app/test-pmd/cmdline.c    |  1 +
>  app/test-pmd/parameters.c |  1 +
>  app/test-pmd/testpmd.c    | 63 ++++++++++++++++++++++++++++-----------
>  app/test-pmd/testpmd.h    |  2 ++
>  4 files changed, 50 insertions(+), 17 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 89034c8b72..8b6b7b6206 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1901,6 +1901,7 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
>                         printf("Unknown parameter\n");
>                         return;
>                 }
> +               update_jumbo_frame_offload(pid, false);

I'm probably missing something, but why isn't this a matter of simply calling
port_mtu_set() here (with mtu computed from max pkt len) and keeping
init_config() as currently implemented?

>         }
>
>         init_port_config();