On Fri, Dec 23, 2016 at 12:19:30PM +0000, Ferruh Yigit wrote:
> On 12/21/2016 3:19 PM, Nelio Laranjeiro wrote:
> > Introduce initial software validation for rte_flow rules.
> >
> > Signed-off-by: Nelio Laranjeiro <nelio.laranje...@6wind.com>
> > ---
> > drivers/net/mlx5/mlx5.h | 2 +
> > drivers/net/mlx5/mlx5_flow.c | 202
> > ++++++++++++++++++++++++++++++++++------
> > drivers/net/mlx5/mlx5_trigger.c | 2 +
> > 3 files changed, 177 insertions(+), 29 deletions(-)
>
> <...>
>
> > + for (; items->type != RTE_FLOW_ITEM_TYPE_END; ++items) {
> > + if (items->type == RTE_FLOW_ITEM_TYPE_VOID) {
> > + continue;
> > + } else if (items->type == RTE_FLOW_ITEM_TYPE_ETH) {
> > + if (ilast)
> > + goto exit_item_not_supported;
> > + ilast = items;
> > + } else if ((items->type == RTE_FLOW_ITEM_TYPE_IPV4) ||
> > + (items->type == RTE_FLOW_ITEM_TYPE_IPV6)) {
> > + if (!ilast)
> > + goto exit_item_not_supported;
> > + else if (ilast->type != RTE_FLOW_ITEM_TYPE_ETH)
> > + goto exit_item_not_supported;
> > + ilast = items;
> > + } else if ((items->type == RTE_FLOW_ITEM_TYPE_UDP) ||
> > + (items->type == RTE_FLOW_ITEM_TYPE_TCP)) {
> > + if (!ilast)
> > + goto exit_item_not_supported;
> > + else if ((ilast->type != RTE_FLOW_ITEM_TYPE_IPV4) &&
> > + (ilast->type != RTE_FLOW_ITEM_TYPE_IPV6))
> > + goto exit_item_not_supported;
> > + ilast = items;
> > + } else {
> > + goto exit_item_not_supported;
> > + }
> > + }
>
> I was thinking rte_flow_validate() is validating rule against hardware /
> PMD, but here the API input validation is also done.
> In patch 3/4 API input validation continues with validating each item
> one by one.
>
> Shouldn't each PMD needs to do this kind of input validation?
> Why not move generic input validation to rte_flow API?
> And if it is valid, call PMD specific one.
I think we'll add one eventually, but such a generic function would be
called by PMDs not by applications. PMDs must have the ability to optimize
validate() and create() however they want.
In the meantime in my opinion it's better to let PMDs implement their own to
determine what can be shared later without cluttering rte_flow from the
start.
--
Adrien Mazarguil
6WIND
