From nobody Mon Apr 29 13:23:04 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) client-ip=205.139.110.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1599140089; cv=none; d=zohomail.com; s=zohoarc; b=edJSEWYbXSGuaeO81P6/8FZNE274NKpY+8GN/hgwVz6deXZswHG9QGLek9i7+Oa+dXqGZ4A2m8VHDVXlEbLd1dE6Gz9Cc9uZKKzswJgF14egjgdtIk+MIYEsOnSFtuO9m2X2tJ+PzUiPcNqdNlpbeMyJNYPfWmOb4BWd0oCBlmw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1599140089; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=Oz+H0oWRyA1utmiyk/Bq6TVAsYuVfHeknYtfOtdVs2U=; b=gyhFGQ3oCuvDf1AOkhj+LmFZQAhvmr9mEvcLNZsYVpihSiwr3WoHaHbObBzncs1WFJ0LWrsdfi5JcBXglftfivoR8Xwt+1xB5FOXmcJDMYgrH7FZxq44IOCpq9Dq0vP4fJAydkM2EFemmTGnaSe/beri91ZtmrQzKA+LJW3k20Q= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by mx.zohomail.com with SMTPS id 1599140089312980.8498360005854; Thu, 3 Sep 2020 06:34:49 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-406-1QgyIndjNbWIejUWsxyPWA-1; Thu, 03 Sep 2020 09:34:45 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E87D818BFECD; Thu, 3 Sep 2020 13:34:38 +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 A155B1C4; Thu, 3 Sep 2020 13:34:38 +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 2674379DBA; Thu, 3 Sep 2020 13:34:38 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 083DYaN1026611 for ; Thu, 3 Sep 2020 09:34:36 -0400 Received: by smtp.corp.redhat.com (Postfix) id 9D55065C74; Thu, 3 Sep 2020 13:34:36 +0000 (UTC) Received: from domokun.gsslab.fab.redhat.com (unknown [10.33.8.110]) by smtp.corp.redhat.com (Postfix) with ESMTP id 60A5262968; Thu, 3 Sep 2020 13:34:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1599140088; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=Oz+H0oWRyA1utmiyk/Bq6TVAsYuVfHeknYtfOtdVs2U=; b=FyJkeAEZe1DMNYyf5sEyA9kw13ERkXPHNyu7vrrPV0Rffstlo+4Nci+3HtqfNef339ls1I 2woBrR7dguacp8leg5og/OBnK5yj2XHtZSNkYfq7k2H9JUMumnC2Llmo1pv0qaX+N9OcHy Gv3GzrPctijae19yFlHe73FWusNjxXo= X-MC-Unique: 1QgyIndjNbWIejUWsxyPWA-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [PATCH] network: drop use of dummy tap device in bridges Date: Thu, 3 Sep 2020 14:34:23 +0100 Message-Id: <20200903133423.3575562-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Cc: Laine Stump 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: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0.003 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) A long time ago we introduced a dummy tap device (e.g. virbr0-nic) that we attached to the bridge device created for virtual networks: commit 5754dbd56d4738112a86776c09e810e32f7c3224 Author: Laine Stump Date: Wed Feb 9 03:28:12 2011 -0500 Give each virtual network bridge its own fixed MAC address This was a hack to workaround a Linux kernel bug where it would not honour any attempt to set a MAC address on a bridge. Instead the bridge would adopt the numerically lowest MAC address of all NICs attached to the bridge. This lead to the MAC addrss of the bridge changing over time as NICs were attached/detached. The Linux bug was actually fixed 3 years before the libvirt workaround was added in: commit 92c0574f11598c8036f81e27d2e8bdd6eed7d76d Author: Stephen Hemminger Date: Tue Jun 17 16:10:06 2008 -0700 bridge: make bridge address settings sticky Normally, the bridge just chooses the smallest mac address as the bridge id and mac address of bridge device. But if the administrator has explictly set the interface address then don't change it. Signed-off-by: Stephen Hemminger Signed-off-by: David S. Miller but libvirt needed to support RHEL-5 kernels at that time, so none the less added the workaround. We have long since dropped support for RHEL-5 vintage distros, so there's no reason to keep the dummy tap device for the purpose of setting the bridge MAC address. Later the dummy TAP device was used for a second purpose related to IPv6 DAD (Duplicate Address Detection) in: commit db488c79173b240459c7754f38c3c6af9b432970 Author: Benjamin Cama Date: Wed Sep 26 21:02:20 2012 +0200 network: fix dnsmasq/radvd binding to IPv6 on recent kernels This was again dealing with a regression in the Linux kernel, where if there were no devices attached to the bridge in the UP state, IPv6 DAD would not be performed. The virbr0-nic was attached but in the DOWN state, so the above libvirt fix tenporarily brought the NIC online. The Linux commit causing the problem was in v2.6.38 commit 1faa4356a3bd89ea11fb92752d897cff3a20ec0e Author: stephen hemminger Date: Mon Mar 7 08:34:06 2011 +0000 bridge: control carrier based on ports online A short while later Linux was tweaked so that DAD would still occur if the bridge had no attached devices at all in 3.1: commit b64b73d7d0c480f75684519c6134e79d50c1b341 Author: stephen hemminger Date: Mon Oct 3 18:14:45 2011 +0000 bridge: leave carrier on for empty bridge IOW, the only reason we need the DAD hack of bringing virbr0-nic online is because virbr0-nic exists. Once it doesn't exist, then we hit the "empty bridge" case which works in Linux. We can rely on distros having Linux kernel >=3D 3.1, so both things that the virbr0-nic are doing are redundant. Fixes https://gitlab.com/libvirt/libvirt/-/issues/53 Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: Laine Stump --- src/network/bridge_driver.c | 58 +++++-------------------------------- 1 file changed, 8 insertions(+), 50 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b016d86b9f..5c00befc16 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2247,8 +2247,7 @@ networkAddAddrToBridge(virNetworkObjPtr obj, =20 =20 static int -networkStartHandleMACTableManagerMode(virNetworkObjPtr obj, - const char *macTapIfName) +networkStartHandleMACTableManagerMode(virNetworkObjPtr obj) { virNetworkDefPtr def =3D virNetworkObjGetDef(obj); const char *brname =3D def->bridge; @@ -2257,12 +2256,6 @@ networkStartHandleMACTableManagerMode(virNetworkObjP= tr obj, def->macTableManager =3D=3D VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_L= IBVIRT) { if (virNetDevBridgeSetVlanFiltering(brname, true) < 0) return -1; - if (macTapIfName) { - if (virNetDevBridgePortSetLearning(brname, macTapIfName, false= ) < 0) - return -1; - if (virNetDevBridgePortSetUnicastFlood(brname, macTapIfName, f= alse) < 0) - return -1; - } } return 0; } @@ -2330,10 +2323,8 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr = driver, virErrorPtr save_err =3D NULL; virNetworkIPDefPtr ipdef; virNetDevIPRoutePtr routedef; - g_autofree char *macTapIfName =3D NULL; virMacMapPtr macmap; g_autofree char *macMapFile =3D NULL; - int tapfd =3D -1; bool dnsmasqStarted =3D false; bool devOnline =3D false; bool firewalRulesAdded =3D false; @@ -2360,29 +2351,6 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr = driver, if (virNetDevBridgeCreate(def->bridge, &def->mac) < 0) return -1; =20 - if (def->mac_specified) { - /* To set a mac for the bridge, we need to define a dummy tap - * device, set its mac, then attach it to the bridge. As long - * as its mac address is lower than any other interface that - * gets attached, the bridge will always maintain this mac - * address. - */ - macTapIfName =3D networkBridgeDummyNicName(def->bridge); - if (!macTapIfName) - goto error; - /* Keep tun fd open and interface up to allow for IPv6 DAD to happ= en */ - if (virNetDevTapCreateInBridgePort(def->bridge, - &macTapIfName, &def->mac, - NULL, NULL, &tapfd, 1, NULL, NU= LL, - VIR_TRISTATE_BOOL_NO, - NULL, def->mtu, NULL, - VIR_NETDEV_TAP_CREATE_USE_MAC_F= OR_BRIDGE | - VIR_NETDEV_TAP_CREATE_IFUP | - VIR_NETDEV_TAP_CREATE_PERSIST) = < 0) { - goto error; - } - } - if (!(macMapFile =3D virMacMapFileName(driver->dnsmasqStateDir, def->bridge)) || !(macmap =3D virMacMapNew(macMapFile))) @@ -2426,7 +2394,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr d= river, goto error; } =20 - if (networkStartHandleMACTableManagerMode(obj, macTapIfName) < 0) + if (networkStartHandleMACTableManagerMode(obj) < 0) goto error; =20 /* Bring up the bridge interface */ @@ -2482,15 +2450,6 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr = driver, if (v6present && networkWaitDadFinish(obj) < 0) goto error; =20 - /* DAD has finished, dnsmasq is now bound to the - * bridge's IPv6 address, so we can set the dummy tun down. - */ - if (tapfd >=3D 0) { - if (virNetDevSetOnline(macTapIfName, false) < 0) - goto error; - VIR_FORCE_CLOSE(tapfd); - } - if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; =20 @@ -2514,16 +2473,11 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr= driver, def->forward.type !=3D VIR_NETWORK_FORWARD_OPEN) networkRemoveFirewallRules(def); =20 - if (macTapIfName) { - VIR_FORCE_CLOSE(tapfd); - ignore_value(virNetDevTapDelete(macTapIfName, NULL)); - } virNetworkObjUnrefMacMap(obj); =20 ignore_value(virNetDevBridgeDelete(def->bridge)); =20 virErrorRestore(&save_err); - /* coverity[leaked_handle] - 'tapfd' is not leaked */ return -1; } =20 @@ -2555,9 +2509,13 @@ networkShutdownNetworkVirtual(virNetworkDriverStateP= tr driver, if (dnsmasqPid > 0) kill(dnsmasqPid, SIGTERM); =20 + /* We no longer create a dummy NIC, but if we've upgraded + * from old libvirt, we still need to delete any dummy NIC + * that might exist. Keep this logic around for a while... + */ if (def->mac_specified) { g_autofree char *macTapIfName =3D networkBridgeDummyNicName(def->b= ridge); - if (macTapIfName) + if (macTapIfName && virNetDevExists(macTapIfName)) ignore_value(virNetDevTapDelete(macTapIfName, NULL)); } =20 @@ -2597,7 +2555,7 @@ networkStartNetworkBridge(virNetworkObjPtr obj) if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; =20 - if (networkStartHandleMACTableManagerMode(obj, NULL) < 0) + if (networkStartHandleMACTableManagerMode(obj) < 0) goto error; =20 return 0; --=20 2.25.4