Re: [PATCH v3 1/3] net/mlx5: use Netlink to add/remove MAC addresses

[Shahaf Shuler <shahafs@mellanox.com>]

Thursday, March 22, 2018 9:35 AM, Shahaf Shuler:
> Hi Nelio,
> 
> Wednesday, March 21, 2018 3:40 PM, Nelio Laranjeiro:
> > VF devices are not able to receive traffic unless it fully requests it
> > though Netlink.  This will cause the request to be processed by the PF
> > which will add/remove the MAC address to the VF table if the VF is trusted.
> >
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > ---
> >  drivers/net/mlx5/Makefile      |   1 +
> >  drivers/net/mlx5/mlx5.c        |  22 ++
> >  drivers/net/mlx5/mlx5.h        |  13 +
> >  drivers/net/mlx5/mlx5_ethdev.c |  27 +++
> >  drivers/net/mlx5/mlx5_mac.c    |  25 +-
> >  drivers/net/mlx5/mlx5_nl.c     | 530
> > +++++++++++++++++++++++++++++++++++++++++
> > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > faacfd9d6..6a7e9f310 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h

Sorry sent to early. 

Another general question - is there required netlink version exists on all the relevant distros? What is the minimal kernel version required? 

> > @@ -78,6 +78,7 @@ struct mlx5_dev_config {
> >  	unsigned int hw_vlan_strip:1; /* VLAN stripping is supported. */
> >  	unsigned int hw_fcs_strip:1; /* FCS stripping is supported. */
> >  	unsigned int hw_padding:1; /* End alignment padding is supported.
> > */
> > +	unsigned int vf:1; /* This is a VF. */
> >  	unsigned int mps:2; /* Multi-packet send supported mode. */
> >  	unsigned int tunnel_en:1;
> >  	/* Whether tunnel stateless offloads are supported. */ @@ -154,6
> > +155,8 @@ struct priv {
> >  	struct mlx5_dev_config config; /* Device configuration. */
> >  	struct mlx5_verbs_alloc_ctx verbs_alloc_ctx;
> >  	/* Context for Verbs allocator. */
> > +	int nl_socket; /* Netlink socket. */
> > +	uint32_t nl_sn; /* Netlink message sequence number. */
> >  };
> >
> >  /* mlx5.c */
> > @@ -163,6 +166,7 @@ int mlx5_getenv_int(const char *);
> >  /* mlx5_ethdev.c */
> >
> >  int mlx5_get_ifname(const struct rte_eth_dev *dev, char
> > (*ifname)[IF_NAMESIZE]);
> > +int mlx5_ifindex(const struct rte_eth_dev *dev);
> >  int mlx5_ifreq(const struct rte_eth_dev *dev, int req, struct ifreq
> > *ifr);  int mlx5_get_mtu(struct rte_eth_dev *dev, uint16_t *mtu);  int
> > mlx5_set_flags(struct rte_eth_dev *dev, unsigned int keep, @@ -297,4
> > +301,13 @@ struct mlx5_mr *mlx5_mr_get(struct rte_eth_dev *dev,
> struct
> > rte_mempool *mp);  int mlx5_mr_release(struct mlx5_mr *mr);  int
> > mlx5_mr_verify(struct rte_eth_dev *dev);
> >
> > +/* mlx5_nl.c */
> > +
> > +int mlx5_nl_init(uint32_t nlgroups);
> > +int mlx5_nl_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr
> > +*mac); int mlx5_nl_mac_addr_remove(struct rte_eth_dev *dev, struct
> > +ether_addr *mac); void mlx5_nl_mac_addr_flush(struct rte_eth_dev
> > *dev);
> 
> I think the two below should be introduced only on the next patch of the
> series.
> 
> > +int mlx5_nl_promisc(struct rte_eth_dev *dev, int enable); int
> > +mlx5_nl_allmulti(struct rte_eth_dev *dev, int enable);
> > +
> >  #endif /* RTE_PMD_MLX5_H_ */
> 
> [...]
> 
> > +/**
> > + * Flush all added MAC addresses.
> > + *
> > + * @param dev
> > + *   Pointer to Ethernet device.
> > + */
> > +void
> > +mlx5_nl_mac_addr_flush(struct rte_eth_dev *dev) {
> > +	int i;
> > +	const struct ether_addr mac_null = { .addr_bytes = { 0 }, };
> > +
> > +	for (i = MLX5_MAX_MAC_ADDRESSES - 1; i >= 0; --i) {
> > +		struct ether_addr *m = &dev->data->mac_addrs[i];
> > +
> > +		if (memcmp(&mac_null, m, ETHER_ADDR_LEN))
> > +			mlx5_nl_mac_addr_remove(dev, m);
> > +	}
> > +}
> 
> What if the DPDK process is terminated ungracefully? I think the MAC table
> will remain with all the MACs which were added.
> The next run of the process may have un-expected results.
> 
> Should we flush the neighbor mac table also on probe to make sure only the
> VF mac exists?
> 
> > --
> > 2.11.0