From nobody Thu May 2 01:16:11 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1571363996; cv=none; d=zoho.com; s=zohoarc; b=mmxpUlaLWpLU0yfcuOOdnfNEe11VlCSAPECvEBG5RQ1TdCFxNI2+BkXFKKTThUAGK1zLwVJx3FRMjGh9Z9cOmGpuEWVooZK3l6mX+t6Gijwlp/gMmgwnqRisCThuRzM1yi+Rt7/lsqhFukaJmWDHZjqYCVIajR/oQgfLbyt5Rs4= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1571363996; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=yDw2Sfq8e+nupVR+gpAfA428/vTnYb4Gg/qVsH5MZgo=; b=ZCjl/VNKPmvdAM1OKF0nxk4dY72ueKRo3Nmgh4CfmPLA4d/WtV4MQNRfnoAPzmPkkY01hgV6FzsJ6RswEtXFcYaST917kvPSuKuB8jG8t37ev2cxU01UQEH/kq6hr1EAiRtIUl5tptzebATkaPUBwY4TGDCPj3QSYoqxpQQZ0+g= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 15713639965699.650862163330203; Thu, 17 Oct 2019 18:59:56 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C84B5300157A; Fri, 18 Oct 2019 01:59:54 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9AFCF60BE1; Fri, 18 Oct 2019 01:59:54 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 50A234E58A; Fri, 18 Oct 2019 01:59:54 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x9I1xrvI027696 for ; Thu, 17 Oct 2019 21:59:53 -0400 Received: by smtp.corp.redhat.com (Postfix) id 4AF5160C63; Fri, 18 Oct 2019 01:59:53 +0000 (UTC) Received: from tilapia.redhat.com (ovpn-124-27.rdu2.redhat.com [10.10.124.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0769560C4E for ; Fri, 18 Oct 2019 01:59:52 +0000 (UTC) From: Laine Stump To: libvir-list@redhat.com Date: Thu, 17 Oct 2019 21:59:48 -0400 Message-Id: <20191018015949.457017-2-laine@laine.org> In-Reply-To: <20191018015949.457017-1-laine@laine.org> References: <20191018015949.457017-1-laine@laine.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCHv2 1/2] util: allow sending mac addr to virNetNewLink without ifindex X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Fri, 18 Oct 2019 01:59:55 +0000 (UTC) Content-Type: text/plain; charset="utf-8" From: Laine Stump Although until now, any use of the extra_args argument (a pointer to a struct containing extra attributes to add the the RTM_NEWLINK message) would always have the ifindex and mac set, so the code could assume it was safe to add both to the message if extra_args !=3D NULL. There is now a use for setting a MAC address in the RTM_NEWLINK without setting the ifindex, so we should check each of these separately. Signed-off-by: Laine Stump Reviewed-by: Jiri Denemark --- src/util/virnetlink.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index ab41b57672..f18946061c 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -548,9 +548,14 @@ virNetlinkNewLink(const char *ifname, NETLINK_MSG_NEST_END(nl_msg, linkinfo); =20 if (extra_args) { - NETLINK_MSG_PUT(nl_msg, IFLA_LINK, + if (extra_args->ifindex) { + NETLINK_MSG_PUT(nl_msg, IFLA_LINK, sizeof(uint32_t), extra_args->ifindex); - NETLINK_MSG_PUT(nl_msg, IFLA_ADDRESS, VIR_MAC_BUFLEN, extra_args->= mac); + } + if (extra_args->mac) { + NETLINK_MSG_PUT(nl_msg, IFLA_ADDRESS, + VIR_MAC_BUFLEN, extra_args->mac); + } } =20 if (virNetlinkCommand(nl_msg, &resp, &buflen, 0, 0, NETLINK_ROUTE, 0) = < 0) --=20 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Thu May 2 01:16:11 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1571363996; cv=none; d=zoho.com; s=zohoarc; b=IlfhzNTgPYdgTOnTRXB7fEqluocPhf1tqmDd/fJ7XG6XL1s8eXxvg9CKgdHJuhYCu8kBuFLwvOlbW2ud7xUB7OrWlHSvPLaOfMs2TlmNNV7i5fpqITOlIMO8j06eNN5Iv8Ho4y+YeXp5qVC9aDkYeK+q8E1CKCsqK4QXk0kWNYE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1571363996; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=9+VKO8VEhmAGEy6dKY8hcSFuNsCuP1Xe4b2HNiaR1yo=; b=d03DFpSM3SdHxWL28aY9jhfkYJrzQDKG5ZdWBkwdQ9S8+pbAs/rp/UjKyOS3tYPAejNgSRz2OlM9rekpHxkOyZy1Vy3Djg0Gk9/omrj5bzL68mTPQ0jm4lwSbVOY+aH8j9xIN2cmrFMDXj+wJS0N6NFeIygNXlBnoN9zPCFTUUY= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1571363996952890.7170352397387; Thu, 17 Oct 2019 18:59:56 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 16A5F8980F1; Fri, 18 Oct 2019 01:59:55 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E7D415C1B5; Fri, 18 Oct 2019 01:59:54 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id B2C5F1803517; Fri, 18 Oct 2019 01:59:54 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x9I1xrUV027702 for ; Thu, 17 Oct 2019 21:59:53 -0400 Received: by smtp.corp.redhat.com (Postfix) id C08B460C63; Fri, 18 Oct 2019 01:59:53 +0000 (UTC) Received: from tilapia.redhat.com (ovpn-124-27.rdu2.redhat.com [10.10.124.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7729560C4E for ; Fri, 18 Oct 2019 01:59:53 +0000 (UTC) From: Laine Stump To: libvir-list@redhat.com Date: Thu, 17 Oct 2019 21:59:49 -0400 Message-Id: <20191018015949.457017-3-laine@laine.org> In-Reply-To: <20191018015949.457017-1-laine@laine.org> References: <20191018015949.457017-1-laine@laine.org> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCHv2 2/2] util: set bridge device MAC address explicitly during virNetDevBridgeCreate X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.67]); Fri, 18 Oct 2019 01:59:55 +0000 (UTC) Content-Type: text/plain; charset="utf-8" From: Laine Stump Remember when the MAC address of libvirt-created bridges weren't stable, and changed as guests were started and stopped? Pepperidge Farms remembers. (No, I would never actually push a comment like that! Just wanted to get your attention). When libvirt first implemented a stable and configurable MAC address for the bridges created for libvirt virtual networks (commit 5754dbd56d, in libvirt v8.8.0) most distro stable releases didn't support explicitly setting the MAC address of a bridge; the bridge just always assumed the lowest numbered MAC of all attached interfaces. Because of this, we stabilized the bridge MAC address by creating a "dummy" tap interface with a MAC address guaranteed to be lower than any of the guest tap devices' MACs (which all started with 0xFE, so it's not difficult to do) and attached it to the bridge - this was the inception of the "virbr0-nic" device that has confused so many people over the years. Even though the linux kernel had recently gained support for explicitly setting a bridge MAC, we deemed it unnecessary to set the MAC that way, because the other (indirect) method worked everywhere. But recently there have been reports that the bridge MAC address was not following the setting in the network config, and mismatched the MAC of the dummy tap device (which was still correct). It turns out that this is due to a change in systemd/udev that persists whatever MAC address is set for a bridge when it's initially started: https://github.com/systemd/systemd/blob/master/NEWS#L499 which was the result of: https://github.com/systemd/systemd/issues/3374 (apparently if there is no MAC saved for a bridge by the name of a bridge being created, the random MAC generated during creation is saved, and then that same MAC is used to explicitly set the MAC each time it is created). Once a bridge has an explicitly set MAC, the "use the lowest numbered MAC of attached devices" rule is ignored, so our dummy tap device is like the goggles - it does nothing! (well, almost). We could whine about changes in default behavior, etc. etc., but because the change was in response to actual user problems, that seems likely a fruitless task. Fortunately, time has marched on, and even distro releases that are old enough that they are no longer supported by upstream libvirt (e.g. RHEL6) have support for explicitly setting a bridge device MAC address, either during creation or with a separate ioctl after creation, so we can now do that. The method is to add a mac arg to virNetDevBridgeCreate(). In the case of platforms where the bridge is created with a netlink RTM_NEWLINK message, we just add the desired mac to the message. For platforms that still use an ioctl (either SIOCBRADDBR or SIOCIFCREATE2), we make a separate call to virNetDevSetMAC() after creating the bridge. This makes the dummy tap pointless for purposes of setting the MAC address, but it is still useful for IPv6 DAD initialization (which apparently requires at least one interface to be attached to the bridge and online), so it hasn't been removed. (I'm considered making another patch to add the dummy tap device only if the network needs IPv6 DAD, but haven't decided yet if it's worthwhile). (NB: we can safely *always* call virNetDevBridgeCreate() with &def->mac from the network driver because, in spite of the existence of a "mac_specified" bool in the config suggesting that it may not always be present, in reality a mac address will always be added to any network that doesn't have one - this is guaranteed in all cases by commit a47ae7c004) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=3D1760851 Signed-off-by: Laine Stump Reviewed-by: Jiri Denemark --- NB: I was unable to test the calling of virNetDevSetMAC() from the SIOCIFCREATE2 (BSD) version of virNetDevBridgeCreate(); even though I managed to get a FreeBSD system setup and libvirt built there, when I tried to start the default network the SIOCIFCREATE2 ioctl itself failed, so it never even got to the virNetDevSetMAC(). I would sincerely appreciate if someone more up to speed with libvirt on FreeBSD/OpenBSD could check that out and let me know if it works properly. src/network/bridge_driver.c | 2 +- src/util/virnetdevbridge.c | 43 ++++++++++++++++++++++++++++++------- src/util/virnetdevbridge.h | 2 +- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6862818698..4f01bf6664 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2498,7 +2498,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr d= river, def->name); return -1; } - if (virNetDevBridgeCreate(def->bridge) < 0) + if (virNetDevBridgeCreate(def->bridge, &def->mac) < 0) return -1; =20 if (def->mac_specified) { diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index edf4cc6236..d3a1e3c13e 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -379,7 +379,7 @@ virNetDevBridgePortSetUnicastFlood(const char *brname G= _GNUC_UNUSED, */ #if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR) static int -virNetDevBridgeCreateWithIoctl(const char *brname) +virNetDevBridgeCreateWithIoctl(const char *brname, const virMacAddr *mac) { VIR_AUTOCLOSE fd =3D -1; =20 @@ -392,22 +392,35 @@ virNetDevBridgeCreateWithIoctl(const char *brname) return -1; } =20 + if (virNetDevSetMAC(brname, mac) < 0) { + virErrorPtr savederr; + + virErrorPreserveLast(&savederr); + ignore_value(ioctl(fd, SIOCBRDELBR, brname)); + virErrorRestore(&savederr); + return -1; + } + return 0; } #endif =20 #if defined(__linux__) && defined(HAVE_LIBNL) int -virNetDevBridgeCreate(const char *brname) +virNetDevBridgeCreate(const char *brname, const virMacAddr *mac) { /* use a netlink RTM_NEWLINK message to create the bridge */ int error =3D 0; + virNetlinkNewLinkData data =3D { + .mac =3D mac, + }; + =20 - if (virNetlinkNewLink(brname, "bridge", NULL, &error) < 0) { + if (virNetlinkNewLink(brname, "bridge", &data, &error) < 0) { # if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR) if (error =3D=3D -EOPNOTSUPP) { /* fallback to ioctl if netlink doesn't support creating bridg= es */ - return virNetDevBridgeCreateWithIoctl(brname); + return virNetDevBridgeCreateWithIoctl(brname, mac); } # endif if (error < 0) @@ -423,15 +436,17 @@ virNetDevBridgeCreate(const char *brname) =20 #elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR) int -virNetDevBridgeCreate(const char *brname) +virNetDevBridgeCreate(const char *brname, + const virMacAddr *mac) { - return virNetDevBridgeCreateWithIoctl(brname); + return virNetDevBridgeCreateWithIoctl(brname, mac); } =20 =20 #elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFCREATE2) int -virNetDevBridgeCreate(const char *brname) +virNetDevBridgeCreate(const char *brname, + const virMacAddr *mac) { struct ifreq ifr; VIR_AUTOCLOSE s =3D -1; @@ -448,10 +463,21 @@ virNetDevBridgeCreate(const char *brname) if (virNetDevSetName(ifr.ifr_name, brname) =3D=3D -1) return -1; =20 + if (virNetDevSetMAC(brname, mac) < 0) { + virErrorPtr savederr; + + virErrorPreserveLast(&savederr); + ignore_value(virNetDevBridgeDelete(brname)); + virErrorRestore(&savederr); + return -1; + } + return 0; } #else -int virNetDevBridgeCreate(const char *brname) +int +virNetDevBridgeCreate(const char *brname, + const virMacAddr *mac ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, _("Unable to create bridge %s"), brname); @@ -528,6 +554,7 @@ virNetDevBridgeDelete(const char *brname) _("Unable to remove bridge %s"), brname); return -1; + } =20 return 0; diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h index 88284d6bed..c47597dec9 100644 --- a/src/util/virnetdevbridge.h +++ b/src/util/virnetdevbridge.h @@ -21,7 +21,7 @@ #include "internal.h" #include "virmacaddr.h" =20 -int virNetDevBridgeCreate(const char *brname) +int virNetDevBridgeCreate(const char *brname, const virMacAddr *mac) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; int virNetDevBridgeDelete(const char *brname) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; --=20 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list