[dpdk-dev] net/mlx5: revert multicast rule Verbs flow type

Message ID 20180213123155.44183-1-shahafs@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Shahaf Shuler Feb. 13, 2018, 12:31 p.m. UTC
  This is to revert the following commits:
commit da646bd93888 ("net/mlx5: fix all multi verification code position")
commit 0a40a1363a4d ("net/mlx5: fix flow type for allmulti rules")

The last one introduced a bug in the following diff:
@ -1262,6 +1274,7 @@ struct ibv_spec_header {
                eth.val.ether_type &= eth.mask.ether_type;
        }
        mlx5_flow_create_copy(parser, &eth, eth_size);
+       parser->allmulti = eth.val.dst_mac[0] & 1;
        return 0;
 }

As broadcast rules will be considered of type allmulti as well.

The patch was originally intended to enable VF to receive all multicast
traffic by using the IBV_FLOW_ATTR_MC_DEFAULT flow type.
Since the support was removed from the kernel there is no point with
fixing this issue, hence the revert.

Cc: nelio.laranjeiro@6wind.com
Cc: rasland@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 13 -------------
 1 file changed, 13 deletions(-)
  

Comments

Nélio Laranjeiro Feb. 13, 2018, 12:41 p.m. UTC | #1
On Tue, Feb 13, 2018 at 02:31:55PM +0200, Shahaf Shuler wrote:
> This is to revert the following commits:
> commit da646bd93888 ("net/mlx5: fix all multi verification code position")
> commit 0a40a1363a4d ("net/mlx5: fix flow type for allmulti rules")
> 
> The last one introduced a bug in the following diff:
> @ -1262,6 +1274,7 @@ struct ibv_spec_header {
>                 eth.val.ether_type &= eth.mask.ether_type;
>         }
>         mlx5_flow_create_copy(parser, &eth, eth_size);
> +       parser->allmulti = eth.val.dst_mac[0] & 1;
>         return 0;
>  }
> 
> As broadcast rules will be considered of type allmulti as well.
> 
> The patch was originally intended to enable VF to receive all multicast
> traffic by using the IBV_FLOW_ATTR_MC_DEFAULT flow type.
> Since the support was removed from the kernel there is no point with
> fixing this issue, hence the revert.
> 
> Cc: nelio.laranjeiro@6wind.com
> Cc: rasland@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>

Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

> ---
>  drivers/net/mlx5/mlx5_flow.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index f88ece123..323dab723 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -391,7 +391,6 @@ static const struct mlx5_flow_items mlx5_flow_items[] = {
>  /** Structure to pass to the conversion function. */
>  struct mlx5_flow_parse {
>  	uint32_t inner; /**< Set once VXLAN is encountered. */
> -	uint32_t allmulti:1; /**< Set once allmulti dst MAC is encountered. */
>  	uint32_t create:1;
>  	/**< Whether resources should remain after a validate. */
>  	uint32_t drop:1; /**< Target is a drop queue. */
> @@ -1128,17 +1127,6 @@ priv_flow_convert(struct priv *priv,
>  			attr->priority +
>  			hash_rxq_init[parser->layer].flow_priority;
>  	}
> -	if (parser->allmulti &&
> -	    parser->layer == HASH_RXQ_ETH) {
> -		for (i = 0; i != hash_rxq_init_n; ++i) {
> -			if (!parser->queue[i].ibv_attr)
> -				continue;
> -			if (parser->queue[i].ibv_attr->num_of_specs != 1)
> -				break;
> -			parser->queue[i].ibv_attr->type =
> -						IBV_FLOW_ATTR_MC_DEFAULT;
> -		}
> -	}
>  exit_free:
>  	/* Only verification is expected, all resources should be released. */
>  	if (!parser->create) {
> @@ -1246,7 +1234,6 @@ mlx5_flow_create_eth(const struct rte_flow_item *item,
>  		eth.val.ether_type &= eth.mask.ether_type;
>  	}
>  	mlx5_flow_create_copy(parser, &eth, eth_size);
> -	parser->allmulti = eth.val.dst_mac[0] & 1;
>  	return 0;
>  }
>  
> -- 
> 2.12.0
>
  
Thomas Monjalon Feb. 13, 2018, 4:03 p.m. UTC | #2
13/02/2018 13:41, Nélio Laranjeiro:
> On Tue, Feb 13, 2018 at 02:31:55PM +0200, Shahaf Shuler wrote:
> > This is to revert the following commits:
> > commit da646bd93888 ("net/mlx5: fix all multi verification code position")
> > commit 0a40a1363a4d ("net/mlx5: fix flow type for allmulti rules")
> > 
> > The last one introduced a bug in the following diff:
> > @ -1262,6 +1274,7 @@ struct ibv_spec_header {
> >                 eth.val.ether_type &= eth.mask.ether_type;
> >         }
> >         mlx5_flow_create_copy(parser, &eth, eth_size);
> > +       parser->allmulti = eth.val.dst_mac[0] & 1;
> >         return 0;
> >  }
> > 
> > As broadcast rules will be considered of type allmulti as well.
> > 
> > The patch was originally intended to enable VF to receive all multicast
> > traffic by using the IBV_FLOW_ATTR_MC_DEFAULT flow type.
> > Since the support was removed from the kernel there is no point with
> > fixing this issue, hence the revert.
> > 
> > Cc: nelio.laranjeiro@6wind.com
> > Cc: rasland@mellanox.com
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> 
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Applied, thanks
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index f88ece123..323dab723 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -391,7 +391,6 @@  static const struct mlx5_flow_items mlx5_flow_items[] = {
 /** Structure to pass to the conversion function. */
 struct mlx5_flow_parse {
 	uint32_t inner; /**< Set once VXLAN is encountered. */
-	uint32_t allmulti:1; /**< Set once allmulti dst MAC is encountered. */
 	uint32_t create:1;
 	/**< Whether resources should remain after a validate. */
 	uint32_t drop:1; /**< Target is a drop queue. */
@@ -1128,17 +1127,6 @@  priv_flow_convert(struct priv *priv,
 			attr->priority +
 			hash_rxq_init[parser->layer].flow_priority;
 	}
-	if (parser->allmulti &&
-	    parser->layer == HASH_RXQ_ETH) {
-		for (i = 0; i != hash_rxq_init_n; ++i) {
-			if (!parser->queue[i].ibv_attr)
-				continue;
-			if (parser->queue[i].ibv_attr->num_of_specs != 1)
-				break;
-			parser->queue[i].ibv_attr->type =
-						IBV_FLOW_ATTR_MC_DEFAULT;
-		}
-	}
 exit_free:
 	/* Only verification is expected, all resources should be released. */
 	if (!parser->create) {
@@ -1246,7 +1234,6 @@  mlx5_flow_create_eth(const struct rte_flow_item *item,
 		eth.val.ether_type &= eth.mask.ether_type;
 	}
 	mlx5_flow_create_copy(parser, &eth, eth_size);
-	parser->allmulti = eth.val.dst_mac[0] & 1;
 	return 0;
 }