[dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

Ananyev, Konstantin konstantin.ananyev at intel.com
Thu Aug 8 18:53:48 CEST 2019


> > > > > > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > > ``DEV_RX_OFFLOAD_RSS``
> > > > > > > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > > > > > >
> > > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
> > > > > > > Acked-by: Andrew Rybchenko <arybchenko at solarflare.com>
> > > > > > > Acked-by: Jerin Jacob <jerinj at marvell.com>
> > > > > > > ---
> > > > > > >  v3 Changes:
> > > > > > >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH
> > (anndrew).
> > > > > > >
> > > > > > >  v2 Changes:
> > > > > > >  - Reword for clarity.
> > > > > > >
> > > > > > >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> > > > > > >  1 file changed, 13 insertions(+)
> > > > > > >
> > > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > > > > b/doc/guides/rel_notes/deprecation.rst
> > > > > > > index 37b8592b6..056c5709f 100644
> > > > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > > > @@ -78,3 +78,16 @@ Deprecation Notices
> > > > > > >    to set new power environment if power environment was
> > > > > > > already
> > > > > > initialized.
> > > > > > >    In this case the function will return -1 unless the
> > > > > > > environment is unset
> > > > > > first
> > > > > > >    (using ``rte_power_unset_env``). Other function usage
> > > > > > > scenarios will not
> > > > > > change.
> > > > > > > +
> > > > > > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > > > +``DEV_RX_OFFLOAD_RSS_HASH``
> > > > > > > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> > > > > >
> > > > > > One question about DEV_RX_OFFLOAD_PTYPE:
> > > > > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be
> > > > > > introduced to indicate that mbuf.packet_type value is set?
> > > > > > Or PMD will have to set  mbuf.packet_type to zero, when
> > > > > > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> > > > >
> > > > > I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> > > > > - mbuf.packet_type will be valid and mbuf.packet_type will have
> > > > > parsed
> > > > packet type.
> > > > > If not set
> > > > > - mbuf.packet_type can be anything application should not use
> > > > mbuf.packet_type field.
> > > >
> > > > But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE
> > > > or so, right?
> > >
> > > Since application has two knobs rte_eth_dev_get_supported_ptypes() and
> > > DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this
> > change. Right?
> > > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application will
> > > get the parsed ptypes by the driver(=
> > rte_eth_dev_get_supported_ptypes()).
> > > So there is no scope ambiguity. Right?
> >
> > I still think there is:
> > Imagine user has 2 eth devices, one does support DEV_RX_OFFLOAD_PTYPE,
> > second doesn't.  Now he has a mix of packets from both devices, that you
> > want t process.
> > How would he figure out for which of them ptype values are valid, and for
> > each are not?
> > Trace back from what port he has received them?
> > Not very convenient, and not always possible.
> 
> I thought so. But in that case, application can always set DEV_RX_OFFLOAD_PTYPE
> Flags for all the ethdev ports. Right? Rather having any complicated ol_flags
> or port based parsing. If limit the _contract_ to following, we are good. Right?
> # when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid
> and mbuf.packet_type will have parsed packet type

Yes sure in principle user can calculate smallest common subset of RX offloads
supported by all devs in the system and use only  them.
Then he can use some global value for ol_flags that will be setup at initialization time,
instead of checking ol_flags for every mbuf. 
Though inside DPDK we don't use that method for other offloads (cksum, vlan, rss).
Why we should do different here?
Again how to deal with hot-plugged devices with such approach?

> 
> or the negative offload(This contract is pretty clear, I don't think any ambiguity at all)
> # when DEV_RX_OFFLOAD_NO_PTYPE(something similar) is set,
> mbuf.packet_type will be invalid.
> 
> > I think we need either to introduce new ol_flag value (as we usually do for
> > other RX offloads),
> > or force PMD to always set ptype value.
> 
> Setting new  ol_flag value may effect performance for existing drivers
> which don't planning to use this offload

If the driver doesn't support DEV_RX_OFFLOAD_PTYPE, it wouldn't need to set anything
(neither ol_flags, neither packet_type).

> and it complicates the
> application to have additional check based on ol_flag. If you see any corner case with above section,
> 
> How about just setting as ptype as 0 incase it is not parsed by driver.

As I said above - ok by me.
AFAIK, this is current behavior, so no changes in PMD will be required.

> Actual lookup is the costly one, writing 0 to pytpe is not costly
> as there are plenty of writes in Rx and it will be write merged(No CPU stall)

Yes packet_type is at first 64B, so shouldn't cause any extra overhead.

> 
> I did not get the complete picture of "rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE? scheme", Does it help?

I thought about it as just a different way to disable(/limit) requested by user PTYPE support.
If let say user is not interested in ptype information at all, he can ask PMD to just 
always set ptype value to 0:
rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_UNKNOWN);

if he is interested just in L2/L3 layer info,
he can ask PMD to provide ptype information only for L2/L3:
rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK);

Or to enable all supported by PMD ptypes: 
rte_eth_dev_set_supported_ptypes(port, UINT32_MAX)




More information about the dev mailing list