From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yongseok Koh <yskoh@mellanox.com>
Subject: Re: [PATCH v2 6/7] net/mlx5: fix reception when VLAN is
	added
Date: Mon, 23 Oct 2017 12:25:45 -0700
Message-ID: <20171023192544.GA19386@yongseok-MBP.local>
References: <cover.1508417257.git.nelio.laranjeiro@6wind.com>
 <cover.1508768946.git.nelio.laranjeiro@6wind.com>
 <d4ed88d2ba0c9905b832d2d6577368b49ac73564.1508768946.git.nelio.laranjeiro@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from EUR02-AM5-obe.outbound.protection.outlook.com
 (mail-eopbgr00049.outbound.protection.outlook.com [40.107.0.49])
 by dpdk.org (Postfix) with ESMTP id 4AB301B6A5
 for <dev@dpdk.org>; Mon, 23 Oct 2017 21:26:01 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <d4ed88d2ba0c9905b832d2d6577368b49ac73564.1508768946.git.nelio.laranjeiro@6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Oct 23, 2017 at 04:49:56PM +0200, Nelio Laranjeiro wrote:
> @@ -261,75 +284,59 @@ priv_dev_traffic_enable(struct priv *priv, struct rte_eth_dev *dev)
>  		};
>  
>  		claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
> -	} else if (dev->data->all_multicast) {
> +		return 0;
> +	}
> +	if (dev->data->all_multicast) {
>  		struct rte_flow_item_eth multicast = {
>  			.dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> -			.src.addr_bytes = "\x01\x00\x00\x00\x00\x00",
> +			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
>  			.type = 0,
>  		};
>  
>  		claim_zero(mlx5_ctrl_flow(dev, &multicast, &multicast));

Just curious. No need to consider VLAN for multicast here?

> -	} else {
> -		struct rte_flow_item_eth bcast = {
> -			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> -		};
> -		struct rte_flow_item_eth ipv6_multi_spec = {
> -			.dst.addr_bytes = "\x33\x33\x00\x00\x00\x00",
> -		};
> -		struct rte_flow_item_eth ipv6_multi_mask = {
> -			.dst.addr_bytes = "\xff\xff\x00\x00\x00\x00",
> -		};
> -		struct rte_flow_item_eth unicast = {
> -			.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
> -		};
> -		struct rte_flow_item_eth unicast_mask = {
> -			.dst.addr_bytes = "\xff\xff\xff\xff\xff\xff",
> -		};
> -		const unsigned int vlan_filter_n = priv->vlan_filter_n;
> -		const struct ether_addr cmp = {
> -			.addr_bytes = "\x00\x00\x00\x00\x00\x00",
> -		};
> -		unsigned int i;
> -		unsigned int j;
> -		unsigned int unicast_flow = 0;
> -		int ret;
> -
> -		for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
> -			struct ether_addr *mac = &dev->data->mac_addrs[i];
> +	}
> +	for (i = 0; i != MLX5_MAX_MAC_ADDRESSES; ++i) {
> +		struct ether_addr *mac = &dev->data->mac_addrs[i];
>  
> -			if (!memcmp(mac, &cmp, sizeof(*mac)))
> -				continue;
> -			memcpy(&unicast.dst.addr_bytes,
> -			       mac->addr_bytes,
> -			       ETHER_ADDR_LEN);
> -			for (j = 0; j != vlan_filter_n; ++j) {
> -				uint16_t vlan = priv->vlan_filter[j];
> +		if (!memcmp(mac, &cmp, sizeof(*mac)))
> +			continue;
> +		memcpy(&unicast.dst.addr_bytes,
> +		       mac->addr_bytes,
> +		       ETHER_ADDR_LEN);
> +		for (j = 0; j != vlan_filter_n; ++j) {
> +			uint16_t vlan = priv->vlan_filter[j];
>  
> -				struct rte_flow_item_vlan vlan_spec = {
> -					.tci = rte_cpu_to_be_16(vlan),
> -				};
> -				struct rte_flow_item_vlan vlan_mask = {
> -					.tci = 0xffff,
> -				};
> +			struct rte_flow_item_vlan vlan_spec = {
> +				.tci = rte_cpu_to_be_16(vlan),
> +			};
> +			struct rte_flow_item_vlan vlan_mask = {
> +				.tci = 0xffff,
> +			};
>  
> -				ret = mlx5_ctrl_flow_vlan(dev, &unicast,
> -							  &unicast_mask,
> -							  &vlan_spec,
> -							  &vlan_mask);
> -				if (ret)
> -					goto error;
> -				unicast_flow = 1;
> -			}
> -			if (!vlan_filter_n) {
> -				ret = mlx5_ctrl_flow(dev, &unicast,
> -						     &unicast_mask);
> -				if (ret)
> -					goto error;
> -				unicast_flow = 1;
> -			}
> +			ret = mlx5_ctrl_flow_vlan(dev, &unicast,
> +						  &unicast_mask,
> +						  &vlan_spec,
> +						  &vlan_mask);
> +			if (ret)
> +				goto error;
> +			ret = mlx5_ctrl_flow_vlan(dev, &bcast, &bcast,
> +						  &vlan_spec, &vlan_mask);
> +			if (ret)
> +				goto error;
> +			ret = mlx5_ctrl_flow_vlan(dev, &ipv6_multi_spec,
> +						  &ipv6_multi_mask,
> +						  &vlan_spec, &vlan_mask);

These (bcast and ipv6_multi_mask) can be duplicated multiple times if there are
multiple MAC addrs, is that intended?


Thanks,
Yongseok