Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

[Pavan Nikhilesh Bhagavatula]

>29/10/2019 16:37, pbhagavatula@marvell.com:
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> --- a/doc/guides/rel_notes/release_19_11.rst
>> +++ b/doc/guides/rel_notes/release_19_11.rst
>> @@ -231,6 +231,14 @@ New Features
>>    * Added a console command to testpmd app, ``show port (port_id)
>ptypes`` which
>>      gives ability to print port supported ptypes in different protocol
>layers.
>>
>> +* **Added ethdev API to set supported packet types**
>> +
>> +  *  Added new API ``rte_eth_dev_set_supported_ptypes`` that
>allows an
>> +     application to inform PMD about packet types classification the
>application
>> +     is interested in
>> +  *  This scheme will allow PMDs to avoid lookup to internal ptype
>table on Rx
>> +     and thereby improve Rx performance if application wishes do so.
>
>You just added or rebased this paragraph at the end.
>As mentioned in the release notes files (top of the section),
>there is an order for presenting features.

Will fix in v16.

>
>>  Removed Items
>>  -------------
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Inform Ethernet device of the packet types classification the
>recipient is
>> + * interested in.
>
>This is a bit convoluted.
>What about this?
>"Optimize driver handling of packet types by reducing its range."

@arybchenko@solarflare.com Thoughts?

>
>> + *
>> + * Application can use this function to set only specific ptypes that it's
>> + * interested. This information can be used by the PMD to optimize
>Rx path.
>> + *
>> + * The function accepts an array `set_ptypes` allocated by the caller
>to
>> + * store the packet types set by the driver, the last element of the
>array
>> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array
>should be
>> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be
>filled
>> + * partially.
>> + *
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @param ptype_mask
>> + *   The ptype family that application is interested in should be
>bitwise OR of
>> + *   RTE_PTYPE_*_MASK or 0.
>> + * @param set_ptypes
>> + *   An array pointer to store set packet types, allocated by caller. The
>> + *   function marks the end of array with RTE_PTYPE_UNKNOWN.
>> + * @param num
>> + *   Size of the array pointed by param ptypes.
>> + *   Should be rte_eth_dev_get_supported_ptypes() + 1 to
>accommodate the
>> + *   set ptypes.
>> + * @return
>> + *   - (0) if Success.
>> + *   - (-ENODEV) if *port_id* invalid.
>> + *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
>> + *     num > 0.
>> + */
>
>John, please you check the English wording?
>
>> +__rte_experimental
>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
>ptype_mask,
>> +				     uint32_t *set_ptypes, unsigned int
>num);
>
>I don't like the name of the function because they are
>not "supported" packet types but "requested".
>What about replacing "set_supported" with "set_allowed", or
>"white_list"?

"white_list" seems ok but hope it doesn't call for blacklisting API.

>

Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

[Andrew Rybchenko]

On 10/31/19 7:38 PM, Pavan Nikhilesh Bhagavatula wrote:
>> 29/10/2019 16:37, pbhagavatula@marvell.com:
>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>  Removed Items
>>>  -------------
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Inform Ethernet device of the packet types classification the
>> recipient is
>>> + * interested in.
>> This is a bit convoluted.
>> What about this?
>> "Optimize driver handling of packet types by reducing its range."
> @arybchenko@solarflare.com Thoughts?

Optimize is a possible side effect of the operation, but there is
no any promise that something will be optimized.
I thought that current description explains what happens.
Below statements try to explain why it may be useful.
Any other options?

>>> + *
>>> + * Application can use this function to set only specific ptypes that it's
>>> + * interested. This information can be used by the PMD to optimize
>> Rx path.
>>> + *
>>> + * The function accepts an array `set_ptypes` allocated by the caller
>> to
>>> + * store the packet types set by the driver, the last element of the
>> array
>>> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array
>> should be
>>> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be
>> filled
>>> + * partially.
>>> + *
>>> + * @param port_id
>>> + *   The port identifier of the Ethernet device.
>>> + * @param ptype_mask
>>> + *   The ptype family that application is interested in should be
>> bitwise OR of
>>> + *   RTE_PTYPE_*_MASK or 0.
>>> + * @param set_ptypes
>>> + *   An array pointer to store set packet types, allocated by caller. The
>>> + *   function marks the end of array with RTE_PTYPE_UNKNOWN.
>>> + * @param num
>>> + *   Size of the array pointed by param ptypes.
>>> + *   Should be rte_eth_dev_get_supported_ptypes() + 1 to
>> accommodate the
>>> + *   set ptypes.
>>> + * @return
>>> + *   - (0) if Success.
>>> + *   - (-ENODEV) if *port_id* invalid.
>>> + *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
>>> + *     num > 0.
>>> + */
>> John, please you check the English wording?
>>
>>> +__rte_experimental
>>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
>> ptype_mask,
>>> +				     uint32_t *set_ptypes, unsigned int
>> num);
>>
>> I don't like the name of the function because they are
>> not "supported" packet types but "requested".
>> What about replacing "set_supported" with "set_allowed", or
>> "white_list"?
> "white_list" seems ok but hope it doesn't call for blacklisting API.

"white_list" suggests that it is guaranteed that nothing else will
be reported. At least for me. However, I agree that set_supported
is not nice and I accepted it just to keep API naming symmetric.
May be it is really misleading here. May be just: rte_eth_dev_set_ptypes()?

Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

[Thomas Monjalon]

01/11/2019 11:55, Andrew Rybchenko:
> On 10/31/19 7:38 PM, Pavan Nikhilesh Bhagavatula wrote:
> >> 29/10/2019 16:37, pbhagavatula@marvell.com:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>  Removed Items
> >>>  -------------
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> +/**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>> + *
> >>> + * Inform Ethernet device of the packet types classification the
> >> recipient is
> >>> + * interested in.
> >> This is a bit convoluted.
> >> What about this?
> >> "Optimize driver handling of packet types by reducing its range."
> > @arybchenko@solarflare.com Thoughts?
> 
> Optimize is a possible side effect of the operation, but there is
> no any promise that something will be optimized.
> I thought that current description explains what happens.
> Below statements try to explain why it may be useful.
> Any other options?

"Reduce range of packet types to handle."

> >>> + * Application can use this function to set only specific ptypes that it's
> >>> + * interested. This information can be used by the PMD to optimize
> >> Rx path.
> >>> + *
> >>> + * The function accepts an array `set_ptypes` allocated by the caller
> >> to
> >>> + * store the packet types set by the driver, the last element of the
> >> array
> >>> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array
> >> should be
> >>> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be
> >> filled
> >>> + * partially.
> >>> + *
> >>> + * @param port_id
> >>> + *   The port identifier of the Ethernet device.
> >>> + * @param ptype_mask
> >>> + *   The ptype family that application is interested in should be
> >> bitwise OR of
> >>> + *   RTE_PTYPE_*_MASK or 0.
> >>> + * @param set_ptypes
> >>> + *   An array pointer to store set packet types, allocated by caller. The
> >>> + *   function marks the end of array with RTE_PTYPE_UNKNOWN.
> >>> + * @param num
> >>> + *   Size of the array pointed by param ptypes.
> >>> + *   Should be rte_eth_dev_get_supported_ptypes() + 1 to
> >> accommodate the
> >>> + *   set ptypes.
> >>> + * @return
> >>> + *   - (0) if Success.
> >>> + *   - (-ENODEV) if *port_id* invalid.
> >>> + *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
> >>> + *     num > 0.
> >>> + */
> >> John, please you check the English wording?
> >>
> >>> +__rte_experimental
> >>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> >> ptype_mask,
> >>> +				     uint32_t *set_ptypes, unsigned int
> >> num);
> >>
> >> I don't like the name of the function because they are
> >> not "supported" packet types but "requested".
> >> What about replacing "set_supported" with "set_allowed", or
> >> "white_list"?
> > "white_list" seems ok but hope it doesn't call for blacklisting API.
> 
> "white_list" suggests that it is guaranteed that nothing else will
> be reported. At least for me. However, I agree that set_supported
> is not nice and I accepted it just to keep API naming symmetric.
> May be it is really misleading here. May be just: rte_eth_dev_set_ptypes()?

Maybe the word "allowed" would better fit?

Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

[Andrew Rybchenko]

On 11/2/19 1:25 AM, Thomas Monjalon wrote:
> 01/11/2019 11:55, Andrew Rybchenko:
>> On 10/31/19 7:38 PM, Pavan Nikhilesh Bhagavatula wrote:
>>>> 29/10/2019 16:37, pbhagavatula@marvell.com:
>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>  Removed Items
>>>>>  -------------
>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>> +/**
>>>>> + * @warning
>>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>>>> + *
>>>>> + * Inform Ethernet device of the packet types classification the
>>>> recipient is
>>>>> + * interested in.
>>>> This is a bit convoluted.
>>>> What about this?
>>>> "Optimize driver handling of packet types by reducing its range."
>>> @arybchenko@solarflare.com Thoughts?
>> Optimize is a possible side effect of the operation, but there is
>> no any promise that something will be optimized.
>> I thought that current description explains what happens.
>> Below statements try to explain why it may be useful.
>> Any other options?
> "Reduce range of packet types to handle."

OK

>>>>> + * Application can use this function to set only specific ptypes that it's
>>>>> + * interested. This information can be used by the PMD to optimize
>>>> Rx path.
>>>>> + *
>>>>> + * The function accepts an array `set_ptypes` allocated by the caller
>>>> to
>>>>> + * store the packet types set by the driver, the last element of the
>>>> array
>>>>> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array
>>>> should be
>>>>> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be
>>>> filled
>>>>> + * partially.
>>>>> + *
>>>>> + * @param port_id
>>>>> + *   The port identifier of the Ethernet device.
>>>>> + * @param ptype_mask
>>>>> + *   The ptype family that application is interested in should be
>>>> bitwise OR of
>>>>> + *   RTE_PTYPE_*_MASK or 0.
>>>>> + * @param set_ptypes
>>>>> + *   An array pointer to store set packet types, allocated by caller. The
>>>>> + *   function marks the end of array with RTE_PTYPE_UNKNOWN.
>>>>> + * @param num
>>>>> + *   Size of the array pointed by param ptypes.
>>>>> + *   Should be rte_eth_dev_get_supported_ptypes() + 1 to
>>>> accommodate the
>>>>> + *   set ptypes.
>>>>> + * @return
>>>>> + *   - (0) if Success.
>>>>> + *   - (-ENODEV) if *port_id* invalid.
>>>>> + *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
>>>>> + *     num > 0.
>>>>> + */
>>>> John, please you check the English wording?
>>>>
>>>>> +__rte_experimental
>>>>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
>>>> ptype_mask,
>>>>> +				     uint32_t *set_ptypes, unsigned int
>>>> num);
>>>>
>>>> I don't like the name of the function because they are
>>>> not "supported" packet types but "requested".
>>>> What about replacing "set_supported" with "set_allowed", or
>>>> "white_list"?
>>> "white_list" seems ok but hope it doesn't call for blacklisting API.
>> "white_list" suggests that it is guaranteed that nothing else will
>> be reported. At least for me. However, I agree that set_supported
>> is not nice and I accepted it just to keep API naming symmetric.
>> May be it is really misleading here. May be just: rte_eth_dev_set_ptypes()?
> Maybe the word "allowed" would better fit?

allowed could be treated as everything else is disallowed. So, I'm not sure.

[dpdk-dev] [PATCH v14 0/6] ethdev: add new Rx offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new Rx offload flags `DEV_RX_OFFLOAD_RSS_HASH` These flags can be used
to enable/disable PMD writes to rte_mbuf fields `hash.rss` and also
`ol_flags:PKT_RX_RSS` and `ol_flags:PKT_RX_FDIR`.

Add new packet type set function `rte_eth_dev_set_supported_ptypes`,
allows application to inform PMDs about the packet types it is interested
in. Based on ptypes requested by application PMDs can optimize the Rx path.

For example, if a given PMD doesn't support any packet types that the
application is interested in then the application can disable[1] writes to
`mbuf.packet_type` done by the PMD and use a software ptype parser.
     [1] rte_eth_dev_set_supported_ptypes(*port_id*, RTE_PTYPE_UNKNOWN,
					  NULL, 0);

v14 Changes:
-----------
- Remove log from drives
- Add log in rte_eth_dev_configure when certain offloads are requested to be
  disabled and PMD cannot honor the request.
- Make changes to default offloads in net/sfc.(Andrew)

v13 Changes:
-----------
- Remove DEV_RX_OFFLOAD_FLOW_MARK from this patchset to allow foreward
  progress will be sent as a seperate patch.
- Use set_supported function only for l2fwd and testpmd.
- Add info log in drivers which expose the DEV_RX_OFFLOAD_RSS_HASH
  indicating that disabling DEV_RX_OFFLOAD_RSS_HASH is not supported.
- Few documentation changes.

v12 Changes:
-----------
- Rebase onto next-net.

v11 Changes:
-----------
- Use RTE_DIM to get array size.
- Since we are using a list of MASKs to validate ptype_mask return -EINVAL
  if any unknown mask is set.
- Rebase to TOT.

v10 Changes:
-----------
- Modify ptype_mask validation in set_supported_ptypes.(Andrew)

v9 Changes:
----------
- Add ptype_mask validation in set_supported_ptypes.(Andrew)
- Make description more verbose.

v8 Changes:
----------
- Make description more verbose.
- Set RTE_PTYPE_UNKNOWN in set_ptypes array when either get ot set ptypes
  is not supported by ethernet device.

v7 Changes:
----------
- Fix unused variable in net/octeontx2

v6 Changes:
----------
- Add additional checks for set supported ptypes.(Andrew)
- Clarify `rte_eth_dev_set_supported_ptypes` documentation.
- Remove DEV_RX_OFFLOAD_FLOW_MARK emulation from net/octeontx2.

v5 Changes:
----------
- Fix typos.

v4 Changes:
----------
- Set the last element in set_ptype array as RTE_PTYPE_UNKNOWN to mark the end
  of array.
- Fix invalid set ptype function call in examples.
- Remove setting rte_eth_dev_set_supported_ptypes to UNKNOWN in l3fwd-power.

v3 Changes:
----------
- Add missing release notes. (Andrew)
- Re-word various descriptions.
- Fix ptype set logic.

v2 Changes:
----------
- Update release notes. (Andrew)
- Redo commit logs. (Andrew)
- Disable ptype parsing for unsupported examples. (Jerin)
- Disable RSS write only in generic mode eventdev_pipeline. (Jerin)
- Modify set_supported_ptypes function to return successfuly set mask
  instead of failure.
- Dropped set_supported_ptypes to drivers by handling in library
  layer, interested PMD can add it in.


Pavan Nikhilesh (7):
  ethdev: add set ptype function
  ethdev: add mbuf RSS update as an offload
  ethdev: log offloads that can't be disabled by PMD
  drivers/net: update Rx RSS hash offload capabilities
  examples/eventdev_pipeline: add new Rx RSS hash offload
  examples/l2fwd: disable ptype parsing
  app/testpmd: add command to set supported ptype mask

 app/test-pmd/cmdline.c                        |  80 +++++++++++
 app/test-pmd/testpmd.c                        |   6 +
 doc/guides/nics/features.rst                  |   9 +-
 doc/guides/rel_notes/release_19_11.rst        |  15 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst   |   7 +
 drivers/net/bnxt/bnxt_ethdev.c                |   8 +-
 drivers/net/cxgbe/cxgbe.h                     |   3 +-
 drivers/net/cxgbe/cxgbe_ethdev.c              |   5 +
 drivers/net/dpaa/dpaa_ethdev.c                |   3 +-
 drivers/net/dpaa2/dpaa2_ethdev.c              |   1 +
 drivers/net/e1000/igb_ethdev.c                |   6 +
 drivers/net/e1000/igb_rxtx.c                  |   3 +-
 drivers/net/enic/enic_ethdev.c                |   5 +
 drivers/net/enic/enic_res.c                   |   3 +-
 drivers/net/fm10k/fm10k_ethdev.c              |   6 +-
 drivers/net/hinic/hinic_pmd_ethdev.c          |   6 +-
 drivers/net/i40e/i40e_ethdev.c                |   6 +-
 drivers/net/iavf/iavf_ethdev.c                |   6 +-
 drivers/net/ice/ice_ethdev.c                  |   6 +-
 drivers/net/ixgbe/ixgbe_ethdev.c              |   7 +
 drivers/net/ixgbe/ixgbe_rxtx.c                |   3 +-
 drivers/net/liquidio/lio_ethdev.c             |   7 +-
 drivers/net/mlx4/mlx4.c                       |   3 +
 drivers/net/mlx4/mlx4_rxq.c                   |   3 +-
 drivers/net/mlx5/mlx5_ethdev.c                |   4 +
 drivers/net/mlx5/mlx5_rxq.c                   |   3 +-
 drivers/net/netvsc/hn_ethdev.c                |   3 +
 drivers/net/netvsc/hn_rndis.c                 |   3 +-
 drivers/net/nfp/nfp_net.c                     |   6 +-
 drivers/net/octeontx2/otx2_ethdev.c           |   3 +-
 drivers/net/octeontx2/otx2_ethdev.h           |  15 +-
 drivers/net/qede/qede_ethdev.c                |   6 +-
 drivers/net/sfc/sfc_ef10_essb_rx.c            |   3 +-
 drivers/net/sfc/sfc_ef10_rx.c                 |   3 +-
 drivers/net/sfc/sfc_rx.c                      |   6 +-
 drivers/net/thunderx/nicvf_ethdev.c           |   3 +
 drivers/net/thunderx/nicvf_ethdev.h           |   3 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c          |   6 +-
 examples/eventdev_pipeline/main.c             | 128 -----------------
 .../pipeline_worker_generic.c                 | 132 ++++++++++++++++++
 .../eventdev_pipeline/pipeline_worker_tx.c    | 128 +++++++++++++++++
 examples/l2fwd/Makefile                       |   1 +
 examples/l2fwd/main.c                         |   2 +
 examples/l2fwd/meson.build                    |   1 +
 lib/librte_ethdev/rte_ethdev.c                | 120 +++++++++++++++-
 lib/librte_ethdev/rte_ethdev.h                |  38 +++++
 lib/librte_ethdev/rte_ethdev_core.h           |  19 +++
 lib/librte_ethdev/rte_ethdev_version.map      |   1 +
 48 files changed, 683 insertions(+), 161 deletions(-)

--
2.17.1

[dpdk-dev] [PATCH v15 0/7] ethdev: add new Rx offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new Rx offload flags `DEV_RX_OFFLOAD_RSS_HASH` These flags can be used
to enable/disable PMD writes to rte_mbuf fields `hash.rss` and also
`ol_flags:PKT_RX_RSS` and `ol_flags:PKT_RX_FDIR`.

Add new packet type set function `rte_eth_dev_set_supported_ptypes`,
allows application to inform PMDs about the packet types it is interested
in. Based on ptypes requested by application PMDs can optimize the Rx path.

For example, if a given PMD doesn't support any packet types that the
application is interested in then the application can disable[1] writes to
`mbuf.packet_type` done by the PMD and use a software ptype parser.
     [1] rte_eth_dev_set_supported_ptypes(*port_id*, RTE_PTYPE_UNKNOWN,
					  NULL, 0);

v15 Changes:
-----------
- Fix sfc RSS_HASH offload check.
- Fix ethdev RSS_HASH offload check when mq_mode is configured with
  MQ_RX_NONE.
- Extend offload validation to return error in the case where application
  has requested an offload to be enabled and PMD couldn't honor it.

v14 Changes:
-----------
- Remove log from drives
- Add log in rte_eth_dev_configure when certain offloads are requested to be
  disabled and PMD cannot honor the request.
- Make changes to default offloads in net/sfc.(Andrew)

v13 Changes:
-----------
- Remove DEV_RX_OFFLOAD_FLOW_MARK from this patchset to allow foreward
  progress will be sent as a seperate patch.
- Use set_supported function only for l2fwd and testpmd.
- Add info log in drivers which expose the DEV_RX_OFFLOAD_RSS_HASH
  indicating that disabling DEV_RX_OFFLOAD_RSS_HASH is not supported.
- Few documentation changes.

v12 Changes:
-----------
- Rebase onto next-net.

v11 Changes:
-----------
- Use RTE_DIM to get array size.
- Since we are using a list of MASKs to validate ptype_mask return -EINVAL
  if any unknown mask is set.
- Rebase to TOT.

v10 Changes:
-----------
- Modify ptype_mask validation in set_supported_ptypes.(Andrew)

v9 Changes:
----------
- Add ptype_mask validation in set_supported_ptypes.(Andrew)
- Make description more verbose.

v8 Changes:
----------
- Make description more verbose.
- Set RTE_PTYPE_UNKNOWN in set_ptypes array when either get ot set ptypes
  is not supported by ethernet device.

v7 Changes:
----------
- Fix unused variable in net/octeontx2

v6 Changes:
----------
- Add additional checks for set supported ptypes.(Andrew)
- Clarify `rte_eth_dev_set_supported_ptypes` documentation.
- Remove DEV_RX_OFFLOAD_FLOW_MARK emulation from net/octeontx2.

v5 Changes:
----------
- Fix typos.

v4 Changes:
----------
- Set the last element in set_ptype array as RTE_PTYPE_UNKNOWN to mark the end
  of array.
- Fix invalid set ptype function call in examples.
- Remove setting rte_eth_dev_set_supported_ptypes to UNKNOWN in l3fwd-power.

v3 Changes:
----------
- Add missing release notes. (Andrew)
- Re-word various descriptions.
- Fix ptype set logic.

v2 Changes:
----------
- Update release notes. (Andrew)
- Redo commit logs. (Andrew)
- Disable ptype parsing for unsupported examples. (Jerin)
- Disable RSS write only in generic mode eventdev_pipeline. (Jerin)
- Modify set_supported_ptypes function to return successfuly set mask
  instead of failure.
- Dropped set_supported_ptypes to drivers by handling in library
  layer, interested PMD can add it in.

Pavan Nikhilesh (7):
  ethdev: add set ptype function
  ethdev: add mbuf RSS update as an offload
  ethdev: add validation to offloads set by PMD
  drivers/net: update Rx RSS hash offload capabilities
  examples/eventdev_pipeline: add new Rx RSS hash offload
  examples/l2fwd: disable ptype parsing
  app/testpmd: add command to set supported ptype mask

 app/test-pmd/cmdline.c                        |  80 +++++++++
 app/test-pmd/testpmd.c                        |   6 +
 doc/guides/nics/features.rst                  |   9 +-
 doc/guides/rel_notes/release_19_11.rst        |  15 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst   |   7 +
 drivers/net/bnxt/bnxt_ethdev.c                |   8 +-
 drivers/net/cxgbe/cxgbe.h                     |   3 +-
 drivers/net/cxgbe/cxgbe_ethdev.c              |   5 +
 drivers/net/dpaa/dpaa_ethdev.c                |   3 +-
 drivers/net/dpaa2/dpaa2_ethdev.c              |   1 +
 drivers/net/e1000/igb_ethdev.c                |   6 +
 drivers/net/e1000/igb_rxtx.c                  |   3 +-
 drivers/net/enic/enic_ethdev.c                |   5 +
 drivers/net/enic/enic_res.c                   |   3 +-
 drivers/net/fm10k/fm10k_ethdev.c              |   6 +-
 drivers/net/hinic/hinic_pmd_ethdev.c          |   6 +-
 drivers/net/i40e/i40e_ethdev.c                |   6 +-
 drivers/net/iavf/iavf_ethdev.c                |   6 +-
 drivers/net/ice/ice_ethdev.c                  |   6 +-
 drivers/net/ixgbe/ixgbe_ethdev.c              |   7 +
 drivers/net/ixgbe/ixgbe_rxtx.c                |   3 +-
 drivers/net/liquidio/lio_ethdev.c             |   7 +-
 drivers/net/mlx4/mlx4.c                       |   3 +
 drivers/net/mlx4/mlx4_rxq.c                   |   3 +-
 drivers/net/mlx5/mlx5_ethdev.c                |   4 +
 drivers/net/mlx5/mlx5_rxq.c                   |   3 +-
 drivers/net/netvsc/hn_ethdev.c                |   3 +
 drivers/net/netvsc/hn_rndis.c                 |   3 +-
 drivers/net/nfp/nfp_net.c                     |   6 +-
 drivers/net/octeontx2/otx2_ethdev.c           |   3 +-
 drivers/net/octeontx2/otx2_ethdev.h           |  15 +-
 drivers/net/qede/qede_ethdev.c                |   6 +-
 drivers/net/sfc/sfc_ef10_essb_rx.c            |   3 +-
 drivers/net/sfc/sfc_ef10_rx.c                 |   3 +-
 drivers/net/sfc/sfc_rx.c                      |   7 +-
 drivers/net/thunderx/nicvf_ethdev.c           |   3 +
 drivers/net/thunderx/nicvf_ethdev.h           |   3 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c          |   6 +-
 examples/eventdev_pipeline/main.c             | 128 --------------
 .../pipeline_worker_generic.c                 | 132 +++++++++++++++
 .../eventdev_pipeline/pipeline_worker_tx.c    | 128 ++++++++++++++
 examples/l2fwd/Makefile                       |   1 +
 examples/l2fwd/main.c                         |   2 +
 examples/l2fwd/meson.build                    |   1 +
 lib/librte_ethdev/rte_ethdev.c                | 158 +++++++++++++++++-
 lib/librte_ethdev/rte_ethdev.h                |  38 +++++
 lib/librte_ethdev/rte_ethdev_core.h           |  19 +++
 lib/librte_ethdev/rte_ethdev_version.map      |   1 +
 48 files changed, 722 insertions(+), 161 deletions(-)

--
2.17.1

[dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add `rte_eth_dev_set_supported_ptypes` function that will allow the
application to inform the PMD the packet types it is interested in.
Based on the ptypes set PMDs can optimize their Rx path.

-If application doesn’t want any ptype information it can call
`rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN, NULL, 0)`
and PMD may skip packet type processing and set rte_mbuf::packet_type to
RTE_PTYPE_UNKNOWN.

-If application doesn’t call `rte_eth_dev_set_supported_ptypes` PMD can
return `rte_mbuf::packet_type` with `rte_eth_dev_get_supported_ptypes`.

-If application is interested only in L2/L3 layer, it can inform the PMD
to update `rte_mbuf::packet_type` with L2/L3 ptype by calling
`rte_eth_dev_set_supported_ptypes(ethdev_id,
		RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK, NULL, 0)`.

Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features.rst             |  7 +-
 doc/guides/rel_notes/release_19_11.rst   |  8 +++
 lib/librte_ethdev/rte_ethdev.c           | 87 +++++++++++++++++++++++-
 lib/librte_ethdev/rte_ethdev.h           | 37 ++++++++++
 lib/librte_ethdev/rte_ethdev_core.h      | 19 ++++++
 lib/librte_ethdev/rte_ethdev_version.map |  1 +
 6 files changed, 156 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index d96696801..6e20bee5f 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -583,9 +583,12 @@ Packet type parsing
 -------------------
 
 Supports packet type parsing and returns a list of supported types.
+Allows application to set ptypes it is interested in.
 
-* **[implements] eth_dev_ops**: ``dev_supported_ptypes_get``.
-* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``.
+* **[implements] eth_dev_ops**: ``dev_supported_ptypes_get``,
+* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``,
+  ``rte_eth_dev_set_supported_ptypes()``, ``dev_supported_ptypes_set``.
+* **[provides]   mbuf**: ``mbuf.packet_type``.
 
 
 .. _nic_features_timesync:
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index ae8e7b2f0..194afc8b9 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -231,6 +231,14 @@ New Features
   * Added a console command to testpmd app, ``show port (port_id) ptypes`` which
     gives ability to print port supported ptypes in different protocol layers.
 
+* **Added ethdev API to set supported packet types**
+
+  *  Added new API ``rte_eth_dev_set_supported_ptypes`` that allows an
+     application to inform PMD about packet types classification the application
+     is interested in
+  *  This scheme will allow PMDs to avoid lookup to internal ptype table on Rx
+     and thereby improve Rx performance if application wishes do so.
+
 
 Removed Items
 -------------
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 7743205d3..6ce8f5e75 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2734,6 +2734,92 @@ rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 	return j;
 }
 
+int
+rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
+				 uint32_t *set_ptypes, unsigned int num)
+{
+	const uint32_t valid_ptype_masks[] = {
+		RTE_PTYPE_L2_MASK,
+		RTE_PTYPE_L3_MASK,
+		RTE_PTYPE_L4_MASK,
+		RTE_PTYPE_TUNNEL_MASK,
+		RTE_PTYPE_INNER_L2_MASK,
+		RTE_PTYPE_INNER_L3_MASK,
+		RTE_PTYPE_INNER_L4_MASK,
+	};
+	const uint32_t *all_ptypes;
+	struct rte_eth_dev *dev;
+	uint32_t unused_mask;
+	unsigned int i, j;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (num > 0 && set_ptypes == NULL)
+		return -EINVAL;
+
+	if (*dev->dev_ops->dev_supported_ptypes_get == NULL ||
+			*dev->dev_ops->dev_supported_ptypes_set == NULL) {
+		ret = 0;
+		goto ptype_unknown;
+	}
+
+	if (ptype_mask == 0) {
+		ret = (*dev->dev_ops->dev_supported_ptypes_set)(dev,
+				ptype_mask);
+		goto ptype_unknown;
+	}
+
+	unused_mask = ptype_mask;
+	for (i = 0; i < RTE_DIM(valid_ptype_masks); i++) {
+		uint32_t mask = ptype_mask & valid_ptype_masks[i];
+		if (mask && mask != valid_ptype_masks[i]) {
+			ret = -EINVAL;
+			goto ptype_unknown;
+		}
+		unused_mask &= ~valid_ptype_masks[i];
+	}
+
+	if (unused_mask) {
+		ret = -EINVAL;
+		goto ptype_unknown;
+	}
+
+	all_ptypes = (*dev->dev_ops->dev_supported_ptypes_get)(dev);
+	if (all_ptypes == NULL) {
+		ret = 0;
+		goto ptype_unknown;
+	}
+
+	/*
+	 * Accodommodate as many set_ptypes as possible. If the supplied
+	 * set_ptypes array is insufficient fill it partially.
+	 */
+	for (i = 0, j = 0; set_ptypes != NULL &&
+				(all_ptypes[i] != RTE_PTYPE_UNKNOWN); ++i) {
+		if (ptype_mask & all_ptypes[i]) {
+			if (j < num - 1) {
+				set_ptypes[j] = all_ptypes[i];
+				j++;
+				continue;
+			}
+			break;
+		}
+	}
+
+	if (set_ptypes != NULL && j < num)
+		set_ptypes[j] = RTE_PTYPE_UNKNOWN;
+
+	return (*dev->dev_ops->dev_supported_ptypes_set)(dev, ptype_mask);
+
+ptype_unknown:
+	if (num > 0)
+		set_ptypes[0] = RTE_PTYPE_UNKNOWN;
+
+	return ret;
+}
+
 int
 rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 {
@@ -2746,7 +2832,6 @@ rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 	return 0;
 }
 
-
 int
 rte_eth_dev_get_mtu(uint16_t port_id, uint16_t *mtu)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index c36c1b631..c4ec36ea7 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2539,6 +2539,43 @@ int rte_eth_dev_fw_version_get(uint16_t port_id,
  */
 int rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				     uint32_t *ptypes, int num);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Inform Ethernet device of the packet types classification the recipient is
+ * interested in.
+ *
+ * Application can use this function to set only specific ptypes that it's
+ * interested. This information can be used by the PMD to optimize Rx path.
+ *
+ * The function accepts an array `set_ptypes` allocated by the caller to
+ * store the packet types set by the driver, the last element of the array
+ * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array should be
+ * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be filled
+ * partially.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param ptype_mask
+ *   The ptype family that application is interested in should be bitwise OR of
+ *   RTE_PTYPE_*_MASK or 0.
+ * @param set_ptypes
+ *   An array pointer to store set packet types, allocated by caller. The
+ *   function marks the end of array with RTE_PTYPE_UNKNOWN.
+ * @param num
+ *   Size of the array pointed by param ptypes.
+ *   Should be rte_eth_dev_get_supported_ptypes() + 1 to accommodate the
+ *   set ptypes.
+ * @return
+ *   - (0) if Success.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
+ *     num > 0.
+ */
+__rte_experimental
+int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
+				     uint32_t *set_ptypes, unsigned int num);
 
 /**
  * Retrieve the MTU of an Ethernet device.
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 392aea8e6..d5245a7d0 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -234,6 +234,23 @@ typedef int (*eth_dev_infos_get_t)(struct rte_eth_dev *dev,
 typedef const uint32_t *(*eth_dev_supported_ptypes_get_t)(struct rte_eth_dev *dev);
 /**< @internal Get supported ptypes of an Ethernet device. */
 
+/**
+ * @internal
+ * Inform an Ethernet device about packet types classifications the recipient
+ * is interested in.
+ *
+ * @param dev
+ *   The Ethernet device identifier.
+ * @param ptype_mask
+ *   The ptype family that application is interested in should be bitwise OR of
+ *   RTE_PTYPE_*_MASK or 0.
+ * @return
+ *   - (0) if Success.
+ *   - (-EINVAL) if *ptype_mask* is invalid.
+ */
+typedef int (*eth_dev_supported_ptypes_set_t)(struct rte_eth_dev *dev,
+					      uint32_t ptype_mask);
+
 typedef int (*eth_queue_start_t)(struct rte_eth_dev *dev,
 				    uint16_t queue_id);
 /**< @internal Start rx and tx of a queue of an Ethernet device. */
@@ -550,6 +567,8 @@ struct eth_dev_ops {
 	eth_fw_version_get_t       fw_version_get; /**< Get firmware version. */
 	eth_dev_supported_ptypes_get_t dev_supported_ptypes_get;
 	/**< Get packet types supported and identified by device. */
+	eth_dev_supported_ptypes_set_t dev_supported_ptypes_set;
+	/**< Inform Ethernet device about packet types the recipient is interested in. */
 
 	vlan_filter_set_t          vlan_filter_set; /**< Filter VLAN Setup. */
 	vlan_tpid_set_t            vlan_tpid_set; /**< Outer/Inner VLAN TPID Setup. */
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index e59d51648..f511294d2 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -288,4 +288,5 @@ EXPERIMENTAL {
 	rte_eth_rx_burst_mode_get;
 	rte_eth_tx_burst_mode_get;
 	rte_eth_burst_mode_option_name;
+	rte_eth_dev_set_supported_ptypes;
 };
-- 
2.17.1

Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function

[Thomas Monjalon]

29/10/2019 16:37, pbhagavatula@marvell.com:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -231,6 +231,14 @@ New Features
>    * Added a console command to testpmd app, ``show port (port_id) ptypes`` which
>      gives ability to print port supported ptypes in different protocol layers.
>  
> +* **Added ethdev API to set supported packet types**
> +
> +  *  Added new API ``rte_eth_dev_set_supported_ptypes`` that allows an
> +     application to inform PMD about packet types classification the application
> +     is interested in
> +  *  This scheme will allow PMDs to avoid lookup to internal ptype table on Rx
> +     and thereby improve Rx performance if application wishes do so.

You just added or rebased this paragraph at the end.
As mentioned in the release notes files (top of the section),
there is an order for presenting features.

>  Removed Items
>  -------------
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Inform Ethernet device of the packet types classification the recipient is
> + * interested in.

This is a bit convoluted.
What about this?
"Optimize driver handling of packet types by reducing its range."

> + *
> + * Application can use this function to set only specific ptypes that it's
> + * interested. This information can be used by the PMD to optimize Rx path.
> + *
> + * The function accepts an array `set_ptypes` allocated by the caller to
> + * store the packet types set by the driver, the last element of the array
> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array should be
> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be filled
> + * partially.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param ptype_mask
> + *   The ptype family that application is interested in should be bitwise OR of
> + *   RTE_PTYPE_*_MASK or 0.
> + * @param set_ptypes
> + *   An array pointer to store set packet types, allocated by caller. The
> + *   function marks the end of array with RTE_PTYPE_UNKNOWN.
> + * @param num
> + *   Size of the array pointed by param ptypes.
> + *   Should be rte_eth_dev_get_supported_ptypes() + 1 to accommodate the
> + *   set ptypes.
> + * @return
> + *   - (0) if Success.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
> + *     num > 0.
> + */

John, please you check the English wording?

> +__rte_experimental
> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
> +				     uint32_t *set_ptypes, unsigned int num);

I don't like the name of the function because they are
not "supported" packet types but "requested".
What about replacing "set_supported" with "set_allowed", or "white_list"?