From 2a51ff7b40ac7ed81d9244120716e1fd38371572 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Mon, 7 Jun 2021 16:27:49 -0400 Subject: [PATCH] openvswitch: don't delete existing OVS port prior to recreating same port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/e21c6643a02c6b446d2fbdfde366ea303b4c2730 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é --- src/util/virnetdevopenvswitch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 21ee4bdd42..eac68d9556 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -155,8 +155,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, } cmd = virNetDevOpenvswitchCreateCmd(); - virCommandAddArgList(cmd, "--", "--if-exists", "del-port", - ifname, "--", "add-port", brname, ifname, NULL); + virCommandAddArgList(cmd, "--", "--may-exist", + "add-port", brname, ifname, NULL); virNetDevOpenvswitchConstructVlans(cmd, virtVlan);