From nobody Tue May 7 10:33:57 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=1623100061; cv=none; d=zohomail.com; s=zohoarc; b=U3qS3oWZxCPkellqLTvVzaVvj2nfri0ZVvMvWQXFeco/0rIG1rzO9idzUlUn1/lIlLyfKvhaFRuS9rUwza5MrNUkFhGdYyZdO1m03G0/NlQs7o2x/m7gSQeBxiRK73xEuDSTvXTvyxjhDkykZF0WXVhHkF3leWBbHcGUqCI48Rc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1623100061; 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=zPiItoCSkScMNu9jszFEES9T183LbTioV7wBLNnU9ZA=; b=UTwxkbl6DOcsVhm3x+CEO1XoDsJ+5B5ZdaEVqA+0WU2vGrEPs5EDVqd9zpCpgrrCMaLr+yDbmKHrOUsWaenAy/M7ijG+/NIstkDVHsTHzoRp21ReX0WzdI+RG7ldFU+FT99k5OF2Z99b034rys6e1d9Hz2JsWIvxSPpjOJ1ju88= 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 1623100061356383.26341453280065; Mon, 7 Jun 2021 14:07:41 -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-593-7pd3J8tUPmesteFSh7s_Ng-1; Mon, 07 Jun 2021 17:07:38 -0400 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id D74C210074A7; Mon, 7 Jun 2021 21:07:31 +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 2F91260C0F; Mon, 7 Jun 2021 21:07: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 4608044A59; Mon, 7 Jun 2021 21:07:24 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 157L7Mqk012829 for ; Mon, 7 Jun 2021 17:07:22 -0400 Received: by smtp.corp.redhat.com (Postfix) id C0DC9E2CC; Mon, 7 Jun 2021 21:07:22 +0000 (UTC) Received: from vhost2.laine.org (ovpn-112-202.phx2.redhat.com [10.3.112.202]) by smtp.corp.redhat.com (Postfix) with ESMTP id 587631A26A; Mon, 7 Jun 2021 21:07:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623100060; 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=zPiItoCSkScMNu9jszFEES9T183LbTioV7wBLNnU9ZA=; b=WciHSwri6JVTW51OxUB+jWzBdmx2/F+lJlhpH6TOm0rZSLpDBnDmxqc4wRWJc/rZgBPpNk vefxjKaWmn70SgyQNejb00kynBQVH9IVcLD+/rTRqAd/hinxl8WTXQTvknJkASTvkRo4o4 ZJfefoB+FFqWSl3ovoDzhNu0PYaBpes= X-MC-Unique: 7pd3J8tUPmesteFSh7s_Ng-1 From: Laine Stump To: libvir-list@redhat.com Subject: [libvirt PATCH] openvswitch: don't delete existing OVS port prior to recreating same port Date: Mon, 7 Jun 2021 17:07:17 -0400 Message-Id: <20210607210717.68642-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Cc: Sofer Athlan-Guyot , Artom Lifshitz 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.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" Connecting a tap device to an Open vSwitch is done by adding a "port" to the switch with the ovs-vsctl "add-port" command. The port will have the same name as the tap device, but it is a separate entity, and can survive beyond the destruction of the tap device (although under normal circumstances the port will be deleted around the same time the tap device is deleted). This makes it possible for a port of a particular name to already exist at the time libvirt calls ovs-vsctl to add that port. The original commit of Open vSwitch support (commit df81004632, libvirt 0.9.10, Feb. 2012) used the "--may-exist" option to the add-port command to indicate that a port of the desired name might already exist, and that it was okay to simply re-use this port (rather than failing with an error message). Then in commit 33445ce8446d9 (libvirt 1.2.7, April 2014) the command was changed to use "--if-exists del-port blah" instead of "--may-exist". The reason given was that there was a bug in OVS where a stale port would be unusable even though it still existed; the workaround was to forcibly delete any existing port prior to adding the new port (of the same name). This is the ovs-vsctl command still in use by libvirt today. It recently came up in the discussion of a bug concerning guest packet loss during OpenStack upgrades (https://bugzilla.redhat.com/1963164) that the bug in OVS that necessitated the del-port workaround was fixed quite a long time ago (August 2015): https://github.com/openvswitch/ovs/commit/e21c6643a02c6b446d2fbdfde366ea3= 03b4c2730 thus rendering the workaround in libvirt unnecessary. The assertion in that discussion is that this workaround is now the cause of the packet loss being experienced during OpenStack upgrades. I'm not convinced this is the case, but it does appear that there is no reason to carry this workaround in libvirt any longer, so this patch reverts the code back to the original behavior (using "--may-exist" instead of "--if-exists del-port"). Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrang=C3=A9 --- In an attempt to sabotage my own patch, I will point out that this makes it a possibility that libvirt could end up using an OVS port created by "someone else" who may have set it up in some way that renders the guest network connection unusable, e.g. if the port had been created with flow filters or something. On the other hand 1) the "someone else" in this scenario would need to have all the same privileges that would also permit them to mess up the connection after it had been created (so there is no *extra* security risk), and 2) this could possibly be *desired* (although difficult for libvirt to support) behavior in the case someone needed libvirt-managed guest network connections to use extra capabilities not directly supported in libvirt's config; additionally pre-creating the OVS port could provide a method of getting the network connection into a usable state (i.e. forwarding traffic) much sooner. src/util/virnetdevopenvswitch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitc= h.c index 21ee4bdd42..eac68d9556 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -155,8 +155,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, con= st char *ifname, } =20 cmd =3D virNetDevOpenvswitchCreateCmd(); - virCommandAddArgList(cmd, "--", "--if-exists", "del-port", - ifname, "--", "add-port", brname, ifname, NULL); + virCommandAddArgList(cmd, "--", "--may-exist", + "add-port", brname, ifname, NULL); =20 virNetDevOpenvswitchConstructVlans(cmd, virtVlan); =20 --=20 2.31.1