Message ID | 20230122100526.2302556-2-lvivier@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp1090345wrn; Sun, 22 Jan 2023 02:20:15 -0800 (PST) X-Google-Smtp-Source: AMrXdXtjjIfkuMI5DUtYywKCp411NIW5kHfYzrPErBOJ3ixby0+NhGPZGY1QCXxqc0xzwi2s6PkJ X-Received: by 2002:a17:90a:d344:b0:229:ef6c:4139 with SMTP id i4-20020a17090ad34400b00229ef6c4139mr11871239pjx.22.1674382815536; Sun, 22 Jan 2023 02:20:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674382815; cv=none; d=google.com; s=arc-20160816; b=dJrzl4TYPmMkINJV1PFZhqEQ2nf2/az2PIdL1+tlNx+3VKoRMhZRuLGdp0km9RmjUM EUrV3GrauZpzBLVTSbADTLA8DhJV3DzK78Ae5iYqidnCBJhvN+ibw9QlWuv1a+mPTJaJ ag7L34Vz+t0/S+x8TlC6I1s8Br8HuqSGxjWUT+YNVM9lxp7wHxkBWNJP8mQt58E9U5yc mJRmouDKuymwRC0dQHdTpMSIIL8AXQqPvXR+A0fjgfx6YaJ2nLeqRjvMOogWMnCDItJn I43x/Oewa84fk5YTXuEF7x0yEEmRvJGiMUnNf15S6Cv92vFZ9QpivPv8nwhHlnrPwypf HxWQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:ui-outboundreport:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from; bh=TTAF4NUG61vUWH38H31o5TVRlO0o1iGHlDoc16+Z74A=; b=ZRY221da01ZWu5Jf55YYo5tO6Zhh/MAjkX5nHlMDzNObh3uUrS+tGVkEsCZqGsYm4C ZE3Ssoe7HE3ERRQ8Zd1dOAznzSuJMY8HIhvLW9+1hSdH0lCN80TyxIFwWWiktFC4IKof la2Jp3yuqN7bRuVArktrosheviFimAG/myjE97Bw1h2J+cYO895j4N86ktiGrxDhC4DC aOJSe9kNtWfMBGLX3IOWOkcDRTJFyiOVZ01yphsnNCq2Tx5iBzoPtrZJ8DjHaejAFGJq KxHzaVM8zLnb0WfqT/Kbt9yXRQhm3ayZgDifcecQR22AWLuNv13gHmL+9Iq+Hf+RiYAe Yn0Q== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id bf18-20020a17090b0b1200b00229189b6fd9si8062993pjb.85.2023.01.22.02.20.03; Sun, 22 Jan 2023 02:20:15 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229963AbjAVKGj (ORCPT <rfc822;ariel.simulevski@gmail.com> + 99 others); Sun, 22 Jan 2023 05:06:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47776 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229890AbjAVKGd (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 22 Jan 2023 05:06:33 -0500 Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.126.130]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A084B1CF6C; Sun, 22 Jan 2023 02:05:45 -0800 (PST) Received: from lenovo-t14s.redhat.com ([82.142.8.70]) by mrelayeu.kundenserver.de (mreue012 [212.227.15.167]) with ESMTPSA (Nemesis) id 1MRVy9-1p53Hj1ciT-00NSmk; Sun, 22 Jan 2023 11:05:30 +0100 From: Laurent Vivier <lvivier@redhat.com> To: linux-kernel@vger.kernel.org Cc: "Michael S. Tsirkin" <mst@redhat.com>, Parav Pandit <parav@nvidia.com>, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, Eli Cohen <elic@nvidia.com>, Jason Wang <jasowang@redhat.com>, Gautam Dawar <gautam.dawar@xilinx.com>, Cindy Lu <lulu@redhat.com>, "David S. Miller" <davem@davemloft.net>, =?utf-8?q?Eugenio_P=C3=A9rez?= <eperezma@redhat.com> Subject: [PATCH 1/4] virtio_net: notify MAC address change on device initialization Date: Sun, 22 Jan 2023 11:05:23 +0100 Message-Id: <20230122100526.2302556-2-lvivier@redhat.com> X-Mailer: git-send-email 2.39.0 In-Reply-To: <20230122100526.2302556-1-lvivier@redhat.com> References: <20230122100526.2302556-1-lvivier@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K1:OvJkQJpHxHaDCPIk3W9i4eIYijK92aR7qOa7YFbWU4FKa/YfJkR E/a6z8dxmCPAKztm0ukyh/oHdFFG7zYa7NElzj3pnNbO1BMzo/7IgFa9I1NikV6ZzztbCV3 E2fJahWqrWBM5gzAUbZWhNFFhiRGzyTdLzqCq6fUs9xJ4pp7Ki6jrCSc0gh3nZNZ5YSb5mm D7PIF++SRA45BlYtN3wpw== UI-OutboundReport: notjunk:1;M01:P0:uI0nKc6Xg14=;V2mW9Yjl2DK8tq+0C6gzIszIg/5 o3q1eas0KF63Bj1HGMfGHZKC0xuc5cpaegmNi9jljig1AveJMDqZ+xRSp/3J5VSVrPcrY3PXw hDtqkTmTbonvTFiqsdrU/uFHuMUVr368KTf3JkgY3MbEtqrdlb46MPQ9ypvUaaoQo8LQF7AXW TX9MwGXOBJy2akXwI1VhwiWkPKGOzrAPdF2UT/GIOlfPoyyhwTMbmWf7pTMf+KPBz8sB3KwOx zyYQ877hiE8zBrwaehPa6PL7QLxIgTKbS9c1Ceo+I6rzs7prqVP7FhZQiixKmSXKIYMCKwa6m wj1A8adlm3mowJ8f5pTMW1LinSu0Swa7ErS0nmmq30z9SwufZx1imMcf1Xpv7OH6y9wnZ5GnW xdUGgBKDrqNc1jLJ/tV+PbkaVUtWTw+mifPUTFIn/rzXQowAqFZ/EPsCJ+s0/4wDQ3UM8zJS+ +XicJtU2EGEPPRbvh9gE28qLvYHibQz7QOsaOMGTeuMViVPvMoR9/1wJPFLF7kaNeraAlvXA7 db/rKUxbBngrNrndSgEK/wlhhXNM4U3wt4K3Ciij1pDt4XoMF65rSSN0Owb4x0fYQWgyN9YG5 4V3MlTq/JdA643f4WoBmkCL7ISjkuQJR7VqL4fd3F/tcdKomV6mSHVy5Y5q3NHT7VAWymh/ry 0Lewed5/mAWn7fJIz1fNou7E+NZoOGGS9SHTOAi8bw== X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1755717635110147346?= X-GMAIL-MSGID: =?utf-8?q?1755717635110147346?= |
Series |
virtio_net: vdpa: update MAC address when it is generated by virtio-net
|
|
Commit Message
Laurent Vivier
Jan. 22, 2023, 10:05 a.m. UTC
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(+)
Comments
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. > + virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) { > + struct scatterlist sg; > + > + sg_init_one(&sg, dev->dev_addr, dev->addr_len); > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC, > + VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) { > + dev_warn(&vdev->dev, "Failed to update MAC address.\n"); > + } > + } > + > return 0; > > free_unregister_netdev:
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)"? Thanks, Laurent
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 >
On Sun, 22 Jan 2023 15:47:08 +0200 Eli Cohen wrote: > > @@ -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. +1, FWIW
On 1/24/23 04:31, Jakub Kicinski wrote: > On Sun, 22 Jan 2023 15:47:08 +0200 Eli Cohen wrote: >>> @@ -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. > > +1, FWIW > v2 sent, but I rely on virtio_has_feature(vdev, VIRTIO_NET_F_MAC) to know if the MAC address is provided by the device or not: https://lore.kernel.org/lkml/20230123120022.2364889-2-lvivier@redhat.com/T/#me9211516e12771001e0346818255c9fb48a2bf4a Thanks, Laurent
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 && + virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) { + struct scatterlist sg; + + sg_init_one(&sg, dev->dev_addr, dev->addr_len); + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC, + VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) { + dev_warn(&vdev->dev, "Failed to update MAC address.\n"); + } + } + return 0; free_unregister_netdev: