From nobody Sat May 4 07:23:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 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=1598243020; cv=none; d=zohomail.com; s=zohoarc; b=nnpERBvQdOa6j5xyAKrbv86KF9CBWM03k9YDWtsso/thad/0cMuq3WoE5NOHayMYEA7Gu01LJoYmZ1Qv0AubOlO0EY1Jzk5YUjd0xkP1OIwl0rQh2mfKRkqJaCqC2JpjNgo0fSyQqJkOBrhCwVaLFKEMAlBeTIFi4Azb3Q2uqQU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1598243020; 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=99Tfi3W3W2zTtP2jPWzBM7lGkoIVOwq5Y3aVq8BCDMM=; b=CNqNzztf99/xSuKJfChaweesDPDpKNGeSxRPhVVhsrA1uiPEt4s0r7nzO+hkDQIzdla+DCvjCRvgVgtPYit9abdWVx6W1G/66nvR2cUpZkx246q7g+LlpfBwG7X9OIx6LwSAinlaq7CDL3aVBhZzv7y9CeYO2C3OBlkW/g4qPvU= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 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-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1598243020152885.0546329773218; Sun, 23 Aug 2020 21:23:40 -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-317-WoAJBWj_P8qESr3T8yAc5A-1; Mon, 24 Aug 2020 00:23:36 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E459E801AB1; Mon, 24 Aug 2020 04:23:29 +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 C3D3D7E69E; Mon, 24 Aug 2020 04:23:28 +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 B58EB9A04B; Mon, 24 Aug 2020 04:23:25 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 07O4NOsY005417 for ; Mon, 24 Aug 2020 00:23:24 -0400 Received: by smtp.corp.redhat.com (Postfix) id 8834D5D9EF; Mon, 24 Aug 2020 04:23:24 +0000 (UTC) Received: from vhost2.laine.org (ovpn-113-64.phx2.redhat.com [10.3.113.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id 420945D9DD for ; Mon, 24 Aug 2020 04:23:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598243018; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=99Tfi3W3W2zTtP2jPWzBM7lGkoIVOwq5Y3aVq8BCDMM=; b=Uaacmfv893FB8xwVwSaPrRKw+SZ6aOKn902zIh7/Cee7k/sQd1+U3oSKNJwjodwnBV9QE8 8kAMfsEvVCaLN5bvv0X9exnQGmVgVcQ6q8kx4M3Af4gk9EXRZhHQSpRWDaHQ6Se+QaH5vj cC906OKGsB4CVe/+Wu7mQooWI7GPtKY= X-MC-Unique: WoAJBWj_P8qESr3T8yAc5A-1 From: Laine Stump To: libvir-list@redhat.com Subject: [libvirt PATCH 1/3] util: make locking versions of virNetDevMacVLan(Reserve|Release)Name() Date: Mon, 24 Aug 2020 00:23:14 -0400 Message-Id: <20200824042316.821783-2-laine@redhat.com> In-Reply-To: <20200824042316.821783-1-laine@redhat.com> References: <20200824042316.821783-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com 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.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0.001 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" When these functions are called from within virnetdevmacvlan.c, they are usually called with virNetDevMacVLanCreateMutex held, but when virNetDevMacVLanReserveName() is called from other places (hypervisor drivers keeping track of already-in-use macvlan/macvtap devices) the lock isn't acquired. This could lead to a situation where one thread is setting a bit in the bitmap to notify of a device already in-use, while another thread is checking/setting/clearing a bit while creating a new macvtap device. In practice this *probably* doesn't happen, because the external calls to virNetDevMacVLan() only happen during hypervisor driver init routines when libvirtd is restarted, but there's no harm in protecting ourselves. (NB: virNetDevMacVLanReleaseName() is actually never called from outside virnetdevmacvlan.c, so it could just as well be static, but I'm leaving it as-is for now. This locking version *is* called from within virnetdevmacvlan.c, since there are a couple places that we used to call the unlocked version after the lock was already released.) Signed-off-by: Laine Stump Reviewed-by: Michal Privoznik --- src/util/virnetdevmacvlan.c | 42 ++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index dcea93a5fe..69a9c784bb 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -196,7 +196,7 @@ virNetDevMacVLanReleaseID(int id, unsigned int flags) =20 =20 /** - * virNetDevMacVLanReserveName: + * virNetDevMacVLanReserveNameInternal: * * @name: already-known name of device * @quietFail: don't log an error if this name is already in-use @@ -208,8 +208,8 @@ virNetDevMacVLanReleaseID(int id, unsigned int flags) * Returns reserved ID# on success, -1 on failure, -2 if the name * doesn't fit the auto-pattern (so not reserveable). */ -int -virNetDevMacVLanReserveName(const char *name, bool quietFail) +static int +virNetDevMacVLanReserveNameInternal(const char *name, bool quietFail) { unsigned int id; unsigned int flags =3D 0; @@ -237,8 +237,21 @@ virNetDevMacVLanReserveName(const char *name, bool qui= etFail) } =20 =20 +int +virNetDevMacVLanReserveName(const char *name, bool quietFail) +{ + /* Call the internal function after locking the macvlan mutex */ + int ret; + + virMutexLock(&virNetDevMacVLanCreateMutex); + ret =3D virNetDevMacVLanReserveNameInternal(name, quietFail); + virMutexUnlock(&virNetDevMacVLanCreateMutex); + return ret; +} + + /** - * virNetDevMacVLanReleaseName: + * virNetDevMacVLanReleaseNameInternal: * * @name: already-known name of device * @@ -248,8 +261,8 @@ virNetDevMacVLanReserveName(const char *name, bool quie= tFail) * * returns 0 on success, -1 on failure */ -int -virNetDevMacVLanReleaseName(const char *name) +static int +virNetDevMacVLanReleaseNameInternal(const char *name) { unsigned int id; unsigned int flags =3D 0; @@ -277,6 +290,19 @@ virNetDevMacVLanReleaseName(const char *name) } =20 =20 +int +virNetDevMacVLanReleaseName(const char *name) +{ + /* Call the internal function after locking the macvlan mutex */ + int ret; + + virMutexLock(&virNetDevMacVLanCreateMutex); + ret =3D virNetDevMacVLanReleaseNameInternal(name); + virMutexUnlock(&virNetDevMacVLanCreateMutex); + return ret; +} + + /** * virNetDevMacVLanIsMacvtap: * @ifname: Name of the interface @@ -967,7 +993,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifna= meRequested, return -1; } if (isAutoName && - (reservedID =3D virNetDevMacVLanReserveName(ifnameRequested, t= rue)) < 0) { + (reservedID =3D virNetDevMacVLanReserveNameInternal(ifnameRequ= ested, true)) < 0) { reservedID =3D -1; goto create_name; } @@ -975,7 +1001,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifn= ameRequested, if (virNetDevMacVLanCreate(ifnameRequested, type, macaddress, linkdev, macvtapMode, &do_retry) < 0) { if (isAutoName) { - virNetDevMacVLanReleaseName(ifnameRequested); + virNetDevMacVLanReleaseNameInternal(ifnameRequested); reservedID =3D -1; goto create_name; } --=20 2.26.2 From nobody Sat May 4 07:23:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) client-ip=205.139.110.61; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 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=1598243049; cv=none; d=zohomail.com; s=zohoarc; b=LPlF5lJW6/7C7UBO7Iur+3E9yDth9QbBMhx3ifRdNwTURbHvCx2cIB3velIvuL+sA+Bpxxsqs6czGPVpKJUl6vPKcWlyacH3i6OfkyYez1bUvtfVMqjfy/cUfzM0ZBFPXUeoXtlmq4PgiMeUy8nuV+yzeGL3sGDQOns8x2y19po= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1598243049; 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=9YRieU5YA4F5Cw4qesnoFBUfztHWj7a8dQVuSKjvxWA=; b=D0CIOzqH4rCAvX2gzqw86oj5m7ogD+NeMkKJi4RoPJ2TTsOlaJ5DosnS+u68R+rgI1bfH2p7YurgH5FTyiY57nhLLpgNwQpZilb9D0ebXvZBpoP59Id8rvHevUmXh3A/JzncRAk99BmeW1ftwG1vSM2Np0ZZjVM7+DaYULy19OY= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 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-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by mx.zohomail.com with SMTPS id 1598243049365711.6003806299997; Sun, 23 Aug 2020 21:24:09 -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-170-InzeYBsWPieVvfodtkXO1Q-1; Mon, 24 Aug 2020 00:24:05 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5FF5F5202; Mon, 24 Aug 2020 04:24:00 +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 3C2FA10027AB; Mon, 24 Aug 2020 04:24:00 +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 07786180C5A2; Mon, 24 Aug 2020 04:24:00 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 07O4NPkk005422 for ; Mon, 24 Aug 2020 00:23:25 -0400 Received: by smtp.corp.redhat.com (Postfix) id 06A9D5D9EF; Mon, 24 Aug 2020 04:23:25 +0000 (UTC) Received: from vhost2.laine.org (ovpn-113-64.phx2.redhat.com [10.3.113.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id B5B245D9DD for ; Mon, 24 Aug 2020 04:23:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598243047; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=9YRieU5YA4F5Cw4qesnoFBUfztHWj7a8dQVuSKjvxWA=; b=DfgF7rmUaYS2ry/W2kYxDQoZlLkWK2mN1XxeVmvs5SE35W4aDIicqftZiz5JOdgrBKcqNt Um81aB3rwi7BKm2pHpIPuvL62/YPK8WHa1h0nJRzxpyFxWFqAY0SpA6Vcs/c8vJM3ivZ+G vpnQenw/1kGL/qWobEwUWWmphmlNA3M= X-MC-Unique: InzeYBsWPieVvfodtkXO1Q-1 From: Laine Stump To: libvir-list@redhat.com Subject: [libvirt PATCH 2/3] util: assign macvtap names using a monotonically increasing integer Date: Mon, 24 Aug 2020 00:23:15 -0400 Message-Id: <20200824042316.821783-3-laine@redhat.com> In-Reply-To: <20200824042316.821783-1-laine@redhat.com> References: <20200824042316.821783-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com 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.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0.002 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" There have been some reports that, due to libvirt always trying to assign the lowest numbered macvtap / tap device name possible, a new guest would sometimes be started using the same tap device name as previously used by another guest that is in the process of being destroyed = *as the new guest is starting. In some cases this has led to, for example, the old guest's qemuProcessStop() code deleting a port from an OVS switch that had just been re-added by the new guest (because the port name is based on only the device name using the port). Similar problems can happen (and I believe have) with nwfilter rules and bandwidth rules (which are both instantiated based on the name of the tap device). A couple patches have been previously proposed to change the ordering of startup and shutdown processing, or to put a mutex around everything related to the tap/macvtap device name usage, but in the end no matter what you do there will still be possible holes, because the device could be deleted outside libvirt's control (for example, regular tap devices are automatically deleted when the qemu process terminates, and that isn't always initiated by libvirt but could instead happen completely asynchronously - libvirt then has no control over the ordering of shutdown operations, and no opportunity to protect it with a mutex.) But this only happens if a new device is created at the same time as one is being deleted. We can effectively eliminate the chance of this happening if we end the practice of always looking for the lowest numbered available device name, and instead just keep an integer that is incremented each time we need a new device name. At some point it will need to wrap back around to 0 (in order to avoid the IFNAMSIZ 15 character limit if nothing else), and we can't guarantee that the new name really will be the *least* recently used name, but "math" suggests that it will be *much* less common that we'll try to re-use the *most* recently used name. This patch implements such a counter for macvtap/macvlan only. It does it on top of the existing "ID reservation" system (I'm thinking about making a followup that gets rid of the bitmap, as it's now overkill and just serves to make the code more confusing). Signed-off-by: Laine Stump Reviewed-by: Michal Privoznik --- src/util/virnetdevmacvlan.c | 67 +++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 69a9c784bb..c086aa3eb0 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -74,6 +74,8 @@ VIR_LOG_INIT("util.netdevmacvlan"); virMutex virNetDevMacVLanCreateMutex =3D VIR_MUTEX_INITIALIZER; virBitmapPtr macvtapIDs =3D NULL; virBitmapPtr macvlanIDs =3D NULL; +static int macvtapLastID =3D -1; +static int macvlanLastID =3D -1; =20 static int virNetDevMacVLanOnceInit(void) @@ -108,12 +110,18 @@ virNetDevMacVLanReserveID(int id, unsigned int flags, bool quietFail, bool nextFree) { virBitmapPtr bitmap; + int *lastID; =20 if (virNetDevMacVLanInitialize() < 0) return -1; =20 - bitmap =3D (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? - macvtapIDs : macvlanIDs; + if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { + bitmap =3D macvtapIDs; + lastID =3D &macvtapLastID; + } else { + bitmap =3D macvlanIDs; + lastID =3D &macvlanLastID; + } =20 if (id > MACVLAN_MAX_ID) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -122,24 +130,49 @@ virNetDevMacVLanReserveID(int id, unsigned int flags, return -1; } =20 - if ((id < 0 || nextFree) && - (id =3D virBitmapNextClearBit(bitmap, id)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("no unused %s names available"), - VIR_NET_GENERATED_PREFIX); - return -1; - } + if (id < 0 || nextFree) { + /* starting with *lastID + 1, do a loop looking for an unused + * device name, wrapping around at MACVLAN_MAX_ID. + */ + int start =3D (++(*lastID)) % (MACVLAN_MAX_ID + 1); + bool found =3D false; =20 - if (virBitmapIsBitSet(bitmap, id)) { - if (quietFail) { - VIR_INFO("couldn't reserve name %s%d - already in use", - VIR_NET_GENERATED_PREFIX, id); - } else { + for (id =3D start; + id + 1 !=3D start; + id =3D (++(*lastID)) % (MACVLAN_MAX_ID + 1)) { + + if (!virBitmapIsBitSet(bitmap, id)) { + found =3D true; + break; + } + } + if (!found) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("couldn't reserve name %s%d - already in use"= ), - VIR_NET_GENERATED_PREFIX, id); + _("no unused %s names available"), + VIR_NET_GENERATED_PREFIX); + return -1; } - return -1; + } else { + /* A specific ID was requested, we just fail if that + * ID isn't available + */ + if (virBitmapIsBitSet(bitmap, id)) { + if (quietFail) { + VIR_INFO("couldn't reserve name %s%d - already in use", + VIR_NET_GENERATED_PREFIX, id); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("couldn't reserve name %s%d - already in = use"), + VIR_NET_GENERATED_PREFIX, id); + } + return -1; + } + /* adjust lastID to not look below this ID (even though + * eventually we will wrap around and look below it - this is + * just a delay tactic + */ + if (*lastID % (MACVLAN_MAX_ID + 1) < id) + *lastID =3D id; } =20 if (virBitmapSetBit(bitmap, id) < 0) { --=20 2.26.2 From nobody Sat May 4 07:23:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 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=1598243020; cv=none; d=zohomail.com; s=zohoarc; b=BmiErL3Zx1nrlZLy36/+3uQ/D02Z0D+z4pnkRK2Zg162rfNDiv6yl7FrF4tbqU0kYaqeVsZuPaL8xJJB9+6jcXByRjkAi7/gn9VM+Pu+CvnAZkCZd5rEavZog4Rmczs1N7ZZp4cH9ItODjX1mLa5e+vBtgO1lPUo33odkFzdBsE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1598243020; 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=gdugcPNraq4kbzIWTy6oWbsVXbpU7CpYH74IlnPjAU4=; b=V7div8Yx/xW+CtRQdtSl3pYhjJ140dEdp5/jBpbP98epym5MzlNs7QP0Jtj+lsEwmJfiEdyKY81LaMi9KJDurZ5TgSF00yGELO2KIN5N0fDhM2DTlcTVWRbp17hJ9nKEIlgrChGat7RxuMyHlwZY+SIlrmiaQBZBl4wYtSsT8Qo= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 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-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1598243020153248.06699045578534; Sun, 23 Aug 2020 21:23:40 -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-255-fiqnQrqkMqOQKXLliZGsKw-1; Mon, 24 Aug 2020 00:23:36 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D3A8D1885DA7; Mon, 24 Aug 2020 04:23:30 +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 346631002382; Mon, 24 Aug 2020 04:23:30 +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 4D5D6180B657; Mon, 24 Aug 2020 04:23:28 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 07O4NPob005430 for ; Mon, 24 Aug 2020 00:23:25 -0400 Received: by smtp.corp.redhat.com (Postfix) id 78F0F5D9EF; Mon, 24 Aug 2020 04:23:25 +0000 (UTC) Received: from vhost2.laine.org (ovpn-113-64.phx2.redhat.com [10.3.113.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id 33ADD5D9DD for ; Mon, 24 Aug 2020 04:23:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598243018; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=gdugcPNraq4kbzIWTy6oWbsVXbpU7CpYH74IlnPjAU4=; b=in8GdTdnnainlwS60ArCkODY5h9EuqC4xhn3ZgBC5RlE77QCA/S/SnSE1esj7swfak0un2 sqm3rb4hYikmYw1kO/PGrs/LxEIW7L5iP406iWoYo+45auMcUgEJ1P2XkIsTUqCOGVfnye 6MsZgiOKjUClficxZwefKO1TCuQnP3w= X-MC-Unique: fiqnQrqkMqOQKXLliZGsKw-1 From: Laine Stump To: libvir-list@redhat.com Subject: [libvirt PATCH 3/3] util: assign tap device names using a monotonically increasing integer Date: Mon, 24 Aug 2020 00:23:16 -0400 Message-Id: <20200824042316.821783-4-laine@redhat.com> In-Reply-To: <20200824042316.821783-1-laine@redhat.com> References: <20200824042316.821783-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com 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.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0.001 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" When creating a standard tap device, if provided with an ifname that contains "%d", rather than taking that literally as the name to use for the new device, the kernel will instead use that string as a template, and search for the lowest number that could be put in place of %d and produce an otherwise unused and unique name for the new device. For example, if there is no tap device name given in the XML, libvirt will always send "vnet%d" as the device name, and the kernel will create new devices named "vnet0", "vnet1", etc. If one of those devices is deleted, creating a "hole" in the name list, the kernel will always attempt to reuse the name in the hole first before using a name with a higher number (i.e. it finds the lowest possible unused number). The problem with this, as described in the previous patch dealing with macvtap device naming, is that it makes "immediate reuse" of a newly freed tap device name *much* more common, and in the aftermath of deleting a tap device, there is some other necessary cleanup of things which are named based on the device name (nwfilter rules, bandwidth rules, OVS switch ports, to name a few) that could end up stomping over the top of the setup of a new device of the same name for a different guest. Since the kernel "create a name based on a template" functionality for tap devices doesn't exist for macvtap, this patch is a bit different from the previous patch - we look for a requested ifname of "vnet%d", and when we see that, we use it as a sprintf format string to find an unused device name ourselves, then pass that exact name on to the kernel; in this way we can avoid the perils of the kernel's "lowest number first" algorithm, and instead use a "hopefully never-before used number" algorithm. (NB: It is still possible for a user to provide their own parameterized template name (e.g. "mytap%d") in the XML, and libvirt will just pass that through to the kernel as it always has.) Signed-off-by: Laine Stump Reviewed-by: Michal Privoznik --- src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 22 ++++++++++- src/util/virnetdevtap.c | 79 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevtap.h | 4 ++ 4 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 01c2e710cd..a9d5af9dde 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2674,6 +2674,7 @@ virNetDevTapGetName; virNetDevTapGetRealDeviceName; virNetDevTapInterfaceStats; virNetDevTapReattachBridge; +virNetDevTapReserveName; =20 =20 # util/virnetdevveth.h diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 832b2e6870..2a1c1a3732 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3319,8 +3319,26 @@ qemuProcessNotifyNets(virDomainDefPtr def) * domain to be unceremoniously killed, which would be *very* * impolite. */ - if (virDomainNetGetActualType(net) =3D=3D VIR_DOMAIN_NET_TYPE_DIRE= CT) - ignore_value(virNetDevMacVLanReserveName(net->ifname, false)); + switch (virDomainNetGetActualType(net)) { + case VIR_DOMAIN_NET_TYPE_DIRECT: + ignore_value(virNetDevMacVLanReserveName(net->ifname, false)); + break; + case VIR_DOMAIN_NET_TYPE_BRIDGE: + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + virNetDevTapReserveName(net->ifname); + break; + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_LAST: + break; + } =20 if (net->type =3D=3D VIR_DOMAIN_NET_TYPE_NETWORK) { if (!conn && !(conn =3D virGetConnectNetwork())) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index c0a7c3019e..4c3b68e582 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -54,6 +54,45 @@ =20 VIR_LOG_INIT("util.netdevtap"); =20 +#define VIR_TAP_MAX_ID 99999 + +virMutex virNetDevTapCreateMutex =3D VIR_MUTEX_INITIALIZER; +static int virNetDevTapLastID =3D -1; + + +/** + * virNetDevTapReserveName: + * @name: name of an existing tap device + * + * Set the value of virNetDevTapLastID to assure that any new tap + * device created with an autogenerated name will use a number higher + * than the number in the given tap device name. + * + * Returns nothing. + */ +void +virNetDevTapReserveName(const char *name) +{ + unsigned int id; + const char *idstr =3D NULL; + + + if (STRPREFIX(name, VIR_NET_GENERATED_TAP_PREFIX)) { + + idstr =3D name + strlen(VIR_NET_GENERATED_TAP_PREFIX); + + if (virStrToLong_ui(idstr, NULL, 10, &id) >=3D 0) { + virMutexLock(&virNetDevTapCreateMutex); + + if (virNetDevTapLastID % (VIR_TAP_MAX_ID + 1) < (int)id) + virNetDevTapLastID =3D id; + + virMutexUnlock(&virNetDevTapCreateMutex); + } + } +} + + /** * virNetDevTapGetName: * @tapfd: a tun/tap file descriptor @@ -230,10 +269,45 @@ int virNetDevTapCreate(char **ifname, size_t tapfdSize, unsigned int flags) { - size_t i; + size_t i =3D 0; struct ifreq ifr; int ret =3D -1; - int fd; + int fd =3D 0; + + virMutexLock(&virNetDevTapCreateMutex); + + /* if ifname is "vnet%d", then create the actual name to use by + * replacing %d with ++virNetDevTapLastID. Keep trying new values + * until one is found that doesn't already exist, or we've + * completed one full circle of the number space. + * + */ + + if (STREQ(*ifname, VIR_NET_GENERATED_TAP_PREFIX "%d")) { + int id; + int start =3D ++virNetDevTapLastID % (VIR_TAP_MAX_ID + 1); + bool found =3D false; + + for (id =3D start; + id + 1 !=3D start; + id =3D ++virNetDevTapLastID % (VIR_TAP_MAX_ID + 1)) { + + g_autofree char *try =3D g_strdup_printf(*ifname, id); + + if (!virNetDevExists(try)) { + g_free(*ifname); + *ifname =3D g_steal_pointer(&try); + found =3D true; + break; + } + } + if (!found) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("no unused %s names available"), + VIR_NET_GENERATED_TAP_PREFIX); + goto cleanup; + } + } =20 if (!tunpath) tunpath =3D "/dev/net/tun"; @@ -302,6 +376,7 @@ int virNetDevTapCreate(char **ifname, ret =3D 0; =20 cleanup: + virMutexUnlock(&virNetDevTapCreateMutex); if (ret < 0) { VIR_FORCE_CLOSE(fd); while (i--) diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index c6bd9285ba..dea8aec3af 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -29,6 +29,10 @@ # define VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP 1 #endif =20 +void +virNetDevTapReserveName(const char *name) + ATTRIBUTE_NONNULL(1); + int virNetDevTapCreate(char **ifname, const char *tunpath, int *tapfd, --=20 2.26.2