On Sun, Aug 05, 2018 at 04:17:21AM -0700, Shahaf Shuler wrote:
> Sunday, August 5, 2018 9:41 AM, Matan Azrad:
> > Subject: RE: [dpdk-dev] [PATCH] net/mlx5: fix sanity check for MPLS-in-GRE
> >
> > Hi Koh
> >
> > From: Yongseok Koh
> > > Multiple tunnel isn't allowed but MPLS over GRE should be accepted.
> > >
> > > Fixes: a4a5cd21d20a ("net/mlx5: add flow MPLS item")
> > >
> > > Signed-off-by: Yongseok Koh <ys...@mellanox.com>
> > > ---
> > > drivers/net/mlx5/mlx5_flow.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > > b/drivers/net/mlx5/mlx5_flow.c index
> > > b7500ec9d6..ca4625b699 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > @@ -1778,7 +1778,9 @@ mlx5_flow_item_mpls(const struct
> > rte_flow_item
> > > *item __rte_unused,
> > > item,
> > > "protocol filtering not compatible"
> > > " with MPLS layer");
> > > - if (flow->layers & MLX5_FLOW_LAYER_TUNNEL)
> > > + /* Multi-tunnel isn't allowed but MPLS over GRE is an exception. */
> > > + if (flow->layers & MLX5_FLOW_LAYER_TUNNEL &&
> > > + (flow->layers & MLX5_FLOW_LAYER_GRE) !=
> >
> > This check is not fully correct because the GRE item must be the last valid
> > item before the mpls, so the next flow
> >
> > eth / ipv4 / gre / ipv4 / mpls
> >
> > is not valid.
> >
> > But the next flows are valid:
> > eth / ipv4 / gre / mpls
> > eth / ipv4 / gre / void / mpls
> > eth / ipv4 / gre / void / void / void / mpls
>
> Spoke w/ Matan on it.
> It is correct this patch is not complete, however considering the release
> schedule it is better to have it in than giveup the MPLS-in-GRE support for
> 18.08.
>
> Hence applying this patch and keep track on subsequent one to fully detect
> all cases.
>
> Applied to next-net-mlx, thanks.
Good to see it is merged, but small comment.
This patch is correct and complete by itself. This patch wasn't intended to add
all the lacking sanity checks complementing the current flow engine but only to
fix a false negative in order to allow MPLSoGRE creation. There're a few more
other cases which are not filtered by validation code. For example, the
following is also accepted by PMD but HW refuses.
pattern eth / ipv4 / udp / gre / end
We don't want to push many lines of code in RC stage but bug fixes. As the flow
engine doesn't store the previous item, it would've needed quite a few lines of
code to address such lacking sanity checks. Those should be done by additional
patch for the next (stable) release if needed.
Thanks,
Yongseok
