Re: [dpdk-dev] [PATCH v3 1/3] ethdev: fix MTU doesn't update when jumbo frame disabled

[Huisong Li <lihuisong@huawei.com>]

Hi Steve,

In the current modification, the MTU is updated based on 
'max_rx_pkt_len' regardless of whether jumbo frame is enabled.

Now, MTU is correct when jumbo frmae is disabled. However, when jumbo 
frame is enabled, the MTU value may be inconsistent with

the definition of the enabled jumbo frame. Like:

1/ DEV_RX_OFFLOAD_JUMBO_FRAME is set;

2/ max_rx_pkt_len = 1200

3/ dev->data->mtu = 1200 - overhead_len(18) = 1182


In rte_eth_dev_configure API, the check for 'max_rx_pkt_len' is as follows:

if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {  //jumbo 
frame enabled
         if (dev_conf->rxmode.max_rx_pkt_len > dev_info.max_rx_pktlen) {
             xxxx
             goto rollback;
         } else if (*dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN*) {
             xxxx
             goto rollback;
         }
} else { //jumbo frame disabled

         if (pktlen < RTE_ETHER_MIN_MTU + overhead_len ||
                     pktlen > RTE_ETHER_MTU + overhead_len)
                         /* Use default value */
dev->data->dev_conf.rxmode.max_rx_pkt_len =
                                                 RTE_ETHER_MTU + 
overhead_len;

}

Since the applicatin sets DEV_RX_OFFLOAD_JUMBO_FRAME to enable jumbo 
frame, and the framework API needs to update

the MTU based on 'max_rx_pkt_len', but the framework API uses 
*RTE_ETHER_MIN_LEN(64)* to verify the boundary value of

'max_rx_pkt_len', instead of "RTE_ETHER_MTU + overhead_len".  As far as 
I know, if the applicatin sets DEV_RX_OFFLOAD_JUMBO_FRAME

and 'max_rx_pkt_len' is 1200, the framework API or driver should return 
a failure. As mentioned in this patch set, the jumbo frame

offload is set only when 'max_rx_pkt_len' requested is greater than 
"RTE_ETHER_MTU + eth_overhead" in testpmd.

I really don't understand it.  How do you understand this behavior?

Thanks.


在 2021/1/22 17:01, Steve Yang 写道:
> The MTU value should be updated to 'max_rx_pkt_len - overhead'
> no matter if the JUMBO FRAME offload enabled. If not update this MTU,
> use will get the wrong MTU info via some command.
> E.g.: 'show port info all' in testpmd tool.
>
> Actually, the 'max_rx_pkt_len' has been used for other purposes in many
> places now, even though the 'max_rx_pkt_len' is expected 'Only used if
> JUMBO_FRAME enabled'.
>
> For examples,
> 'max_rx_pkt_len' perhaps can be used as the 'rx_ctx.rxmax' in i40e.
>
> Fixes: bf0f90d92d30 ("ethdev: fix max Rx packet length check")
>
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index daf5f24f7e..42857e3b67 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1421,10 +1421,6 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   			ret = -EINVAL;
>   			goto rollback;
>   		}
> -
> -		/* Scale the MTU size to adapt max_rx_pkt_len */
> -		dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len -
> -				overhead_len;
>   	} else {
>   		uint16_t pktlen = dev_conf->rxmode.max_rx_pkt_len;
>   		if (pktlen < RTE_ETHER_MIN_MTU + overhead_len ||
> @@ -1434,6 +1430,10 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   						RTE_ETHER_MTU + overhead_len;
>   	}
>   
> +	/* Scale the MTU size to adapt max_rx_pkt_len */
> +	dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len -
> +				overhead_len;
> +
>   	/*
>   	 * If LRO is enabled, check that the maximum aggregated packet
>   	 * size is supported by the configured device.