Re: [dpdk-dev] [PATCH v2 01/22] ethdev: fix MTU size exceeds max rx packet length

[oulijun <oulijun@huawei.com>]

在 2020/12/17 17:22, Steve Yang 写道:
> If max rx packet length is smaller then MTU + Ether overhead, that will
> drop all MTU size packets.
> 
> Update the MTU size according to the max rx packet and Ether overhead.
> 
> Fixes: 59d0ecdbf0e1 ("ethdev: MTU accessors")
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 17ddacc78d..ff6a1e675f 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1292,6 +1292,7 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   	struct rte_eth_dev *dev;
>   	struct rte_eth_dev_info dev_info;
>   	struct rte_eth_conf orig_conf;
> +	uint16_t overhead_len;
>   	int diag;
>   	int ret;
>   
> @@ -1323,6 +1324,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   	if (ret != 0)
>   		goto rollback;
>   
> +	/* Get the real Ethernet overhead length */
> +	if (dev_info.max_mtu &&
> +	    dev_info.max_mtu != UINT16_MAX &&
> +	    dev_info.max_rx_pktlen &&
> +	    dev_info.max_rx_pktlen > dev_info.max_mtu)
> +		overhead_len = dev_info.max_rx_pktlen - dev_info.max_mtu;
> +	else
> +		overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> +
>   	/* If number of queues specified by application for both Rx and Tx is
>   	 * zero, use driver preferred values. This cannot be done individually
>   	 * as it is valid for either Tx or Rx (but not both) to be zero.
> @@ -1410,13 +1420,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   			goto rollback;
>   		}
>   	} else {
> -		if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN ||
> -			dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
> +		uint16_t pktlen = dev_conf->rxmode.max_rx_pkt_len;
> +		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_MAX_LEN;
> +						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;
> +
Hi
   I think the dev->data->mtu should be updated after configured 
success. So the update in this position seems unreasonable. 
'max_rx_pkt_len' is only used when jumbo_frame enabled, as follows:
struct rte_eth_rxmode {
	.....
	uint32_t max_rx_pkt_len;  /**< Only used if JUMBO_FRAME enabled. */
	/** Maximum allowed size of LRO aggregated packet. */
	....};

So If DEV_RX_OFFLOAD_JUMBO_FRAME is set to rxmode.offload, driver should 
configure mtu to hardware according to 'max_rx_pkt_len' and update 
dev->data->mtu. This seems more reasonable. And some PMD drivers are 
already doing this.

In addition, validity check for 'max_rx_pkt_len' in 
rte_eth_dev_configure API may be error. It should be greater than 
'RTE_ETHER_MTU + overhead_len'.
Because driver must enable DEV_RX_OFFLOAD_JUMBO_FRAME when user set mtu 
with greater than 1500 by 'rte_eth_dev_set_mtu' API.

What do you think?

Thanks
Lijun Ou

>   	/*
>   	 * If LRO is enabled, check that the maximum aggregated packet
>   	 * size is supported by the configured device.
>