[dpdk-dev,v4,03/11] net/mlx5: support L3 VXLAN flow

Message ID 20180417151436.161374-4-xuemingl@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Shahaf Shuler
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Xueming Li April 17, 2018, 3:14 p.m. UTC
  This patch support L3 VXLAN, no inner L2 header comparing to standard
VXLAN protocol. L3 VXLAN using specific overlay UDP destination port to
discriminate against standard VXLAN, FW has to be configured to support
it:
  sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
  sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Nélio Laranjeiro April 18, 2018, 6:48 a.m. UTC | #1
On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> This patch support L3 VXLAN, no inner L2 header comparing to standard
> VXLAN protocol. L3 VXLAN using specific overlay UDP destination port to
> discriminate against standard VXLAN, FW has to be configured to support
> it:
>   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
>   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>
> 
> Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
>  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
>  	},
>  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
> -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
>  		.actions = valid_actions,
>  		.mask = &(const struct rte_flow_item_vxlan){
>  			.vni = "\xff\xff\xff",
> -- 
> 2.13.3

Such support must be under device parameter has it depends on the
configuration of the firmware.  If the firmware is not correctly
configured the PMD must refuse such rule.

Thanks,
  
Xueming Li April 18, 2018, 2:43 p.m. UTC | #2
> -----Original Message-----

> From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> Sent: Wednesday, April 18, 2018 2:49 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> 

> On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:

> > This patch support L3 VXLAN, no inner L2 header comparing to standard

> > VXLAN protocol. L3 VXLAN using specific overlay UDP destination port

> > to discriminate against standard VXLAN, FW has to be configured to

> > support

> > it:

> >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1

> >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>

> >

> > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff 100644

> > --- a/drivers/net/mlx5/mlx5_flow.c

> > +++ b/drivers/net/mlx5/mlx5_flow.c

> > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {

> >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),

> >  	},

> >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {

> > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),

> > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,

> > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */

> > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */

> >  		.actions = valid_actions,

> >  		.mask = &(const struct rte_flow_item_vxlan){

> >  			.vni = "\xff\xff\xff",

> > --

> > 2.13.3

> 

> Such support must be under device parameter has it depends on the configuration of the firmware.  If

> the firmware is not correctly configured the PMD must refuse such rule.

> 

> Thanks,

> 

> --

> Nélio Laranjeiro

> 6WIND


Are you suggesting Verbs parameter? I'm afraid we can't have it in short time, need new patch in later 
release when Verbs ready.
  
Nélio Laranjeiro April 18, 2018, 3:08 p.m. UTC | #3
On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Sent: Wednesday, April 18, 2018 2:49 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > 
> > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> > > This patch support L3 VXLAN, no inner L2 header comparing to standard
> > > VXLAN protocol. L3 VXLAN using specific overlay UDP destination port
> > > to discriminate against standard VXLAN, FW has to be configured to
> > > support
> > > it:
> > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
> > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>
> > >
> > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
> > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > >  	},
> > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
> > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
> > >  		.actions = valid_actions,
> > >  		.mask = &(const struct rte_flow_item_vxlan){
> > >  			.vni = "\xff\xff\xff",
> > > --
> > > 2.13.3
> > 
> > Such support must be under device parameter has it depends on the configuration of the firmware.  If
> > the firmware is not correctly configured the PMD must refuse such rule.
> > 
> > Thanks,
> > 
> > --
> > Nélio Laranjeiro
> > 6WIND
> 
> Are you suggesting Verbs parameter? I'm afraid we can't have it in
> short time, need new patch in later release when Verbs ready.

Take a look at [1], this is what I mean.

Regards,

[1] https://dpdk.org/doc/guides/nics/mlx5.html#run-time-configuration
  
Xueming Li April 19, 2018, 6:20 a.m. UTC | #4
> -----Original Message-----

> From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> Sent: Wednesday, April 18, 2018 11:09 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> 

> On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:

> >

> >

> > > -----Original Message-----

> > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > Sent: Wednesday, April 18, 2018 2:49 PM

> > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > >

> > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:

> > > > This patch support L3 VXLAN, no inner L2 header comparing to

> > > > standard VXLAN protocol. L3 VXLAN using specific overlay UDP

> > > > destination port to discriminate against standard VXLAN, FW has to

> > > > be configured to support

> > > > it:

> > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1

> > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>

> > > >

> > > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff 100644

> > > > --- a/drivers/net/mlx5/mlx5_flow.c

> > > > +++ b/drivers/net/mlx5/mlx5_flow.c

> > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {

> > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),

> > > >  	},

> > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {

> > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),

> > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,

> > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */

> > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */

> > > >  		.actions = valid_actions,

> > > >  		.mask = &(const struct rte_flow_item_vxlan){

> > > >  			.vni = "\xff\xff\xff",

> > > > --

> > > > 2.13.3

> > >

> > > Such support must be under device parameter has it depends on the

> > > configuration of the firmware.  If the firmware is not correctly configured the PMD must refuse

> such rule.

> > >

> > > Thanks,

> > >

> > > --

> > > Nélio Laranjeiro

> > > 6WIND

> >

> > Are you suggesting Verbs parameter? I'm afraid we can't have it in

> > short time, need new patch in later release when Verbs ready.

> 

> Take a look at [1], this is what I mean.


Enabling a new device parameter can't make L3 VXLAN packet get received if fw configuration not set.
On the other hand, if fw continuation enabled and device parameter not set, packet could be received
but failed to create rule. I'm afraid that a device parameter will introduce complexity of using 
this feature w/o real benefits.

> 

> Regards,

> 

> [1]

> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdpdk.org%2Fdoc%2Fguides%2Fnics%2Fml

> x5.html%23run-time-

> configuration&data=02%7C01%7Cxuemingl%40mellanox.com%7Ce3b4395f998a447f054308d5a53e3afc%7Ca652971c7d2e

> 4d9ba6a4d149256f461b%7C0%7C0%7C636596609057090801&sdata=VrTSE0favcGEhpyURZ8thHjBxdlR8f80%2BOmDy1a1vZU%

> 3D&reserved=0

> 

> --

> Nélio Laranjeiro

> 6WIND
  
Nélio Laranjeiro April 19, 2018, 6:55 a.m. UTC | #5
On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Sent: Wednesday, April 18, 2018 11:09 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > 
> > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > Sent: Wednesday, April 18, 2018 2:49 PM
> > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > >
> > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> > > > > This patch support L3 VXLAN, no inner L2 header comparing to
> > > > > standard VXLAN protocol. L3 VXLAN using specific overlay UDP
> > > > > destination port to discriminate against standard VXLAN, FW has to
> > > > > be configured to support
> > > > > it:
> > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
> > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>
> > > > >
> > > > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff 100644
> > > > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
> > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > > > >  	},
> > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
> > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
> > > > >  		.actions = valid_actions,
> > > > >  		.mask = &(const struct rte_flow_item_vxlan){
> > > > >  			.vni = "\xff\xff\xff",
> > > > > --
> > > > > 2.13.3
> > > >
> > > > Such support must be under device parameter has it depends on the
> > > > configuration of the firmware.  If the firmware is not correctly configured the PMD must refuse
> > such rule.
> > > >
> > > > Thanks,
> > > >
> > > > --
> > > > Nélio Laranjeiro
> > > > 6WIND
> > >
> > > Are you suggesting Verbs parameter? I'm afraid we can't have it in
> > > short time, need new patch in later release when Verbs ready.
> > 
> > Take a look at [1], this is what I mean.
> 
> Enabling a new device parameter can't make L3 VXLAN packet get
> received if fw configuration not set.

So you expect than the user will enable a feature without reading the
PMD documentation?
If it is the case, the answer it pretty simple, it is the same as above,
read the PMD documentation.

> On the other hand, if fw continuation enabled and device parameter not
> set, packet could be received but failed to create rule.

Again a user using a NIC should read the documentation.

> I'm afraid that a device parameter will introduce complexity of using 
> this feature w/o real benefits.

Add this missing device parameter and update accordingly the
documentation, or wait for Verbs to add the missing query feature.

If the firmware it not configured this rule must be refused, as there is
no way in the PMD to know if the firmware is configured, it must rely on
a device parameter.

Regards,
  
Xueming Li April 19, 2018, 10:21 a.m. UTC | #6
> -----Original Message-----

> From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> Sent: Thursday, April 19, 2018 2:56 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> 

> On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:

> >

> >

> > > -----Original Message-----

> > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > Sent: Wednesday, April 18, 2018 11:09 PM

> > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > >

> > > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > > > Sent: Wednesday, April 18, 2018 2:49 PM

> > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > > > >

> > > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:

> > > > > > This patch support L3 VXLAN, no inner L2 header comparing to

> > > > > > standard VXLAN protocol. L3 VXLAN using specific overlay UDP

> > > > > > destination port to discriminate against standard VXLAN, FW

> > > > > > has to be configured to support

> > > > > > it:

> > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1

> > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>

> > > > > >

> > > > > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff

> > > > > > 100644

> > > > > > --- a/drivers/net/mlx5/mlx5_flow.c

> > > > > > +++ b/drivers/net/mlx5/mlx5_flow.c

> > > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {

> > > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),

> > > > > >  	},

> > > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {

> > > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),

> > > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,

> > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */

> > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */

> > > > > >  		.actions = valid_actions,

> > > > > >  		.mask = &(const struct rte_flow_item_vxlan){

> > > > > >  			.vni = "\xff\xff\xff",

> > > > > > --

> > > > > > 2.13.3

> > > > >

> > > > > Such support must be under device parameter has it depends on

> > > > > the configuration of the firmware.  If the firmware is not

> > > > > correctly configured the PMD must refuse

> > > such rule.

> > > > >

> > > > > Thanks,

> > > > >

> > > > > --

> > > > > Nélio Laranjeiro

> > > > > 6WIND

> > > >

> > > > Are you suggesting Verbs parameter? I'm afraid we can't have it in

> > > > short time, need new patch in later release when Verbs ready.

> > >

> > > Take a look at [1], this is what I mean.

> >

> > Enabling a new device parameter can't make L3 VXLAN packet get

> > received if fw configuration not set.

> 

> So you expect than the user will enable a feature without reading the PMD documentation?

> If it is the case, the answer it pretty simple, it is the same as above, read the PMD documentation.

> 

> > On the other hand, if fw continuation enabled and device parameter not

> > set, packet could be received but failed to create rule.

> 

> Again a user using a NIC should read the documentation.


If a user read the document, fw should be configured correctly to enable this feature.

> 

> > I'm afraid that a device parameter will introduce complexity of using

> > this feature w/o real benefits.

> 

> Add this missing device parameter and update accordingly the documentation, or wait for Verbs to add

> the missing query feature.

> 

> If the firmware it not configured this rule must be refused, as there is no way in the PMD to know if

> the firmware is configured, it must rely on a device parameter.


Let's keep the design simple, users know exactly what they are doing and should not expecting 
such flow working by reading document.

> 

> Regards,

> 

> --

> Nélio Laranjeiro

> 6WIND
  
Nélio Laranjeiro April 19, 2018, 11:15 a.m. UTC | #7
On Thu, Apr 19, 2018 at 10:21:26AM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Sent: Thursday, April 19, 2018 2:56 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > 
> > On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > Sent: Wednesday, April 18, 2018 11:09 PM
> > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > >
> > > > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > > > Sent: Wednesday, April 18, 2018 2:49 PM
> > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > > > >
> > > > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> > > > > > > This patch support L3 VXLAN, no inner L2 header comparing to
> > > > > > > standard VXLAN protocol. L3 VXLAN using specific overlay UDP
> > > > > > > destination port to discriminate against standard VXLAN, FW
> > > > > > > has to be configured to support
> > > > > > > it:
> > > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
> > > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_PORT=<port>
> > > > > > >
> > > > > > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff
> > > > > > > 100644
> > > > > > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > > > > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
> > > > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > > > > > >  	},
> > > > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
> > > > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> > > > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
> > > > > > >  		.actions = valid_actions,
> > > > > > >  		.mask = &(const struct rte_flow_item_vxlan){
> > > > > > >  			.vni = "\xff\xff\xff",
> > > > > > > --
> > > > > > > 2.13.3
> > > > > >
> > > > > > Such support must be under device parameter has it depends on
> > > > > > the configuration of the firmware.  If the firmware is not
> > > > > > correctly configured the PMD must refuse
> > > > such rule.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > --
> > > > > > Nélio Laranjeiro
> > > > > > 6WIND
> > > > >
> > > > > Are you suggesting Verbs parameter? I'm afraid we can't have it in
> > > > > short time, need new patch in later release when Verbs ready.
> > > >
> > > > Take a look at [1], this is what I mean.
> > >
> > > Enabling a new device parameter can't make L3 VXLAN packet get
> > > received if fw configuration not set.
> > 
> > So you expect than the user will enable a feature without reading the PMD documentation?
> > If it is the case, the answer it pretty simple, it is the same as above, read the PMD documentation.
> > 
> > > On the other hand, if fw continuation enabled and device parameter not
> > > set, packet could be received but failed to create rule.
> > 
> > Again a user using a NIC should read the documentation.
> 
> If a user read the document, fw should be configured correctly to enable this feature.

And a user which does not read this document must not be able to create
rules the NIC cannot handle because the firmware is not configured.

> > > I'm afraid that a device parameter will introduce complexity of using
> > > this feature w/o real benefits.
> > 
> > Add this missing device parameter and update accordingly the documentation, or wait for Verbs to add
> > the missing query feature.
> > 
> > If the firmware it not configured this rule must be refused, as there is no way in the PMD to know if
> > the firmware is configured, it must rely on a device parameter.
> 
> Let's keep the design simple, users know exactly what they are doing and should not expecting 
> such flow working by reading document.

This is exactly the opposite, users never read documentation even
today I've already spotted a new user to such documentation [1].

For this same reason a functionality not enabled by default in the
firmware must not be used by the PMD.  No device parameter no feature.

Add the device parameter and the according documentation.

Regards,

[1] https://dpdk.org/ml/archives/users/2018-April/003020.html
  
Xueming Li April 19, 2018, 11:53 a.m. UTC | #8
> -----Original Message-----

> From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> Sent: Thursday, April 19, 2018 7:15 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> 

> On Thu, Apr 19, 2018 at 10:21:26AM +0000, Xueming(Steven) Li wrote:

> >

> >

> > > -----Original Message-----

> > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > Sent: Thursday, April 19, 2018 2:56 PM

> > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > >

> > > On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > > > Sent: Wednesday, April 18, 2018 11:09 PM

> > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > > > >

> > > > > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > > > > > Sent: Wednesday, April 18, 2018 2:49 PM

> > > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN

> > > > > > > flow

> > > > > > >

> > > > > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:

> > > > > > > > This patch support L3 VXLAN, no inner L2 header comparing

> > > > > > > > to standard VXLAN protocol. L3 VXLAN using specific

> > > > > > > > overlay UDP destination port to discriminate against

> > > > > > > > standard VXLAN, FW has to be configured to support

> > > > > > > > it:

> > > > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1

> > > > > > > >   sudo mlxconfig -d <device> -y s

> > > > > > > > IP_OVER_VXLAN_PORT=<port>

> > > > > > > >

> > > > > > > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff

> > > > > > > > 100644

> > > > > > > > --- a/drivers/net/mlx5/mlx5_flow.c

> > > > > > > > +++ b/drivers/net/mlx5/mlx5_flow.c

> > > > > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {

> > > > > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),

> > > > > > > >  	},

> > > > > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {

> > > > > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),

> > > > > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,

> > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */

> > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */

> > > > > > > >  		.actions = valid_actions,

> > > > > > > >  		.mask = &(const struct rte_flow_item_vxlan){

> > > > > > > >  			.vni = "\xff\xff\xff",

> > > > > > > > --

> > > > > > > > 2.13.3

> > > > > > >

> > > > > > > Such support must be under device parameter has it depends

> > > > > > > on the configuration of the firmware.  If the firmware is

> > > > > > > not correctly configured the PMD must refuse

> > > > > such rule.

> > > > > > >

> > > > > > > Thanks,

> > > > > > >

> > > > > > > --

> > > > > > > Nélio Laranjeiro

> > > > > > > 6WIND

> > > > > >

> > > > > > Are you suggesting Verbs parameter? I'm afraid we can't have

> > > > > > it in short time, need new patch in later release when Verbs ready.

> > > > >

> > > > > Take a look at [1], this is what I mean.

> > > >

> > > > Enabling a new device parameter can't make L3 VXLAN packet get

> > > > received if fw configuration not set.

> > >

> > > So you expect than the user will enable a feature without reading the PMD documentation?

> > > If it is the case, the answer it pretty simple, it is the same as above, read the PMD

> documentation.

> > >

> > > > On the other hand, if fw continuation enabled and device parameter

> > > > not set, packet could be received but failed to create rule.

> > >

> > > Again a user using a NIC should read the documentation.

> >

> > If a user read the document, fw should be configured correctly to enable this feature.

> 

> And a user which does not read this document must not be able to create rules the NIC cannot handle

> because the firmware is not configured.

> 

> > > > I'm afraid that a device parameter will introduce complexity of

> > > > using this feature w/o real benefits.

> > >

> > > Add this missing device parameter and update accordingly the

> > > documentation, or wait for Verbs to add the missing query feature.

> > >

> > > If the firmware it not configured this rule must be refused, as

> > > there is no way in the PMD to know if the firmware is configured, it must rely on a device

> parameter.

> >

> > Let's keep the design simple, users know exactly what they are doing

> > and should not expecting such flow working by reading document.

> 

> This is exactly the opposite, users never read documentation even today I've already spotted a new

> user to such documentation [1].


  "So you expect than the user will enable a feature without reading the PMD documentation?
   If it is the case, the answer it pretty simple, it is the same as above, read the PMD documentation.
   Again a user using a NIC should read the documentation."

> 

> For this same reason a functionality not enabled by default in the firmware must not be used by the

> PMD.  No device parameter no feature.


Unlike other functionality, this feature related to supporting a new tunnel type, w/o fw configuration, 
L3 VXLAN packet certainly be treated as normal packet, it doesn't hurt. How do you think?

> 

> Add the device parameter and the according documentation.

> 

> Regards,

> 

> [1]

> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdpdk.org%2Fml%2Farchives%2Fusers%2F

> 2018-

> April%2F003020.html&data=02%7C01%7Cxuemingl%40mellanox.com%7C7b417f2ace1044d6858a08d5a5e6c502%7Ca65297

> 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636597332927305548&sdata=7sUJ8okKx2yKTQz9nXfi%2B2cDwYVUbMa41glmez

> SYQyQ%3D&reserved=0

> 

> --

> Nélio Laranjeiro

> 6WIND
  
Nélio Laranjeiro April 19, 2018, 12:18 p.m. UTC | #9
On Thu, Apr 19, 2018 at 11:53:05AM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Sent: Thursday, April 19, 2018 7:15 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > 
> > On Thu, Apr 19, 2018 at 10:21:26AM +0000, Xueming(Steven) Li wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > Sent: Thursday, April 19, 2018 2:56 PM
> > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > >
> > > > On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > > > Sent: Wednesday, April 18, 2018 11:09 PM
> > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > > > >
> > > > > > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > > > > > Sent: Wednesday, April 18, 2018 2:49 PM
> > > > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN
> > > > > > > > flow
> > > > > > > >
> > > > > > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> > > > > > > > > This patch support L3 VXLAN, no inner L2 header comparing
> > > > > > > > > to standard VXLAN protocol. L3 VXLAN using specific
> > > > > > > > > overlay UDP destination port to discriminate against
> > > > > > > > > standard VXLAN, FW has to be configured to support
> > > > > > > > > it:
> > > > > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
> > > > > > > > >   sudo mlxconfig -d <device> -y s
> > > > > > > > > IP_OVER_VXLAN_PORT=<port>
> > > > > > > > >
> > > > > > > > > Signed-off-by: Xueming Li <xuemingl@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 771d5f14d..d7a921dff
> > > > > > > > > 100644
> > > > > > > > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > > > > > > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > > > > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
> > > > > > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > > > > > > > >  	},
> > > > > > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
> > > > > > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> > > > > > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> > > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
> > > > > > > > >  		.actions = valid_actions,
> > > > > > > > >  		.mask = &(const struct rte_flow_item_vxlan){
> > > > > > > > >  			.vni = "\xff\xff\xff",
> > > > > > > > > --
> > > > > > > > > 2.13.3
> > > > > > > >
> > > > > > > > Such support must be under device parameter has it depends
> > > > > > > > on the configuration of the firmware.  If the firmware is
> > > > > > > > not correctly configured the PMD must refuse
> > > > > > such rule.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > >
> > > > > > > > --
> > > > > > > > Nélio Laranjeiro
> > > > > > > > 6WIND
> > > > > > >
> > > > > > > Are you suggesting Verbs parameter? I'm afraid we can't have
> > > > > > > it in short time, need new patch in later release when Verbs ready.
> > > > > >
> > > > > > Take a look at [1], this is what I mean.
> > > > >
> > > > > Enabling a new device parameter can't make L3 VXLAN packet get
> > > > > received if fw configuration not set.
> > > >
> > > > So you expect than the user will enable a feature without reading the PMD documentation?
> > > > If it is the case, the answer it pretty simple, it is the same as above, read the PMD
> > documentation.
> > > >
> > > > > On the other hand, if fw continuation enabled and device parameter
> > > > > not set, packet could be received but failed to create rule.
> > > >
> > > > Again a user using a NIC should read the documentation.
> > >
> > > If a user read the document, fw should be configured correctly to enable this feature.
> > 
> > And a user which does not read this document must not be able to create rules the NIC cannot handle
> > because the firmware is not configured.
> > 
> > > > > I'm afraid that a device parameter will introduce complexity of
> > > > > using this feature w/o real benefits.
> > > >
> > > > Add this missing device parameter and update accordingly the
> > > > documentation, or wait for Verbs to add the missing query feature.
> > > >
> > > > If the firmware it not configured this rule must be refused, as
> > > > there is no way in the PMD to know if the firmware is configured, it must rely on a device
> > parameter.
> > >
> > > Let's keep the design simple, users know exactly what they are doing
> > > and should not expecting such flow working by reading document.
> > 
> > This is exactly the opposite, users never read documentation even today I've already spotted a new
> > user to such documentation [1].
> 
>   "So you expect than the user will enable a feature without reading the PMD documentation?
>    If it is the case, the answer it pretty simple, it is the same as above, read the PMD documentation.
>    Again a user using a NIC should read the documentation."
> 
> > 
> > For this same reason a functionality not enabled by default in the firmware must not be used by the
> > PMD.  No device parameter no feature.
> 
> Unlike other functionality, this feature related to supporting a new tunnel type, w/o fw configuration, 
> L3 VXLAN packet certainly be treated as normal packet, it doesn't hurt. How do you think?

 flow create 0 ingress eth / ipv4 / end action queue index 3 end

but the packet ends in queue 0, will it hurt?

Any rule *accepted* by the PMD *must* follow the user request, otherwise
it is a bug.

Add the device parameter and the according documentation.

Regards,
  
Xueming Li April 19, 2018, 12:49 p.m. UTC | #10
> -----Original Message-----

> From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> Sent: Thursday, April 19, 2018 8:19 PM

> To: Xueming(Steven) Li <xuemingl@mellanox.com>

> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> 

> On Thu, Apr 19, 2018 at 11:53:05AM +0000, Xueming(Steven) Li wrote:

> >

> >

> > > -----Original Message-----

> > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > Sent: Thursday, April 19, 2018 7:15 PM

> > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > >

> > > On Thu, Apr 19, 2018 at 10:21:26AM +0000, Xueming(Steven) Li wrote:

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > > > Sent: Thursday, April 19, 2018 2:56 PM

> > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow

> > > > >

> > > > > On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:

> > > > > >

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > > > > > Sent: Wednesday, April 18, 2018 11:09 PM

> > > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN

> > > > > > > flow

> > > > > > >

> > > > > > > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:

> > > > > > > >

> > > > > > > >

> > > > > > > > > -----Original Message-----

> > > > > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>

> > > > > > > > > Sent: Wednesday, April 18, 2018 2:49 PM

> > > > > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>

> > > > > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org

> > > > > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN

> > > > > > > > > flow

> > > > > > > > >

> > > > > > > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:

> > > > > > > > > > This patch support L3 VXLAN, no inner L2 header

> > > > > > > > > > comparing to standard VXLAN protocol. L3 VXLAN using

> > > > > > > > > > specific overlay UDP destination port to discriminate

> > > > > > > > > > against standard VXLAN, FW has to be configured to

> > > > > > > > > > support

> > > > > > > > > > it:

> > > > > > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1

> > > > > > > > > >   sudo mlxconfig -d <device> -y s

> > > > > > > > > > IP_OVER_VXLAN_PORT=<port>

> > > > > > > > > >

> > > > > > > > > > Signed-off-by: Xueming Li <xuemingl@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

> > > > > > > > > > 771d5f14d..d7a921dff

> > > > > > > > > > 100644

> > > > > > > > > > --- a/drivers/net/mlx5/mlx5_flow.c

> > > > > > > > > > +++ b/drivers/net/mlx5/mlx5_flow.c

> > > > > > > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {

> > > > > > > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),

> > > > > > > > > >  	},

> > > > > > > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {

> > > > > > > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),

> > > > > > > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,

> > > > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */

> > > > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN.

> > > > > > > > > > +*/

> > > > > > > > > >  		.actions = valid_actions,

> > > > > > > > > >  		.mask = &(const struct rte_flow_item_vxlan){

> > > > > > > > > >  			.vni = "\xff\xff\xff",

> > > > > > > > > > --

> > > > > > > > > > 2.13.3

> > > > > > > > >

> > > > > > > > > Such support must be under device parameter has it

> > > > > > > > > depends on the configuration of the firmware.  If the

> > > > > > > > > firmware is not correctly configured the PMD must refuse

> > > > > > > such rule.

> > > > > > > > >

> > > > > > > > > Thanks,

> > > > > > > > >

> > > > > > > > > --

> > > > > > > > > Nélio Laranjeiro

> > > > > > > > > 6WIND

> > > > > > > >

> > > > > > > > Are you suggesting Verbs parameter? I'm afraid we can't

> > > > > > > > have it in short time, need new patch in later release when Verbs ready.

> > > > > > >

> > > > > > > Take a look at [1], this is what I mean.

> > > > > >

> > > > > > Enabling a new device parameter can't make L3 VXLAN packet get

> > > > > > received if fw configuration not set.

> > > > >

> > > > > So you expect than the user will enable a feature without reading the PMD documentation?

> > > > > If it is the case, the answer it pretty simple, it is the same

> > > > > as above, read the PMD

> > > documentation.

> > > > >

> > > > > > On the other hand, if fw continuation enabled and device

> > > > > > parameter not set, packet could be received but failed to create rule.

> > > > >

> > > > > Again a user using a NIC should read the documentation.

> > > >

> > > > If a user read the document, fw should be configured correctly to enable this feature.

> > >

> > > And a user which does not read this document must not be able to

> > > create rules the NIC cannot handle because the firmware is not configured.

> > >

> > > > > > I'm afraid that a device parameter will introduce complexity

> > > > > > of using this feature w/o real benefits.

> > > > >

> > > > > Add this missing device parameter and update accordingly the

> > > > > documentation, or wait for Verbs to add the missing query feature.

> > > > >

> > > > > If the firmware it not configured this rule must be refused, as

> > > > > there is no way in the PMD to know if the firmware is

> > > > > configured, it must rely on a device

> > > parameter.

> > > >

> > > > Let's keep the design simple, users know exactly what they are

> > > > doing and should not expecting such flow working by reading document.

> > >

> > > This is exactly the opposite, users never read documentation even

> > > today I've already spotted a new user to such documentation [1].

> >

> >   "So you expect than the user will enable a feature without reading the PMD documentation?

> >    If it is the case, the answer it pretty simple, it is the same as above, read the PMD

> documentation.

> >    Again a user using a NIC should read the documentation."

> >

> > >

> > > For this same reason a functionality not enabled by default in the

> > > firmware must not be used by the PMD.  No device parameter no feature.

> >

> > Unlike other functionality, this feature related to supporting a new

> > tunnel type, w/o fw configuration,

> > L3 VXLAN packet certainly be treated as normal packet, it doesn't hurt. How do you think?

> 

>  flow create 0 ingress eth / ipv4 / end action queue index 3 end

> 

> but the packet ends in queue 0, will it hurt?


This is the correct example: 

flow create 0 ingress pattern eth / ipv4 / udp dst is 4789 / vxlan / ipv4 / end actions rss queues 1 2  end / end

Users should never create such rule and expect it to work because it doesn't meet and VXLAN RFC.
If users want to match L3 VXLAN, read document and configure fw to get correct result.

> 

> Any rule *accepted* by the PMD *must* follow the user request, otherwise it is a bug.


I'd beg you to consider from user's perspective, the motivation of this design is to sale rte flow
by replacing device parameter, now we are making the flow usage awkward.

> 

> Add the device parameter and the according documentation.

> 

> Regards,

> 

> --

> Nélio Laranjeiro

> 6WIND
  
Nélio Laranjeiro April 19, 2018, 1:40 p.m. UTC | #11
On Thu, Apr 19, 2018 at 12:49:41PM +0000, Xueming(Steven) Li wrote:
> 
> 
> > -----Original Message-----
> > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Sent: Thursday, April 19, 2018 8:19 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > 
> > On Thu, Apr 19, 2018 at 11:53:05AM +0000, Xueming(Steven) Li wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > Sent: Thursday, April 19, 2018 7:15 PM
> > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > >
> > > > On Thu, Apr 19, 2018 at 10:21:26AM +0000, Xueming(Steven) Li wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > > > Sent: Thursday, April 19, 2018 2:56 PM
> > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN flow
> > > > > >
> > > > > > On Thu, Apr 19, 2018 at 06:20:50AM +0000, Xueming(Steven) Li wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > > > > > Sent: Wednesday, April 18, 2018 11:09 PM
> > > > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN
> > > > > > > > flow
> > > > > > > >
> > > > > > > > On Wed, Apr 18, 2018 at 02:43:30PM +0000, Xueming(Steven) Li wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > > > > > > > > Sent: Wednesday, April 18, 2018 2:49 PM
> > > > > > > > > > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > > > > > > > > > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > > > > > > > > > Subject: Re: [PATCH v4 03/11] net/mlx5: support L3 VXLAN
> > > > > > > > > > flow
> > > > > > > > > >
> > > > > > > > > > On Tue, Apr 17, 2018 at 11:14:28PM +0800, Xueming Li wrote:
> > > > > > > > > > > This patch support L3 VXLAN, no inner L2 header
> > > > > > > > > > > comparing to standard VXLAN protocol. L3 VXLAN using
> > > > > > > > > > > specific overlay UDP destination port to discriminate
> > > > > > > > > > > against standard VXLAN, FW has to be configured to
> > > > > > > > > > > support
> > > > > > > > > > > it:
> > > > > > > > > > >   sudo mlxconfig -d <device> -y s IP_OVER_VXLAN_EN=1
> > > > > > > > > > >   sudo mlxconfig -d <device> -y s
> > > > > > > > > > > IP_OVER_VXLAN_PORT=<port>
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Xueming Li <xuemingl@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
> > > > > > > > > > > 771d5f14d..d7a921dff
> > > > > > > > > > > 100644
> > > > > > > > > > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > > > > > > > > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > > > > > > > > > @@ -413,7 +413,9 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
> > > > > > > > > > >  		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
> > > > > > > > > > >  	},
> > > > > > > > > > >  	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
> > > > > > > > > > > -		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
> > > > > > > > > > > +		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
> > > > > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
> > > > > > > > > > > +			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN.
> > > > > > > > > > > +*/
> > > > > > > > > > >  		.actions = valid_actions,
> > > > > > > > > > >  		.mask = &(const struct rte_flow_item_vxlan){
> > > > > > > > > > >  			.vni = "\xff\xff\xff",
> > > > > > > > > > > --
> > > > > > > > > > > 2.13.3
> > > > > > > > > >
> > > > > > > > > > Such support must be under device parameter has it
> > > > > > > > > > depends on the configuration of the firmware.  If the
> > > > > > > > > > firmware is not correctly configured the PMD must refuse
> > > > > > > > such rule.
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Nélio Laranjeiro
> > > > > > > > > > 6WIND
> > > > > > > > >
> > > > > > > > > Are you suggesting Verbs parameter? I'm afraid we can't
> > > > > > > > > have it in short time, need new patch in later release when Verbs ready.
> > > > > > > >
> > > > > > > > Take a look at [1], this is what I mean.
> > > > > > >
> > > > > > > Enabling a new device parameter can't make L3 VXLAN packet get
> > > > > > > received if fw configuration not set.
> > > > > >
> > > > > > So you expect than the user will enable a feature without reading the PMD documentation?
> > > > > > If it is the case, the answer it pretty simple, it is the same
> > > > > > as above, read the PMD
> > > > documentation.
> > > > > >
> > > > > > > On the other hand, if fw continuation enabled and device
> > > > > > > parameter not set, packet could be received but failed to create rule.
> > > > > >
> > > > > > Again a user using a NIC should read the documentation.
> > > > >
> > > > > If a user read the document, fw should be configured correctly to enable this feature.
> > > >
> > > > And a user which does not read this document must not be able to
> > > > create rules the NIC cannot handle because the firmware is not configured.
> > > >
> > > > > > > I'm afraid that a device parameter will introduce complexity
> > > > > > > of using this feature w/o real benefits.
> > > > > >
> > > > > > Add this missing device parameter and update accordingly the
> > > > > > documentation, or wait for Verbs to add the missing query feature.
> > > > > >
> > > > > > If the firmware it not configured this rule must be refused, as
> > > > > > there is no way in the PMD to know if the firmware is
> > > > > > configured, it must rely on a device
> > > > parameter.
> > > > >
> > > > > Let's keep the design simple, users know exactly what they are
> > > > > doing and should not expecting such flow working by reading document.
> > > >
> > > > This is exactly the opposite, users never read documentation even
> > > > today I've already spotted a new user to such documentation [1].
> > >
> > >   "So you expect than the user will enable a feature without reading the PMD documentation?
> > >    If it is the case, the answer it pretty simple, it is the same as above, read the PMD
> > documentation.
> > >    Again a user using a NIC should read the documentation."
> > >
> > > >
> > > > For this same reason a functionality not enabled by default in the
> > > > firmware must not be used by the PMD.  No device parameter no feature.
> > >
> > > Unlike other functionality, this feature related to supporting a new
> > > tunnel type, w/o fw configuration,
> > > L3 VXLAN packet certainly be treated as normal packet, it doesn't hurt. How do you think?
> > 
> >  flow create 0 ingress eth / ipv4 / end action queue index 3 end
> > 
> > but the packet ends in queue 0, will it hurt?
> 
> This is the correct example: 
> 
> flow create 0 ingress pattern eth / ipv4 / udp dst is 4789 / vxlan / ipv4 / end actions rss queues 1 2  end / end
> 
> Users should never create such rule and expect it to work because it doesn't meet and VXLAN RFC.
> If users want to match L3 VXLAN, read document and configure fw to get correct result.
>
> > Any rule *accepted* by the PMD *must* follow the user request, otherwise it is a bug.
>
> I'd beg you to consider from user's perspective, the motivation of this design is to sale rte flow
> by replacing device parameter, now we are making the flow usage awkward.

DPDK application are generic, they can still create specific rules for
some devices, but has it don't have any knowledge on the underlying NIC
it must query them through the flow API and devices not supporting the
flow must answer with an error.

"Users should never" in reality the flow API is present for the exact
opposite.

Coming back to this specific patch, you are only giving more arguments
to refuse it and this, for the "user".

I cannot accept this patch as in some situation the user request cannot
be offloaded by the hardware.

There are only two possibilities for me to accept such feature in the
PMD:

1. the PMD can query the firmware and know if such feature is enabled
   (as it does for the (E)MPS and others) and thus refuse the flow if
   not.
2. a device parameter to enable such feature from the PMD perspective
   (does not mean it will be available from the hardware one).

I would prefer #1, but has you have mentioned there is no possibility
from Verbs team to provide such solution so fast.

Remains the #2 solution.  Add it and I can accept the patch.

Regards,
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 771d5f14d..d7a921dff 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -413,7 +413,9 @@  static const struct mlx5_flow_items mlx5_flow_items[] = {
 		.dst_sz = sizeof(struct ibv_flow_spec_tunnel),
 	},
 	[RTE_FLOW_ITEM_TYPE_VXLAN] = {
-		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH),
+		.items = ITEMS(RTE_FLOW_ITEM_TYPE_ETH,
+			       RTE_FLOW_ITEM_TYPE_IPV4, /* For L3 VXLAN. */
+			       RTE_FLOW_ITEM_TYPE_IPV6), /* For L3 VXLAN. */
 		.actions = valid_actions,
 		.mask = &(const struct rte_flow_item_vxlan){
 			.vni = "\xff\xff\xff",