Re: [dpdk-dev] [PATCH 2/4] ethdev: move jumbo frame offload check to library

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 7/9/21 8:29 PM, Ferruh Yigit wrote:
> Setting MTU bigger than RTE_ETHER_MTU requires the jumbo frame support,
> and application should enable the jumbo frame offload support for it.
> 
> When jumbo frame offload is not enabled by application, but MTU bigger
> than RTE_ETHER_MTU is requested there are two options, either fail or
> enable jumbo frame offload implicitly.
> 
> Enabling jumbo frame offload implicitly is selected by many drivers
> since setting a big MTU value already implies it, and this increases
> usability.
> 
> This patch moves this logic from drivers to the library, both to reduce
> the duplicated code in the drivers and to make behaviour more visible.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Very good cleanup, many thanks.

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[snip]

> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 3451125639f9..d649a5dd69a9 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3625,6 +3625,7 @@ rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
>  	int ret;
>  	struct rte_eth_dev_info dev_info;
>  	struct rte_eth_dev *dev;
> +	int is_jumbo_frame_capable = 0;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  	dev = &rte_eth_devices[port_id];
> @@ -3643,12 +3644,27 @@ rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
>  
>  		if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu)
>  			return -EINVAL;
> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME)
> +			is_jumbo_frame_capable = 1;
>  	}
>  
> +	if (mtu > RTE_ETHER_MTU && is_jumbo_frame_capable == 0)
> +		return -EINVAL;
> +
>  	ret = (*dev->dev_ops->mtu_set)(dev, mtu);
> -	if (!ret)
> +	if (!ret) {

Since line it updated anyway, may I ask to use explicit
comparison vs 0 as coding style says.

>  		dev->data->mtu = mtu;
>  
> +		/* switch to jumbo mode if needed */
> +		if (mtu > RTE_ETHER_MTU)
> +			dev->data->dev_conf.rxmode.offloads |=
> +				DEV_RX_OFFLOAD_JUMBO_FRAME;
> +		else
> +			dev->data->dev_conf.rxmode.offloads &=
> +				~DEV_RX_OFFLOAD_JUMBO_FRAME;
> +	}
> +
>  	return eth_err(port_id, ret);
>  }
>  
>