Re: [dpdk-dev] [PATCH v8 2/2] doc: annouce deprecation of jumbo frame flag condition

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

On 11/24/20 8:46 PM, Ferruh Yigit wrote:
> On 11/3/2020 9:07 AM, Yang, SteveX wrote:
>>> -----Original Message-----
>>> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>>> Sent: Tuesday, November 3, 2020 12:05 AM
>>> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew Rybchenko
>>> <andrew.rybchenko@oktetlabs.ru>; Yang, SteveX <stevex.yang@intel.com>;
>>> dev@dpdk.org
>>> Cc: Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo
>>> <wenzhuo.lu@intel.com>; Iremonger, Bernard
>>> <bernard.iremonger@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
>>> mdr@ashroe.eu; nhorman@tuxdriver.com
>>> Subject: RE: [dpdk-dev] [PATCH v8 2/2] doc: annouce deprecation of jumbo
>>> frame flag condition
>>>
>>>> On 11/2/2020 1:18 PM, Andrew Rybchenko wrote:
>>>>> On 11/2/20 11:52 AM, SteveX Yang wrote:
>>>>>> Annouce to replace 'RTE_ETHER_MAX_LEN' with 'RTE_ETHER_MTU' as type
>>>>>> condition of jumbo frame. Involved scopes:
>>>>>> - rte_ethdev;
>>>>>> - app, e.g.: test-pmd, test-eventdev;
>>>>>> - examples, e.g.: ipsec-secgw, l3fwd, vhost;
>>>>>> - net PMDs which support VLAN tag(s) within overhead, e.g.: i40e,
>>>>>> ixgbe;
>>>>>>
>>>>>> Signed-off-by: SteveX Yang <stevex.yang@intel.com>
>>>>>> ---
>>>>>>    doc/guides/rel_notes/deprecation.rst | 12 ++++++++++++
>>>>>>    1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>> index 2e082499b..fae139f01 100644
>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>> @@ -138,6 +138,18 @@ Deprecation Notices
>>>>>>      will be limited to maximum 256 queues.
>>>>>>      Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
>>>>>> +* ethdev: Offload flag ``DEV_RX_OFFLOAD_JUMBO_FRAME`` will be set
>>>>>> +according to
>>>>>> +  ``RTE_ETHER_MTU`` in next release. Currently, the jumbo frame
>>>>>> +uses the
>>>>>> +  ``RTE_ETHER_MAX_LEN`` as boundary condition. When the MTU (1500)
>>>>>> +set, the
>>>>>> +  frame type of rx packet will be different if used different
>>>>>> +overhead, it will
>>>>>> +  cause the consistency issue. Hence, using fixed value
>>>>>> +``RTE_ETHER_MTU`` can
>>>>>> +  avoid this issue.
>>>>>> +  Following scopes will be changed:
>>>>>> +  - ``rte_ethdev``
>>>>>> +  - ``app``, e.g.: ``test-pmd``, ``test-eventdev``;
>>>>>> +  - ``examples``, e.g.: ``ipsec-secgw``, ``l3fwd``, ``vhost``;
>>>>>> +  - net PMDs which support VLAN tag(s) within overhead, e.g.:
>>>>>> +``i40e``;
>>>>>> +
>>>>>>    * cryptodev: support for using IV with all sizes is added, J0
>>>>>> still can
>>>>>>      be used but only when IV length in following structs
>>>>>> ``rte_crypto_auth_xform``,
>>>>>>      ``rte_crypto_aead_xform`` is set to zero. When IV length is
>>>>>> greater or equal
>>>>>>
>>>>>
>>>>> If so, what's the point to have the offload? May be just deprecate
>>>>> and later remove it?
>>>>>
>>>
>>> Same thought actually, can we remove DEV_RX_OFFLOAD_JUMBO_FRAME
>>> flag completely?
>>> PMD can decide based on provided MTU size does segmentation, etc. is
>>> needed.
>>>
>>
>> Yes, it seems can be removed when base on MTU size.
>> I've checked related code of using DEV_RX_OFFLOAD_JUMBO_FRAME.
>> Most of them use for checking boundary of max packet length and set
>> 'dev->data->mtu'.
>>
> 
> Steve already sent the RFC for above fix:
> https://patches.dpdk.org/patch/84486/
> 
> We can consider removing 'DEV_RX_OFFLOAD_JUMBO_FRAME' but anyway it is
> for 21.11.
> 
> This deprecation notice is required to fix the ethdev in next release,
> as in the above RFC.
> 
> I cc'ed a few more relevant people, can you please review this
> deprecation notice.
> 
> Thanks,
> ferruh
> 
> 
>>>>
>>>> Above just changes the condition of what is called jumbo frame, nothing more.
>>>>
>>>> ethdev assumes max frame size (without jumbo frame support) can be
>>>> 'RTE_ETHER_MAX_LEN' (1518)
>>>>
>>>> But a PMD can support double VLAN, and it can have RTE_ETHER_MAX_LEN +
>>>> 8 bytes frame size and still may not support jumbo frame.
>>>>
>>>> In that case ethdev overwrites the frame size as RTE_ETHER_MAX_LEN,
>>>> and this will reduce the supported MTU to 1492 (instead of expected
>>>> 1500).
>>>> Above deprecation notice is to fix this.

My problem with the deprecation notice is that I don't actually
understand what will be done to address it.

The idea explained by Ferruh in details makes sense. But I
don't understand how the deprecation notice description
corresponding to it. I read
  "Offload flag ``DEV_RX_OFFLOAD_JUMBO_FRAME`` will be set.."
as an enforcement of the offload flag based on other
parameters. I think it is incorrect. Or I still don't
understand something...

Looking at [1] adds more confusion since I don't understand why
we care about dev_conf->rxmode.max_rx_pkt_len when JUMBO_FRAME
offload is disabled. As far as I know JUMBO_FRAME offload
enable means that driver should take a look at it and apply.
Otherwise, just ignore it.

[1]
http://lxr.dpdk.org/dpdk/v20.08/source/lib/librte_ethdev/rte_ethdev.c#L1345