diff mbox series

[ovs-dev,3/3] Revert "ofproto-dpif: Let the dpif report when a port is a duplicate."

Message ID 20181213142448.8504-4-fbl@redhat.com
State Accepted
Headers show
Series revert port duplicate checking optimization | expand

Commit Message

Flavio Leitner Dec. 13, 2018, 2:24 p.m. UTC 
This reverts commit 7521e0cf9e88a62f2feff4e7253654557f94877e.

This patch introduced a regression in OSP environments using internal
ports in other netns. Their networking configuration is lost when
the service is restarted because the ports are recreated now.

Before the patch it checked using netlink if the port with a specific
"name" was already there. The check is a lookup in all ports attached
to the DP regardless of the port's netns.

After the patch it relies on the kernel to identify that situation.
Unfortunately the only protection there is register_netdevice() which
fails only if the port with that name exists in the current netns.

If the port is in another netns, it will get a new dp_port and because
of that userspace will delete the old port. At this point the original
port is gone from the other netns and there a fresh port in the current
netns.

Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 lib/dpif.c             | 9 ++-------
 ofproto/ofproto-dpif.c | 7 ++++---
 2 files changed, 6 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/lib/dpif.c b/lib/dpif.c
index e35f11147..457c9bfb9 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -592,13 +592,8 @@  dpif_port_add(struct dpif *dpif, struct netdev *netdev, odp_port_t *port_nop)
             netdev_ports_insert(netdev, dpif->dpif_class, &dpif_port);
         }
     } else {
-        if (error != EEXIST) {
-            VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s",
-                         dpif_name(dpif), netdev_name, ovs_strerror(error));
-        } else {
-            /* It's fairly common for upper layers to try to add a duplicate
-             * port, and they know how to handle it properly. */
-        }
+        VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s",
+                     dpif_name(dpif), netdev_name, ovs_strerror(error));
         port_no = ODPP_NONE;
     }
     if (port_nop) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f40f157ca..252e8add6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3662,10 +3662,11 @@  port_add(struct ofproto *ofproto_, struct netdev *netdev)
     }
 
     dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
+    if (!dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
+        odp_port_t port_no = ODPP_NONE;
+        int error;
 
-    odp_port_t port_no = ODPP_NONE;
-    int error = dpif_port_add(ofproto->backer->dpif, netdev, &port_no);
-    if (error != EEXIST) {
+        error = dpif_port_add(ofproto->backer->dpif, netdev, &port_no);
         if (error) {
             return error;
         }