From nobody Mon Nov 25 17:39:08 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 --- 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