On 1/25/2021 3:46 PM, Lance Richardson wrote:
On Mon, Jan 25, 2021 at 3:35 AM Steve Yang <[email protected]> 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 <[email protected]> --- 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?
They are very similar, calling the 'port_mtu_set()' can be an option, but it is headache to get dev->info first and calculate overhead to be able to call 'port_mtu_set()'; Since this is already done in 'init_config()', converting it to a function and reuse it also looks valid as done here.
I am updating that function and sending a new version soon.
