Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: fix reception when VLAN is added

["Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>]

On Tue, Oct 24, 2017 at 09:11:42AM +0200, Nélio Laranjeiro wrote:
> On Mon, Oct 23, 2017 at 12:25:45PM -0700, Yongseok Koh wrote:
> > 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?
> 
> According to the lib documentation no [1]
> 
>  "Enable the receipt of any multicast frame by an Ethernet device"
> 
> > [...]
> > These (bcast and ipv6_multi_mask) can be duplicated multiple times if there are
> > multiple MAC addrs, is that intended?
> 
> There is in fact an issue in this series, it does not match my final code.
> 
> I'll send a v3.
> 
> [1] http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n2304

I've wrongly read your last comment, the patch is correct, it won't add
multiple time the broadcast multicast, it will add one per expected
VLAN.

Example:

 testpmd> set promisc all off
 testpmd> set allmulti all off
 testpmd> rx_vlan add 0 1330
 testpmd> rx_vlan add 0 1331

Will cause this code to add a broadcast flow with VLAN TCI 1330 and
another broadcast flow with VLAN TCI 1331, others won't be received.

The user will only receive broadcast packets with VLAN TCI 1330 and
1331.  It is what he expects.

In case not VLAN is configured, the broadcast and muilticast flow
insertion are under the condition:

 "if (!dev->data->all_multicast && !vlan_filter_n)"

Thanks,

-- 
Nélio Laranjeiro
6WIND