diff mbox series

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

Message ID 20200930091854.19768-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 success coding style OK

Commit Message

Gregory Etelson Sept. 30, 2020, 9:18 a.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 library functions cannot work with PMD private items and
actions (elements) because RTE flow has no API to query PMD flow
object size. In the patch, PMD flow elements use object pointer.
RTE flow library functions handle PMD element object size as
size of a pointer. PMD handles its objects internally.

Signed-off-by: Gregory Etelson <getelson@mellanox.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 lib/librte_ethdev/rte_flow.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Comments

Ajit Khaparde Oct. 4, 2020, 5:40 a.m. UTC | #1 
Gregory,
Please see inline.

On Wed, Sep 30, 2020 at 2:19 AM Gregory Etelson <getelson@nvidia.com> wrote:
>
> 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 library functions cannot work with PMD private items and
> actions (elements) because RTE flow has no API to query PMD flow
> object size. In the patch, PMD flow elements use object pointer.
> RTE flow library functions handle PMD element object size as
> size of a pointer. PMD handles its objects internally.

This is important information. Apart from the commit log,
this should also be added  in the rte_flow API documentation.
The comment in the code/API could be elaborated with this info as well.

>
> Signed-off-by: Gregory Etelson <getelson@mellanox.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
>  lib/librte_ethdev/rte_flow.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> 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");
> --
> 2.25.1
>
Gregory Etelson Oct. 4, 2020, 9:24 a.m. UTC | #2 
Hello Ajit,

[snip]
> > RTE flow library functions cannot work with PMD private items and
> > actions (elements) because RTE flow has no API to query PMD flow
> > object size. In the patch, PMD flow elements use object pointer.
> > RTE flow library functions handle PMD element object size as
> > size of a pointer. PMD handles its objects internally.
> 
> This is important information. Apart from the commit log,
> this should also be added  in the rte_flow API documentation.
> The comment in the code/API could be elaborated with this info as well.
> 

I'll update code comments & rte_flow  API documentation in the next patch update.
The update will be ready this week.

> >
> > Signed-off-by: Gregory Etelson <getelson@mellanox.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > ---
> >  lib/librte_ethdev/rte_flow.c | 28 ++++++++++++++++++++++------
> >  1 file changed, 22 insertions(+), 6 deletions(-)
> >
> > 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");
> > --
> > 2.25.1
> >
diff mbox series

Patch

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");