diff mbox

[dpdk-dev] net/mlx5: fix parsing all-multicast from flow item

Message ID 20180111075153.33537-1-yskoh@mellanox.com (mailing list archive)
State Rejected, archived
Delegated to: Shahaf Shuler
Headers

Checks

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

Commit Message

Yongseok Koh Jan. 11, 2018, 7:51 a.m. UTC 
  As the dst_mac of allmulti is already masked with the mask, it has 0x01 in
the first octet. Checking the least significant bit only can't distinguish
it from broadcast or IPv6 multicast.

Fixes: bb47fb6e6067 ("net/mlx5: fix flow type for allmulti rules")
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nélio Laranjeiro Jan. 11, 2018, 8:20 a.m. UTC | #1 
On Wed, Jan 10, 2018 at 11:51:53PM -0800, Yongseok Koh wrote:
> As the dst_mac of allmulti is already masked with the mask, it has 0x01 in
> the first octet. Checking the least significant bit only can't distinguish
> it from broadcast or IPv6 multicast.
>
> Fixes: bb47fb6e6067 ("net/mlx5: fix flow type for allmulti rules")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 305b2ec01..d01c8069b 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -1281,7 +1281,7 @@ 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;
> +	parser->allmulti = eth.val.dst_mac[0] == 0x01;
>  	return 0;
>  }
>  
> -- 
> 2.11.0
> 

Seems you are introducing a bug, for broadcast Mac addresses, this will
not work i.e. 0xff != 0x01 but it as the multicast bit set.  From my
understanding, Verbs flow attribute must also be modified in such
situation.

Are you sure about this change?

Thanks,
Yongseok Koh Jan. 12, 2018, 10:36 p.m. UTC | #2 
> On Jan 11, 2018, at 12:20 AM, Nélio Laranjeiro <nelio.laranjeiro@6wind.com> wrote:

> 

> On Wed, Jan 10, 2018 at 11:51:53PM -0800, Yongseok Koh wrote:

>> As the dst_mac of allmulti is already masked with the mask, it has 0x01 in

>> the first octet. Checking the least significant bit only can't distinguish

>> it from broadcast or IPv6 multicast.

>> 

>> Fixes: bb47fb6e6067 ("net/mlx5: fix flow type for allmulti rules")

>> Cc: stable@dpdk.org

>> 

>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

>> ---

>> drivers/net/mlx5/mlx5_flow.c | 2 +-

>> 1 file changed, 1 insertion(+), 1 deletion(-)

>> 

>> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c

>> index 305b2ec01..d01c8069b 100644

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

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

>> @@ -1281,7 +1281,7 @@ 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;

>> +	parser->allmulti = eth.val.dst_mac[0] == 0x01;

>> 	return 0;

>> }

>> 

>> -- 

>> 2.11.0

>> 

> 

> Seems you are introducing a bug, for broadcast Mac addresses, this will

> not work i.e. 0xff != 0x01 but it as the multicast bit set.  From my

> understanding, Verbs flow attribute must also be modified in such

> situation.

> 

> Are you sure about this change?


Indeed. I was trying to fix a bug about the control flow. In
priv_dev_traffic_enable(), if VLAN filter is configured, it isn't properly set
for broadcast. Looks like code should be changed a lot regarding allmulti. And I
heard Raslan is preparing it. I'm taking back this patch so that Raslan include
the fix in his patch set.

Thanks,
Yongseok
diff mbox

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 305b2ec01..d01c8069b 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -1281,7 +1281,7 @@  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;
+	parser->allmulti = eth.val.dst_mac[0] == 0x01;
 	return 0;
 }