Messages in this thread | | | Date | Mon, 23 Jan 2023 12:05:05 +0200 | Subject | Re: [PATCH 1/4] virtio_net: notify MAC address change on device initialization | From | Eli Cohen <> |
| |
On 23/01/2023 11:52, Laurent Vivier wrote: > On 1/22/23 14:47, Eli Cohen wrote: >> >> On 22/01/2023 12:05, Laurent Vivier wrote: >>> In virtnet_probe(), if the device doesn't provide a MAC address the >>> driver assigns a random one. >>> As we modify the MAC address we need to notify the device to allow it >>> to update all the related information. >>> >>> The problem can be seen with vDPA and mlx5_vdpa driver as it doesn't >>> assign a MAC address by default. The virtio_net device uses a random >>> MAC address (we can see it with "ip link"), but we can't ping a net >>> namespace from another one using the virtio-vdpa device because the >>> new MAC address has not been provided to the hardware. >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> --- >>> drivers/net/virtio_net.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>> index 7723b2a49d8e..25511a86590e 100644 >>> --- a/drivers/net/virtio_net.c >>> +++ b/drivers/net/virtio_net.c >>> @@ -3800,6 +3800,8 @@ static int virtnet_probe(struct virtio_device >>> *vdev) >>> eth_hw_addr_set(dev, addr); >>> } else { >>> eth_hw_addr_random(dev); >>> + dev_info(&vdev->dev, "Assigned random MAC address %pM\n", >>> + dev->dev_addr); >>> } >>> /* Set up our device-specific information */ >>> @@ -3956,6 +3958,18 @@ static int virtnet_probe(struct virtio_device >>> *vdev) >>> pr_debug("virtnet: registered device %s with %d RX and TX >>> vq's\n", >>> dev->name, max_queue_pairs); >>> + /* a random MAC address has been assigned, notify the device */ >>> + if (dev->addr_assign_type == NET_ADDR_RANDOM && >> Maybe it's better to not count on addr_assign_type and use a local >> variable to indicate that virtnet_probe assigned random MAC. The >> reason is that the hardware driver might have done that as well and >> does not need notification. > > eth_hw_addr_random() sets explicitly NET_ADDR_RANDOM, while > eth_hw_addr_set() doesn't change addr_assign_type so it doesn't seem > this value is set by the hardware driver. > > So I guess it's the default value (NET_ADDR_PERM) in this case (even > if it's a random address from the point of view of the hardware). > > If you prefer I can replace it by "!virtio_has_feature(vdev, > VIRTIO_NET_F_MAC)"? > My point is this. What if the hardware driver decides to choose a random address by calling eth_hw_addr_random(). In this case dev->addr_assign_type = NET_ADDR_RANDOM is executed unconditionally. But now you will needlessly execute the control command because the hardware address already knows about this address.
> Thanks, > Laurent >
| |