Michael S. Tsirkin
2021-Mar-01 16:12 UTC
[PATCH linux-next 7/9] vdpa/mlx5: Provide device generated random MAC address
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 at 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 at nvidia.com> > > > > > Reviewed-by: Parav Pandit <parav at nvidia.com> > > > > > > > > > > > > Acked-by: Jason Wang <jasowang at 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) > > > > > >
Jason Wang
2021-Mar-02 04:10 UTC
[PATCH linux-next 7/9] vdpa/mlx5: Provide device generated random MAC address
On 2021/3/2 12:12 ??, Michael S. Tsirkin wrote:> 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 at 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 at nvidia.com> >>>>>> Reviewed-by: Parav Pandit <parav at nvidia.com> >>>>> >>>>> Acked-by: Jason Wang <jasowang at 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?I think the point is that, if userspace doens't specify a mac, it can still work. Thanks> > >>> 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)