[v3,1/4] ethdev: allow negative values in flow rule types
Checks
Commit Message
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
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
>
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
> >
@@ -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");