Re: [dpdk-dev] [PATCH] net/mlx5: support MPLS-in-GRE and MPLS-in-UDP

[Matan Azrad <matan@mellanox.com>]

Hi Nelio

From: Nélio Laranjeiro
> Hi Matan,
> 
> I have a single comment, please see bellow,
> 
> On Mon, May 14, 2018 at 01:41:20PM +0000, Matan Azrad wrote:
> > Add support for MPLS over GRE and MPLS over UDP tunnel types as
> > described in the next RFCs:
> > 1.
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftoo
> >
> ls.ietf.org%2Fhtml%2Frfc4023&data=02%7C01%7Cmatan%40mellanox.com%7C
> 702
> >
> 706112b0942d2e61808d5ba478dae%7Ca652971c7d2e4d9ba6a4d149256f461b
> %7C0%7
> >
> C0%7C636619738845367511&sdata=junDKbO1BJ%2Bh%2FZVV7P5zSsxQGDgcR
> UGcfhac
> > XaC502E%3D&reserved=0 2.
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftoo
> >
> ls.ietf.org%2Fhtml%2Frfc7510&data=02%7C01%7Cmatan%40mellanox.com%7C
> 702
> >
> 706112b0942d2e61808d5ba478dae%7Ca652971c7d2e4d9ba6a4d149256f461b
> %7C0%7
> >
> C0%7C636619738845367511&sdata=5i8IQBXYRqOtc9Pda2Dq7na9mzfwffi3iuxq
> huZS
> > JTQ%3D&reserved=0 3.
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftoo
> >
> ls.ietf.org%2Fhtml%2Frfc4385&data=02%7C01%7Cmatan%40mellanox.com%7C
> 702
> >
> 706112b0942d2e61808d5ba478dae%7Ca652971c7d2e4d9ba6a4d149256f461b
> %7C0%7
> >
> C0%7C636619738845367511&sdata=RG80NBHW3Yv3o%2BzyKFb6p41LsoBgwo
> gB6m4X3I
> > fshes%3D&reserved=0
> >
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> > ---
> >  doc/guides/nics/mlx5.rst     |   4 +-
> >  drivers/net/mlx5/Makefile    |   5 ++
> >  drivers/net/mlx5/mlx5.c      |  13 ++++
> >  drivers/net/mlx5/mlx5.h      |   1 +
> >  drivers/net/mlx5/mlx5_flow.c | 154
> >+++++++++++++++++++++++++++++++++++++++++--
> >  5 files changed, 170 insertions(+), 7 deletions(-) [...]  @@ -896,7
> >+928,9 @@ struct ibv_spec_header {
> >  		if (ret)
> >  			goto exit_item_not_supported;
> >  		if (IS_TUNNEL(items->type)) {
> > -			if (parser->tunnel) {
> > +			if (parser->tunnel &&
> > +			   !((items - 1)->type == RTE_FLOW_ITEM_TYPE_GRE
> &&
> > +			     items->type == RTE_FLOW_ITEM_TYPE_MPLS)) {
> >  				rte_flow_error_set(error, ENOTSUP,
> >
> RTE_FLOW_ERROR_TYPE_ITEM,
> >  						   items,
> > @@ -904,6 +938,16 @@ struct ibv_spec_header {
> >  						   " tunnel encapsulations.");
> >  				return -rte_errno;
> >  			}
> > +			if (items->type == RTE_FLOW_ITEM_TYPE_MPLS &&
> > +			    !priv->config.mpls_en) {
> > +				rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_ITEM,
> > +						   items,
> > +						   "MPLS not supported or"
> > +						   " disabled in firmware"
> > +						   " configuration.");
> > +				return -rte_errno;
> > +			}
> >  			if (!priv->config.tunnel_en &&
> >  			    parser->rss_conf.level > 1) {
> >  				rte_flow_error_set(error, ENOTSUP,
> 
> Doing such comparison will not work in all cases like
> 
>  GRE / VOID / MPLS
> 
> which is totally valid from rte_flow perspective.

Yes, you right, will send v2 to handle the voids.

> 
> Regards,
> 
> --
> Nélio Laranjeiro
> 6WIND