From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yongseok Koh Subject: Re: [PATCH] net/mlx5: fix GRE flow rule Date: Wed, 23 May 2018 03:01:22 -0700 Message-ID: <20180523100116.GA11530@minint-98vp2qg> References: <20180523015157.35716-1-yskoh@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Shahaf Shuler , Adrien Mazarguil , =?iso-8859-1?Q?N=E9lio?= Laranjeiro , "dev@dpdk.org" , "stable@dpdk.org" To: Matan Azrad Return-path: Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, May 22, 2018 at 10:36:43PM -0700, Matan Azrad wrote: > Hi Yongseok > > From: Yongseok Koh > > Creating a flow having pattern from the middle of a packet is allowed. For > > example, > > > > testpmd> flow create 0 ingress pattern vxlan vni is 20 / end actions ... > > > > Device can parse GRE header but without proper support from library and > > firmware (HAVE_IBV_DEVICE_MPLS_SUPPORT), a field in GRE header can't be > > specified when creating a rule. As a result, the following rule will be > > interpreted as a wildcard rule, which always matches any packet. > > > > testpmd> flow create 0 ingress pattern gre / end actions ... > > Fixes: 96c6c65a10d2 ("net/mlx5: support GRE tunnel flow") > > Fixes: 1f106da2bf7b ("net/mlx5: support MPLS-in-GRE and MPLS-in-UDP") > > Cc: stable@dpdk.org > > > > Signed-off-by: Yongseok Koh > > --- > > drivers/net/mlx5/mlx5_flow.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index > > 994be05be..526fe6b0e 100644 > > --- a/drivers/net/mlx5/mlx5_flow.c > > +++ b/drivers/net/mlx5/mlx5_flow.c > > @@ -330,9 +330,11 @@ static const enum rte_flow_action_type > > valid_actions[] = { static const struct mlx5_flow_items mlx5_flow_items[] = { > > [RTE_FLOW_ITEM_TYPE_END] = { > > .items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH, > > +#ifdef HAVE_IBV_DEVICE_MPLS_SUPPORT > > The GRE item was here even before the MPLSoGRE support Yes, this bug has existed before adding MPLSoGRE support. > so I think that this is not the correct fix and even that it can hurt the > support of GRE for the current customers use it. How can it hurt? Please clarify. > Looks like you must specify at least 1 spec in the GRE to apply it correctly > as you did for VXLAN, Can you try empty vxlan and fully gre (with protocol > field)? That's exactly the reason why I'm taking this out. If you look at the code, it doesn't even set any field for GRE if HAVE_IBV_DEVICE_MPLS_SUPPORT isn't supported. Thus, it is considered as a wildcard (all-matching) rule. But if it has HAVE_IBV_DEVICE_MPLS_SUPPORT, such pattern can be allowed. Having pattern 'vxlan' without vni isn't allowed by mlx5 PMD because zero VNI is never accepted. Thanks, Yongseok > > + RTE_FLOW_ITEM_TYPE_GRE, > > +#endif > > RTE_FLOW_ITEM_TYPE_VXLAN, > > - RTE_FLOW_ITEM_TYPE_VXLAN_GPE, > > - RTE_FLOW_ITEM_TYPE_GRE), > > + RTE_FLOW_ITEM_TYPE_VXLAN_GPE), > > }, > > [RTE_FLOW_ITEM_TYPE_ETH] = { > > .items = ITEMS(RTE_FLOW_ITEM_TYPE_VLAN, > > >