On Tue, Sep 8, 2020 at 1:16 PM Gregory Etelson <getel...@nvidia.com> wrote:
> From: Gregory Etelson <getel...@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.
>
While we are reviewing this, some quick comment/questions..
Doesn't this go against the above "PMD
items & actions usually are not exposed to application and are not
used to create RTE flows."?
Why would an application try to use PMD specific private types?
Isn't this contrary to having a standard API?
>
> 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 <getel...@mellanox.com>
> Acked-by: Ori Kam <or...@mellanox.com>
> ---
> v2:
> * Update commit log
> ---
> lib/librte_ethdev/rte_flow.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index f8fdd68fe9..9905426bc9 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -564,7 +564,12 @@ 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 = (uint32_t)item->type <= INT_MAX ?
> + rte_flow_desc_item[item->type].size :
> + sizeof(void *);
> rte_memcpy(buf, data, (size > off ? off : size));
> break;
> }
> @@ -667,7 +672,12 @@ 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 = (uint32_t)action->type <= INT_MAX ?
> + rte_flow_desc_action[action->type].size :
> + sizeof(void *);
> rte_memcpy(buf, action->conf, (size > off ? off : size));
> break;
> }
> @@ -709,8 +719,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 (((uint32_t)src->type <= INT_MAX) &&
> + ((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 +812,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 (((uint32_t)src->type <= INT_MAX) &&
> + ((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
>
>
