diff mbox

[ovs-dev,1/1] netdev: Fix netdev_open() to track and recreate classless interfaces

Message ID edb5fc61576927e4d4ec0bd3fb3a4e83db65da13.1500035569.git.echaudro@redhat.com
State Accepted
Headers show

Commit Message

Eelco Chaudron July 14, 2017, 12:33 p.m. UTC
Due to commit 67ac844 an existing issue with OVS persisten ports
surfaced. If we revert the commit we no longer get the error, and
basic traffic will flow. However the wrong netdev class is used, hence
the wrong callbacks get called.

The main issue is with netdev_open() being called with type = NULL
before the interface is actually configured in the system. This patch
tracks these "auto" generated interfaces, and once netdev_open() gets
called with a valid type, re-configures (re-create) it.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/netdev-provider.h |  4 ++++
 lib/netdev.c          | 30 +++++++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

Comments

Ben Pfaff Aug. 2, 2017, 8:58 p.m. UTC | #1
On Fri, Jul 14, 2017 at 02:33:27PM +0200, Eelco Chaudron wrote:
> Due to commit 67ac844 an existing issue with OVS persisten ports
> surfaced. If we revert the commit we no longer get the error, and
> basic traffic will flow. However the wrong netdev class is used, hence
> the wrong callbacks get called.
> 
> The main issue is with netdev_open() being called with type = NULL
> before the interface is actually configured in the system. This patch
> tracks these "auto" generated interfaces, and once netdev_open() gets
> called with a valid type, re-configures (re-create) it.
> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>

OK, let's try this.  I applied it to master.

If it doesn't solve the issues, then we'll have to revert both this and
67ac844.

Thanks,

Ben.
diff mbox

Patch

diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 3c3c181..b3c57d5 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -45,6 +45,10 @@  struct netdev {
     const struct netdev_class *netdev_class; /* Functions to control
                                                 this device. */
 
+    /* If this is 'true' the user did not specify a netdev_class when
+     * opening this device, and therefore got assigned to the "system" class */
+    bool auto_classified;
+
     /* A sequence number which indicates changes in one of 'netdev''s
      * properties.   It must be nonzero so that users have a value which
      * they may use as a reset when tracking 'netdev'.
diff --git a/lib/netdev.c b/lib/netdev.c
index a7840a8..89afa71 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -361,7 +361,7 @@  netdev_open(const char *name, const char *type, struct netdev **netdevp)
     OVS_EXCLUDED(netdev_mutex)
 {
     struct netdev *netdev;
-    int error;
+    int error = 0;
 
     if (!name[0]) {
         /* Reject empty names.  This saves the providers having to do this.  At
@@ -375,6 +375,29 @@  netdev_open(const char *name, const char *type, struct netdev **netdevp)
 
     ovs_mutex_lock(&netdev_mutex);
     netdev = shash_find_data(&netdev_shash, name);
+
+    if (netdev &&
+        type && type[0] && strcmp(type, netdev->netdev_class->type)) {
+
+        if (netdev->auto_classified) {
+            /* If this device was first created without a classification type,
+             * for example due to routing or tunneling code, and they keep a
+             * reference, a "classified" call to open will fail. In this case
+             * we remove the classless device, and re-add it below. We remove
+             * the netdev from the shash, and change the sequence, so owners of
+             * the old classless device can release/cleanup. */
+            if (netdev->node) {
+                shash_delete(&netdev_shash, netdev->node);
+                netdev->node = NULL;
+                netdev_change_seq_changed(netdev);
+            }
+
+            netdev = NULL;
+        } else {
+            error = EEXIST;
+        }
+    }
+
     if (!netdev) {
         struct netdev_registered_class *rc;
 
@@ -384,6 +407,7 @@  netdev_open(const char *name, const char *type, struct netdev **netdevp)
             if (netdev) {
                 memset(netdev, 0, sizeof *netdev);
                 netdev->netdev_class = rc->class;
+                netdev->auto_classified = type && type[0] ? false : true;
                 netdev->name = xstrdup(name);
                 netdev->change_seq = 1;
                 netdev->reconfigure_seq = seq_create();
@@ -416,10 +440,6 @@  netdev_open(const char *name, const char *type, struct netdev **netdevp)
                       name, type);
             error = EAFNOSUPPORT;
         }
-    } else if (type && type[0] && strcmp(type, netdev->netdev_class->type)) {
-        error = EEXIST;
-    } else {
-        error = 0;
     }
 
     if (!error) {