On 7/13/2021 2:48 PM, Andrew Rybchenko wrote:
> 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.yi...@intel.com>
>
> Very good cleanup, many thanks.
>
> Reviewed-by: Andrew Rybchenko <andrew.rybche...@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.
>
ack, will fix all occurrences
>> 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);
>> }
>>
>>
>
