DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
@ 2019-08-07  2:04 Jerin Jacob Kollanukkaran
  2019-08-07  8:32 ` Andrew Rybchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-08-07  2:04 UTC (permalink / raw)
  To: Stephen Hemminger, Andrew Rybchenko
  Cc: Pavan Nikhilesh Bhagavatula, Hemant Agrawal, dev

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, August 7, 2019 4:45 AM
> To: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
> 
> On Tue, 6 Aug 2019 12:06:35 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> > On 8/6/19 11:47 AM, Pavan Nikhilesh Bhagavatula wrote:
> > >
> > >> -----Original Message-----
> > >> From: Hemant Agrawal <hemant.agrawal@nxp.com>
> > >> Sent: Tuesday, August 6, 2019 1:49 PM
> > >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
> > >> Jacob Kollanukkaran <jerinj@marvell.com>
> > >> Cc: dev@dpdk.org
> > >> Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
> > >>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
> > >>>
> > >>> Currently, most of the NICs already support PTYPE parsing and
> > >>> update
> > >> the
> > >>> mbuf->packet_type through an internal lookup table, but there is
> > >>> mbuf->no
> > >> way to
> > >>> disable the lookup if the application is not intrested in ptypes
> > >> returned by
> > >>> `rte_eth_dev_get_supported_ptypes`.
> > >>>
> > >> [Hemant]  it will also mean introducing another check in datapath,
> > >> if the application has asked for PTYPE offload - copy the results
> > >> to mbuf-
> > >>> packet_type otherwise don't do it.
> > > I think that having the check would give better performance than
> > > loading ptype table to L1 doing  a lookup and copying it to mbuf when the
> application doesn't need it.
> >
> > Anyway, if PMD decides that it is better to always provide packet type
> > information - there is no harm. Basically if the offload is not
> > requested it makes packet_type undefined in mbuf.
> >
> > >> Your second patch is incomplete in the sense that it only adds the
> > >> capability. But it does not disable the lookups?
> > > It is upto the maintainer of the PMD to disable the lookup in data
> > > path. If there is a scope of optimization then they could do it. There is no
> harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.
> > > I was hesitant to touch data path as it would be impossible to verify
> performance effect on all NICs.
> >
> > I think it is the right way to approach it especially taking
> > transition into account.
> >
> 
> With hardline API policy, this has to fail on compile for old applications.

Not specific to this API change. That's is the propriety any new symbol addition
to the code base.

Planning to make this API change available from v19.11 LTS.

> You can't magically assume that applications using ptype will set new feature.

When OFFLOAD flags got introduced, we decided to disable all offloads by default.
So, need to add positive logic here to enable offload instead of enable something by
Default and disable if required to get have synergy with other offloads.

Will update the release note as usual to document the change. 
Since there is NO ABI change, IMO, we don't need deprecation notice.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-07  2:04 [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload Jerin Jacob Kollanukkaran
@ 2019-08-07  8:32 ` Andrew Rybchenko
  2019-08-07 15:22   ` Stephen Hemminger
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Rybchenko @ 2019-08-07  8:32 UTC (permalink / raw)
  To: Jerin Jacob Kollanukkaran, Stephen Hemminger
  Cc: Pavan Nikhilesh Bhagavatula, Hemant Agrawal, dev

On 8/7/19 5:04 AM, Jerin Jacob Kollanukkaran wrote:
>> -----Original Message-----
>> From: Stephen Hemminger <stephen@networkplumber.org>
>> Sent: Wednesday, August 7, 2019 4:45 AM
>> To: Andrew Rybchenko <arybchenko@solarflare.com>
>> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>; dev@dpdk.org
>> Subject: [EXT] Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
>>
>> On Tue, 6 Aug 2019 12:06:35 +0300
>> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>>
>>> On 8/6/19 11:47 AM, Pavan Nikhilesh Bhagavatula wrote:
>>>>> -----Original Message-----
>>>>> From: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>>> Sent: Tuesday, August 6, 2019 1:49 PM
>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>>>>> Jacob Kollanukkaran <jerinj@marvell.com>
>>>>> Cc: dev@dpdk.org
>>>>> Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
>>>>>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
>>>>>>
>>>>>> Currently, most of the NICs already support PTYPE parsing and
>>>>>> update
>>>>> the
>>>>>> mbuf->packet_type through an internal lookup table, but there is
>>>>>> mbuf->no
>>>>> way to
>>>>>> disable the lookup if the application is not intrested in ptypes
>>>>> returned by
>>>>>> `rte_eth_dev_get_supported_ptypes`.
>>>>>>
>>>>> [Hemant]  it will also mean introducing another check in datapath,
>>>>> if the application has asked for PTYPE offload - copy the results
>>>>> to mbuf-
>>>>>> packet_type otherwise don't do it.
>>>> I think that having the check would give better performance than
>>>> loading ptype table to L1 doing  a lookup and copying it to mbuf when the
>> application doesn't need it.
>>> Anyway, if PMD decides that it is better to always provide packet type
>>> information - there is no harm. Basically if the offload is not
>>> requested it makes packet_type undefined in mbuf.
>>>
>>>>> Your second patch is incomplete in the sense that it only adds the
>>>>> capability. But it does not disable the lookups?
>>>> It is upto the maintainer of the PMD to disable the lookup in data
>>>> path. If there is a scope of optimization then they could do it. There is no
>> harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.
>>>> I was hesitant to touch data path as it would be impossible to verify
>> performance effect on all NICs.
>>> I think it is the right way to approach it especially taking
>>> transition into account.
>>>
>> With hardline API policy, this has to fail on compile for old applications.

Stephen, could you explain a bit more why.

> Not specific to this API change. That's is the propriety any new symbol addition
> to the code base.
>
> Planning to make this API change available fromv19.11 LTS.

The only way to to require applications to enable PTYPE offload to get
ptypes in mbuf since 19.11 LTS is to have deprecation notice in 19.08.

>> You can't magically assume that applications using ptype will set new feature.
> When OFFLOAD flags got introduced, we decided to disable all offloads by default.
> So, need to add positive logic here to enable offload instead of enable something by
> Default and disable if required to get have synergy with other offloads.
>
> Will update the release note as usual to document the change.
> Since there is NO ABI change, IMO, we don't need deprecation notice.

Sorry, but it is a behaviour change. Before an application does not need
to enable ptype offload, but now it is required. It means that application
will be broken and, therefore, it requires deprecation notice.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-07  8:32 ` Andrew Rybchenko
@ 2019-08-07 15:22   ` Stephen Hemminger
  2019-08-07 15:44     ` Andrew Rybchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2019-08-07 15:22 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Jerin Jacob Kollanukkaran, Pavan Nikhilesh Bhagavatula,
	Hemant Agrawal, dev

On Wed, 7 Aug 2019 11:32:35 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 8/7/19 5:04 AM, Jerin Jacob Kollanukkaran wrote:
> >> -----Original Message-----
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Sent: Wednesday, August 7, 2019 4:45 AM
> >> To: Andrew Rybchenko <arybchenko@solarflare.com>
> >> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Hemant
> >> Agrawal <hemant.agrawal@nxp.com>; Jerin Jacob Kollanukkaran
> >> <jerinj@marvell.com>; dev@dpdk.org
> >> Subject: [EXT] Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
> >>
> >> On Tue, 6 Aug 2019 12:06:35 +0300
> >> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> >>  
> >>> On 8/6/19 11:47 AM, Pavan Nikhilesh Bhagavatula wrote:  
> >>>>> -----Original Message-----
> >>>>> From: Hemant Agrawal <hemant.agrawal@nxp.com>
> >>>>> Sent: Tuesday, August 6, 2019 1:49 PM
> >>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
> >>>>> Jacob Kollanukkaran <jerinj@marvell.com>
> >>>>> Cc: dev@dpdk.org
> >>>>> Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload  
> >>>>>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
> >>>>>>
> >>>>>> Currently, most of the NICs already support PTYPE parsing and
> >>>>>> update  
> >>>>> the  
> >>>>>> mbuf->packet_type through an internal lookup table, but there is
> >>>>>> mbuf->no  
> >>>>> way to  
> >>>>>> disable the lookup if the application is not intrested in ptypes  
> >>>>> returned by  
> >>>>>> `rte_eth_dev_get_supported_ptypes`.
> >>>>>>  
> >>>>> [Hemant]  it will also mean introducing another check in datapath,
> >>>>> if the application has asked for PTYPE offload - copy the results
> >>>>> to mbuf-  
> >>>>>> packet_type otherwise don't do it.  
> >>>> I think that having the check would give better performance than
> >>>> loading ptype table to L1 doing  a lookup and copying it to mbuf when the  
> >> application doesn't need it.  
> >>> Anyway, if PMD decides that it is better to always provide packet type
> >>> information - there is no harm. Basically if the offload is not
> >>> requested it makes packet_type undefined in mbuf.
> >>>  
> >>>>> Your second patch is incomplete in the sense that it only adds the
> >>>>> capability. But it does not disable the lookups?  
> >>>> It is upto the maintainer of the PMD to disable the lookup in data
> >>>> path. If there is a scope of optimization then they could do it. There is no  
> >> harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.  
> >>>> I was hesitant to touch data path as it would be impossible to verify  
> >> performance effect on all NICs.  
> >>> I think it is the right way to approach it especially taking
> >>> transition into account.
> >>>  
> >> With hardline API policy, this has to fail on compile for old applications.  
> 
> Stephen, could you explain a bit more why.

Existing releases packets will be received with ptype for hardware that
supports it. We should not require users to change their application to
continue to get mbufs with ptype.  If your change would break that, and
require application to change; then your change should break the API in
a hard way that causes compile rather than runtime failure.

The best solution would be to just keep old applications running and compiling
without breaking anything. That means ptype should still be received.

If (as an optimization) you want to allow application to turn of getting
ptype; then that would be a useful. Probably best done at the port level
as part of configuration.

> 
> > Not specific to this API change. That's is the propriety any new symbol addition
> > to the code base.
> >
> > Planning to make this API change available fromv19.11 LTS.  
> 
> The only way to to require applications to enable PTYPE offload to get
> ptypes in mbuf since 19.11 LTS is to have deprecation notice in 19.08.
> 
> >> You can't magically assume that applications using ptype will set new feature.  
> > When OFFLOAD flags got introduced, we decided to disable all offloads by default.
> > So, need to add positive logic here to enable offload instead of enable something by
> > Default and disable if required to get have synergy with other offloads.
> >
> > Will update the release note as usual to document the change.
> > Since there is NO ABI change, IMO, we don't need deprecation notice.  
> 
> Sorry, but it is a behaviour change. Before an application does not need
> to enable ptype offload, but now it is required. It means that application
> will be broken and, therefore, it requires deprecation notice.

The DPDK development community has to make not breaking applications
a higher priority than adding marginal enhancements


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-07 15:22   ` Stephen Hemminger
@ 2019-08-07 15:44     ` Andrew Rybchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Rybchenko @ 2019-08-07 15:44 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jerin Jacob Kollanukkaran, Pavan Nikhilesh Bhagavatula,
	Hemant Agrawal, dev

On 8/7/19 6:22 PM, Stephen Hemminger wrote:
> On Wed, 7 Aug 2019 11:32:35 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
>> On 8/7/19 5:04 AM, Jerin Jacob Kollanukkaran wrote:
>>>> -----Original Message-----
>>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>>> Sent: Wednesday, August 7, 2019 4:45 AM
>>>> To: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Hemant
>>>> Agrawal <hemant.agrawal@nxp.com>; Jerin Jacob Kollanukkaran
>>>> <jerinj@marvell.com>; dev@dpdk.org
>>>> Subject: [EXT] Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
>>>>
>>>> On Tue, 6 Aug 2019 12:06:35 +0300
>>>> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>>>>   
>>>>> On 8/6/19 11:47 AM, Pavan Nikhilesh Bhagavatula wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>>>>> Sent: Tuesday, August 6, 2019 1:49 PM
>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>>>>>>> Jacob Kollanukkaran <jerinj@marvell.com>
>>>>>>> Cc: dev@dpdk.org
>>>>>>> Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
>>>>>>>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
>>>>>>>>
>>>>>>>> Currently, most of the NICs already support PTYPE parsing and
>>>>>>>> update
>>>>>>> the
>>>>>>>> mbuf->packet_type through an internal lookup table, but there is
>>>>>>>> mbuf->no
>>>>>>> way to
>>>>>>>> disable the lookup if the application is not intrested in ptypes
>>>>>>> returned by
>>>>>>>> `rte_eth_dev_get_supported_ptypes`.
>>>>>>>>   
>>>>>>> [Hemant]  it will also mean introducing another check in datapath,
>>>>>>> if the application has asked for PTYPE offload - copy the results
>>>>>>> to mbuf-
>>>>>>>> packet_type otherwise don't do it.
>>>>>> I think that having the check would give better performance than
>>>>>> loading ptype table to L1 doing  a lookup and copying it to mbuf when the
>>>> application doesn't need it.
>>>>> Anyway, if PMD decides that it is better to always provide packet type
>>>>> information - there is no harm. Basically if the offload is not
>>>>> requested it makes packet_type undefined in mbuf.
>>>>>   
>>>>>>> Your second patch is incomplete in the sense that it only adds the
>>>>>>> capability. But it does not disable the lookups?
>>>>>> It is upto the maintainer of the PMD to disable the lookup in data
>>>>>> path. If there is a scope of optimization then they could do it. There is no
>>>> harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.
>>>>>> I was hesitant to touch data path as it would be impossible to verify
>>>> performance effect on all NICs.
>>>>> I think it is the right way to approach it especially taking
>>>>> transition into account.
>>>>>   
>>>> With hardline API policy, this has to fail on compile for old applications.
>>
>> Stephen, could you explain a bit more why.
> 
> Existing releases packets will be received with ptype for hardware that
> supports it. We should not require users to change their application to
> continue to get mbufs with ptype.  If your change would break that, and
> require application to change; then your change should break the API in
> a hard way that causes compile rather than runtime failure.

Many thanks, I got it.

> The best solution would be to just keep old applications running and compiling
> without breaking anything. That means ptype should still be received.
> 
> If (as an optimization) you want to allow application to turn of getting
> ptype; then that would be a useful. Probably best done at the port level
> as part of configuration.

I see, but it contradicts to the existing practice that offloads should
be disabled by default and a way to enable should be provided.
May be techboard should discuss it and make a decision (covering RSS
hash information and Rx mark mentioned in my review notes).

>>> Not specific to this API change. That's is the propriety any new symbol addition
>>> to the code base.
>>>
>>> Planning to make this API change available fromv19.11 LTS.
>>
>> The only way to to require applications to enable PTYPE offload to get
>> ptypes in mbuf since 19.11 LTS is to have deprecation notice in 19.08.
>>
>>>> You can't magically assume that applications using ptype will set new feature.
>>> When OFFLOAD flags got introduced, we decided to disable all offloads by default.
>>> So, need to add positive logic here to enable offload instead of enable something by
>>> Default and disable if required to get have synergy with other offloads.
>>>
>>> Will update the release note as usual to document the change.
>>> Since there is NO ABI change, IMO, we don't need deprecation notice.
>>
>> Sorry, but it is a behaviour change. Before an application does not need
>> to enable ptype offload, but now it is required. It means that application
>> will be broken and, therefore, it requires deprecation notice.
> 
> The DPDK development community has to make not breaking applications
> a higher priority than adding marginal enhancements

Fair, but where is marginal enhancements boundary?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-06  9:06     ` Andrew Rybchenko
@ 2019-08-06 23:15       ` Stephen Hemminger
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2019-08-06 23:15 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: Pavan Nikhilesh Bhagavatula, Hemant Agrawal,
	Jerin Jacob Kollanukkaran, dev

On Tue, 6 Aug 2019 12:06:35 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 8/6/19 11:47 AM, Pavan Nikhilesh Bhagavatula wrote:
> >  
> >> -----Original Message-----
> >> From: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> Sent: Tuesday, August 6, 2019 1:49 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
> >> Jacob Kollanukkaran <jerinj@marvell.com>
> >> Cc: dev@dpdk.org
> >> Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload  
> >>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
> >>>
> >>> Currently, most of the NICs already support PTYPE parsing and update  
> >> the  
> >>> mbuf->packet_type through an internal lookup table, but there is no  
> >> way to  
> >>> disable the lookup if the application is not intrested in ptypes  
> >> returned by  
> >>> `rte_eth_dev_get_supported_ptypes`.
> >>>  
> >> [Hemant]  it will also mean introducing another check in datapath, if the
> >> application has asked for PTYPE offload - copy the results to mbuf-  
> >>> packet_type otherwise don't do it.  
> > I think that having the check would give better performance than loading ptype table to L1
> > doing  a lookup and copying it to mbuf when the application doesn't need it.  
> 
> Anyway, if PMD decides that it is better to always provide packet type
> information - there is no harm. Basically if the offload is not requested
> it makes packet_type undefined in mbuf.
> 
> >> Your second patch is incomplete in the sense that it only adds the
> >> capability. But it does not disable the lookups?  
> > It is upto the maintainer of the PMD to disable the lookup in data path. If there is a scope of optimization
> > then they could do it. There is no harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.
> > I was hesitant to touch data path as it would be impossible to verify performance effect on all NICs.  
> 
> I think it is the right way to approach it especially taking transition 
> into account.
> 

With hardline API policy, this has to fail on compile for old applications.
You can't magically assume that applications using ptype will set new feature.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-06  8:47   ` Pavan Nikhilesh Bhagavatula
@ 2019-08-06  9:06     ` Andrew Rybchenko
  2019-08-06 23:15       ` Stephen Hemminger
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Rybchenko @ 2019-08-06  9:06 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Hemant Agrawal, Jerin Jacob Kollanukkaran
  Cc: dev

On 8/6/19 11:47 AM, Pavan Nikhilesh Bhagavatula wrote:
>
>> -----Original Message-----
>> From: Hemant Agrawal <hemant.agrawal@nxp.com>
>> Sent: Tuesday, August 6, 2019 1:49 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>> Jacob Kollanukkaran <jerinj@marvell.com>
>> Cc: dev@dpdk.org
>> Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
>>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
>>>
>>> Currently, most of the NICs already support PTYPE parsing and update
>> the
>>> mbuf->packet_type through an internal lookup table, but there is no
>> way to
>>> disable the lookup if the application is not intrested in ptypes
>> returned by
>>> `rte_eth_dev_get_supported_ptypes`.
>>>
>> [Hemant]  it will also mean introducing another check in datapath, if the
>> application has asked for PTYPE offload - copy the results to mbuf-
>>> packet_type otherwise don't do it.
> I think that having the check would give better performance than loading ptype table to L1
> doing  a lookup and copying it to mbuf when the application doesn't need it.

Anyway, if PMD decides that it is better to always provide packet type
information - there is no harm. Basically if the offload is not requested
it makes packet_type undefined in mbuf.

>> Your second patch is incomplete in the sense that it only adds the
>> capability. But it does not disable the lookups?
> It is upto the maintainer of the PMD to disable the lookup in data path. If there is a scope of optimization
> then they could do it. There is no harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.
> I was hesitant to touch data path as it would be impossible to verify performance effect on all NICs.

I think it is the right way to approach it especially taking transition 
into account.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-06  8:19 ` Hemant Agrawal
@ 2019-08-06  8:47   ` Pavan Nikhilesh Bhagavatula
  2019-08-06  9:06     ` Andrew Rybchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2019-08-06  8:47 UTC (permalink / raw)
  To: Hemant Agrawal, Jerin Jacob Kollanukkaran; +Cc: dev



>-----Original Message-----
>From: Hemant Agrawal <hemant.agrawal@nxp.com>
>Sent: Tuesday, August 6, 2019 1:49 PM
>To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>Jacob Kollanukkaran <jerinj@marvell.com>
>Cc: dev@dpdk.org
>Subject: RE: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
>>
>> Add PTYPE to DEV_RX_OFFLOAD_* flags.
>>
>> Currently, most of the NICs already support PTYPE parsing and update
>the
>> mbuf->packet_type through an internal lookup table, but there is no
>way to
>> disable the lookup if the application is not intrested in ptypes
>returned by
>> `rte_eth_dev_get_supported_ptypes`.
>>
>[Hemant]  it will also mean introducing another check in datapath, if the
>application has asked for PTYPE offload - copy the results to mbuf-
>>packet_type otherwise don't do it.
>

I think that having the check would give better performance than loading ptype table to L1 
doing  a lookup and copying it to mbuf when the application doesn't need it.

>Your second patch is incomplete in the sense that it only adds the
>capability. But it does not disable the lookups?

It is upto the maintainer of the PMD to disable the lookup in data path. If there is a scope of optimization 
then they could do it. There is no harm in exposing  PTYPE even RX_OFFLOAD_PTYPE is not enabled.
I was hesitant to touch data path as it would be impossible to verify performance effect on all NICs.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload
  2019-08-06  8:02 pbhagavatula
@ 2019-08-06  8:19 ` Hemant Agrawal
  2019-08-06  8:47   ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 9+ messages in thread
From: Hemant Agrawal @ 2019-08-06  8:19 UTC (permalink / raw)
  To: pbhagavatula, jerinj; +Cc: dev

> 
> Add PTYPE to DEV_RX_OFFLOAD_* flags.
> 
> Currently, most of the NICs already support PTYPE parsing and update the
> mbuf->packet_type through an internal lookup table, but there is no way to
> disable the lookup if the application is not intrested in ptypes returned by
> `rte_eth_dev_get_supported_ptypes`.
> 
[Hemant]  it will also mean introducing another check in datapath, if the application has asked for PTYPE offload - copy the results to mbuf->packet_type otherwise don't do it. 

Your second patch is incomplete in the sense that it only adds the capability. But it does not disable the lookups? 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev]  [RFC 0/3] ethdev: add ptype as Rx offload
@ 2019-08-06  8:02 pbhagavatula
  2019-08-06  8:19 ` Hemant Agrawal
  0 siblings, 1 reply; 9+ messages in thread
From: pbhagavatula @ 2019-08-06  8:02 UTC (permalink / raw)
  To: jerinj; +Cc: dev, Pavan Nikhilesh

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add PTYPE to DEV_RX_OFFLOAD_* flags.

Currently, most of the NICs already support PTYPE parsing and update
the mbuf->packet_type through an internal lookup table, but there is
no way to disable the lookup if the application is not intrested in
ptypes returned by `rte_eth_dev_get_supported_ptypes`.

Pavan Nikhilesh (3):
  ethdev: add ptype as an Rx offload
  net: update Rx offload capabilities
  examples: add Rx ptype offload

 doc/guides/nics/features.rst                    |  3 +++
 drivers/net/atlantic/atl_ethdev.c               |  3 ++-
 drivers/net/bnxt/bnxt_ethdev.c                  |  3 ++-
 drivers/net/cxgbe/cxgbe.h                       |  3 ++-
 drivers/net/dpaa/dpaa_ethdev.c                  |  3 ++-
 drivers/net/dpaa2/dpaa2_ethdev.c                |  3 ++-
 drivers/net/e1000/em_rxtx.c                     |  3 ++-
 drivers/net/e1000/igb_rxtx.c                    |  3 ++-
 drivers/net/enetc/enetc_ethdev.c                |  3 ++-
 drivers/net/enic/enic_res.c                     |  3 ++-
 drivers/net/failsafe/failsafe_ops.c             |  6 ++++--
 drivers/net/fm10k/fm10k_ethdev.c                | 15 ++++++++-------
 drivers/net/i40e/i40e_ethdev.c                  |  3 ++-
 drivers/net/iavf/iavf_ethdev.c                  |  3 ++-
 drivers/net/ice/ice_ethdev.c                    |  3 ++-
 drivers/net/ixgbe/ixgbe_rxtx.c                  |  3 ++-
 drivers/net/mlx4/mlx4_rxq.c                     |  3 ++-
 drivers/net/mlx5/mlx5_rxq.c                     |  3 ++-
 drivers/net/mvneta/mvneta_ethdev.h              |  3 ++-
 drivers/net/mvpp2/mrvl_ethdev.c                 |  3 ++-
 drivers/net/netvsc/hn_rndis.c                   |  3 ++-
 drivers/net/nfp/nfp_net.c                       |  3 ++-
 drivers/net/octeontx/octeontx_ethdev.h          |  3 ++-
 drivers/net/octeontx2/otx2_ethdev.c             |  5 +++++
 drivers/net/octeontx2/otx2_ethdev.h             | 15 ++++++++-------
 drivers/net/qede/qede_ethdev.c                  |  3 ++-
 drivers/net/tap/rte_eth_tap.c                   |  3 ++-
 drivers/net/thunderx/nicvf_ethdev.h             |  3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c            |  3 ++-
 examples/ip_fragmentation/main.c                |  7 +++++++
 examples/l3fwd-power/main.c                     |  8 ++++++++
 examples/l3fwd/main.c                           |  9 +++++++++
 examples/performance-thread/l3fwd-thread/main.c |  9 +++++++++
 examples/tep_termination/vxlan_setup.c          |  1 +
 lib/librte_ethdev/rte_ethdev.h                  |  1 +
 35 files changed, 111 insertions(+), 40 deletions(-)

--
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-08-07 15:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  2:04 [dpdk-dev] [RFC 0/3] ethdev: add ptype as Rx offload Jerin Jacob Kollanukkaran
2019-08-07  8:32 ` Andrew Rybchenko
2019-08-07 15:22   ` Stephen Hemminger
2019-08-07 15:44     ` Andrew Rybchenko
  -- strict thread matches above, loose matches on Subject: below --
2019-08-06  8:02 pbhagavatula
2019-08-06  8:19 ` Hemant Agrawal
2019-08-06  8:47   ` Pavan Nikhilesh Bhagavatula
2019-08-06  9:06     ` Andrew Rybchenko
2019-08-06 23:15       ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).