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

Bruce Richardson bruce.richardson at intel.com
Fri Nov 27 18:08:42 CET 2020


On Fri, Nov 27, 2020 at 03:19:43PM +0300, Andrew Rybchenko wrote:
> 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 at intel.com>
> >>> Sent: Tuesday, November 3, 2020 12:05 AM
> >>> To: Yigit, Ferruh <ferruh.yigit at intel.com>; Andrew Rybchenko
> >>> <andrew.rybchenko at oktetlabs.ru>; Yang, SteveX <stevex.yang at intel.com>;
> >>> dev at dpdk.org
> >>> Cc: Xing, Beilei <beilei.xing at intel.com>; Lu, Wenzhuo
> >>> <wenzhuo.lu at intel.com>; Iremonger, Bernard
> >>> <bernard.iremonger at intel.com>; Yang, Qiming <qiming.yang at intel.com>;
> >>> mdr at ashroe.eu; nhorman at 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 at 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.
> 

I agree with the comment here - my understanding is the same that if the
JUMBO_FRAME offload flag is not set, then the max_rx_pkt_len should be
ignored (which for me implies that it should be set to 0 or similar
sentinal value in ethdev to ensure drivers don't accidentally use it).

In terms of the deprecation notice, I also think it's fairly confusing, and
after talking with Ferruh, I'm not convinced we need one. It seems that the
planned changes based on this are just bug fixes, where packets that
should not have been dropped were dropped. Perhaps someone could comment on
the specific behaviour change that would affect apps (where it's not just
plain buggy behaviour!)

However, it does appear that this area is in need of clean-up generally.
The suggestion to drop the jumbo frame flag, packet_len/mtu value from the
ethdev config, and just use the existing API calls, sounds interesting. If
that is not the approach taken, I'd like to see the existing approach kept,
so that a zero-initialized config is acceptable for packet size setting,
i.e. no jumbo frame flag and zero max-length == default ethernet MTU.

Just my 2c.
/Bruce


More information about the dev mailing list