[dpdk-dev] [RFC] net/ionic: update MTU calculations
Ferruh Yigit
ferruh.yigit at intel.com
Fri Apr 23 13:42:12 CEST 2021
On 12/15/2020 12:26 PM, Ferruh Yigit wrote:
> On 12/10/2020 2:46 AM, Andrew Boyer wrote:
>> This RFC is in response to the threads about testpmd mtu settings
>> and the deprecation of max-rx-pkt-len.
>>
>> It took us a while to figure out what we were supposed to be doing
>> in this part of the API. It is not clear if max_rx_pkt_len should be
>> an input to or an output from the PMD.
>
> 'max_rx_pkt_len' is input to the PMD, but it needs to be in sync with MTU value,
> that is why PMDs update this value when MTU is updated to keep the sync.
>
>>
>> The code below represents what we believe should happen today, and also
>> happens to pass the DTS tests which were failing prior to this change
>> (checksum and jumbo_frame at least).
>>
Hi Andrew,
I am updating the status of the patch as "change requested", what is the status
of this patch, will there be a new version?
Is DTS still failing?
Please see a few comments below if there will be new version.
<...>
>> diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
>> index 925da3e29..7000de3f9 100644
>> --- a/drivers/net/ionic/ionic_ethdev.c
>> +++ b/drivers/net/ionic/ionic_ethdev.c
>> @@ -357,25 +357,19 @@ ionic_dev_mtu_set(struct rte_eth_dev *eth_dev, uint16_t
>> mtu)
>> int err;
>> IONIC_PRINT_CALL();
>> + IONIC_PRINT(INFO, "Setting mtu %u\n", mtu);
>> - /*
>> - * Note: mtu check against IONIC_MIN_MTU, IONIC_MAX_MTU
>> - * is done by the the API.
>> - */
>> -
>> - /*
>> - * Max frame size is MTU + Ethernet header + VLAN + QinQ
>> - * (plus ETHER_CRC_LEN if the adapter is able to keep CRC)
>> - */
>> - max_frame_size = mtu + RTE_ETHER_HDR_LEN + 4 + 4;
>> -
>> - if (eth_dev->data->dev_conf.rxmode.max_rx_pkt_len < max_frame_size)
>> - return -EINVAL;
>
> The max frame size calculation depends on what HW support, but if VLAN header is
> supported above calculation and check is correct.
>
Removing above check seems correct thing to do, instead should check against
'dev_info.max_mtu' which is already done in ethdev layer, so nothing needed here.
>> -
>> + /* Note: mtu check against min/max is done by the API */
>> err = ionic_lif_change_mtu(lif, mtu);
>> if (err)
>> return err;
>> + /* Update max frame size */
>> + max_frame_size = mtu + RTE_ETHER_HDR_LEN;
>
> I guess you are removing the CRC because your HW strips the CRC in the Rx
> buffer, but this limit is not for Rx buffer, it is for the frame size HW
> accepts, and since recevied frame will have the CRC it should be included into
> the calculation.
>
>> + eth_dev->data->dev_conf.rxmode.max_rx_pkt_len = max_frame_size;
>> +
Although 'rxmode.max_rx_pkt_len' is input to driver, it is related with MTU,
which is also input from application in this function, so OK to update
'rxmode.max_rx_pkt_len' here.
>> + ionic_lif_set_rx_buf_size(lif);
>> +
This seems to keep the local copy of 'rxmode.max_rx_pkt_len' and use local copy
instead, is this just refactoring or is there any other reason for local copy?
More information about the dev
mailing list