[v4,1/4] ethdev: allow negative values in flow rule types

Message ID 20201004135040.10307-2-getelson@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series Tunnel Offload API |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Etelson, Gregory Oct. 4, 2020, 1:50 p.m. UTC
From: Gregory Etelson <getelson@mellanox.com>

RTE flow items & actions use positive values in item & action type.
Negative values are reserved for PMD private types. PMD
items & actions usually are not exposed to application and are not
used to create RTE flows.

The patch allows applications with access to PMD flow
items & actions ability to integrate RTE and PMD items & actions
and use them to create flow rule.

RTE flow item or action coversion library accepts positive known
element types with predefined sizes only. Private PMD items and
actions do not fit into this scheme becase PMD type values are
negative, each PMD has it's own types numeration and element types and
their sizes are not visible at RTE level.  To resolve these
limitations the patch proposes this solution:
1. PMD can to expose elements of pointer size only.  RTE flow
   conversion functions will use pointer size for each configuration
   object in private PMD element it processes;
2. RTE flow verification will not reject elements with negative type.

Signed-off-by: Gregory Etelson <getelson@mellanox.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

---
v4:
* update the 'Negative types' section in the rtre_flow.rst

* update the patch comment
---
 doc/guides/prog_guide/rte_flow.rst |  3 +++
 lib/librte_ethdev/rte_flow.c       | 28 ++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 6 deletions(-)
  

Comments

Thomas Monjalon Oct. 14, 2020, 11:40 p.m. UTC | #1
04/10/2020 15:50, Gregory Etelson:
> From: Gregory Etelson <getelson@mellanox.com>
> 
> RTE flow items & actions use positive values in item & action type.
> Negative values are reserved for PMD private types. PMD
> items & actions usually are not exposed to application and are not
> used to create RTE flows.
> 
> The patch allows applications with access to PMD flow
> items & actions ability to integrate RTE and PMD items & actions
> and use them to create flow rule.
> 
> RTE flow item or action coversion library accepts positive known

typo: coversion

> element types with predefined sizes only. Private PMD items and
> actions do not fit into this scheme becase PMD type values are
> negative, each PMD has it's own types numeration and element types and
> their sizes are not visible at RTE level.  To resolve these
> limitations the patch proposes this solution:
> 1. PMD can to expose elements of pointer size only.  RTE flow

typo: "can to"

>    conversion functions will use pointer size for each configuration
>    object in private PMD element it processes;
> 2. RTE flow verification will not reject elements with negative type.
> 
> Signed-off-by: Gregory Etelson <getelson@mellanox.com>

Don't you want to use nvidia email?

> Acked-by: Ori Kam <orika@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

I was not aware of negative PMD types in rte_flow.
Allowing to use these opaque PMD types for magic complex rule looks OK.
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 119b128739..df71bd2eeb 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2678,6 +2678,9 @@  identifiers they are not aware of.
 
 A method to generate them remains to be defined.
 
+Application may use PMD dynamic items or actions in flow rules. In that case
+size of configuration object in dynamic element must be a pointer size.
+
 Planned types
 ~~~~~~~~~~~~~
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index f8fdd68fe9..c8c6d62a8b 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -564,7 +564,11 @@  rte_flow_conv_item_spec(void *buf, const size_t size,
 		}
 		break;
 	default:
-		off = rte_flow_desc_item[item->type].size;
+		/**
+		 * allow PMD private flow item
+		 */
+		off = (int)item->type >= 0 ?
+		      rte_flow_desc_item[item->type].size : sizeof(void *);
 		rte_memcpy(buf, data, (size > off ? off : size));
 		break;
 	}
@@ -667,7 +671,11 @@  rte_flow_conv_action_conf(void *buf, const size_t size,
 		}
 		break;
 	default:
-		off = rte_flow_desc_action[action->type].size;
+		/**
+		 * allow PMD private flow action
+		 */
+		off = (int)action->type >= 0 ?
+		      rte_flow_desc_action[action->type].size : sizeof(void *);
 		rte_memcpy(buf, action->conf, (size > off ? off : size));
 		break;
 	}
@@ -709,8 +717,12 @@  rte_flow_conv_pattern(struct rte_flow_item *dst,
 	unsigned int i;
 
 	for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) {
-		if ((size_t)src->type >= RTE_DIM(rte_flow_desc_item) ||
-		    !rte_flow_desc_item[src->type].name)
+		/**
+		 * allow PMD private flow item
+		 */
+		if (((int)src->type >= 0) &&
+			((size_t)src->type >= RTE_DIM(rte_flow_desc_item) ||
+		    !rte_flow_desc_item[src->type].name))
 			return rte_flow_error_set
 				(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ITEM, src,
 				 "cannot convert unknown item type");
@@ -798,8 +810,12 @@  rte_flow_conv_actions(struct rte_flow_action *dst,
 	unsigned int i;
 
 	for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) {
-		if ((size_t)src->type >= RTE_DIM(rte_flow_desc_action) ||
-		    !rte_flow_desc_action[src->type].name)
+		/**
+		 * allow PMD private flow action
+		 */
+		if (((int)src->type >= 0) &&
+		    ((size_t)src->type >= RTE_DIM(rte_flow_desc_action) ||
+		    !rte_flow_desc_action[src->type].name))
 			return rte_flow_error_set
 				(error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION,
 				 src, "cannot convert unknown action type");