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

[dpdk-dev] [PATCH 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` and
`DEV_RX_OFFLOAD_FLOW_MARK`. These flags can be used to enable/disable PMD
writes to rte_mbuf fields `hash.rss` and `hash.fdir.hi` 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*, 0);


Pavan Nikhilesh (7):
  ethdev: add set ptype function
  ethdev: add mbuf RSS update as a offload
  ethdev: add flow action type update as an offload
  net: update Rx RSS hash offload capabilities
  net: update Rx flow action offload capabilities
  net: add ptype set default functionality
  examples/eventdev_pipeline: add new Rx RSS hash offload

 doc/guides/nics/features.rst            | 24 ++++++++++++++++++---
 drivers/net/atlantic/atl_ethdev.c       | 12 +++++++++++
 drivers/net/bnxt/bnxt_ethdev.c          | 14 ++++++++++++-
 drivers/net/cxgbe/cxgbe.h               |  3 ++-
 drivers/net/cxgbe/cxgbe_ethdev.c        | 10 +++++++++
 drivers/net/dpaa/dpaa_ethdev.c          | 13 +++++++++++-
 drivers/net/dpaa2/dpaa2_ethdev.c        | 13 +++++++++++-
 drivers/net/e1000/igb_ethdev.c          | 13 ++++++++++++
 drivers/net/e1000/igb_rxtx.c            |  3 ++-
 drivers/net/enetc/enetc_ethdev.c        | 10 +++++++++
 drivers/net/enic/enic_ethdev.c          | 10 +++++++++
 drivers/net/enic/enic_res.c             |  4 +++-
 drivers/net/failsafe/failsafe_ops.c     | 10 +++++++++
 drivers/net/fm10k/fm10k_ethdev.c        | 13 +++++++++++-
 drivers/net/hinic/hinic_pmd_ethdev.c    |  3 ++-
 drivers/net/i40e/i40e_ethdev.c          |  5 ++++-
 drivers/net/i40e/i40e_ethdev_vf.c       |  1 +
 drivers/net/i40e/i40e_rxtx.c            |  9 ++++++++
 drivers/net/i40e/i40e_rxtx.h            |  1 +
 drivers/net/iavf/iavf_ethdev.c          | 16 +++++++++++++-
 drivers/net/ice/ice_ethdev.c            |  5 ++++-
 drivers/net/ice/ice_rxtx.c              |  9 ++++++++
 drivers/net/ice/ice_rxtx.h              |  1 +
 drivers/net/ixgbe/ixgbe_ethdev.c        | 13 ++++++++++++
 drivers/net/ixgbe/ixgbe_rxtx.c          |  4 +++-
 drivers/net/liquidio/lio_ethdev.c       |  3 ++-
 drivers/net/mlx4/mlx4.c                 |  1 +
 drivers/net/mlx4/mlx4.h                 |  1 +
 drivers/net/mlx4/mlx4_ethdev.c          | 21 +++++++++++++++++++
 drivers/net/mlx4/mlx4_rxq.c             |  3 ++-
 drivers/net/mlx5/mlx5.c                 |  2 ++
 drivers/net/mlx5/mlx5.h                 |  1 +
 drivers/net/mlx5/mlx5_ethdev.c          | 21 +++++++++++++++++++
 drivers/net/mlx5/mlx5_rxq.c             |  4 +++-
 drivers/net/mvneta/mvneta_ethdev.c      | 22 +++++++++++++++++++
 drivers/net/mvpp2/mrvl_ethdev.c         | 22 +++++++++++++++++++
 drivers/net/netvsc/hn_ethdev.c          |  3 ++-
 drivers/net/netvsc/hn_rndis.c           |  3 ++-
 drivers/net/netvsc/hn_var.h             |  3 ++-
 drivers/net/netvsc/hn_vf.c              | 10 ++++++++-
 drivers/net/nfp/nfp_net.c               | 13 +++++++++++-
 drivers/net/octeontx/octeontx_ethdev.c  | 10 +++++++++
 drivers/net/octeontx2/otx2_ethdev.c     |  4 +++-
 drivers/net/octeontx2/otx2_ethdev.h     | 17 ++++++++-------
 drivers/net/octeontx2/otx2_flow_parse.c |  3 ++-
 drivers/net/octeontx2/otx2_lookup.c     |  9 ++++++++
 drivers/net/qede/qede_ethdev.c          | 14 ++++++++++++-
 drivers/net/sfc/sfc_ethdev.c            | 11 ++++++++++
 drivers/net/sfc/sfc_rx.c                |  4 +++-
 drivers/net/tap/rte_eth_tap.c           | 10 +++++++++
 drivers/net/thunderx/nicvf_ethdev.c     | 10 +++++++++
 drivers/net/thunderx/nicvf_ethdev.h     |  3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c    | 15 ++++++++++++-
 examples/eventdev_pipeline/main.c       |  4 ++++
 lib/librte_ethdev/rte_ethdev.c          | 28 +++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.h          | 19 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h     |  6 ++++++
 57 files changed, 483 insertions(+), 36 deletions(-)

--
2.22.0

[dpdk-dev] [PATCH 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)` and PMD
will set rte_mbuf::packet_type to 0.

-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)`.

Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 doc/guides/nics/features.rst        | 12 ++++++++----
 lib/librte_ethdev/rte_ethdev.c      | 28 ++++++++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev.h      | 17 +++++++++++++++++
 lib/librte_ethdev/rte_ethdev_core.h |  6 ++++++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index c4e128d2f..d4d55f721 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -582,10 +582,14 @@ Supports inner packet L4 checksum.
 Packet type parsing
 -------------------

-Supports packet type parsing and returns a list of supported types.
-
-* **[implements] eth_dev_ops**: ``dev_supported_ptypes_get``.
-* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``.
+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``,
+  ``dev_supported_ptypes_set``.
+* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``,
+  ``rte_eth_dev_set_supported_ptypes()``.
+* **[provides]   mbuf**: ``mbuf.packet_type``.


 .. _nic_features_timesync:
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 17d183e1f..72fe660c3 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -2602,6 +2602,34 @@ 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)
+{
+	int i;
+	struct rte_eth_dev *dev;
+	const uint32_t *all_ptypes;
+	uint32_t all_ptype_mask = 0;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_set,
+				-ENOTSUP);
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_get,
+				-ENOTSUP);
+	all_ptypes = (*dev->dev_ops->dev_supported_ptypes_get)(dev);
+
+	if (!all_ptypes)
+		return -ENOTSUP;
+
+	for (i = 0; all_ptypes[i] != RTE_PTYPE_UNKNOWN; ++i)
+		all_ptype_mask |= all_ptypes[i];
+
+	if ((all_ptype_mask & ptype_mask) != ptype_mask)
+		return -ENOTSUP;
+
+	return (*dev->dev_ops->dev_supported_ptypes_set)(dev, ptype_mask);
+}
+
 void
 rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
 {
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index dc6596bc9..f97f0a6e5 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -2431,6 +2431,23 @@ 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);
+/**
+ * Request Ethernet device to set only specific packet types in the packet.
+ *
+ * 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.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param ptype_mask
+ *   The ptype family that application is interested in.
+ * @return
+ *   - (0) Successfully set supported ptypes.
+ *   - (-ENODEV) if *port_id* is invalid.
+ *   - (-ENOTSUP) Packet type mask supplied is not supported by the Ethernet
+ *		  device.
+ */
+int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t ptype_mask);

 /**
  * 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 2922d5b7c..02ee7c12c 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -110,6 +110,10 @@ typedef void (*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. */

+typedef int (*eth_dev_supported_ptypes_set_t)(struct rte_eth_dev *dev,
+					      uint32_t ptype_mask);
+/**< @internal Set required ptypes of an Ethernet device. */
+
 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. */
@@ -421,6 +425,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 device about the interested ptypes. */

 	vlan_filter_set_t          vlan_filter_set; /**< Filter VLAN Setup. */
 	vlan_tpid_set_t            vlan_tpid_set; /**< Outer/Inner VLAN TPID Setup. */
--
2.22.0

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

[Andrew Rybchenko]

The patch should add item in release notes (as well as all other
subsequent patches which add more features).

On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> 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)` and PMD
> will set rte_mbuf::packet_type to 0.

As I understand PMD may provide more reach classification
than set. So, it is not obliged to set packet type to 0.

> -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)`.
>
> Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>   doc/guides/nics/features.rst        | 12 ++++++++----
>   lib/librte_ethdev/rte_ethdev.c      | 28 ++++++++++++++++++++++++++++
>   lib/librte_ethdev/rte_ethdev.h      | 17 +++++++++++++++++
>   lib/librte_ethdev/rte_ethdev_core.h |  6 ++++++
>   4 files changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index c4e128d2f..d4d55f721 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -582,10 +582,14 @@ Supports inner packet L4 checksum.
>   Packet type parsing
>   -------------------
>
> -Supports packet type parsing and returns a list of supported types.
> -
> -* **[implements] eth_dev_ops**: ``dev_supported_ptypes_get``.
> -* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``.
> +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``,
> +  ``dev_supported_ptypes_set``.
> +* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``,
> +  ``rte_eth_dev_set_supported_ptypes()``.
> +* **[provides]   mbuf**: ``mbuf.packet_type``.
>
>
>   .. _nic_features_timesync:
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 17d183e1f..72fe660c3 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -2602,6 +2602,34 @@ 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)
> +{
> +	int i;
> +	struct rte_eth_dev *dev;
> +	const uint32_t *all_ptypes;
> +	uint32_t all_ptype_mask = 0;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_set,
> +				-ENOTSUP);
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_supported_ptypes_get,
> +				-ENOTSUP);
> +	all_ptypes = (*dev->dev_ops->dev_supported_ptypes_get)(dev);
> +
> +	if (!all_ptypes)

If I remember correctly DPDK style prefers to compare vs 0.

> +		return -ENOTSUP;
> +
> +	for (i = 0; all_ptypes[i] != RTE_PTYPE_UNKNOWN; ++i)
> +		all_ptype_mask |= all_ptypes[i];
> +
> +	if ((all_ptype_mask & ptype_mask) != ptype_mask)
> +		return -ENOTSUP;

Do we really want so strict check here? May be just check
that intersection is not empty if ptype_mask is not empty?

I think that setting ptype_mask to 0 could be pretty often and
may be it makes sense to avoid get in the case. Also consider
to return OK even if there is no get/set callbacks at all.

> +
> +	return (*dev->dev_ops->dev_supported_ptypes_set)(dev, ptype_mask);
> +}
> +
>   void
>   rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr)
>   {
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index dc6596bc9..f97f0a6e5 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2431,6 +2431,23 @@ 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);
> +/**
> + * Request Ethernet device to set only specific packet types in the packet.
> + *
> + * 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.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param ptype_mask
> + *   The ptype family that application is interested in.
> + * @return
> + *   - (0) Successfully set supported ptypes.
> + *   - (-ENODEV) if *port_id* is invalid.
> + *   - (-ENOTSUP) Packet type mask supplied is not supported by the Ethernet
> + *		  device.
> + */
> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t ptype_mask);

Should it be experimental?

Please, add it to .map file.

>   /**
>    * 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 2922d5b7c..02ee7c12c 100644
> --- a/lib/librte_ethdev/rte_ethdev_core.h
> +++ b/lib/librte_ethdev/rte_ethdev_core.h
> @@ -110,6 +110,10 @@ typedef void (*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. */
>
> +typedef int (*eth_dev_supported_ptypes_set_t)(struct rte_eth_dev *dev,
> +					      uint32_t ptype_mask);
> +/**< @internal Set required ptypes of an Ethernet device. */
> +
>   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. */
> @@ -421,6 +425,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 device about the interested ptypes. */
>
>   	vlan_filter_set_t          vlan_filter_set; /**< Filter VLAN Setup. */
>   	vlan_tpid_set_t            vlan_tpid_set; /**< Outer/Inner VLAN TPID Setup. */
> --
> 2.22.0
>

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

[Pavan Nikhilesh Bhagavatula]

>-----Original Message-----
>From: Andrew Rybchenko <arybchenko@solarflare.com>
>Sent: Friday, August 16, 2019 1:53 PM
>To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>Jacob Kollanukkaran <jerinj@marvell.com>; ferruh.yigit@intel.com;
>John McNamara <john.mcnamara@intel.com>; Marko Kovacevic
><marko.kovacevic@intel.com>; Thomas Monjalon
><thomas@monjalon.net>
>Cc: dev@dpdk.org
>Subject: [EXT] Re: [dpdk-dev] [PATCH 1/7] ethdev: add set ptype
>function
>
>External Email
>
>----------------------------------------------------------------------
>The patch should add item in release notes (as well as all other
>subsequent patches which add more features).

Will update release notes in v2.
>
>On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>> 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)` and PMD
>> will set rte_mbuf::packet_type to 0.
>
>As I understand PMD may provide more reach classification
>than set. So, it is not obliged to set packet type to 0.

Yes it would be undefined. 
>
>> -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)`.
>>
>> Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> ---
>>   doc/guides/nics/features.rst        | 12 ++++++++----
>>   lib/librte_ethdev/rte_ethdev.c      | 28
>++++++++++++++++++++++++++++
>>   lib/librte_ethdev/rte_ethdev.h      | 17 +++++++++++++++++
>>   lib/librte_ethdev/rte_ethdev_core.h |  6 ++++++
>>   4 files changed, 59 insertions(+), 4 deletions(-)
>>
>> diff --git a/doc/guides/nics/features.rst
>b/doc/guides/nics/features.rst
>> index c4e128d2f..d4d55f721 100644
>> --- a/doc/guides/nics/features.rst
>> +++ b/doc/guides/nics/features.rst
>> @@ -582,10 +582,14 @@ Supports inner packet L4 checksum.
>>   Packet type parsing
>>   -------------------
>>
>> -Supports packet type parsing and returns a list of supported types.
>> -
>> -* **[implements] eth_dev_ops**: ``dev_supported_ptypes_get``.
>> -* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``.
>> +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``,
>> +  ``dev_supported_ptypes_set``.
>> +* **[related]    API**: ``rte_eth_dev_get_supported_ptypes()``,
>> +  ``rte_eth_dev_set_supported_ptypes()``.
>> +* **[provides]   mbuf**: ``mbuf.packet_type``.
>>
>>
>>   .. _nic_features_timesync:
>> diff --git a/lib/librte_ethdev/rte_ethdev.c
>b/lib/librte_ethdev/rte_ethdev.c
>> index 17d183e1f..72fe660c3 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -2602,6 +2602,34 @@
>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)
>> +{
>> +	int i;
>> +	struct rte_eth_dev *dev;
>> +	const uint32_t *all_ptypes;
>> +	uint32_t all_ptype_mask = 0;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
>>dev_supported_ptypes_set,
>> +				-ENOTSUP);
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
>>dev_supported_ptypes_get,
>> +				-ENOTSUP);
>> +	all_ptypes = (*dev->dev_ops-
>>dev_supported_ptypes_get)(dev);
>> +
>> +	if (!all_ptypes)
>
>If I remember correctly DPDK style prefers to compare vs 0.
>
>> +		return -ENOTSUP;
>> +
>> +	for (i = 0; all_ptypes[i] != RTE_PTYPE_UNKNOWN; ++i)
>> +		all_ptype_mask |= all_ptypes[i];
>> +
>> +	if ((all_ptype_mask & ptype_mask) != ptype_mask)
>> +		return -ENOTSUP;
>
>Do we really want so strict check here? May be just check
>that intersection is not empty if ptype_mask is not empty?
>
I was thinking the same maybe we can return the successfully set mask?  
And the application can use software parser to parse the rest?.

>I think that setting ptype_mask to 0 could be pretty often and
>may be it makes sense to avoid get in the case.

Agreed.

>Also consider
>to return OK even if there is no get/set callbacks at all.

I think if we are ok with the above approach i.e. returning successfully set ptypes, 
we will return 0 if there are no supported get/set call backs and the application 
can softparse the ptypes.

>
>> +
>> +	return (*dev->dev_ops->dev_supported_ptypes_set)(dev,
>ptype_mask);
>> +}
>> +
>>   void
>>   rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr
>*mac_addr)
>>   {
>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>b/lib/librte_ethdev/rte_ethdev.h
>> index dc6596bc9..f97f0a6e5 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -2431,6 +2431,23 @@ 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);
>> +/**
>> + * Request Ethernet device to set only specific packet types in the
>packet.
>> + *
>> + * 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.
>> + *
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @param ptype_mask
>> + *   The ptype family that application is interested in.
>> + * @return
>> + *   - (0) Successfully set supported ptypes.
>> + *   - (-ENODEV) if *port_id* is invalid.
>> + *   - (-ENOTSUP) Packet type mask supplied is not supported by the
>Ethernet
>> + *		  device.
>> + */
>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
>ptype_mask);
>
>Should it be experimental?
>
>Please, add it to .map file.

Will fix in v2.
>
>>   /**
>>    * 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 2922d5b7c..02ee7c12c 100644
>> --- a/lib/librte_ethdev/rte_ethdev_core.h
>> +++ b/lib/librte_ethdev/rte_ethdev_core.h
>> @@ -110,6 +110,10 @@ typedef void (*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. */
>>
>> +typedef int (*eth_dev_supported_ptypes_set_t)(struct
>rte_eth_dev *dev,
>> +					      uint32_t ptype_mask);
>> +/**< @internal Set required ptypes of an Ethernet device. */
>> +
>>   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. */
>> @@ -421,6 +425,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 device about the interested ptypes. */
>>
>>   	vlan_filter_set_t          vlan_filter_set; /**< Filter VLAN Setup. */
>>   	vlan_tpid_set_t            vlan_tpid_set; /**< Outer/Inner VLAN
>TPID Setup. */
>> --
>> 2.22.0
>>

[dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be used to
enable/disable PMDs write to `rte_mbuf::hash::rss`.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 doc/guides/nics/features.rst   | 2 ++
 lib/librte_ethdev/rte_ethdev.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index d4d55f721..f79b69b38 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -274,6 +274,7 @@ Supports RSS hashing on RX.
 
 * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``ETH_MQ_RX_RSS_FLAG``.
 * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
+* **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
 * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
 * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
 
@@ -286,6 +287,7 @@ Inner RSS
 Supports RX RSS hashing on Inner headers.
 
 * **[uses]    rte_flow_action_rss**: ``level``.
+* **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
 * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
 
 
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index f97f0a6e5..889486a11 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1013,6 +1013,7 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
 #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
 #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
+#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
 
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
-- 
2.22.0

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Andrew Rybchenko]

On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be used to
> enable/disable PMDs write to `rte_mbuf::hash::rss`.

It should be highlighted that presence of the RSS hash is indicated
by PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have
a way to check that RSS hash delivery is supported and should
enable the offload if RSS hash is used. PMD may still provide the
hash even if the offload is not enabled.

> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

with above and one note below fixed.

> ---
>   doc/guides/nics/features.rst   | 2 ++
>   lib/librte_ethdev/rte_ethdev.h | 1 +
>   2 files changed, 3 insertions(+)
>
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index d4d55f721..f79b69b38 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
>   
>   * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` = ``ETH_MQ_RX_RSS_FLAG``.
>   * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>   * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>   * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
>   
> @@ -286,6 +287,7 @@ Inner RSS
>   Supports RX RSS hashing on Inner headers.
>   
>   * **[uses]    rte_flow_action_rss**: ``level``.
> +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>   * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
>   
>   
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index f97f0a6e5..889486a11 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
>   #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
>   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000

Should be added to rte_rx_offload_names in lib/librte_ethdev/rte_ethdev.c.

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Pavan Nikhilesh Bhagavatula]

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Andrew Rybchenko
>Sent: Friday, August 16, 2019 1:18 PM
>To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>Jacob Kollanukkaran <jerinj@marvell.com>; ferruh.yigit@intel.com;
>John McNamara <john.mcnamara@intel.com>; Marko Kovacevic
><marko.kovacevic@intel.com>; Thomas Monjalon
><thomas@monjalon.net>
>Cc: dev@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
>offload
>
>On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be
>used to
>> enable/disable PMDs write to `rte_mbuf::hash::rss`.
>
>It should be highlighted that presence of the RSS hash is indicated
>by PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have
>a way to check that RSS hash delivery is supported and should
>enable the offload if RSS hash is used. PMD may still provide the
>hash even if the offload is not enabled.
>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
>Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
>with above and one note below fixed.
>
>> ---
>>   doc/guides/nics/features.rst   | 2 ++
>>   lib/librte_ethdev/rte_ethdev.h | 1 +
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/doc/guides/nics/features.rst
>b/doc/guides/nics/features.rst
>> index d4d55f721..f79b69b38 100644
>> --- a/doc/guides/nics/features.rst
>> +++ b/doc/guides/nics/features.rst
>> @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
>>
>>   * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>``ETH_MQ_RX_RSS_FLAG``.
>>   * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>>   * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>   * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
>``mbuf.rss``.
>>
>> @@ -286,6 +287,7 @@ Inner RSS
>>   Supports RX RSS hashing on Inner headers.
>>
>>   * **[uses]    rte_flow_action_rss**: ``level``.
>> +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>>   * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
>``mbuf.rss``.
>>
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>b/lib/librte_ethdev/rte_ethdev.h
>> index f97f0a6e5..889486a11 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
>>   #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
>>   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>> +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>
>Should be added to rte_rx_offload_names in
>lib/librte_ethdev/rte_ethdev.c.

Thanks for the heads up Andrew, will fix in v2.

Pavan

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Shahaf Shuler]

Friday, August 16, 2019 10:48 AM, Andrew Rybchenko:
> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
> offload
> 
> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be used
> to
> > enable/disable PMDs write to `rte_mbuf::hash::rss`.
> 
> It should be highlighted that presence of the RSS hash is indicated by
> PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have a way to
> check that RSS hash delivery is supported and should enable the offload if
> RSS hash is used. PMD may still provide the hash even if the offload is not
> enabled.

I don't understand how PMDs should act w/ this addition when considering the API breakage to application. 

Currently application don't set this flag, and expect to get the RSS hash result on mbuf. 
If PMDs will not set the RSS hash result when flag is not present then applications might break.
If they will always set, then there is no meaning for it.

as I understand the motivation to save few cycles on the PMD receive path, if we want to include it we should treat it as API breakage and documents it on the release notes.
My option is that some offload should just be usable (OOB) by the fact user enabled them (e.g. RSS). no need to complicate the user by checking and set this field. 


> 
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> with above and one note below fixed.
> 
> > ---
> >   doc/guides/nics/features.rst   | 2 ++
> >   lib/librte_ethdev/rte_ethdev.h | 1 +
> >   2 files changed, 3 insertions(+)
> >
> > diff --git a/doc/guides/nics/features.rst
> > b/doc/guides/nics/features.rst index d4d55f721..f79b69b38 100644
> > --- a/doc/guides/nics/features.rst
> > +++ b/doc/guides/nics/features.rst
> > @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
> >
> >   * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
> ``ETH_MQ_RX_RSS_FLAG``.
> >   * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
> > +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
> >   * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
> >   * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
> >
> > @@ -286,6 +287,7 @@ Inner RSS
> >   Supports RX RSS hashing on Inner headers.
> >
> >   * **[uses]    rte_flow_action_rss**: ``level``.
> > +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
> >   * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
> >
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.h
> > b/lib/librte_ethdev/rte_ethdev.h index f97f0a6e5..889486a11 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
> >   #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
> >   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
> >   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> > +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> 
> Should be added to rte_rx_offload_names in
> lib/librte_ethdev/rte_ethdev.c.

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Andrew Rybchenko]

On 8/18/19 7:52 AM, Shahaf Shuler wrote:
> Friday, August 16, 2019 10:48 AM, Andrew Rybchenko:
>> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
>> offload
>>
>> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>
>>> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be used
>> to
>>> enable/disable PMDs write to `rte_mbuf::hash::rss`.
>> It should be highlighted that presence of the RSS hash is indicated by
>> PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have a way to
>> check that RSS hash delivery is supported and should enable the offload if
>> RSS hash is used. PMD may still provide the hash even if the offload is not
>> enabled.
> I don't understand how PMDs should act w/ this addition when considering the API breakage to application.

There is a deprecation notice for it.
I mentioned in my review notes for one of patches in the series
that the change should be highlighted in release notes.
Yes, it is absolutely required if these patches are accepted.

> Currently application don't set this flag, and expect to get the RSS hash result on mbuf.
> If PMDs will not set the RSS hash result when flag is not present then applications might break.
> If they will always set, then there is no meaning for it.
>
> as I understand the motivation to save few cycles on the PMD receive path, if we want to include it we should treat it as API breakage and documents it on the release notes.
> My option is that some offload should just be usable (OOB) by the fact user enabled them (e.g. RSS). no need to complicate the user by checking and set this field.

What I don't understand is why some offloads should just work but
another requires action to enable it. Just because it is the current
state of things - I don't think it is a good motivation. Sorry.
I think more applications use checksum offloads than RSS hash,
but it is still required to enable it. It looks like no single DPDK example
uses RSS hash. So, I guess it not widely used by applications as well.
Anyway these 2 patches for flow action and RSS hash make all Rx
offloads consistent - if you need something, enable it.

And the question is not to save few cycles in the PMD receive path.
It makes is possible to not deliver both from NIC to host.
8 bytes (4 RSS hash and 4 flow mark) are more than 10% for the
smallest packets.

>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>
>> with above and one note below fixed.
>>
>>> ---
>>>    doc/guides/nics/features.rst   | 2 ++
>>>    lib/librte_ethdev/rte_ethdev.h | 1 +
>>>    2 files changed, 3 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/features.rst
>>> b/doc/guides/nics/features.rst index d4d55f721..f79b69b38 100644
>>> --- a/doc/guides/nics/features.rst
>>> +++ b/doc/guides/nics/features.rst
>>> @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
>>>
>>>    * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>> ``ETH_MQ_RX_RSS_FLAG``.
>>>    * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>>>    * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>    * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
>>>
>>> @@ -286,6 +287,7 @@ Inner RSS
>>>    Supports RX RSS hashing on Inner headers.
>>>
>>>    * **[uses]    rte_flow_action_rss**: ``level``.
>>> +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>>>    * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``.
>>>
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>> b/lib/librte_ethdev/rte_ethdev.h index f97f0a6e5..889486a11 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
>>>    #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
>>>    #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>>    #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>> +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>> Should be added to rte_rx_offload_names in
>> lib/librte_ethdev/rte_ethdev.c.

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Shahaf Shuler]

Sunday, August 18, 2019 8:39 AM, Andrew Rybchenko:
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
> offload
> 
> On 8/18/19 7:52 AM, Shahaf Shuler wrote:
> > Friday, August 16, 2019 10:48 AM, Andrew Rybchenko:
> >> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
> >> offload
> >>
> >> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>
> >>> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be
> used
> >> to
> >>> enable/disable PMDs write to `rte_mbuf::hash::rss`.
> >> It should be highlighted that presence of the RSS hash is indicated
> >> by PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have a way
> >> to check that RSS hash delivery is supported and should enable the
> >> offload if RSS hash is used. PMD may still provide the hash even if
> >> the offload is not enabled.
> > I don't understand how PMDs should act w/ this addition when considering
> the API breakage to application.
> 
> There is a deprecation notice for it.
> I mentioned in my review notes for one of patches in the series that the
> change should be highlighted in release notes.
> Yes, it is absolutely required if these patches are accepted.
> 
> > Currently application don't set this flag, and expect to get the RSS hash
> result on mbuf.
> > If PMDs will not set the RSS hash result when flag is not present then
> applications might break.
> > If they will always set, then there is no meaning for it.
> >
> > as I understand the motivation to save few cycles on the PMD receive path,
> if we want to include it we should treat it as API breakage and documents it
> on the release notes.
> > My option is that some offload should just be usable (OOB) by the fact user
> enabled them (e.g. RSS). no need to complicate the user by checking and set
> this field.
> 
> What I don't understand is why some offloads should just work but another
> requires action to enable it. Just because it is the current state of things - I
> don't think it is a good motivation. Sorry.

Not because it is the current state of things, because it makes user experience much simpler. 

You enabled RSS -> you get full RSS behavior 
You set a flow rule w/ mark -> you get full flow mark behavior
You set checksum -> you get full csum behavior. 

> I think more applications use checksum offloads than RSS hash, but it is still
> required to enable it. It looks like no single DPDK example uses RSS hash. So,
> I guess it not widely used by applications as well.

Well there is at least one called ovs-dpdk, that use the RSS result as the key to access the EMC.
I know of few more, not upstream, ones. 

> Anyway these 2 patches for flow action and RSS hash make all Rx offloads
> consistent - if you need something, enable it.

But the user enabled it -
It enabled RSS by setting ETH_MQ_RX_RSS, why does it need to enable another flag? 

Same for flow mark. 

> 
> And the question is not to save few cycles in the PMD receive path.
> It makes is possible to not deliver both from NIC to host.
> 8 bytes (4 RSS hash and 4 flow mark) are more than 10% for the smallest
> packets.

There is always the line between how much tight control we want to provide to user (to save cycles/ to save PCI BW) and how much it will be simple for the user to work on top.
My opinion is that we need to have some basics. 

> 
> >>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>
> >> with above and one note below fixed.
> >>
> >>> ---
> >>>    doc/guides/nics/features.rst   | 2 ++
> >>>    lib/librte_ethdev/rte_ethdev.h | 1 +
> >>>    2 files changed, 3 insertions(+)
> >>>
> >>> diff --git a/doc/guides/nics/features.rst
> >>> b/doc/guides/nics/features.rst index d4d55f721..f79b69b38 100644
> >>> --- a/doc/guides/nics/features.rst
> >>> +++ b/doc/guides/nics/features.rst
> >>> @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
> >>>
> >>>    * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
> >> ``ETH_MQ_RX_RSS_FLAG``.
> >>>    * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
> >>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
> >> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
> >>>    * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
> >>>    * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
> ``mbuf.rss``.
> >>>
> >>> @@ -286,6 +287,7 @@ Inner RSS
> >>>    Supports RX RSS hashing on Inner headers.
> >>>
> >>>    * **[uses]    rte_flow_action_rss**: ``level``.
> >>> +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
> >> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
> >>>    * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
> ``mbuf.rss``.
> >>>
> >>>
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.h
> >>> b/lib/librte_ethdev/rte_ethdev.h index f97f0a6e5..889486a11 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
> >>>    #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
> >>>    #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
> >>>    #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> >>> +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> >> Should be added to rte_rx_offload_names in
> >> lib/librte_ethdev/rte_ethdev.c.

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Andrew Rybchenko]

On 8/18/19 9:18 AM, Shahaf Shuler wrote:
> Sunday, August 18, 2019 8:39 AM, Andrew Rybchenko:
>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
>> offload
>>
>> On 8/18/19 7:52 AM, Shahaf Shuler wrote:
>>> Friday, August 16, 2019 10:48 AM, Andrew Rybchenko:
>>>> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
>>>> offload
>>>>
>>>> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>
>>>>> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be
>> used
>>>> to
>>>>> enable/disable PMDs write to `rte_mbuf::hash::rss`.
>>>> It should be highlighted that presence of the RSS hash is indicated
>>>> by PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have a way
>>>> to check that RSS hash delivery is supported and should enable the
>>>> offload if RSS hash is used. PMD may still provide the hash even if
>>>> the offload is not enabled.
>>> I don't understand how PMDs should act w/ this addition when considering
>> the API breakage to application.
>>
>> There is a deprecation notice for it.
>> I mentioned in my review notes for one of patches in the series that the
>> change should be highlighted in release notes.
>> Yes, it is absolutely required if these patches are accepted.
>>
>>> Currently application don't set this flag, and expect to get the RSS hash
>> result on mbuf.
>>> If PMDs will not set the RSS hash result when flag is not present then
>> applications might break.
>>> If they will always set, then there is no meaning for it.
>>>
>>> as I understand the motivation to save few cycles on the PMD receive path,
>> if we want to include it we should treat it as API breakage and documents it
>> on the release notes.
>>> My option is that some offload should just be usable (OOB) by the fact user
>> enabled them (e.g. RSS). no need to complicate the user by checking and set
>> this field.
>>
>> What I don't understand is why some offloads should just work but another
>> requires action to enable it. Just because it is the current state of things - I
>> don't think it is a good motivation. Sorry.
> Not because it is the current state of things, because it makes user experience much simpler.

If so, it would be simpler to have no controls at all and always have
everything possible.

> You enabled RSS -> you get full RSS behavior

You enable distribution across many queues here.
RSS hash availability is a side effect here.

> You set a flow rule w/ mark -> you get full flow mark behavior
> You set checksum -> you get full csum behavior.
>
>> I think more applications use checksum offloads than RSS hash, but it is still
>> required to enable it. It looks like no single DPDK example uses RSS hash. So,
>> I guess it not widely used by applications as well.
> Well there is at least one called ovs-dpdk, that use the RSS result as the key to access the EMC.
> I know of few more, not upstream, ones.
>
>> Anyway these 2 patches for flow action and RSS hash make all Rx offloads
>> consistent - if you need something, enable it.
> But the user enabled it -
> It enabled RSS by setting ETH_MQ_RX_RSS, why does it need to enable another flag?

Answered above. If you need distribution it does not mean
that you need RSS hash information. There are really many
examples when you don't really need it.

> Same for flow mark.
>
>> And the question is not to save few cycles in the PMD receive path.
>> It makes is possible to not deliver both from NIC to host.
>> 8 bytes (4 RSS hash and 4 flow mark) are more than 10% for the smallest
>> packets.
> There is always the line between how much tight control we want to provide to user (to save cycles/ to save PCI BW) and how much it will be simple for the user to work on top.
> My opinion is that we need to have some basics.

Many thanks, your arguments make sense. I vote for
consistency and more fine grained control which allows
more optimizations and allow to squeeze more performance
from HW and SW. So, my line is a bit lower.  I don't think
that these two patches make user control over-complicated.

>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>
>>>> with above and one note below fixed.
>>>>
>>>>> ---
>>>>>     doc/guides/nics/features.rst   | 2 ++
>>>>>     lib/librte_ethdev/rte_ethdev.h | 1 +
>>>>>     2 files changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/doc/guides/nics/features.rst
>>>>> b/doc/guides/nics/features.rst index d4d55f721..f79b69b38 100644
>>>>> --- a/doc/guides/nics/features.rst
>>>>> +++ b/doc/guides/nics/features.rst
>>>>> @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
>>>>>
>>>>>     * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
>>>> ``ETH_MQ_RX_RSS_FLAG``.
>>>>>     * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
>>>>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>>>>>     * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
>>>>>     * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
>> ``mbuf.rss``.
>>>>> @@ -286,6 +287,7 @@ Inner RSS
>>>>>     Supports RX RSS hashing on Inner headers.
>>>>>
>>>>>     * **[uses]    rte_flow_action_rss**: ``level``.
>>>>> +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
>>>>>     * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
>> ``mbuf.rss``.
>>>>>
>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>>>> b/lib/librte_ethdev/rte_ethdev.h index f97f0a6e5..889486a11 100644
>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>> @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
>>>>>     #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
>>>>>     #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>>>>     #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>>>> +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>>>> Should be added to rte_rx_offload_names in
>>>> lib/librte_ethdev/rte_ethdev.c.

Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload

[Shahaf Shuler]

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Sunday, August 18, 2019 10:01 AM
> To: Shahaf Shuler <shahafs@mellanox.com>; pbhagavatula@marvell.com;
> jerinj@marvell.com; ferruh.yigit@intel.com; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
> offload
> 
> On 8/18/19 9:18 AM, Shahaf Shuler wrote:
> > Sunday, August 18, 2019 8:39 AM, Andrew Rybchenko:
> >> <marko.kovacevic@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a
> >> offload
> >>
> >> On 8/18/19 7:52 AM, Shahaf Shuler wrote:
> >>> Friday, August 16, 2019 10:48 AM, Andrew Rybchenko:
> >>>> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as
> >>>> a offload
> >>>>
> >>>> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> >>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>
> >>>>> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be
> >> used
> >>>> to
> >>>>> enable/disable PMDs write to `rte_mbuf::hash::rss`.
> >>>> It should be highlighted that presence of the RSS hash is indicated
> >>>> by PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have a
> way
> >>>> to check that RSS hash delivery is supported and should enable the
> >>>> offload if RSS hash is used. PMD may still provide the hash even if
> >>>> the offload is not enabled.
> >>> I don't understand how PMDs should act w/ this addition when
> >>> considering
> >> the API breakage to application.
> >>
> >> There is a deprecation notice for it.
> >> I mentioned in my review notes for one of patches in the series that
> >> the change should be highlighted in release notes.
> >> Yes, it is absolutely required if these patches are accepted.
> >>
> >>> Currently application don't set this flag, and expect to get the RSS
> >>> hash
> >> result on mbuf.
> >>> If PMDs will not set the RSS hash result when flag is not present
> >>> then
> >> applications might break.
> >>> If they will always set, then there is no meaning for it.
> >>>
> >>> as I understand the motivation to save few cycles on the PMD receive
> >>> path,
> >> if we want to include it we should treat it as API breakage and
> >> documents it on the release notes.
> >>> My option is that some offload should just be usable (OOB) by the
> >>> fact user
> >> enabled them (e.g. RSS). no need to complicate the user by checking
> >> and set this field.
> >>
> >> What I don't understand is why some offloads should just work but
> >> another requires action to enable it. Just because it is the current
> >> state of things - I don't think it is a good motivation. Sorry.
> > Not because it is the current state of things, because it makes user
> experience much simpler.
> 
> If so, it would be simpler to have no controls at all and always have
> everything possible.
> 
> > You enabled RSS -> you get full RSS behavior
> 
> You enable distribution across many queues here.
> RSS hash availability is a side effect here.

I disagree.

There is at least one RSS spec (Microsoft NDIS) that define it part of the RSS action. [1]
Bottom of page " The NIC always passes on the 32-bit hash value." 

Am not spec pedantic, just to emphasize that natively drivers expects to have the RSS hash on the packet descriptor. 

As for the rest - I think you spread the two different approach correctly.

[1]
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/introduction-to-receive-side-scaling


> 
> > You set a flow rule w/ mark -> you get full flow mark behavior You set
> > checksum -> you get full csum behavior.
> >
> >> I think more applications use checksum offloads than RSS hash, but it
> >> is still required to enable it. It looks like no single DPDK example
> >> uses RSS hash. So, I guess it not widely used by applications as well.
> > Well there is at least one called ovs-dpdk, that use the RSS result as the key
> to access the EMC.
> > I know of few more, not upstream, ones.
> >
> >> Anyway these 2 patches for flow action and RSS hash make all Rx
> >> offloads consistent - if you need something, enable it.
> > But the user enabled it -
> > It enabled RSS by setting ETH_MQ_RX_RSS, why does it need to enable
> another flag?
> 
> Answered above. If you need distribution it does not mean that you need
> RSS hash information. There are really many examples when you don't really
> need it.
> 
> > Same for flow mark.
> >
> >> And the question is not to save few cycles in the PMD receive path.
> >> It makes is possible to not deliver both from NIC to host.
> >> 8 bytes (4 RSS hash and 4 flow mark) are more than 10% for the
> >> smallest packets.
> > There is always the line between how much tight control we want to
> provide to user (to save cycles/ to save PCI BW) and how much it will be
> simple for the user to work on top.
> > My opinion is that we need to have some basics.
> 
> Many thanks, your arguments make sense. I vote for consistency and more
> fine grained control which allows more optimizations and allow to squeeze
> more performance from HW and SW. So, my line is a bit lower.  I don't think
> that these two patches make user control over-complicated.
> 
> >>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>>>
> >>>> with above and one note below fixed.
> >>>>
> >>>>> ---
> >>>>>     doc/guides/nics/features.rst   | 2 ++
> >>>>>     lib/librte_ethdev/rte_ethdev.h | 1 +
> >>>>>     2 files changed, 3 insertions(+)
> >>>>>
> >>>>> diff --git a/doc/guides/nics/features.rst
> >>>>> b/doc/guides/nics/features.rst index d4d55f721..f79b69b38 100644
> >>>>> --- a/doc/guides/nics/features.rst
> >>>>> +++ b/doc/guides/nics/features.rst
> >>>>> @@ -274,6 +274,7 @@ Supports RSS hashing on RX.
> >>>>>
> >>>>>     * **[uses]     user config**: ``dev_conf.rxmode.mq_mode`` =
> >>>> ``ETH_MQ_RX_RSS_FLAG``.
> >>>>>     * **[uses]     user config**: ``dev_conf.rx_adv_conf.rss_conf``.
> >>>>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
> >>>> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
> >>>>>     * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``.
> >>>>>     * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
> >> ``mbuf.rss``.
> >>>>> @@ -286,6 +287,7 @@ Inner RSS
> >>>>>     Supports RX RSS hashing on Inner headers.
> >>>>>
> >>>>>     * **[uses]    rte_flow_action_rss**: ``level``.
> >>>>> +* **[uses]    rte_eth_rxconf,rte_eth_rxmode**:
> >>>> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``.
> >>>>>     * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``,
> >> ``mbuf.rss``.
> >>>>>
> >>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
> >>>>> b/lib/librte_ethdev/rte_ethdev.h index f97f0a6e5..889486a11 100644
> >>>>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>>>> @@ -1013,6 +1013,7 @@ struct rte_eth_conf {
> >>>>>     #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
> >>>>>     #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
> >>>>>     #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> >>>>> +#define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> >>>> Should be added to rte_rx_offload_names in
> >>>> lib/librte_ethdev/rte_ethdev.c.

[dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an offload

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be used to
enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 doc/guides/nics/features.rst   | 12 ++++++++++++
 lib/librte_ethdev/rte_ethdev.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index f79b69b38..d67430d90 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -594,6 +594,18 @@ application to set ptypes it is interested in.
 * **[provides]   mbuf**: ``mbuf.packet_type``.
 
 
+.. _nic_features_flow_action_type_update:
+
+Flow type update
+----------------
+
+Supports flow action type update to ``mbuf.ol_flags`` and ``mbuf.hash.fdir.hi``.
+
+* **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
+* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``, ``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
+  ``mbuf.hash.fdir.hi``
+
+
 .. _nic_features_timesync:
 
 Timesync
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 889486a11..4a0cff830 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1014,6 +1014,7 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
 #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
 #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
+#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
 
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
-- 
2.22.0

Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an offload

[Andrew Rybchenko]

On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be used to
> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.

Notes similar to RSS hash.

It requires better motivation why. It lets Rx queue know that
it will be used as flow action MARK target and the queue should
be configured to deliver the mark from NIC to PMD and processed
in the driver.

Also I think that flow API action MARK documentation should be
updated to mentioned the offload.

> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>   doc/guides/nics/features.rst   | 12 ++++++++++++
>   lib/librte_ethdev/rte_ethdev.h |  1 +
>   2 files changed, 13 insertions(+)
>
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index f79b69b38..d67430d90 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
>   * **[provides]   mbuf**: ``mbuf.packet_type``.
>   
>   
> +.. _nic_features_flow_action_type_update:

May be  _nic_features_flow_mark ?

> +
> +Flow type update

May be "Flow mark delivery" ?

> +----------------
> +
> +Supports flow action type update to ``mbuf.ol_flags`` and ``mbuf.hash.fdir.hi``.
> +
> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.

DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.



> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``, ``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
> +  ``mbuf.hash.fdir.hi``
> +
> +
>   .. _nic_features_timesync:
>   
>   Timesync
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 889486a11..4a0cff830 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
>   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>   #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
>   
>   #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>   				 DEV_RX_OFFLOAD_UDP_CKSUM | \

Add to rte_rx_offload_names

Re: [dpdk-dev] [EXT] Re: [PATCH 3/7] ethdev: add flow action type update as an offload

[Pavan Nikhilesh Bhagavatula]

>> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.
>
>Notes similar to RSS hash.
>
>It requires better motivation why. It lets Rx queue know that
>it will be used as flow action MARK target and the queue should
>be configured to deliver the mark from NIC to PMD and processed
>in the driver.
>
>Also I think that flow API action MARK documentation should be
>updated to mentioned the offload.

Ack will reword the commit log and update the documentation.
>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> ---
>>   doc/guides/nics/features.rst   | 12 ++++++++++++
>>   lib/librte_ethdev/rte_ethdev.h |  1 +
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/doc/guides/nics/features.rst
>b/doc/guides/nics/features.rst
>> index f79b69b38..d67430d90 100644
>> --- a/doc/guides/nics/features.rst
>> +++ b/doc/guides/nics/features.rst
>> @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
>>   * **[provides]   mbuf**: ``mbuf.packet_type``.
>>
>>
>> +.. _nic_features_flow_action_type_update:
>
>May be  _nic_features_flow_mark ?
>
>> +
>> +Flow type update
>
>May be "Flow mark delivery" ?

I named it flow action type update because we use the same flag to represent two different
flow types i.e. RTE_FLOW_ACTION_TYPE_FLAG and RTE_FLOW_ACTION_TYPE_MARK.

I'm not so sure about the name RX_OFFLOAD_FLOW_MARK too because mbuf might have ol_flags
marked for ACTION_TYPE_FLAG. Let me know your thoughts on this. 

I will modify the name once we come to an agreement. 

>
>> +----------------
>> +
>> +Supports flow action type update to ``mbuf.ol_flags`` and
>``mbuf.hash.fdir.hi``.
>> +
>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
>
>DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.

Ack, will fix in v2.

>
>
>
>> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
>``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
>> +  ``mbuf.hash.fdir.hi``
>> +
>> +
>>   .. _nic_features_timesync:
>>
>>   Timesync
>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>b/lib/librte_ethdev/rte_ethdev.h
>> index 889486a11..4a0cff830 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
>>   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>   #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
>>
>>   #define DEV_RX_OFFLOAD_CHECKSUM
>(DEV_RX_OFFLOAD_IPV4_CKSUM | \
>>   				 DEV_RX_OFFLOAD_UDP_CKSUM | \
>
>Add to rte_rx_offload_names

Re: [dpdk-dev] [EXT] Re: [PATCH 3/7] ethdev: add flow action type update as an offload

[Andrew Rybchenko]

On 8/17/19 5:23 PM, Pavan Nikhilesh Bhagavatula wrote:
>>> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.
>> Notes similar to RSS hash.
>>
>> It requires better motivation why. It lets Rx queue know that
>> it will be used as flow action MARK target and the queue should
>> be configured to deliver the mark from NIC to PMD and processed
>> in the driver.
>>
>> Also I think that flow API action MARK documentation should be
>> updated to mentioned the offload.
> Ack will reword the commit log and update the documentation.
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>> ---
>>>    doc/guides/nics/features.rst   | 12 ++++++++++++
>>>    lib/librte_ethdev/rte_ethdev.h |  1 +
>>>    2 files changed, 13 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/features.rst
>> b/doc/guides/nics/features.rst
>>> index f79b69b38..d67430d90 100644
>>> --- a/doc/guides/nics/features.rst
>>> +++ b/doc/guides/nics/features.rst
>>> @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
>>>    * **[provides]   mbuf**: ``mbuf.packet_type``.
>>>
>>>
>>> +.. _nic_features_flow_action_type_update:
>> May be  _nic_features_flow_mark ?
>>
>>> +
>>> +Flow type update
>> May be "Flow mark delivery" ?
> I named it flow action type update because we use the same flag to represent two different
> flow types i.e. RTE_FLOW_ACTION_TYPE_FLAG and RTE_FLOW_ACTION_TYPE_MARK.
>
> I'm not so sure about the name RX_OFFLOAD_FLOW_MARK too because mbuf might have ol_flags
> marked for ACTION_TYPE_FLAG. Let me know your thoughts on this.

I forgot about RTE_FLOW_ACTION_TYPE_FLAG and really care about mark only
since it is 32-bit vs one. Anyway may be it is more correct to care 
about both.
If so,  may be _nic_features_flow_flag_mark and "Flow flag/mark update"?

> I will modify the name once we come to an agreement.
>
>>> +----------------
>>> +
>>> +Supports flow action type update to ``mbuf.ol_flags`` and
>> ``mbuf.hash.fdir.hi``.
>>> +
>>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>> ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
>>
>> DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.
> Ack, will fix in v2.
>
>>
>>
>>> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
>> ``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
>>> +  ``mbuf.hash.fdir.hi``
>>> +
>>> +
>>>    .. _nic_features_timesync:
>>>
>>>    Timesync
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>> b/lib/librte_ethdev/rte_ethdev.h
>>> index 889486a11..4a0cff830 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
>>>    #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>>    #define DEV_RX_OFFLOAD_OUTER_Udrivers/net/sfc/base/rhead_ev.c.origDP_CKSUM  0x00040000
>>>    #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>>> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
>>>
>>>    #define DEV_RX_OFFLOAD_CHECKSUM
>> (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>>>    				 DEV_RX_OFFLOAD_UDP_CKSUM | \
>> Add to rte_rx_offload_names

Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an offload

[Shahaf Shuler]

Friday, August 16, 2019 11:05 AM, Andrew Rybchenko:
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as
> an offload
> 
> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be
> used to
> > enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.
> 
> Notes similar to RSS hash.
> 
> It requires better motivation why. It lets Rx queue know that it will be used
> as flow action MARK target and the queue should be configured to deliver
> the mark from NIC to PMD and processed in the driver.

This one is even worse than the RSS (sorry). 

First - the API breakage exists also here and if we want to include such patch we should have proper doc on RN. 
Second - the user explicitly inserted a rte_flow rule w/ action mark. Its expectation is to receive his mark on the mbuf (otherwise why would it set this action?).  it is not expected from the user to set another offload flag just to enable the mark set on the mbuf. It makes the user experience very convoluted.   
Third - so far we never reported rte_flow capabilities, and there is a good reason for it - the cap matrix is too big. For rte_flow the chosen approach was trail and error. If we start w/ the flow mark, the amount of cap bits the application will need to monitor will be huge. 

My suggestion here, for PMD that wants to optimize their datapath is to check if flow w/ mark action was inserted on the queue. So long there is no such flow they can disable the set of the mark.


> 
> Also I think that flow API action MARK documentation should be updated to
> mentioned the offload.
> 
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> >   doc/guides/nics/features.rst   | 12 ++++++++++++
> >   lib/librte_ethdev/rte_ethdev.h |  1 +
> >   2 files changed, 13 insertions(+)
> >
> > diff --git a/doc/guides/nics/features.rst
> > b/doc/guides/nics/features.rst index f79b69b38..d67430d90 100644
> > --- a/doc/guides/nics/features.rst
> > +++ b/doc/guides/nics/features.rst
> > @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
> >   * **[provides]   mbuf**: ``mbuf.packet_type``.
> >
> >
> > +.. _nic_features_flow_action_type_update:
> 
> May be  _nic_features_flow_mark ?
> 
> > +
> > +Flow type update
> 
> May be "Flow mark delivery" ?
> 
> > +----------------
> > +
> > +Supports flow action type update to ``mbuf.ol_flags`` and
> ``mbuf.hash.fdir.hi``.
> > +
> > +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
> ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
> 
> DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.
> 
> 
> 
> > +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
> > +``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
> > +  ``mbuf.hash.fdir.hi``
> > +
> > +
> >   .. _nic_features_timesync:
> >
> >   Timesync
> > diff --git a/lib/librte_ethdev/rte_ethdev.h
> > b/lib/librte_ethdev/rte_ethdev.h index 889486a11..4a0cff830 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
> >   #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
> >   #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> >   #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> > +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
> >
> >   #define DEV_RX_OFFLOAD_CHECKSUM
> (DEV_RX_OFFLOAD_IPV4_CKSUM | \
> >   				 DEV_RX_OFFLOAD_UDP_CKSUM | \
> 
> Add to rte_rx_offload_names

Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an offload

[Andrew Rybchenko]

On 8/18/19 7:59 AM, Shahaf Shuler wrote:
> Friday, August 16, 2019 11:05 AM, Andrew Rybchenko:
>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as
>> an offload
>>
>> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>
>>> Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be
>> used to
>>> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.
>> Notes similar to RSS hash.
>>
>> It requires better motivation why. It lets Rx queue know that it will be used
>> as flow action MARK target and the queue should be configured to deliver
>> the mark from NIC to PMD and processed in the driver.
> This one is even worse than the RSS (sorry).
>
> First - the API breakage exists also here and if we want to include such patch we should have proper doc on RN.

Yes, there is a deprecation notice for it in v19.08 and I already
mentioned in review notes for the patch [1/7] that release notes
are required.

> Second - the user explicitly inserted a rte_flow rule w/ action mark. Its expectation is to receive his mark on the mbuf (otherwise why would it set this action?).  it is not expected from the user to set another offload flag just to enable the mark set on the mbuf. It makes the user experience very convoluted.
> Third - so far we never reported rte_flow capabilities, and there is a good reason for it - the cap matrix is too big. For rte_flow the chosen approach was trail and error. If we start w/ the flow mark, the amount of cap bits the application will need to monitor will be huge.

The feature differs a lot from other rte_flow features since it
adds Rx meta information.
And yes, rte_flow rule to set mark should fail with appropriate
diagnostics if the offload is supported by not enabled or not
supported at all. So, application will be informed and I think
it is less worse than RSS hash.

> My suggestion here, for PMD that wants to optimize their datapath is to check if flow w/ mark action was inserted on the queue. So long there is no such flow they can disable the set of the mark.

Unfortunately the information is required on Rx queue setup
stage and rte_rule insertion is too late.

Anyway, the important point here is all Rx offloads consistency:
want something - enable it. The only remaining exception is
packet type which default behaviour is preserved since really
flexible solution suggested by Konstantin.

>> Also I think that flow API action MARK documentation should be updated to
>> mentioned the offload.
>>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>> ---
>>>    doc/guides/nics/features.rst   | 12 ++++++++++++
>>>    lib/librte_ethdev/rte_ethdev.h |  1 +
>>>    2 files changed, 13 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/features.rst
>>> b/doc/guides/nics/features.rst index f79b69b38..d67430d90 100644
>>> --- a/doc/guides/nics/features.rst
>>> +++ b/doc/guides/nics/features.rst
>>> @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
>>>    * **[provides]   mbuf**: ``mbuf.packet_type``.
>>>
>>>
>>> +.. _nic_features_flow_action_type_update:
>> May be  _nic_features_flow_mark ?
>>
>>> +
>>> +Flow type update
>> May be "Flow mark delivery" ?
>>
>>> +----------------
>>> +
>>> +Supports flow action type update to ``mbuf.ol_flags`` and
>> ``mbuf.hash.fdir.hi``.
>>> +
>>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>> ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
>>
>> DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.
>>
>>
>>
>>> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
>>> +``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
>>> +  ``mbuf.hash.fdir.hi``
>>> +
>>> +
>>>    .. _nic_features_timesync:
>>>
>>>    Timesync
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>> b/lib/librte_ethdev/rte_ethdev.h index 889486a11..4a0cff830 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
>>>    #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>>    #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>>    #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>>> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
>>>
>>>    #define DEV_RX_OFFLOAD_CHECKSUM
>> (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>>>    				 DEV_RX_OFFLOAD_UDP_CKSUM | \
>> Add to rte_rx_offload_names

Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an offload

[Shahaf Shuler]

Sunday, August 18, 2019 8:57 AM, Andrew Rybchenko:
> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as
> an offload
> 
> On 8/18/19 7:59 AM, Shahaf Shuler wrote:
> > Friday, August 16, 2019 11:05 AM, Andrew Rybchenko:
> >> <marko.kovacevic@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type
> >> update as an offload
> >>
> >> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>
> >>> Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be
> >> used to
> >>> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.
> >> Notes similar to RSS hash.
> >>
> >> It requires better motivation why. It lets Rx queue know that it will
> >> be used as flow action MARK target and the queue should be configured
> >> to deliver the mark from NIC to PMD and processed in the driver.
> > This one is even worse than the RSS (sorry).
> >
> > First - the API breakage exists also here and if we want to include such
> patch we should have proper doc on RN.
> 
> Yes, there is a deprecation notice for it in v19.08 and I already mentioned in
> review notes for the patch [1/7] that release notes are required.
> 
> > Second - the user explicitly inserted a rte_flow rule w/ action mark. Its
> expectation is to receive his mark on the mbuf (otherwise why would it set
> this action?).  it is not expected from the user to set another offload flag just
> to enable the mark set on the mbuf. It makes the user experience very
> convoluted.
> > Third - so far we never reported rte_flow capabilities, and there is a good
> reason for it - the cap matrix is too big. For rte_flow the chosen approach was
> trail and error. If we start w/ the flow mark, the amount of cap bits the
> application will need to monitor will be huge.
> 
> The feature differs a lot from other rte_flow features since it adds Rx meta
> information.
> And yes, rte_flow rule to set mark should fail with appropriate diagnostics if
> the offload is supported by not enabled or not supported at all. So,
> application will be informed and I think it is less worse than RSS hash.
> 
> > My suggestion here, for PMD that wants to optimize their datapath is to
> check if flow w/ mark action was inserted on the queue. So long there is no
> such flow they can disable the set of the mark.
> 
> Unfortunately the information is required on Rx queue setup stage and
> rte_rule insertion is too late.

See my comments on the RSS patch. Same point of discussion.

The main question we need to answer -
User set flow mark action by rte_flow (that was accepted). Why does it need to set more flag? 

> 
> Anyway, the important point here is all Rx offloads consistency:
> want something - enable it. The only remaining exception is packet type
> which default behaviour is preserved since really flexible solution suggested
> by Konstantin.
> 
> >> Also I think that flow API action MARK documentation should be
> >> updated to mentioned the offload.
> >>
> >>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>> ---
> >>>    doc/guides/nics/features.rst   | 12 ++++++++++++
> >>>    lib/librte_ethdev/rte_ethdev.h |  1 +
> >>>    2 files changed, 13 insertions(+)
> >>>
> >>> diff --git a/doc/guides/nics/features.rst
> >>> b/doc/guides/nics/features.rst index f79b69b38..d67430d90 100644
> >>> --- a/doc/guides/nics/features.rst
> >>> +++ b/doc/guides/nics/features.rst
> >>> @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
> >>>    * **[provides]   mbuf**: ``mbuf.packet_type``.
> >>>
> >>>
> >>> +.. _nic_features_flow_action_type_update:
> >> May be  _nic_features_flow_mark ?
> >>
> >>> +
> >>> +Flow type update
> >> May be "Flow mark delivery" ?
> >>
> >>> +----------------
> >>> +
> >>> +Supports flow action type update to ``mbuf.ol_flags`` and
> >> ``mbuf.hash.fdir.hi``.
> >>> +
> >>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
> >> ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
> >>
> >> DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.
> >>
> >>
> >>
> >>> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
> >>> +``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
> >>> +  ``mbuf.hash.fdir.hi``
> >>> +
> >>> +
> >>>    .. _nic_features_timesync:
> >>>
> >>>    Timesync
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.h
> >>> b/lib/librte_ethdev/rte_ethdev.h index 889486a11..4a0cff830 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
> >>>    #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
> >>>    #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
> >>>    #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
> >>> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
> >>>
> >>>    #define DEV_RX_OFFLOAD_CHECKSUM
> >> (DEV_RX_OFFLOAD_IPV4_CKSUM | \
> >>>    				 DEV_RX_OFFLOAD_UDP_CKSUM | \
> >> Add to rte_rx_offload_names

Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an offload

[Andrew Rybchenko]

On 8/18/19 9:20 AM, Shahaf Shuler wrote:
> Sunday, August 18, 2019 8:57 AM, Andrew Rybchenko:
>> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as
>> an offload
>>
>> On 8/18/19 7:59 AM, Shahaf Shuler wrote:
>>> Friday, August 16, 2019 11:05 AM, Andrew Rybchenko:
>>>> <marko.kovacevic@intel.com>; Thomas Monjalon
>> <thomas@monjalon.net>
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type
>>>> update as an offload
>>>>
>>>> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>
>>>>> Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be
>>>> used to
>>>>> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`.
>>>> Notes similar to RSS hash.
>>>>
>>>> It requires better motivation why. It lets Rx queue know that it will
>>>> be used as flow action MARK target and the queue should be configured
>>>> to deliver the mark from NIC to PMD and processed in the driver.
>>> This one is even worse than the RSS (sorry).
>>>
>>> First - the API breakage exists also here and if we want to include such
>> patch we should have proper doc on RN.
>>
>> Yes, there is a deprecation notice for it in v19.08 and I already mentioned in
>> review notes for the patch [1/7] that release notes are required.
>>
>>> Second - the user explicitly inserted a rte_flow rule w/ action mark. Its
>> expectation is to receive his mark on the mbuf (otherwise why would it set
>> this action?).  it is not expected from the user to set another offload flag just
>> to enable the mark set on the mbuf. It makes the user experience very
>> convoluted.
>>> Third - so far we never reported rte_flow capabilities, and there is a good
>> reason for it - the cap matrix is too big. For rte_flow the chosen approach was
>> trail and error. If we start w/ the flow mark, the amount of cap bits the
>> application will need to monitor will be huge.
>>
>> The feature differs a lot from other rte_flow features since it adds Rx meta
>> information.
>> And yes, rte_flow rule to set mark should fail with appropriate diagnostics if
>> the offload is supported by not enabled or not supported at all. So,
>> application will be informed and I think it is less worse than RSS hash.
>>
>>> My suggestion here, for PMD that wants to optimize their datapath is to
>> check if flow w/ mark action was inserted on the queue. So long there is no
>> such flow they can disable the set of the mark.
>>
>> Unfortunately the information is required on Rx queue setup stage and
>> rte_rule insertion is too late.
> See my comments on the RSS patch. Same point of discussion.
>
> The main question we need to answer -
> User set flow mark action by rte_flow (that was accepted). Why does it need to set more flag?

I think all arguments are on the table.
Mine:
  - possibility to squeeze more performance from HW and SW
  - consistency (at least as I see it) in filling in of the information 
in mbuf
    (direct using corresponding offloads)
Your:
  - more complexity in control (which is less painful in the case of 
flow mark,
    since it should be an error on flow rule setup, and a bit more 
painful in
    in the case of RSS hash since there is simply no RSS hash if not 
enabled)
  - API breakage (but there is deprecation notice in v19.08)

Many thanks for the discussion.

>> Anyway, the important point here is all Rx offloads consistency:
>> want something - enable it. The only remaining exception is packet type
>> which default behaviour is preserved since really flexible solution suggested
>> by Konstantin.
>>
>>>> Also I think that flow API action MARK documentation should be
>>>> updated to mentioned the offload.
>>>>
>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>> ---
>>>>>     doc/guides/nics/features.rst   | 12 ++++++++++++
>>>>>     lib/librte_ethdev/rte_ethdev.h |  1 +
>>>>>     2 files changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/doc/guides/nics/features.rst
>>>>> b/doc/guides/nics/features.rst index f79b69b38..d67430d90 100644
>>>>> --- a/doc/guides/nics/features.rst
>>>>> +++ b/doc/guides/nics/features.rst
>>>>> @@ -594,6 +594,18 @@ application to set ptypes it is interested in.
>>>>>     * **[provides]   mbuf**: ``mbuf.packet_type``.
>>>>>
>>>>>
>>>>> +.. _nic_features_flow_action_type_update:
>>>> May be  _nic_features_flow_mark ?
>>>>
>>>>> +
>>>>> +Flow type update
>>>> May be "Flow mark delivery" ?
>>>>
>>>>> +----------------
>>>>> +
>>>>> +Supports flow action type update to ``mbuf.ol_flags`` and
>>>> ``mbuf.hash.fdir.hi``.
>>>>> +
>>>>> +* **[uses]     rte_eth_rxconf,rte_eth_rxmode**:
>>>> ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``.
>>>>
>>>> DEV_RX_OFFLOAD_FLOW_MARK, not TYPE.
>>>>
>>>>
>>>>
>>>>> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``,
>>>>> +``mbuf.ol_flags:PKT_RX_FDIR_ID;``,
>>>>> +  ``mbuf.hash.fdir.hi``
>>>>> +
>>>>> +
>>>>>     .. _nic_features_timesync:
>>>>>
>>>>>     Timesync
>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>>>>> b/lib/librte_ethdev/rte_ethdev.h index 889486a11..4a0cff830 100644
>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>> @@ -1014,6 +1014,7 @@ struct rte_eth_conf {
>>>>>     #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
>>>>>     #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
>>>>>     #define DEV_RX_OFFLOAD_RSS_HASH		0x00080000
>>>>> +#define DEV_RX_OFFLOAD_FLOW_MARK	0x00100000
>>>>>
>>>>>     #define DEV_RX_OFFLOAD_CHECKSUM
>>>> (DEV_RX_OFFLOAD_IPV4_CKSUM | \
>>>>>     				 DEV_RX_OFFLOAD_UDP_CKSUM | \
>>>> Add to rte_rx_offload_names

[dpdk-dev] [PATCH 4/7] net: update Rx RSS hash offload capabilities

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add DEV_RX_OFFLOAD_RSS_HASH flag for all supported NICs.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 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/igb_rxtx.c         |  3 ++-
 drivers/net/enic/enic_res.c          |  3 ++-
 drivers/net/fm10k/fm10k_ethdev.c     |  3 ++-
 drivers/net/hinic/hinic_pmd_ethdev.c |  3 ++-
 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/liquidio/lio_ethdev.c    |  3 ++-
 drivers/net/mlx4/mlx4_rxq.c          |  3 ++-
 drivers/net/mlx5/mlx5_rxq.c          |  3 ++-
 drivers/net/netvsc/hn_rndis.c        |  3 ++-
 drivers/net/nfp/nfp_net.c            |  3 ++-
 drivers/net/octeontx2/otx2_ethdev.c  |  3 ++-
 drivers/net/octeontx2/otx2_ethdev.h  | 15 ++++++++-------
 drivers/net/qede/qede_ethdev.c       |  3 ++-
 drivers/net/sfc/sfc_rx.c             |  3 ++-
 drivers/net/thunderx/nicvf_ethdev.h  |  3 ++-
 drivers/net/vmxnet3/vmxnet3_ethdev.c |  3 ++-
 23 files changed, 52 insertions(+), 29 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6685ee7d9..6c106baf7 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -160,7 +160,8 @@ static const struct rte_pci_id bnxt_pci_id_map[] = {
 				     DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM | \
 				     DEV_RX_OFFLOAD_JUMBO_FRAME | \
 				     DEV_RX_OFFLOAD_KEEP_CRC | \
-				     DEV_RX_OFFLOAD_TCP_LRO)
+				     DEV_RX_OFFLOAD_TCP_LRO | \
+				     DEV_RX_OFFLOAD_RSS_HASH)
 
 static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
 static void bnxt_print_link_info(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/cxgbe/cxgbe.h b/drivers/net/cxgbe/cxgbe.h
index 3f97fa58b..22e61a55c 100644
--- a/drivers/net/cxgbe/cxgbe.h
+++ b/drivers/net/cxgbe/cxgbe.h
@@ -47,7 +47,8 @@
 			   DEV_RX_OFFLOAD_UDP_CKSUM | \
 			   DEV_RX_OFFLOAD_TCP_CKSUM | \
 			   DEV_RX_OFFLOAD_JUMBO_FRAME | \
-			   DEV_RX_OFFLOAD_SCATTER)
+			   DEV_RX_OFFLOAD_SCATTER | \
+			   DEV_RX_OFFLOAD_RSS_HASH)
 
 
 #define CXGBE_DEVARG_KEEP_OVLAN "keep_ovlan"
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 7154fb9b4..18c7bd0d5 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -49,7 +49,8 @@
 /* Supported Rx offloads */
 static uint64_t dev_rx_offloads_sup =
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
-		DEV_RX_OFFLOAD_SCATTER;
+		DEV_RX_OFFLOAD_SCATTER |
+		DEV_RX_OFFLOAD_RSS_HASH;
 
 /* Rx offloads which cannot be disabled */
 static uint64_t dev_rx_offloads_nodis =
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index dd6a78f9f..55a1c4455 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -38,7 +38,8 @@ static uint64_t dev_rx_offloads_sup =
 		DEV_RX_OFFLOAD_TCP_CKSUM |
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
-		DEV_RX_OFFLOAD_JUMBO_FRAME;
+		DEV_RX_OFFLOAD_JUMBO_FRAME |
+		DEV_RX_OFFLOAD_RSS_HASH;
 
 /* Rx offloads which cannot be disabled */
 static uint64_t dev_rx_offloads_nodis =
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index c5606de5d..684fa4ad8 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1646,7 +1646,8 @@ igb_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 			  DEV_RX_OFFLOAD_TCP_CKSUM   |
 			  DEV_RX_OFFLOAD_JUMBO_FRAME |
 			  DEV_RX_OFFLOAD_KEEP_CRC    |
-			  DEV_RX_OFFLOAD_SCATTER;
+			  DEV_RX_OFFLOAD_SCATTER     |
+			  DEV_RX_OFFLOAD_RSS_HASH;
 
 	return rx_offload_capa;
 }
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 9405e1933..607a085f8 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -198,7 +198,8 @@ int enic_get_vnic_config(struct enic *enic)
 		DEV_RX_OFFLOAD_VLAN_STRIP |
 		DEV_RX_OFFLOAD_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_UDP_CKSUM |
-		DEV_RX_OFFLOAD_TCP_CKSUM;
+		DEV_RX_OFFLOAD_TCP_CKSUM |
+		DEV_RX_OFFLOAD_RSS_HASH;
 	enic->tx_offload_mask =
 		PKT_TX_IPV6 |
 		PKT_TX_IPV4 |
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index db4d72129..ba9b174cf 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1797,7 +1797,8 @@ static uint64_t fm10k_get_rx_port_offloads_capa(struct rte_eth_dev *dev)
 			   DEV_RX_OFFLOAD_UDP_CKSUM   |
 			   DEV_RX_OFFLOAD_TCP_CKSUM   |
 			   DEV_RX_OFFLOAD_JUMBO_FRAME |
-			   DEV_RX_OFFLOAD_HEADER_SPLIT);
+			   DEV_RX_OFFLOAD_HEADER_SPLIT |
+			   DEV_RX_OFFLOAD_RSS_HASH);
 }
 
 static int
diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hinic_pmd_ethdev.c
index 044af9053..53bd2b9ae 100644
--- a/drivers/net/hinic/hinic_pmd_ethdev.c
+++ b/drivers/net/hinic/hinic_pmd_ethdev.c
@@ -680,7 +680,8 @@ hinic_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
 	info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
 				DEV_RX_OFFLOAD_IPV4_CKSUM |
 				DEV_RX_OFFLOAD_UDP_CKSUM |
-				DEV_RX_OFFLOAD_TCP_CKSUM;
+				DEV_RX_OFFLOAD_TCP_CKSUM |
+				DEV_RX_OFFLOAD_RSS_HASH;
 
 	info->tx_queue_offload_capa = 0;
 	info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT |
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 4e40b7ab5..7058e0213 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3511,7 +3511,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_SCATTER |
 		DEV_RX_OFFLOAD_VLAN_EXTEND |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
-		DEV_RX_OFFLOAD_JUMBO_FRAME;
+		DEV_RX_OFFLOAD_JUMBO_FRAME |
+		DEV_RX_OFFLOAD_RSS_HASH;
 
 	dev_info->tx_queue_offload_capa = DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 	dev_info->tx_offload_capa =
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 8f3907378..aef91a79b 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -517,7 +517,8 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_SCATTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
-		DEV_RX_OFFLOAD_VLAN_FILTER;
+		DEV_RX_OFFLOAD_VLAN_FILTER |
+		DEV_RX_OFFLOAD_RSS_HASH;
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_VLAN_INSERT |
 		DEV_TX_OFFLOAD_QINQ_INSERT |
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 44a14cb8a..fc0f0003f 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2133,7 +2133,8 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 			DEV_RX_OFFLOAD_TCP_CKSUM |
 			DEV_RX_OFFLOAD_QINQ_STRIP |
 			DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
-			DEV_RX_OFFLOAD_VLAN_EXTEND;
+			DEV_RX_OFFLOAD_VLAN_EXTEND |
+			DEV_RX_OFFLOAD_RSS_HASH;
 		dev_info->tx_offload_capa |=
 			DEV_TX_OFFLOAD_QINQ_INSERT |
 			DEV_TX_OFFLOAD_IPV4_CKSUM |
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index edcfa60ce..fa572d184 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2872,7 +2872,8 @@ ixgbe_get_rx_port_offloads(struct rte_eth_dev *dev)
 		   DEV_RX_OFFLOAD_KEEP_CRC    |
 		   DEV_RX_OFFLOAD_JUMBO_FRAME |
 		   DEV_RX_OFFLOAD_VLAN_FILTER |
-		   DEV_RX_OFFLOAD_SCATTER;
+		   DEV_RX_OFFLOAD_SCATTER |
+		   DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (hw->mac.type == ixgbe_mac_82598EB)
 		offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c
index c25dab00c..ff118586e 100644
--- a/drivers/net/liquidio/lio_ethdev.c
+++ b/drivers/net/liquidio/lio_ethdev.c
@@ -406,7 +406,8 @@ lio_dev_info_get(struct rte_eth_dev *eth_dev,
 	devinfo->rx_offload_capa = (DEV_RX_OFFLOAD_IPV4_CKSUM		|
 				    DEV_RX_OFFLOAD_UDP_CKSUM		|
 				    DEV_RX_OFFLOAD_TCP_CKSUM		|
-				    DEV_RX_OFFLOAD_VLAN_STRIP);
+				    DEV_RX_OFFLOAD_VLAN_STRIP		|
+				    DEV_RX_OFFLOAD_RSS_HASH);
 	devinfo->tx_offload_capa = (DEV_TX_OFFLOAD_IPV4_CKSUM		|
 				    DEV_TX_OFFLOAD_UDP_CKSUM		|
 				    DEV_TX_OFFLOAD_TCP_CKSUM		|
diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c
index f45c1ff85..4a6fbd922 100644
--- a/drivers/net/mlx4/mlx4_rxq.c
+++ b/drivers/net/mlx4/mlx4_rxq.c
@@ -685,7 +685,8 @@ mlx4_get_rx_queue_offloads(struct mlx4_priv *priv)
 {
 	uint64_t offloads = DEV_RX_OFFLOAD_SCATTER |
 			    DEV_RX_OFFLOAD_KEEP_CRC |
-			    DEV_RX_OFFLOAD_JUMBO_FRAME;
+			    DEV_RX_OFFLOAD_JUMBO_FRAME |
+			    DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (priv->hw_csum)
 		offloads |= DEV_RX_OFFLOAD_CHECKSUM;
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index a1fdeef2a..b5fd57693 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -368,7 +368,8 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev *dev)
 	struct mlx5_dev_config *config = &priv->config;
 	uint64_t offloads = (DEV_RX_OFFLOAD_SCATTER |
 			     DEV_RX_OFFLOAD_TIMESTAMP |
-			     DEV_RX_OFFLOAD_JUMBO_FRAME);
+			     DEV_RX_OFFLOAD_JUMBO_FRAME |
+			     DEV_RX_OFFLOAD_RSS_HASH);
 
 	if (config->hw_fcs_strip)
 		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
diff --git a/drivers/net/netvsc/hn_rndis.c b/drivers/net/netvsc/hn_rndis.c
index a67bc7a79..2b4714042 100644
--- a/drivers/net/netvsc/hn_rndis.c
+++ b/drivers/net/netvsc/hn_rndis.c
@@ -897,7 +897,8 @@ int hn_rndis_get_offload(struct hn_data *hv,
 	    == HN_NDIS_LSOV2_CAP_IP6)
 		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
 
-	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP |
+				    DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (hwcaps.ndis_csum.ndis_ip4_rxcsum & NDIS_RXCSUM_CAP_IP4)
 		dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_IPV4_CKSUM;
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index f1a3ef2f9..230d64c8a 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1226,7 +1226,8 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 					     DEV_RX_OFFLOAD_UDP_CKSUM |
 					     DEV_RX_OFFLOAD_TCP_CKSUM;
 
-	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_JUMBO_FRAME |
+				     DEV_RX_OFFLOAD_RSS_HASH;
 
 	if (hw->cap & NFP_NET_CFG_CTRL_TXVLAN)
 		dev_info->tx_offload_capa = DEV_TX_OFFLOAD_VLAN_INSERT;
diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
index b84128fef..2e88d1844 100644
--- a/drivers/net/octeontx2/otx2_ethdev.c
+++ b/drivers/net/octeontx2/otx2_ethdev.c
@@ -569,7 +569,8 @@ nix_rx_offload_flags(struct rte_eth_dev *eth_dev)
 	struct rte_eth_rxmode *rxmode = &conf->rxmode;
 	uint16_t flags = 0;
 
-	if (rxmode->mq_mode == ETH_MQ_RX_RSS)
+	if (rxmode->mq_mode == ETH_MQ_RX_RSS &&
+			(dev->rx_offloads & DEV_RX_OFFLOAD_RSS_HASH))
 		flags |= NIX_RX_OFFLOAD_RSS_F;
 
 	if (dev->rx_offloads & (DEV_RX_OFFLOAD_TCP_CKSUM |
diff --git a/drivers/net/octeontx2/otx2_ethdev.h b/drivers/net/octeontx2/otx2_ethdev.h
index 7b15d6bc8..0cca6746d 100644
--- a/drivers/net/octeontx2/otx2_ethdev.h
+++ b/drivers/net/octeontx2/otx2_ethdev.h
@@ -122,8 +122,8 @@
 	DEV_TX_OFFLOAD_MT_LOCKFREE	| \
 	DEV_TX_OFFLOAD_VLAN_INSERT	| \
 	DEV_TX_OFFLOAD_QINQ_INSERT	| \
-	DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM | \
-	DEV_TX_OFFLOAD_OUTER_UDP_CKSUM  | \
+	DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM	| \
+	DEV_TX_OFFLOAD_OUTER_UDP_CKSUM	| \
 	DEV_TX_OFFLOAD_TCP_CKSUM	| \
 	DEV_TX_OFFLOAD_UDP_CKSUM	| \
 	DEV_TX_OFFLOAD_SCTP_CKSUM	| \
@@ -136,11 +136,12 @@
 	DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM | \
 	DEV_RX_OFFLOAD_SCATTER		| \
 	DEV_RX_OFFLOAD_JUMBO_FRAME	| \
-	DEV_RX_OFFLOAD_OUTER_UDP_CKSUM | \
-	DEV_RX_OFFLOAD_VLAN_STRIP | \
-	DEV_RX_OFFLOAD_VLAN_FILTER | \
-	DEV_RX_OFFLOAD_QINQ_STRIP | \
-	DEV_RX_OFFLOAD_TIMESTAMP)
+	DEV_RX_OFFLOAD_OUTER_UDP_CKSUM	| \
+	DEV_RX_OFFLOAD_VLAN_STRIP	| \
+	DEV_RX_OFFLOAD_VLAN_FILTER	| \
+	DEV_RX_OFFLOAD_QINQ_STRIP	| \
+	DEV_RX_OFFLOAD_TIMESTAMP	| \
+	DEV_RX_OFFLOAD_RSS_HASH)
 
 #define NIX_DEFAULT_RSS_CTX_GROUP  0
 #define NIX_DEFAULT_RSS_MCAM_IDX  -1
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 528b33e8c..da25b26df 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1291,7 +1291,8 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
 				     DEV_RX_OFFLOAD_SCATTER	|
 				     DEV_RX_OFFLOAD_JUMBO_FRAME |
 				     DEV_RX_OFFLOAD_VLAN_FILTER |
-				     DEV_RX_OFFLOAD_VLAN_STRIP);
+				     DEV_RX_OFFLOAD_VLAN_STRIP  |
+				     DEV_RX_OFFLOAD_RSS_HASH);
 	dev_info->rx_queue_offload_capa = 0;
 
 	/* TX offloads are on a per-packet basis, so it is applicable
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index e6809bb64..695580b22 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -618,7 +618,8 @@ struct sfc_dp_rx sfc_efx_rx = {
 	},
 	.features		= SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= DEV_RX_OFFLOAD_CHECKSUM,
-	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER,
+	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER |
+				  DEV_RX_OFFLOAD_RSS_HASH,
 	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
 	.qcreate		= sfc_efx_rx_qcreate,
 	.qdestroy		= sfc_efx_rx_qdestroy,
diff --git a/drivers/net/thunderx/nicvf_ethdev.h b/drivers/net/thunderx/nicvf_ethdev.h
index c0bfbf848..391411799 100644
--- a/drivers/net/thunderx/nicvf_ethdev.h
+++ b/drivers/net/thunderx/nicvf_ethdev.h
@@ -41,7 +41,8 @@
 	DEV_RX_OFFLOAD_CHECKSUM    | \
 	DEV_RX_OFFLOAD_VLAN_STRIP  | \
 	DEV_RX_OFFLOAD_JUMBO_FRAME | \
-	DEV_RX_OFFLOAD_SCATTER)
+	DEV_RX_OFFLOAD_SCATTER     | \
+	DEV_RX_OFFLOAD_RSS_HASH)
 
 #define NICVF_DEFAULT_RX_FREE_THRESH    224
 #define NICVF_DEFAULT_TX_FREE_THRESH    224
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 57feb3773..a1bdf3482 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -58,7 +58,8 @@
 	 DEV_RX_OFFLOAD_UDP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_CKSUM |	\
 	 DEV_RX_OFFLOAD_TCP_LRO |	\
-	 DEV_RX_OFFLOAD_JUMBO_FRAME)
+	 DEV_RX_OFFLOAD_JUMBO_FRAME |   \
+	 DEV_RX_OFFLOAD_RSS_HASH)
 
 static int eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev);
-- 
2.22.0

Re: [dpdk-dev] [PATCH 4/7] net: update Rx RSS hash offload capabilities

[Andrew Rybchenko]

Prefix "net: " is typically used for lib/librte_net. It should be 
"drivers/net: " here.

On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Add DEV_RX_OFFLOAD_RSS_HASH flag for all supported NICs.

"all supported NICs" sounds wrong here.  I'd say
"all PMDs which support RSS hash delivery" or something like this.

> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---

[snip]

> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
> index e6809bb64..695580b22 100644
> --- a/drivers/net/sfc/sfc_rx.c
> +++ b/drivers/net/sfc/sfc_rx.c
> @@ -618,7 +618,8 @@ struct sfc_dp_rx sfc_efx_rx = {
>   	},
>   	.features		= SFC_DP_RX_FEAT_INTR,
>   	.dev_offload_capa	= DEV_RX_OFFLOAD_CHECKSUM,
> -	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER,
> +	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER |
> +				  DEV_RX_OFFLOAD_RSS_HASH,
>   	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
>   	.qcreate		= sfc_efx_rx_qcreate,
>   	.qdestroy		= sfc_efx_rx_qdestroy,
>

Similar changes are required in sfc_ef10_essb_rx
(drivers/net/sfc/sfc_ef10_essb_rx.c) and sfc_ef10_rx
(drivers/net/sfc/sfc_ef10_rx.c).

Re: [dpdk-dev] [PATCH 4/7] net: update Rx RSS hash offload capabilities

[Pavan Nikhilesh Bhagavatula]

>Subject: Re: [dpdk-dev] [PATCH 4/7] net: update Rx RSS hash offload
>capabilities
>
>Prefix "net: " is typically used for lib/librte_net. It should be
>"drivers/net: " here.

Ack will fix in v2.

>
>On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Add DEV_RX_OFFLOAD_RSS_HASH flag for all supported NICs.
>
>"all supported NICs" sounds wrong here.  I'd say
>"all PMDs which support RSS hash delivery" or something like this.
>

Will reword in v2.

>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> ---
>
>[snip]
>
>> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
>> index e6809bb64..695580b22 100644
>> --- a/drivers/net/sfc/sfc_rx.c
>> +++ b/drivers/net/sfc/sfc_rx.c
>> @@ -618,7 +618,8 @@ struct sfc_dp_rx sfc_efx_rx = {
>>   	},
>>   	.features		= SFC_DP_RX_FEAT_INTR,
>>   	.dev_offload_capa	= DEV_RX_OFFLOAD_CHECKSUM,
>> -	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER,
>> +	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER |
>> +				  DEV_RX_OFFLOAD_RSS_HASH,
>>   	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
>>   	.qcreate		= sfc_efx_rx_qcreate,
>>   	.qdestroy		= sfc_efx_rx_qdestroy,
>>
>
>Similar changes are required in sfc_ef10_essb_rx
>(drivers/net/sfc/sfc_ef10_essb_rx.c) and sfc_ef10_rx
>(drivers/net/sfc/sfc_ef10_rx.c).

Thanks for the heads up will update these files in v2. 

Pavan

[dpdk-dev] [PATCH 5/7] net: update Rx flow action offload capabilities

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add DEV_RX_OFFLOAD_FLOW_MARK flag for all supported NICs.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 drivers/net/bnxt/bnxt_ethdev.c          | 3 ++-
 drivers/net/enic/enic_res.c             | 3 ++-
 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/mlx5/mlx5_rxq.c             | 3 ++-
 drivers/net/octeontx2/otx2_ethdev.h     | 3 ++-
 drivers/net/octeontx2/otx2_flow_parse.c | 3 ++-
 drivers/net/sfc/sfc_rx.c                | 3 ++-
 10 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 6c106baf7..fd1fb7eda 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -161,7 +161,8 @@ static const struct rte_pci_id bnxt_pci_id_map[] = {
 				     DEV_RX_OFFLOAD_JUMBO_FRAME | \
 				     DEV_RX_OFFLOAD_KEEP_CRC | \
 				     DEV_RX_OFFLOAD_TCP_LRO | \
-				     DEV_RX_OFFLOAD_RSS_HASH)
+				     DEV_RX_OFFLOAD_RSS_HASH | \
+				     DEV_RX_OFFLOAD_FLOW_MARK)

 static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
 static void bnxt_print_link_info(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 607a085f8..3503d5d7e 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -199,7 +199,8 @@ int enic_get_vnic_config(struct enic *enic)
 		DEV_RX_OFFLOAD_IPV4_CKSUM |
 		DEV_RX_OFFLOAD_UDP_CKSUM |
 		DEV_RX_OFFLOAD_TCP_CKSUM |
-		DEV_RX_OFFLOAD_RSS_HASH;
+		DEV_RX_OFFLOAD_RSS_HASH |
+		DEV_RX_OFFLOAD_FLOW_MARK;
 	enic->tx_offload_mask =
 		PKT_TX_IPV6 |
 		PKT_TX_IPV4 |
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7058e0213..6311943be 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3512,7 +3512,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_VLAN_EXTEND |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
-		DEV_RX_OFFLOAD_RSS_HASH;
+		DEV_RX_OFFLOAD_RSS_HASH |
+		DEV_RX_OFFLOAD_FLOW_MARK;

 	dev_info->tx_queue_offload_capa = DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 	dev_info->tx_offload_capa =
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index aef91a79b..7bdaa87b1 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -518,7 +518,8 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_RX_OFFLOAD_SCATTER |
 		DEV_RX_OFFLOAD_JUMBO_FRAME |
 		DEV_RX_OFFLOAD_VLAN_FILTER |
-		DEV_RX_OFFLOAD_RSS_HASH;
+		DEV_RX_OFFLOAD_RSS_HASH |
+		DEV_RX_OFFLOAD_FLOW_MARK;
 	dev_info->tx_offload_capa =
 		DEV_TX_OFFLOAD_VLAN_INSERT |
 		DEV_TX_OFFLOAD_QINQ_INSERT |
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index fc0f0003f..8b8d55e4a 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2134,7 +2134,8 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 			DEV_RX_OFFLOAD_QINQ_STRIP |
 			DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
 			DEV_RX_OFFLOAD_VLAN_EXTEND |
-			DEV_RX_OFFLOAD_RSS_HASH;
+			DEV_RX_OFFLOAD_RSS_HASH |
+			DEV_RX_OFFLOAD_FLOW_MARK;
 		dev_info->tx_offload_capa |=
 			DEV_TX_OFFLOAD_QINQ_INSERT |
 			DEV_TX_OFFLOAD_IPV4_CKSUM |
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index fa572d184..1481e2426 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2873,7 +2873,8 @@ ixgbe_get_rx_port_offloads(struct rte_eth_dev *dev)
 		   DEV_RX_OFFLOAD_JUMBO_FRAME |
 		   DEV_RX_OFFLOAD_VLAN_FILTER |
 		   DEV_RX_OFFLOAD_SCATTER |
-		   DEV_RX_OFFLOAD_RSS_HASH;
+		   DEV_RX_OFFLOAD_RSS_HASH |
+		   DEV_RX_OFFLOAD_FLOW_MARK;

 	if (hw->mac.type == ixgbe_mac_82598EB)
 		offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index b5fd57693..1bf01bda3 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -369,7 +369,8 @@ mlx5_get_rx_queue_offloads(struct rte_eth_dev *dev)
 	uint64_t offloads = (DEV_RX_OFFLOAD_SCATTER |
 			     DEV_RX_OFFLOAD_TIMESTAMP |
 			     DEV_RX_OFFLOAD_JUMBO_FRAME |
-			     DEV_RX_OFFLOAD_RSS_HASH);
+			     DEV_RX_OFFLOAD_RSS_HASH |
+			     DEV_RX_OFFLOAD_FLOW_MARK);

 	if (config->hw_fcs_strip)
 		offloads |= DEV_RX_OFFLOAD_KEEP_CRC;
diff --git a/drivers/net/octeontx2/otx2_ethdev.h b/drivers/net/octeontx2/otx2_ethdev.h
index 0cca6746d..97c1a636a 100644
--- a/drivers/net/octeontx2/otx2_ethdev.h
+++ b/drivers/net/octeontx2/otx2_ethdev.h
@@ -141,7 +141,8 @@
 	DEV_RX_OFFLOAD_VLAN_FILTER	| \
 	DEV_RX_OFFLOAD_QINQ_STRIP	| \
 	DEV_RX_OFFLOAD_TIMESTAMP	| \
-	DEV_RX_OFFLOAD_RSS_HASH)
+	DEV_RX_OFFLOAD_RSS_HASH		| \
+	DEV_RX_OFFLOAD_FLOW_MARK)

 #define NIX_DEFAULT_RSS_CTX_GROUP  0
 #define NIX_DEFAULT_RSS_MCAM_IDX  -1
diff --git a/drivers/net/octeontx2/otx2_flow_parse.c b/drivers/net/octeontx2/otx2_flow_parse.c
index 6670c1a70..bf0a160be 100644
--- a/drivers/net/octeontx2/otx2_flow_parse.c
+++ b/drivers/net/octeontx2/otx2_flow_parse.c
@@ -979,7 +979,8 @@ otx2_flow_parse_actions(struct rte_eth_dev *dev,
 	if (mark)
 		flow->npc_action |= (uint64_t)mark << 40;

-	if (rte_atomic32_read(&npc->mark_actions) == 1) {
+	if (rte_atomic32_read(&npc->mark_actions) == 1 &&
+			(hw->rx_offloads & DEV_RX_OFFLOAD_FLOW_MARK)) {
 		hw->rx_offload_flags |=
 			NIX_RX_OFFLOAD_MARK_UPDATE_F;
 		otx2_eth_set_rx_function(dev);
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index 695580b22..0f842e9e9 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -619,7 +619,8 @@ struct sfc_dp_rx sfc_efx_rx = {
 	.features		= SFC_DP_RX_FEAT_INTR,
 	.dev_offload_capa	= DEV_RX_OFFLOAD_CHECKSUM,
 	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER |
-				  DEV_RX_OFFLOAD_RSS_HASH,
+				  DEV_RX_OFFLOAD_RSS_HASH |
+				  DEV_RX_OFFLOAD_FLOW_MARK,
 	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
 	.qcreate		= sfc_efx_rx_qcreate,
 	.qdestroy		= sfc_efx_rx_qdestroy,
--
2.22.0

Re: [dpdk-dev] [PATCH 5/7] net: update Rx flow action offload capabilities

[Andrew Rybchenko]

Prefix "net: " is typically used for lib/librte_net. It should be 
"drivers/net: " here.

"Rx flow action offload" sounds strange. May be "Rx flow mark offload".

On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Add DEV_RX_OFFLOAD_FLOW_MARK flag for all supported NICs.

"all supported NICs" sounds wrong here as well.
It sounds like all NICs supported by DPDK.

> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

[snip]

> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
> index 695580b22..0f842e9e9 100644
> --- a/drivers/net/sfc/sfc_rx.c
> +++ b/drivers/net/sfc/sfc_rx.c
> @@ -619,7 +619,8 @@ struct sfc_dp_rx sfc_efx_rx = {
>   	.features		= SFC_DP_RX_FEAT_INTR,
>   	.dev_offload_capa	= DEV_RX_OFFLOAD_CHECKSUM,
>   	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER |
> -				  DEV_RX_OFFLOAD_RSS_HASH,
> +				  DEV_RX_OFFLOAD_RSS_HASH |
> +				  DEV_RX_OFFLOAD_FLOW_MARK,
>   	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
>   	.qcreate		= sfc_efx_rx_qcreate,
>   	.qdestroy		= sfc_efx_rx_qdestroy,

In the case of net/sfc it is supported by sfc_ef10_essb_rx only
(drivers/net/sfc/sfc_ef10_essb_rx.c).

[dpdk-dev] [PATCH 6/7] net: add ptype set default functionality

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add ptype set default functionality i.e. enable ptype by default and
report ptypes even if application disables ptypes.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 drivers/net/atlantic/atl_ethdev.c      | 12 ++++++++++++
 drivers/net/bnxt/bnxt_ethdev.c         | 10 ++++++++++
 drivers/net/cxgbe/cxgbe_ethdev.c       | 10 ++++++++++
 drivers/net/dpaa/dpaa_ethdev.c         | 10 ++++++++++
 drivers/net/dpaa2/dpaa2_ethdev.c       | 10 ++++++++++
 drivers/net/e1000/igb_ethdev.c         | 13 +++++++++++++
 drivers/net/enetc/enetc_ethdev.c       | 10 ++++++++++
 drivers/net/enic/enic_ethdev.c         | 10 ++++++++++
 drivers/net/failsafe/failsafe_ops.c    | 10 ++++++++++
 drivers/net/fm10k/fm10k_ethdev.c       | 10 ++++++++++
 drivers/net/i40e/i40e_ethdev.c         |  1 +
 drivers/net/i40e/i40e_ethdev_vf.c      |  1 +
 drivers/net/i40e/i40e_rxtx.c           |  9 +++++++++
 drivers/net/i40e/i40e_rxtx.h           |  1 +
 drivers/net/iavf/iavf_ethdev.c         | 12 ++++++++++++
 drivers/net/ice/ice_ethdev.c           |  1 +
 drivers/net/ice/ice_rxtx.c             |  9 +++++++++
 drivers/net/ice/ice_rxtx.h             |  1 +
 drivers/net/ixgbe/ixgbe_ethdev.c       | 13 +++++++++++++
 drivers/net/mlx4/mlx4.c                |  1 +
 drivers/net/mlx4/mlx4.h                |  1 +
 drivers/net/mlx4/mlx4_ethdev.c         | 21 +++++++++++++++++++++
 drivers/net/mlx5/mlx5.c                |  2 ++
 drivers/net/mlx5/mlx5.h                |  1 +
 drivers/net/mlx5/mlx5_ethdev.c         | 21 +++++++++++++++++++++
 drivers/net/mvneta/mvneta_ethdev.c     | 22 ++++++++++++++++++++++
 drivers/net/mvpp2/mrvl_ethdev.c        | 22 ++++++++++++++++++++++
 drivers/net/netvsc/hn_ethdev.c         |  3 ++-
 drivers/net/netvsc/hn_var.h            |  3 ++-
 drivers/net/netvsc/hn_vf.c             | 10 +++++++++-
 drivers/net/nfp/nfp_net.c              | 10 ++++++++++
 drivers/net/octeontx/octeontx_ethdev.c | 10 ++++++++++
 drivers/net/octeontx2/otx2_ethdev.c    |  1 +
 drivers/net/octeontx2/otx2_ethdev.h    |  1 +
 drivers/net/octeontx2/otx2_lookup.c    |  9 +++++++++
 drivers/net/qede/qede_ethdev.c         | 11 +++++++++++
 drivers/net/sfc/sfc_ethdev.c           | 11 +++++++++++
 drivers/net/tap/rte_eth_tap.c          | 10 ++++++++++
 drivers/net/thunderx/nicvf_ethdev.c    | 10 ++++++++++
 drivers/net/vmxnet3/vmxnet3_ethdev.c   | 12 ++++++++++++
 40 files changed, 342 insertions(+), 3 deletions(-)

diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index 3c1b349df..f9b630e17 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -49,6 +49,8 @@ static void atl_dev_info_get(struct rte_eth_dev *dev,
 			       struct rte_eth_dev_info *dev_info);
 
 static const uint32_t *atl_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+static int atl_dev_supported_ptypes_set(struct rte_eth_dev *dev,
+					uint32_t ptype_mask);
 
 static int atl_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
 
@@ -296,6 +298,7 @@ static const struct eth_dev_ops atl_eth_dev_ops = {
 	.fw_version_get       = atl_fw_version_get,
 	.dev_infos_get	      = atl_dev_info_get,
 	.dev_supported_ptypes_get = atl_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = atl_dev_supported_ptypes_set,
 
 	.mtu_set              = atl_dev_mtu_set,
 
@@ -1142,6 +1145,15 @@ atl_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+atl_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static void
 atl_dev_delayed_handler(void *param)
 {
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index fd1fb7eda..98e851d70 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2865,6 +2865,15 @@ bnxt_dev_supported_ptypes_get_op(struct rte_eth_dev *dev)
 	return ptypes;
 }
 
+static int
+bnxt_dev_supported_ptypes_set_op(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int bnxt_map_regs(struct bnxt *bp, uint32_t *reg_arr, int count,
 			 int reg_win)
 {
@@ -3316,6 +3325,7 @@ static const struct eth_dev_ops bnxt_dev_ops = {
 	.tx_queue_stop = bnxt_tx_queue_stop,
 	.filter_ctrl = bnxt_filter_ctrl_op,
 	.dev_supported_ptypes_get = bnxt_dev_supported_ptypes_get_op,
+	.dev_supported_ptypes_set = bnxt_dev_supported_ptypes_set_op,
 	.get_eeprom_length    = bnxt_get_eeprom_length_op,
 	.get_eeprom           = bnxt_get_eeprom_op,
 	.set_eeprom           = bnxt_set_eeprom_op,
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 3a373ec1d..f9b148fbc 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -804,6 +804,15 @@ cxgbe_dev_supported_ptypes_get(struct rte_eth_dev *eth_dev)
 	return NULL;
 }
 
+static int
+cxgbe_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /* Update RSS hash configuration
  */
 static int cxgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
@@ -1081,6 +1090,7 @@ static const struct eth_dev_ops cxgbe_eth_dev_ops = {
 	.dev_configure		= cxgbe_dev_configure,
 	.dev_infos_get		= cxgbe_dev_info_get,
 	.dev_supported_ptypes_get = cxgbe_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = cxgbe_dev_supported_ptypes_set,
 	.link_update		= cxgbe_dev_link_update,
 	.dev_set_link_up        = cxgbe_dev_set_link_up,
 	.dev_set_link_down      = cxgbe_dev_set_link_down,
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 18c7bd0d5..c35279f20 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -268,6 +268,15 @@ dpaa_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+dpaa_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int dpaa_eth_dev_start(struct rte_eth_dev *dev)
 {
 	struct dpaa_if *dpaa_intf = dev->data->dev_private;
@@ -986,6 +995,7 @@ static struct eth_dev_ops dpaa_devops = {
 	.dev_close		  = dpaa_eth_dev_close,
 	.dev_infos_get		  = dpaa_eth_dev_info,
 	.dev_supported_ptypes_get = dpaa_supported_ptypes_get,
+	.dev_supported_ptypes_set = dpaa_supported_ptypes_set,
 
 	.rx_queue_setup		  = dpaa_eth_rx_queue_setup,
 	.tx_queue_setup		  = dpaa_eth_tx_queue_setup,
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 55a1c4455..9bcddc87a 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -751,6 +751,15 @@ dpaa2_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+dpaa2_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /**
  * Dpaa2 link Interrupt handler
  *
@@ -1968,6 +1977,7 @@ static struct eth_dev_ops dpaa2_ethdev_ops = {
 	.fw_version_get	   = dpaa2_fw_version_get,
 	.dev_infos_get	   = dpaa2_dev_info_get,
 	.dev_supported_ptypes_get = dpaa2_supported_ptypes_get,
+	.dev_supported_ptypes_set = dpaa2_supported_ptypes_set,
 	.mtu_set           = dpaa2_dev_mtu_set,
 	.vlan_filter_set      = dpaa2_vlan_filter_set,
 	.vlan_offload_set     = dpaa2_vlan_offload_set,
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index fec2b4289..5505d5649 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -105,6 +105,8 @@ static int eth_igb_fw_version_get(struct rte_eth_dev *dev,
 static void eth_igb_infos_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static const uint32_t *eth_igb_supported_ptypes_get(struct rte_eth_dev *dev);
+static int eth_igb_supported_ptypes_set(struct rte_eth_dev *dev,
+					uint32_t ptype_mask);
 static void eth_igbvf_infos_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static int  eth_igb_flow_ctrl_get(struct rte_eth_dev *dev,
@@ -372,6 +374,7 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.fw_version_get       = eth_igb_fw_version_get,
 	.dev_infos_get        = eth_igb_infos_get,
 	.dev_supported_ptypes_get = eth_igb_supported_ptypes_get,
+	.dev_supported_ptypes_set = eth_igb_supported_ptypes_set,
 	.mtu_set              = eth_igb_mtu_set,
 	.vlan_filter_set      = eth_igb_vlan_filter_set,
 	.vlan_tpid_set        = eth_igb_vlan_tpid_set,
@@ -439,6 +442,7 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = {
 	.vlan_filter_set      = igbvf_vlan_filter_set,
 	.dev_infos_get        = eth_igbvf_infos_get,
 	.dev_supported_ptypes_get = eth_igb_supported_ptypes_get,
+	.dev_supported_ptypes_set = eth_igb_supported_ptypes_set,
 	.rx_queue_setup       = eth_igb_rx_queue_setup,
 	.rx_queue_release     = eth_igb_rx_queue_release,
 	.rx_descriptor_done   = eth_igb_rx_descriptor_done,
@@ -2320,6 +2324,15 @@ eth_igb_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+eth_igb_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static void
 eth_igbvf_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
diff --git a/drivers/net/enetc/enetc_ethdev.c b/drivers/net/enetc/enetc_ethdev.c
index 6c5501acb..5530fac78 100644
--- a/drivers/net/enetc/enetc_ethdev.c
+++ b/drivers/net/enetc/enetc_ethdev.c
@@ -80,6 +80,15 @@ enetc_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+enetc_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /* return 0 means link status changed, -1 means not changed */
 static int
 enetc_link_update(struct rte_eth_dev *dev, int wait_to_complete __rte_unused)
@@ -795,6 +804,7 @@ static const struct eth_dev_ops enetc_ops = {
 	.tx_queue_stop        = enetc_tx_queue_stop,
 	.tx_queue_release     = enetc_tx_queue_release,
 	.dev_supported_ptypes_get = enetc_supported_ptypes_get,
+	.dev_supported_ptypes_set = enetc_supported_ptypes_set,
 };
 
 /**
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 06dc67122..b1a04cc02 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -601,6 +601,15 @@ static const uint32_t *enicpmd_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int enicpmd_dev_supported_ptypes_set(struct rte_eth_dev *dev,
+					    uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static void enicpmd_dev_promiscuous_enable(struct rte_eth_dev *eth_dev)
 {
 	struct enic *enic = pmd_priv(eth_dev);
@@ -1064,6 +1073,7 @@ static const struct eth_dev_ops enicpmd_eth_dev_ops = {
 	.queue_stats_mapping_set = NULL,
 	.dev_infos_get        = enicpmd_dev_info_get,
 	.dev_supported_ptypes_get = enicpmd_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = enicpmd_dev_supported_ptypes_set,
 	.mtu_set              = enicpmd_mtu_set,
 	.vlan_filter_set      = NULL,
 	.vlan_tpid_set        = NULL,
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 96e05d4dc..18daf8d6c 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -971,6 +971,15 @@ fs_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return ret;
 }
 
+static int
+fs_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 fs_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 {
@@ -1235,6 +1244,7 @@ const struct eth_dev_ops failsafe_ops = {
 	.stats_reset = fs_stats_reset,
 	.dev_infos_get = fs_dev_infos_get,
 	.dev_supported_ptypes_get = fs_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = fs_dev_supported_ptypes_set,
 	.mtu_set = fs_mtu_set,
 	.vlan_filter_set = fs_vlan_filter_set,
 	.rx_queue_start = fs_rx_queue_start,
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index ba9b174cf..755afa227 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1489,6 +1489,15 @@ fm10k_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 }
 #endif
 
+static int
+fm10k_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {
@@ -2817,6 +2826,7 @@ static const struct eth_dev_ops fm10k_eth_dev_ops = {
 	.link_update		= fm10k_link_update,
 	.dev_infos_get		= fm10k_dev_infos_get,
 	.dev_supported_ptypes_get = fm10k_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = fm10k_dev_supported_ptypes_set,
 	.vlan_filter_set	= fm10k_vlan_filter_set,
 	.vlan_offload_set	= fm10k_vlan_offload_set,
 	.mac_addr_add		= fm10k_macaddr_add,
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 6311943be..6f0659776 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -456,6 +456,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.fw_version_get               = i40e_fw_version_get,
 	.dev_infos_get                = i40e_dev_info_get,
 	.dev_supported_ptypes_get     = i40e_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set     = i40e_dev_supported_ptypes_set,
 	.vlan_filter_set              = i40e_vlan_filter_set,
 	.vlan_tpid_set                = i40e_vlan_tpid_set,
 	.vlan_offload_set             = i40e_vlan_offload_set,
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 308fb9835..d48f90b0b 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -188,6 +188,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
 	.dev_reset	      = i40evf_dev_reset,
 	.dev_infos_get        = i40evf_dev_info_get,
 	.dev_supported_ptypes_get = i40e_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = i40e_dev_supported_ptypes_set,
 	.vlan_filter_set      = i40evf_vlan_filter_set,
 	.vlan_offload_set     = i40evf_vlan_offload_set,
 	.rx_queue_start       = i40evf_dev_rx_queue_start,
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 692c3bab4..a94e556d8 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1687,6 +1687,15 @@ i40e_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+int
+i40e_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 i40e_dev_first_queue(uint16_t idx, void **queues, int num)
 {
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index 3fc619af9..2e1adda4c 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -171,6 +171,7 @@ int i40e_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int i40e_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
 int i40e_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
 const uint32_t *i40e_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+int i40e_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 int i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 			    uint16_t queue_idx,
 			    uint16_t nb_desc,
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 7bdaa87b1..291b41c1a 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -40,6 +40,8 @@ static void iavf_dev_close(struct rte_eth_dev *dev);
 static void iavf_dev_info_get(struct rte_eth_dev *dev,
 			     struct rte_eth_dev_info *dev_info);
 static const uint32_t *iavf_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+static int iavf_dev_supported_ptypes_set(struct rte_eth_dev *dev,
+					uint32_t ptype_mask);
 static int iavf_dev_stats_get(struct rte_eth_dev *dev,
 			     struct rte_eth_stats *stats);
 static void iavf_dev_stats_reset(struct rte_eth_dev *dev);
@@ -88,6 +90,7 @@ static const struct eth_dev_ops iavf_eth_dev_ops = {
 	.dev_close                  = iavf_dev_close,
 	.dev_infos_get              = iavf_dev_info_get,
 	.dev_supported_ptypes_get   = iavf_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set   = iavf_dev_supported_ptypes_set,
 	.link_update                = iavf_dev_link_update,
 	.stats_get                  = iavf_dev_stats_get,
 	.stats_reset                = iavf_dev_stats_reset,
@@ -577,6 +580,15 @@ iavf_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+iavf_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 int
 iavf_dev_link_update(struct rte_eth_dev *dev,
 		    __rte_unused int wait_to_complete)
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 8b8d55e4a..55186bb4c 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -125,6 +125,7 @@ static const struct eth_dev_ops ice_eth_dev_ops = {
 	.tx_queue_release             = ice_tx_queue_release,
 	.dev_infos_get                = ice_dev_info_get,
 	.dev_supported_ptypes_get     = ice_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set     = ice_dev_supported_ptypes_set,
 	.link_update                  = ice_link_update,
 	.mtu_set                      = ice_mtu_set,
 	.mac_addr_set                 = ice_macaddr_set,
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 0282b5375..e018ae583 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -1535,6 +1535,15 @@ ice_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+int
+ice_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 int
 ice_rx_descriptor_status(void *rx_queue, uint16_t offset)
 {
diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h
index e9214110c..0a0deea2d 100644
--- a/drivers/net/ice/ice_rxtx.h
+++ b/drivers/net/ice/ice_rxtx.h
@@ -170,6 +170,7 @@ int ice_rx_descriptor_status(void *rx_queue, uint16_t offset);
 int ice_tx_descriptor_status(void *tx_queue, uint16_t offset);
 void ice_set_default_ptype_table(struct rte_eth_dev *dev);
 const uint32_t *ice_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+int ice_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 
 int ice_rx_vec_dev_check(struct rte_eth_dev *dev);
 int ice_tx_vec_dev_check(struct rte_eth_dev *dev);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 03fc1f717..507796c2a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -185,6 +185,8 @@ static int ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
 static void ixgbe_dev_info_get(struct rte_eth_dev *dev,
 			       struct rte_eth_dev_info *dev_info);
 static const uint32_t *ixgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+static int ixgbe_dev_supported_ptypes_set(struct rte_eth_dev *dev,
+					  uint32_t ptype_mask);
 static void ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 				 struct rte_eth_dev_info *dev_info);
 static int ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
@@ -532,6 +534,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.fw_version_get       = ixgbe_fw_version_get,
 	.dev_infos_get        = ixgbe_dev_info_get,
 	.dev_supported_ptypes_get = ixgbe_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = ixgbe_dev_supported_ptypes_set,
 	.mtu_set              = ixgbe_dev_mtu_set,
 	.vlan_filter_set      = ixgbe_vlan_filter_set,
 	.vlan_tpid_set        = ixgbe_vlan_tpid_set,
@@ -615,6 +618,7 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
 	.allmulticast_disable = ixgbevf_dev_allmulticast_disable,
 	.dev_infos_get        = ixgbevf_dev_info_get,
 	.dev_supported_ptypes_get = ixgbe_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = ixgbe_dev_supported_ptypes_set,
 	.mtu_set              = ixgbevf_dev_set_mtu,
 	.vlan_filter_set      = ixgbevf_vlan_filter_set,
 	.vlan_strip_queue_set = ixgbevf_vlan_strip_queue_set,
@@ -3902,6 +3906,15 @@ ixgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+ixgbe_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static void
 ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 		     struct rte_eth_dev_info *dev_info)
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 931e4f4fe..40c67a56b 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -419,6 +419,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
 	.fw_version_get = mlx4_fw_version_get,
 	.dev_infos_get = mlx4_dev_infos_get,
 	.dev_supported_ptypes_get = mlx4_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = mlx4_dev_supported_ptypes_set,
 	.vlan_filter_set = mlx4_vlan_filter_set,
 	.rx_queue_setup = mlx4_rx_queue_setup,
 	.tx_queue_setup = mlx4_tx_queue_setup,
diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
index 982357716..750bf3fdf 100644
--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h
@@ -227,6 +227,7 @@ int mlx4_flow_ctrl_get(struct rte_eth_dev *dev,
 int mlx4_flow_ctrl_set(struct rte_eth_dev *dev,
 		       struct rte_eth_fc_conf *fc_conf);
 const uint32_t *mlx4_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+int mlx4_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 int mlx4_is_removed(struct rte_eth_dev *dev);
 
 /* mlx4_intr.c */
diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index 5d28c0116..bc41013e4 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -944,6 +944,27 @@ mlx4_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+/**
+ * DPDK callback to set the packet types that the application is interested in.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param ptype_mask
+ *   Packet type mask that the application is interested in.
+ *
+ * @return
+ *   0 if packet types requested are successfully set.
+ *   -ENOTSUP if packet types requested are not supported.
+ */
+static int
+mlx4_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /**
  * Check if mlx4 device was removed.
  *
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 962c0d509..7b4c097de 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -937,6 +937,7 @@ const struct eth_dev_ops mlx5_dev_ops = {
 	.dev_infos_get = mlx5_dev_infos_get,
 	.read_clock = mlx5_read_clock,
 	.dev_supported_ptypes_get = mlx5_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = mlx5_dev_supported_ptypes_set,
 	.vlan_filter_set = mlx5_vlan_filter_set,
 	.rx_queue_setup = mlx5_rx_queue_setup,
 	.tx_queue_setup = mlx5_tx_queue_setup,
@@ -998,6 +999,7 @@ const struct eth_dev_ops mlx5_dev_ops_isolate = {
 	.fw_version_get = mlx5_fw_version_get,
 	.dev_infos_get = mlx5_dev_infos_get,
 	.dev_supported_ptypes_get = mlx5_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = mlx5_dev_supported_ptypes_set,
 	.vlan_filter_set = mlx5_vlan_filter_set,
 	.rx_queue_setup = mlx5_rx_queue_setup,
 	.tx_queue_setup = mlx5_tx_queue_setup,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index f559f835d..e9e97acae 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -689,6 +689,7 @@ void mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info);
 int mlx5_read_clock(struct rte_eth_dev *dev, uint64_t *clock);
 int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size);
 const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+int mlx5_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 int mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete);
 int mlx5_force_link_status_change(struct rte_eth_dev *dev, int status);
 int mlx5_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 33a32fc2f..766de8e61 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -695,6 +695,27 @@ mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+/**
+ * Set packet types interested in.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param ptype_mask
+ *   Packet type mask that the application is interested in.
+ *
+ * @return
+ *   0 if packet types requested are successfully set.
+ *   -ENOTSUP if packet types requested are not supported.
+ */
+static int
+mlx5_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /**
  * Retrieve the master device for representor in the same switch domain.
  *
diff --git a/drivers/net/mvneta/mvneta_ethdev.c b/drivers/net/mvneta/mvneta_ethdev.c
index a8def8fc4..bcae910c6 100644
--- a/drivers/net/mvneta/mvneta_ethdev.c
+++ b/drivers/net/mvneta/mvneta_ethdev.c
@@ -213,6 +213,27 @@ mvneta_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+/**
+ * Set packet types interested in.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param ptype_mask
+ *   Packet type mask that the application is interested in.
+ *
+ * @return
+ *   0 if packet types requested are successfully set.
+ *   -ENOTSUP if packet types requested are not supported.
+ */
+static int
+mvneta_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /**
  * DPDK callback to change the MTU.
  *
@@ -758,6 +779,7 @@ static const struct eth_dev_ops mvneta_ops = {
 	.stats_reset = mvneta_stats_reset,
 	.dev_infos_get = mvneta_dev_infos_get,
 	.dev_supported_ptypes_get = mvneta_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = mvneta_dev_supported_ptypes_set,
 	.rxq_info_get = mvneta_rxq_info_get,
 	.txq_info_get = mvneta_txq_info_get,
 	.rx_queue_setup = mvneta_rx_queue_setup,
diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c
index 810a703fc..6fc246308 100644
--- a/drivers/net/mvpp2/mrvl_ethdev.c
+++ b/drivers/net/mvpp2/mrvl_ethdev.c
@@ -1488,6 +1488,27 @@ mrvl_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+/**
+ * Set packet types interested in.
+ *
+ * @param dev
+ *   Pointer to Ethernet device structure.
+ * @param ptype_mask
+ *   Packet type mask that the application is interested in.
+ *
+ * @return
+ *   0 if packet types requested are successfully set.
+ *   -ENOTSUP if packet types requested are not supported.
+ */
+static int
+mrvl_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /**
  * DPDK callback to get information about specific receive queue.
  *
@@ -2048,6 +2069,7 @@ static const struct eth_dev_ops mrvl_ops = {
 	.xstats_get_names = mrvl_xstats_get_names,
 	.dev_infos_get = mrvl_dev_infos_get,
 	.dev_supported_ptypes_get = mrvl_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = mrvl_dev_supported_ptypes_set,
 	.rxq_info_get = mrvl_rxq_info_get,
 	.txq_info_get = mrvl_txq_info_get,
 	.vlan_filter_set = mrvl_vlan_filter_set,
diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index 2a90d674e..0c009bf61 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -818,7 +818,8 @@ static const struct eth_dev_ops hn_eth_dev_ops = {
 	.dev_stop		= hn_dev_stop,
 	.dev_close		= hn_dev_close,
 	.dev_infos_get		= hn_dev_info_get,
-	.dev_supported_ptypes_get = hn_vf_supported_ptypes,
+	.dev_supported_ptypes_get = hn_vf_supported_ptypes_get,
+	.dev_supported_ptypes_set = hn_vf_supported_ptypes_set,
 	.promiscuous_enable     = hn_dev_promiscuous_enable,
 	.promiscuous_disable    = hn_dev_promiscuous_disable,
 	.allmulticast_enable    = hn_dev_allmulticast_enable,
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 3f5e8dfa7..57bdef8b2 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -206,7 +206,8 @@ void	hn_vf_info_get(struct hn_data *hv,
 int	hn_vf_add(struct rte_eth_dev *dev, struct hn_data *hv);
 int	hn_vf_configure(struct rte_eth_dev *dev,
 			const struct rte_eth_conf *dev_conf);
-const uint32_t *hn_vf_supported_ptypes(struct rte_eth_dev *dev);
+const uint32_t *hn_vf_supported_ptypes_get(struct rte_eth_dev *dev);
+int hn_vf_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 int	hn_vf_start(struct rte_eth_dev *dev);
 void	hn_vf_reset(struct rte_eth_dev *dev);
 void	hn_vf_stop(struct rte_eth_dev *dev);
diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c
index 03f855b94..d5b06b68f 100644
--- a/drivers/net/netvsc/hn_vf.c
+++ b/drivers/net/netvsc/hn_vf.c
@@ -302,7 +302,7 @@ int hn_vf_configure(struct rte_eth_dev *dev,
 	return ret;
 }
 
-const uint32_t *hn_vf_supported_ptypes(struct rte_eth_dev *dev)
+const uint32_t *hn_vf_supported_ptypes_get(struct rte_eth_dev *dev)
 {
 	struct hn_data *hv = dev->data->dev_private;
 	struct rte_eth_dev *vf_dev;
@@ -317,6 +317,14 @@ const uint32_t *hn_vf_supported_ptypes(struct rte_eth_dev *dev)
 	return ptypes;
 }
 
+int hn_vf_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 int hn_vf_start(struct rte_eth_dev *dev)
 {
 	struct hn_data *hv = dev->data->dev_private;
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 230d64c8a..3a9349ae4 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1295,6 +1295,15 @@ nfp_net_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+nfp_net_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static uint32_t
 nfp_net_rx_queue_count(struct rte_eth_dev *dev, uint16_t queue_idx)
 {
@@ -2705,6 +2714,7 @@ static const struct eth_dev_ops nfp_net_eth_dev_ops = {
 	.stats_reset		= nfp_net_stats_reset,
 	.dev_infos_get		= nfp_net_infos_get,
 	.dev_supported_ptypes_get = nfp_net_supported_ptypes_get,
+	.dev_supported_ptypes_set = nfp_net_supported_ptypes_set,
 	.mtu_set		= nfp_net_dev_mtu_set,
 	.mac_addr_set           = nfp_set_mac_addr,
 	.vlan_offload_set	= nfp_net_vlan_offload_set,
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index 27eed47bb..d3d5e04d0 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -955,6 +955,15 @@ octeontx_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+octeontx_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 octeontx_pool_ops(struct rte_eth_dev *dev, const char *pool)
 {
@@ -986,6 +995,7 @@ static const struct eth_dev_ops octeontx_dev_ops = {
 	.rx_queue_setup		 = octeontx_dev_rx_queue_setup,
 	.rx_queue_release	 = octeontx_dev_rx_queue_release,
 	.dev_supported_ptypes_get = octeontx_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = octeontx_dev_supported_ptypes_set,
 	.pool_ops_supported      = octeontx_pool_ops,
 };
 
diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
index 2e88d1844..d5768df2f 100644
--- a/drivers/net/octeontx2/otx2_ethdev.c
+++ b/drivers/net/octeontx2/otx2_ethdev.c
@@ -1620,6 +1620,7 @@ static const struct eth_dev_ops otx2_eth_dev_ops = {
 	.dev_set_link_up          = otx2_nix_dev_set_link_up,
 	.dev_set_link_down        = otx2_nix_dev_set_link_down,
 	.dev_supported_ptypes_get = otx2_nix_supported_ptypes_get,
+	.dev_supported_ptypes_set = otx2_nix_supported_ptypes_set,
 	.dev_reset                = otx2_nix_dev_reset,
 	.stats_get                = otx2_nix_dev_stats_get,
 	.stats_reset              = otx2_nix_dev_stats_reset,
diff --git a/drivers/net/octeontx2/otx2_ethdev.h b/drivers/net/octeontx2/otx2_ethdev.h
index 97c1a636a..3fc76bfb4 100644
--- a/drivers/net/octeontx2/otx2_ethdev.h
+++ b/drivers/net/octeontx2/otx2_ethdev.h
@@ -502,6 +502,7 @@ void *otx2_nix_fastpath_lookup_mem_get(void);
 
 /* PTYPES */
 const uint32_t *otx2_nix_supported_ptypes_get(struct rte_eth_dev *dev);
+int otx2_nix_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 
 /* Mac address handling */
 int otx2_nix_mac_addr_set(struct rte_eth_dev *eth_dev,
diff --git a/drivers/net/octeontx2/otx2_lookup.c b/drivers/net/octeontx2/otx2_lookup.c
index 99199d08a..0dec1f1c4 100644
--- a/drivers/net/octeontx2/otx2_lookup.c
+++ b/drivers/net/octeontx2/otx2_lookup.c
@@ -61,6 +61,15 @@ otx2_nix_supported_ptypes_get(struct rte_eth_dev *eth_dev)
 		return NULL;
 }
 
+int
+otx2_nix_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 /*
  * +------------------ +------------------ +
  * |  | IL4 | IL3| IL2 | TU | L4 | L3 | L2 |
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index da25b26df..7b3119e00 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1945,6 +1945,15 @@ qede_dev_supported_ptypes_get(struct rte_eth_dev *eth_dev)
 	return NULL;
 }
 
+static int
+qede_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static void qede_init_rss_caps(uint8_t *rss_caps, uint64_t hf)
 {
 	*rss_caps = 0;
@@ -2338,6 +2347,7 @@ static const struct eth_dev_ops qede_eth_dev_ops = {
 	.flow_ctrl_set = qede_flow_ctrl_set,
 	.flow_ctrl_get = qede_flow_ctrl_get,
 	.dev_supported_ptypes_get = qede_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = qede_dev_supported_ptypes_set,
 	.rss_hash_update = qede_rss_hash_update,
 	.rss_hash_conf_get = qede_rss_hash_conf_get,
 	.reta_update  = qede_rss_reta_update,
@@ -2376,6 +2386,7 @@ static const struct eth_dev_ops qede_eth_vf_dev_ops = {
 	.vlan_offload_set = qede_vlan_offload_set,
 	.vlan_filter_set = qede_vlan_filter_set,
 	.dev_supported_ptypes_get = qede_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = qede_dev_supported_ptypes_set,
 	.rss_hash_update = qede_rss_hash_update,
 	.rss_hash_conf_get = qede_rss_hash_conf_get,
 	.reta_update  = qede_rss_reta_update,
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 1f78a3d8a..2e562b070 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -194,6 +194,15 @@ sfc_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return sap->dp_rx->supported_ptypes_get(sap->shared->tunnel_encaps);
 }
 
+static int
+sfc_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 sfc_dev_configure(struct rte_eth_dev *dev)
 {
@@ -1782,6 +1791,7 @@ static const struct eth_dev_ops sfc_eth_dev_ops = {
 	.xstats_get_names		= sfc_xstats_get_names,
 	.dev_infos_get			= sfc_dev_infos_get,
 	.dev_supported_ptypes_get	= sfc_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set       = sfc_dev_supported_ptypes_set,
 	.mtu_set			= sfc_dev_set_mtu,
 	.rx_queue_start			= sfc_rx_queue_start,
 	.rx_queue_stop			= sfc_rx_queue_stop,
@@ -1987,6 +1997,7 @@ sfc_eth_dev_clear_ops(struct rte_eth_dev *dev)
 
 static const struct eth_dev_ops sfc_eth_dev_secondary_ops = {
 	.dev_supported_ptypes_get	= sfc_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set       = sfc_dev_supported_ptypes_set,
 	.rx_queue_count			= sfc_rx_queue_count,
 	.rx_descriptor_done		= sfc_rx_descriptor_done,
 	.rx_descriptor_status		= sfc_rx_descriptor_status,
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 64bd04911..c374bb1cf 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -1561,6 +1561,15 @@ tap_dev_supported_ptypes_get(struct rte_eth_dev *dev __rte_unused)
 	return ptypes;
 }
 
+static int
+tap_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 tap_flow_ctrl_get(struct rte_eth_dev *dev __rte_unused,
 		  struct rte_eth_fc_conf *fc_conf)
@@ -1671,6 +1680,7 @@ static const struct eth_dev_ops ops = {
 	.stats_get              = tap_stats_get,
 	.stats_reset            = tap_stats_reset,
 	.dev_supported_ptypes_get = tap_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = tap_dev_supported_ptypes_set,
 	.rss_hash_update        = tap_rss_hash_update,
 	.filter_ctrl            = tap_dev_filter_ctrl,
 };
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index 56769ef85..9b42ecf9a 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -362,6 +362,15 @@ nicvf_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return ptypes;
 }
 
+static int
+nicvf_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static void
 nicvf_dev_stats_reset(struct rte_eth_dev *dev)
 {
@@ -1987,6 +1996,7 @@ static const struct eth_dev_ops nicvf_eth_dev_ops = {
 	.promiscuous_enable       = nicvf_dev_promisc_enable,
 	.dev_infos_get            = nicvf_dev_info_get,
 	.dev_supported_ptypes_get = nicvf_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = nicvf_dev_supported_ptypes_set,
 	.mtu_set                  = nicvf_dev_set_mtu,
 	.vlan_offload_set         = nicvf_vlan_offload_set,
 	.reta_update              = nicvf_dev_reta_update,
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index a1bdf3482..85bbc75c9 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -89,6 +89,8 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
 				 struct rte_eth_dev_info *dev_info);
 static const uint32_t *
 vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+static int
+vmxnet3_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask);
 static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
 				       uint16_t vid, int on);
 static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
@@ -126,6 +128,7 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.mac_addr_set         = vmxnet3_mac_addr_set,
 	.dev_infos_get        = vmxnet3_dev_info_get,
 	.dev_supported_ptypes_get = vmxnet3_dev_supported_ptypes_get,
+	.dev_supported_ptypes_set = vmxnet3_dev_supported_ptypes_set,
 	.vlan_filter_set      = vmxnet3_dev_vlan_filter_set,
 	.vlan_offload_set     = vmxnet3_dev_vlan_offload_set,
 	.rx_queue_setup       = vmxnet3_dev_rx_queue_setup,
@@ -1205,6 +1208,15 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
+static int
+vmxnet3_dev_supported_ptypes_set(struct rte_eth_dev *dev, uint32_t ptype_mask)
+{
+	RTE_SET_USED(dev);
+	RTE_SET_USED(ptype_mask);
+
+	return 0;
+}
+
 static int
 vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr)
 {
-- 
2.22.0

Re: [dpdk-dev] [PATCH 6/7] net: add ptype set default functionality

[Andrew Rybchenko]

On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Add ptype set default functionality i.e. enable ptype by default and
> report ptypes even if application disables ptypes.
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

May be it is better to have rte_ethdev_supported_ptype_set_dummy()
in ethdev library and use it in PMDs instead of duplicating the function
in all drivers. Or even treat NULL as dummy? It will remove the patch
completely.

Re: [dpdk-dev] [PATCH 6/7] net: add ptype set default functionality

[Pavan Nikhilesh Bhagavatula]

>>
>> Add ptype set default functionality i.e. enable ptype by default and
>> report ptypes even if application disables ptypes.
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
>May be it is better to have
>rte_ethdev_supported_ptype_set_dummy()
>in ethdev library and use it in PMDs instead of duplicating the function
>in all drivers. Or even treat NULL as dummy? It will remove the patch
>completely.

But how would we differentiate drivers that actually support ptypes vs drivers that don’t do ptypes at all?.
Maybe we could return the OR'd result of get_ptypes for now?


To summarize:

If we agree that rte_eth_dev_set_supported_ptypes() will return  successfully set ptype mask then:

1. If PMD doesn’t support get then we can return 0, application will softparse ptypes.
2. If a subset of ptypes requested by the application through `set_supported_ptypes` are not supported then
 it will return subset of ptypes that are supported set and rest will be parsed in software.
3. If we choose to not have [6/7] i.e. if a PMD has a get function but not set function we can match the masks 
 in  `rte_eth_dev_set_supported_ptypes` and return supported ptype mask.
4. If application calls `rte_eth_dev_set_supported_ptypes` with mask as '0' then packet_type might be undefined.


Let me know your thoughts.

Thanks,
Pavan
  

[dpdk-dev] [PATCH 7/7] examples/eventdev_pipeline: add new Rx RSS hash offload

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Since pipeline_generic uses `rte_mbuf::hash::rss` add the new Rx offload
flag `DEV_RX_OFFLOAD_RSS_HASH` to inform PMD to copy the RSS hash result
into the mbuf.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 examples/eventdev_pipeline/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/examples/eventdev_pipeline/main.c b/examples/eventdev_pipeline/main.c
index f4e57f541..f08253cc3 100644
--- a/examples/eventdev_pipeline/main.c
+++ b/examples/eventdev_pipeline/main.c
@@ -278,6 +278,10 @@ port_init(uint8_t port, struct rte_mempool *mbuf_pool)
 	if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
 		port_conf.txmode.offloads |=
 			DEV_TX_OFFLOAD_MBUF_FAST_FREE;
+
+	if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_RSS_HASH)
+		port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_RSS_HASH;
+
 	rx_conf = dev_info.default_rxconf;
 	rx_conf.offloads = port_conf.rxmode.offloads;
 
-- 
2.22.0

Re: [dpdk-dev] [PATCH 7/7] examples/eventdev_pipeline: add new Rx RSS hash offload

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Friday, August 16, 2019 11:25 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; ferruh.yigit@intel.com;
> Harry van Haaren <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: [dpdk-dev] [PATCH 7/7] examples/eventdev_pipeline: add new Rx
> RSS hash offload
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Since pipeline_generic uses `rte_mbuf::hash::rss` add the new Rx offload flag
> `DEV_RX_OFFLOAD_RSS_HASH` to inform PMD to copy the RSS hash result
> into the mbuf.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  examples/eventdev_pipeline/main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/examples/eventdev_pipeline/main.c
> b/examples/eventdev_pipeline/main.c
> index f4e57f541..f08253cc3 100644
> --- a/examples/eventdev_pipeline/main.c
> +++ b/examples/eventdev_pipeline/main.c
> @@ -278,6 +278,10 @@ port_init(uint8_t port, struct rte_mempool
> *mbuf_pool)
>  	if (dev_info.tx_offload_capa &
> DEV_TX_OFFLOAD_MBUF_FAST_FREE)
>  		port_conf.txmode.offloads |=
>  			DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> +
> +	if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_RSS_HASH)
> +		port_conf.rxmode.offloads |=
> DEV_RX_OFFLOAD_RSS_HASH;

We need this only when Rx adapter in SW mode.

> +
>  	rx_conf = dev_info.default_rxconf;
>  	rx_conf.offloads = port_conf.rxmode.offloads;
> 
> --
> 2.22.0

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

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Friday, August 16, 2019 11:25 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; ferruh.yigit@intel.com
> Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: [dpdk-dev] [PATCH 0/7] ethdev: add new Rx offload flags
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new Rx offload flags `DEV_RX_OFFLOAD_RSS_HASH` and
> `DEV_RX_OFFLOAD_FLOW_MARK`. These flags can be used to
> enable/disable PMD writes to rte_mbuf fields `hash.rss` and `hash.fdir.hi`
> 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*, 0);
> 
> 
> Pavan Nikhilesh (7):
>   ethdev: add set ptype function
>   ethdev: add mbuf RSS update as a offload
>   ethdev: add flow action type update as an offload
>   net: update Rx RSS hash offload capabilities
>   net: update Rx flow action offload capabilities
>   net: add ptype set default functionality
>   examples/eventdev_pipeline: add new Rx RSS hash offload

Missing the examples update to disable ptype when not needed.

Re: [dpdk-dev] [PATCH 5/7] net: update Rx flow action offload capabilities

[Pavan Nikhilesh Bhagavatula]

>Prefix "net: " is typically used for lib/librte_net. It should be
>"drivers/net: " here.
>
>"Rx flow action offload" sounds strange. May be "Rx flow mark offload".
>
>On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote:
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Add DEV_RX_OFFLOAD_FLOW_MARK flag for all supported NICs.
>
>"all supported NICs" sounds wrong here as well.
>It sounds like all NICs supported by DPDK.
>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
>[snip]
>
>> diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
>> index 695580b22..0f842e9e9 100644
>> --- a/drivers/net/sfc/sfc_rx.c
>> +++ b/drivers/net/sfc/sfc_rx.c
>> @@ -619,7 +619,8 @@ struct sfc_dp_rx sfc_efx_rx = {
>>   	.features		= SFC_DP_RX_FEAT_INTR,
>>   	.dev_offload_capa	= DEV_RX_OFFLOAD_CHECKSUM,
>>   	.queue_offload_capa	= DEV_RX_OFFLOAD_SCATTER |
>> -				  DEV_RX_OFFLOAD_RSS_HASH,
>> +				  DEV_RX_OFFLOAD_RSS_HASH |
>> +				  DEV_RX_OFFLOAD_FLOW_MARK,
>>   	.qsize_up_rings		= sfc_efx_rx_qsize_up_rings,
>>   	.qcreate		= sfc_efx_rx_qcreate,
>>   	.qdestroy		= sfc_efx_rx_qdestroy,
>
>In the case of net/sfc it is supported by sfc_ef10_essb_rx only
>(drivers/net/sfc/sfc_ef10_essb_rx.c).

Will fix in v2.

Pavan.