[v3,1/2] ethdev: add random item support

Message ID 20231130163215.666239-2-michaelba@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: add random item support |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Michael Baum Nov. 30, 2023, 4:32 p.m. UTC
Add support for a new item type "RTE_FLOW_ITEM_TYPE_RANDOM".
This item enables to match on some random value as a part of flow rule.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/nics/features/default.ini   |  1 +
 doc/guides/prog_guide/rte_flow.rst     | 13 ++++++++++
 doc/guides/rel_notes/release_24_03.rst |  4 +++
 lib/ethdev/rte_flow.c                  |  1 +
 lib/ethdev/rte_flow.h                  | 35 +++++++++++++++++++++++++-
 5 files changed, 53 insertions(+), 1 deletion(-)
  

Comments

Dariusz Sosnowski Dec. 8, 2023, 6:54 p.m. UTC | #1
Hi Michael,

> +Item: ``RANDOM``
> +^^^^^^^^^^^^^^^^
> +
> +Matches a random value.
> +
> +The rundom number is generated by PMD,
s/rundom/random

I'm not sure that mentioning PMD here is fully correct, because in my opinion it implies that SW generates it.
HW, SW and system clock were mentioned as examples of sources of randomness in previous discussions on this API.

Also, I think it's worth adding that "number == unsigned integer with at most 32 bits."
It gives some leeway for any driver implementing this API - value is uint32_t but not all bits must be used.
For example, some HW may support only 16-bit random number generation.
Such HW might implement validation on mask, where mask with more than 16 bits would be rejected.

What do you think about the following proposal based on those comments?
 
"A random unsigned integer (at most 32-bit) is generated for each packet
during flow rule processing, by either HW, SW or some external source.
Application can match on either exact value or range of values."

> +Application shouldn't assume that this value is kept during the life
> +time of the packet.
s/life time/lifetime

Best regards,
Dariusz Sosnowski
  
Dariusz Sosnowski Dec. 8, 2023, 7:03 p.m. UTC | #2
> +       RTE_FLOW_FIELD_RANDOM           /**< Random value. */
I think that this new modify field type should be mentioned in release notes as well.

Best regards,
Dariusz Sosnowski
  
Michael Baum Dec. 14, 2023, 10:32 a.m. UTC | #3
On 12/8/2023 8:54 PM, 0 Dariusz Sosnowski wrote:
> 
> Hi Michael,
> 
> > +Item: ``RANDOM``
> > +^^^^^^^^^^^^^^^^
> > +
> > +Matches a random value.
> > +
> > +The rundom number is generated by PMD,
> s/rundom/random
Ack, thank you.

> 
> I'm not sure that mentioning PMD here is fully correct, because in my opinion it
> implies that SW generates it.
> HW, SW and system clock were mentioned as examples of sources of randomness
> in previous discussions on this API.
> 
> Also, I think it's worth adding that "number == unsigned integer with at most 32
> bits."
> It gives some leeway for any driver implementing this API - value is uint32_t but
> not all bits must be used.
> For example, some HW may support only 16-bit random number generation.
> Such HW might implement validation on mask, where mask with more than 16
> bits would be rejected.
> 
> What do you think about the following proposal based on those comments?
> 
> "A random unsigned integer (at most 32-bit) is generated for each packet during
> flow rule processing, by either HW, SW or some external source.
> Application can match on either exact value or range of values."
I think your suggestion is good, but the words "for each packet" imply that the generated random value is oriented to the packet.
So I'm taking it and keeping the next lines: 
" This value is not based on the packet data/headers.
Application shouldn't assume that this value is kept during the lifetime of the packet."
> 
> > +Application shouldn't assume that this value is kept during the life
> > +time of the packet.
> s/life time/lifetime
Ack, thank you.

> 
> Best regards,
> Dariusz Sosnowski
  
Dariusz Sosnowski Dec. 14, 2023, 10:54 a.m. UTC | #4
> >
> > I'm not sure that mentioning PMD here is fully correct, because in my
> > opinion it implies that SW generates it.
> > HW, SW and system clock were mentioned as examples of sources of
> > randomness in previous discussions on this API.
> >
> > Also, I think it's worth adding that "number == unsigned integer with
> > at most 32 bits."
> > It gives some leeway for any driver implementing this API - value is
> > uint32_t but not all bits must be used.
> > For example, some HW may support only 16-bit random number generation.
> > Such HW might implement validation on mask, where mask with more than
> > 16 bits would be rejected.
> >
> > What do you think about the following proposal based on those comments?
> >
> > "A random unsigned integer (at most 32-bit) is generated for each
> > packet during flow rule processing, by either HW, SW or some external source.
> > Application can match on either exact value or range of values."
> I think your suggestion is good, but the words "for each packet" imply that the
> generated random value is oriented to the packet.
> So I'm taking it and keeping the next lines:
> " This value is not based on the packet data/headers.
> Application shouldn't assume that this value is kept during the lifetime of the
> packet."
Sounds good to me.

Best regards,
Dariusz Sosnowski
  

Patch

diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index 806cb033ff..6d50236292 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -140,6 +140,7 @@  pppoe_proto_id       =
 ptype                =
 quota                =
 raw                  =
+random               =
 represented_port     =
 sctp                 =
 tag                  =
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 627b845bfb..fd7fddb6cd 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1573,6 +1573,19 @@  Matches the packet type as defined in rte_mbuf_ptype.
 
 - ``packet_type``: L2/L3/L4 and tunnel information.
 
+Item: ``RANDOM``
+^^^^^^^^^^^^^^^^
+
+Matches a random value.
+
+The rundom number is generated by PMD,
+application can match on either exact value or range of values.
+This value is not based on the packet data/headers.
+Application shouldn't assume that this value is kept during the life time of
+the packet.
+
+- ``value``: Specific value to match.
+
 Actions
 ~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst
index e9c9717706..ab91ce2b21 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -55,6 +55,10 @@  New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added flow matching of random value.**
+
+  Added ``RTE_FLOW_ITEM_RANDOM`` to match random value.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 549e329558..090b936ca9 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -136,6 +136,7 @@  static const struct rte_flow_desc_data rte_flow_desc_item[] = {
 		     sizeof(struct rte_flow_item_icmp6_nd_opt_tla_eth)),
 	MK_FLOW_ITEM(MARK, sizeof(struct rte_flow_item_mark)),
 	MK_FLOW_ITEM(META, sizeof(struct rte_flow_item_meta)),
+	MK_FLOW_ITEM(RANDOM, sizeof(struct rte_flow_item_random)),
 	MK_FLOW_ITEM(TAG, sizeof(struct rte_flow_item_tag)),
 	MK_FLOW_ITEM(GRE_KEY, sizeof(rte_be32_t)),
 	MK_FLOW_ITEM(GRE_OPTION, sizeof(struct rte_flow_item_gre_opt)),
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index affdc8121b..887401bb86 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -704,6 +704,19 @@  enum rte_flow_item_type {
 	 *
 	 */
 	RTE_FLOW_ITEM_TYPE_PTYPE,
+
+	/**
+	 * [META]
+	 *
+	 * Matches a random value.
+	 *
+	 * This value is not based on the packet data/headers.
+	 * Application shouldn't assume that this value is kept during the life
+	 * time of the packet.
+	 *
+	 * @see struct rte_flow_item_random.
+	 */
+	RTE_FLOW_ITEM_TYPE_RANDOM,
 };
 
 /**
@@ -2047,6 +2060,25 @@  static const struct rte_flow_item_ib_bth rte_flow_item_ib_bth_mask = {
 };
 #endif
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ITEM_TYPE_RANDOM
+ *
+ * Matches a random value.
+ */
+struct rte_flow_item_random {
+	uint32_t value;
+};
+
+/** Default mask for RTE_FLOW_ITEM_TYPE_RANDOM. */
+#ifndef __cplusplus
+static const struct rte_flow_item_random rte_flow_item_random_mask = {
+	.value = UINT32_MAX,
+};
+#endif
+
 /**
  * Matching pattern item definition.
  *
@@ -3903,7 +3935,8 @@  enum rte_flow_field_id {
 	RTE_FLOW_FIELD_TCP_DATA_OFFSET,	/**< TCP data offset. */
 	RTE_FLOW_FIELD_IPV4_IHL,	/**< IPv4 IHL. */
 	RTE_FLOW_FIELD_IPV4_TOTAL_LEN,	/**< IPv4 total length. */
-	RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN	/**< IPv6 payload length. */
+	RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN,/**< IPv6 payload length. */
+	RTE_FLOW_FIELD_RANDOM		/**< Random value. */
 };
 
 /**