Re: [PATCH linux-next 7/9] vdpa/mlx5: Provide device generated random MAC address

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Mar 01, 2021 at 03:19:51PM +0200, Eli Cohen wrote:
> On Mon, Mar 01, 2021 at 08:09:48AM -0500, Michael S. Tsirkin wrote:
> > On Mon, Mar 01, 2021 at 09:08:28AM +0200, Eli Cohen wrote:
> > > On Wed, Feb 24, 2021 at 05:11:23PM +0800, Jason Wang wrote:
> > > > 
> > > > On 2021/2/24 2:18 下午, Parav Pandit wrote:
> > > > > From: Eli Cohen <elic@nvidia.com>
> > > > > 
> > > > > Use a randomly generated MAC address to be applied in case it is not
> > > > > configured by management tool.
> > > > > 
> > > > > The value queried through mlx5_query_nic_vport_mac_address() is not
> > > > > relelavnt to vdpa since it is the mac that should be used by the regular
> > > > > NIC driver.
> > > > > 
> > > > > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > > > > Reviewed-by: Parav Pandit <parav@nvidia.com>
> > > > 
> > > > 
> > > > Acked-by: Jason Wang <jasowang@redhat.com>
> > > > 
> > > > 
> > > > > ---
> > > > >   drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 +----
> > > > >   1 file changed, 1 insertion(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > index b67bba581dfd..ece2183e7b20 100644
> > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > @@ -2005,10 +2005,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
> > > > >   	if (err)
> > > > >   		goto err_mtu;
> > > > > -	err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
> > > > > -	if (err)
> > > > > -		goto err_mtu;
> > > > > -
> > > > > +	eth_random_addr(config->mac);
> > > 
> > > I think this patch is missing setting VIRTIO_NET_F_MTU. I will post v2
> > > with the other fixes in this series.
> > 
> > I don't really understand why this is a good idea.
> > 
> > If userspace wants a random mac it can set it, with this
> > patch it is impossible to know whether the mac is
> > a hardware one (which will be persistent e.g. across reboots)
> > or a random one.
> > 
> 
> You can still get a persistent MAC set by the vdpa tool. e.g
> 
> vdpa dev config set vdpa0 mac 00:11:22:33:44:55
> 
> If you don't use vdpa tool, the device assigns a random MAC. This MAC is
> used to filter imcoming unicast traffic (done in a subsequent patch).

Well previously device learned the MAC from outgoing traffic
and used that for the filter.
Is changing that to a random value really all that useful as
a default?  Why not do the randomization in userspace?


> > E.g. there is a patch configuring a userspace supplied
> > mac if the hardware mac is zero.
> > 
> > This patch will break it.
> > 
> > > > >   	mvdev->vdev.dma_dev = mdev->device;
> > > > >   	err = mlx5_vdpa_alloc_resources(&ndev->mvdev);
> > > > >   	if (err)
> > > > 
> > 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization