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

[Ferruh Yigit]

On 1/14/2021 9:45 AM, Steve Yang wrote:
> Ethdev is using default Ethernet overhead to decide if provided
> 'max_rx_pkt_len' value is bigger than max (non jumbo) MTU value,
> and limits it to MAX if it is.
> 
> Since the application/driver used Ethernet overhead is different than
> the ethdev one, check result is wrong.
> 
> If the driver is using Ethernet overhead bigger than the default one,
> the provided 'max_rx_pkt_len' is trimmed down, and in the driver when
> correct Ethernet overhead is used to convert back, the resulting MTU is
> less than the intended one, causing some packets to be dropped.
> 
> Like,
> app     -> max_rx_pkt_len = 1500/*mtu*/ + 22/*overhead*/ = 1522
> ethdev  -> 1522 > 1518/*MAX*/; max_rx_pkt_len = 1518
> driver  -> MTU = 1518 - 22 = 1496
> Packets with size 1497-1500 are dropped although intention is to be able
> to send/receive them.
> 
> The fix is to make ethdev use the correct Ethernet overhead for port,
> instead of default one.
> 
> Fixes: 59d0ecdbf0e1 ("ethdev: MTU accessors")
> 
> Signed-off-by: Steve Yang <stevex.yang at intel.com>

<...>

> @@ -1410,11 +1422,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;

What do you think removing the above check, the else block, completely?
Since the 'max_rx_pkt_len' should not be used when jumbo frame is not set.

> +	}
> +
> +	/* Scale the MTU size to adapt max_rx_pkt_len */
> +	if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
> +		dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len -
> +				overhead_len;
>   	}

Above if block has exact same check, why not move it above block?