[v2,3/7] ethdev: add flow action type update as an offload

Message ID 20190821204755.1990-4-pbhagavatula@marvell.com
State New
Delegated to: Ferruh Yigit
Headers show
Series
  • ethdev: add new Rx offload flags
Related show

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues
ci/checkpatch success coding style OK

Commit Message

Pavan Nikhilesh Bhagavatula Aug. 21, 2019, 8:47 p.m.
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` and
`rte_mbuf::ol_flags` when flow actions `RTE_FLOW_ACTION_MARK` and
`RTE_FLOW_ACTION_FLAG` are enabled.

PMDs notify the validity of `rte_mbuf::hash:fdir::hi` to the applcation
by enabling `PKT_RX_FDIR_ID` flag in `rte_mbuf::ol_flags`.

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

Comments

Andrew Rybchenko Aug. 23, 2019, 10 a.m. | #1
On 8/21/19 11:47 PM, 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` and
> `rte_mbuf::ol_flags` when flow actions `RTE_FLOW_ACTION_MARK` and
> `RTE_FLOW_ACTION_FLAG` are enabled.
>
> PMDs notify the validity of `rte_mbuf::hash:fdir::hi` to the applcation
> by enabling `PKT_RX_FDIR_ID` flag in `rte_mbuf::ol_flags`.
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

LGTM, few minor notes below, otherwise

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

However, it is a long discussion of it in v1 which should be taken into 
account.

<snip>

> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index b66bf1495..5d9d88d76 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -1316,7 +1316,8 @@ enum rte_flow_action_type {
>   
>   	/**
>   	 * Attaches an integer value to packets and sets PKT_RX_FDIR and
> -	 * PKT_RX_FDIR_ID mbuf flags.
> +	 * PKT_RX_FDIR_ID mbuf flags when
> +	 * `rx_mode:offloads:DEV_RX_OFFLOAD_FLOW_MARK` is set.

I'd not concentrate on rx_mode offloads here. I'd say:
when Rx offload DEV_RX_OFFLOAD_FLOW_MARK is enabled.

>   	 *
>   	 * See struct rte_flow_action_mark.
>   	 */
> @@ -1324,7 +1325,8 @@ enum rte_flow_action_type {
>   
>   	/**
>   	 * Flags packets. Similar to MARK without a specific value; only
> -	 * sets the PKT_RX_FDIR mbuf flag.
> +	 * sets the PKT_RX_FDIR mbuf flag when
> +	 * `rx_mode:offloads:DEV_RX_OFFLOAD_FLOW_MARK` is set

Same as above.

>   	 *
>   	 * No associated configuration structure.
>   	 */

Patch

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index f79b69b38..338b19e03 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_flag_mark:
+
+Flow flag/mark 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_MARK``.
+* **[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.c b/lib/librte_ethdev/rte_ethdev.c
index 9c5517d5f..bcbe06c5c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -130,6 +130,7 @@  static const struct {
 	RTE_RX_OFFLOAD_BIT2STR(SCTP_CKSUM),
 	RTE_RX_OFFLOAD_BIT2STR(OUTER_UDP_CKSUM),
 	RTE_RX_OFFLOAD_BIT2STR(RSS_HASH),
+	RTE_RX_OFFLOAD_BIT2STR(FLOW_MARK),
 };
 
 #undef RTE_RX_OFFLOAD_BIT2STR
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 836b30074..44686ec21 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 | \
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index b66bf1495..5d9d88d76 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -1316,7 +1316,8 @@  enum rte_flow_action_type {
 
 	/**
 	 * Attaches an integer value to packets and sets PKT_RX_FDIR and
-	 * PKT_RX_FDIR_ID mbuf flags.
+	 * PKT_RX_FDIR_ID mbuf flags when
+	 * `rx_mode:offloads:DEV_RX_OFFLOAD_FLOW_MARK` is set.
 	 *
 	 * See struct rte_flow_action_mark.
 	 */
@@ -1324,7 +1325,8 @@  enum rte_flow_action_type {
 
 	/**
 	 * Flags packets. Similar to MARK without a specific value; only
-	 * sets the PKT_RX_FDIR mbuf flag.
+	 * sets the PKT_RX_FDIR mbuf flag when
+	 * `rx_mode:offloads:DEV_RX_OFFLOAD_FLOW_MARK` is set
 	 *
 	 * No associated configuration structure.
 	 */