Re: [dpdk-dev] [PATCH v3 1/4] ethdev: allow negative values in flow rule types

[Gregory Etelson <getelson@nvidia.com>]

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
> >