netdev: Make netdev arguments fetchable, and implement for netdev-vport.

This gives network device implementations the opportunity to fetch an
existing device's configuration and store it as their arguments, so that
netdev clients can find out how an existing device is configured.

So far netdev-vport is the only implementation that needs to use this.

The next commit will add use by clients.

Reviewed by Justin Pettit.

lib/netdev-dummy.c

@@ -76,14 +76,14 @@ netdev_dummy_cast(const struct netdev *netdev)
 
 static int
 netdev_dummy_create(const struct netdev_class *class, const char *name,
-                    const struct shash *args OVS_UNUSED,
+                    const struct shash *args,
                     struct netdev_dev **netdev_devp)
 {
     static unsigned int n = 0xaa550000;
     struct netdev_dev_dummy *netdev_dev;
 
     netdev_dev = xzalloc(sizeof *netdev_dev);
-    netdev_dev_init(&netdev_dev->netdev_dev, name, class);
+    netdev_dev_init(&netdev_dev->netdev_dev, name, args, class);
     netdev_dev->hwaddr[0] = 0xaa;
     netdev_dev->hwaddr[1] = 0x55;
     netdev_dev->hwaddr[2] = n >> 24;

lib/netdev-linux.c

@@ -523,7 +523,7 @@ netdev_linux_create(const struct netdev_class *class,
     cache_notifier_refcount++;
 
     netdev_dev = xzalloc(sizeof *netdev_dev);
-    netdev_dev_init(&netdev_dev->netdev_dev, name, class);
+    netdev_dev_init(&netdev_dev->netdev_dev, name, args, class);
 
     *netdev_devp = &netdev_dev->netdev_dev;
     return 0;
@@ -577,7 +577,7 @@ netdev_linux_create_tap(const struct netdev_class *class OVS_UNUSED,
         goto error;
     }
 
-    netdev_dev_init(&netdev_dev->netdev_dev, name, &netdev_tap_class);
+    netdev_dev_init(&netdev_dev->netdev_dev, name, args, &netdev_tap_class);
     *netdev_devp = &netdev_dev->netdev_dev;
     return 0;
 
@@ -2201,7 +2201,7 @@ netdev_linux_poll_remove(struct netdev_notifier *notifier_)
                                                                 \
     CREATE,                                                     \
     netdev_linux_destroy,                                       \
-    NULL,                       /* reconfigure */               \
+    NULL,                       /* set_config */                \
                                                                 \
     netdev_linux_open,                                          \
     netdev_linux_close,                                         \

lib/netdev-provider.h

@@ -43,6 +43,7 @@ struct netdev_dev {
 };
 
 void netdev_dev_init(struct netdev_dev *, const char *name,
+                     const struct shash *args,
                      const struct netdev_class *);
 void netdev_dev_uninit(struct netdev_dev *, bool destroy);
 const char *netdev_dev_get_type(const struct netdev_dev *);
@@ -137,12 +138,12 @@ struct netdev_class {
      * called. */
     void (*destroy)(struct netdev_dev *netdev_dev);
 
-    /* Reconfigures the device 'netdev_dev' with 'args'.
+    /* Changes the device 'netdev_dev''s configuration to 'args'.
      *
      * If this netdev class does not support reconfiguring a netdev
      * device, this may be a null pointer.
      */
-    int (*reconfigure)(struct netdev_dev *netdev_dev, const struct shash *args);
+    int (*set_config)(struct netdev_dev *netdev_dev, const struct shash *args);
 
     /* Attempts to open a network device.  On success, sets 'netdevp'
      * to the new network device.

lib/netdev-vport.c

@@ -72,8 +72,10 @@ struct netdev_vport {
 
 struct vport_class {
     struct netdev_class netdev_class;
-    int (*parse_config)(const struct netdev_dev *, const struct shash *args,
-                        void *config);
+    int (*parse_config)(const char *name, const char *type,
+                        const struct shash *args, void *config);
+    int (*unparse_config)(const char *name, const char *type,
+                          const void *config, struct shash *args);
 };
 
 static struct shash netdev_vport_notifiers =
@@ -146,19 +148,58 @@ netdev_vport_create(const struct netdev_class *netdev_class, const char *name,
                     struct netdev_dev **netdev_devp)
 {
     const struct vport_class *vport_class = vport_class_cast(netdev_class);
-    struct netdev_dev_vport *dev;
+    uint64_t config[VPORT_CONFIG_SIZE / 8];
+    struct shash fetched_args;
     int error;
 
-    dev = xmalloc(sizeof *dev);
-    *netdev_devp = &dev->netdev_dev;
-    netdev_dev_init(&dev->netdev_dev, name, netdev_class);
+    memset(config, 0, sizeof config);
+    shash_init(&fetched_args);
 
-    memset(dev->config, 0, sizeof dev->config);
-    error = vport_class->parse_config(&dev->netdev_dev, args, dev->config);
+    if (!shash_is_empty(args)) {
+        /* Parse the provided configuration. */
+        error = vport_class->parse_config(name, netdev_class->type,
+                                          args, config);
+    } else {
+        /* Fetch an existing configuration from the kernel.
+         *
+         * This case could be ambiguous with initializing a new vport with an
+         * empty configuration, but none of the existing vport classes accept
+         * an empty configuration. */
+        struct odp_port odp_port;
 
-    if (error) {
-        netdev_dev_uninit(&dev->netdev_dev, true);
+        memset(&odp_port, 0, sizeof odp_port);
+        strncpy(odp_port.devname, name, sizeof odp_port.devname);
+        error = netdev_vport_do_ioctl(ODP_VPORT_QUERY, &odp_port);
+        if (!error) {
+            /* XXX verify correct type */
+            memcpy(config, odp_port.config, sizeof config);
+            error = vport_class->unparse_config(name, netdev_class->type,
+                                                odp_port.config,
+                                                &fetched_args);
+            if (error) {
+                VLOG_ERR_RL(&rl, "%s: failed to parse kernel config (%s)",
+                            name, strerror(error));
+            }
+        } else {
+            VLOG_ERR_RL(&rl, "%s: vport query failed (%s)",
+                        name, strerror(error));
+        }
     }
+
+    if (!error) {
+        struct netdev_dev_vport *dev;
+
+        dev = xmalloc(sizeof *dev);
+        netdev_dev_init(&dev->netdev_dev, name,
+                        shash_is_empty(&fetched_args) ? args : &fetched_args,
+                        netdev_class);
+        memcpy(dev->config, config, sizeof dev->config);
+
+        *netdev_devp = &dev->netdev_dev;
+    }
+
+    shash_destroy(&fetched_args);
+
     return error;
 }
 
@@ -192,8 +233,7 @@ netdev_vport_close(struct netdev *netdev_)
 }
 
 static int
-netdev_vport_reconfigure(struct netdev_dev *dev_,
-                         const struct shash *args)
+netdev_vport_set_config(struct netdev_dev *dev_, const struct shash *args)
 {
     const struct netdev_class *netdev_class = netdev_dev_get_class(dev_);
     const struct vport_class *vport_class = vport_class_cast(netdev_class);
@@ -204,7 +244,9 @@ netdev_vport_reconfigure(struct netdev_dev *dev_,
     memset(&port, 0, sizeof port);
     strncpy(port.devname, netdev_dev_get_name(dev_), sizeof port.devname);
     strncpy(port.type, netdev_dev_get_type(dev_), sizeof port.type);
-    error = vport_class->parse_config(dev_, args, port.config);
+    error = vport_class->parse_config(netdev_dev_get_name(dev_),
+                                      netdev_dev_get_type(dev_),
+                                      args, port.config);
     if (!error && memcmp(port.config, dev->config, sizeof dev->config)) {
         error = netdev_vport_do_ioctl(ODP_VPORT_MOD, &port);
         if (!error || error == ENODEV) {
@@ -633,11 +675,9 @@ netdev_vport_poll_notify(const struct netdev *netdev)
 /* Code specific to individual vport types. */
 
 static int
-parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args,
-                    void *configp)
+parse_tunnel_config(const char *name, const char *type,
+                    const struct shash *args, void *configp)
 {
-    const char *name = netdev_dev_get_name(dev);
-    const char *type = netdev_dev_get_type(dev);
     bool is_gre = false;
     bool is_ipsec = false;
     struct tnl_port_config config;
@@ -777,10 +817,63 @@ parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args,
 }
 
 static int
-parse_patch_config(const struct netdev_dev *dev, const struct shash *args,
-                   void *configp)
+unparse_tunnel_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
+                      const void *config_, struct shash *args)
+{
+    const struct tnl_port_config *config = config_;
+
+    if (!(config->flags & TNL_F_HDR_CACHE) == !(config->flags & TNL_F_IPSEC)) {
+        smap_add(args, "header_cache",
+                 config->flags & TNL_F_HDR_CACHE ? "true" : "false");
+    }
+    shash_add(args, "remote_ip", xasprintf(IP_FMT, IP_ARGS(&config->daddr)));
+    if (config->saddr) {
+        shash_add(args, "local_ip",
+                  xasprintf(IP_FMT, IP_ARGS(&config->saddr)));
+    }
+
+    if ((config->flags & (TNL_F_IN_KEY_MATCH | TNL_F_OUT_KEY_ACTION))
+                      == (TNL_F_IN_KEY_MATCH | TNL_F_OUT_KEY_ACTION)) {
+        smap_add(args, "key", "flow");
+    } else if (config->in_key && config->in_key == config->out_key) {
+        shash_add(args, "key",
+                  xasprintf("%"PRIu64, ntohll(config->in_key)));
+    } else {
+        if (config->flags & TNL_F_IN_KEY_MATCH) {
+            smap_add(args, "in_key", "flow");
+        } else if (config->in_key) {
+            shash_add(args, "in_key",
+                      xasprintf("%"PRIu64, ntohll(config->in_key)));
+        }
+
+        if (config->flags & TNL_F_OUT_KEY_ACTION) {
+            smap_add(args, "out_key", "flow");
+        } else if (config->out_key) {
+            shash_add(args, "out_key",
+                      xasprintf("%"PRIu64, ntohll(config->out_key)));
+        }
+    }
+
+    if (config->flags & TNL_F_TTL_INHERIT) {
+        smap_add(args, "tos", "inherit");
+    } else if (config->ttl) {
+        shash_add(args, "tos", xasprintf("%"PRIu8, config->ttl));
+    }
+
+    if (config->flags & TNL_F_CSUM) {
+        smap_add(args, "csum", "true");
+    }
+    if (!(config->flags & TNL_F_PMTUD)) {
+        smap_add(args, "pmtud", "false");
+    }
+
+    return 0;
+}
+
+static int
+parse_patch_config(const char *name, const char *type OVS_UNUSED,
+                   const struct shash *args, void *configp)
 {
-    const char *name = netdev_dev_get_name(dev);
     const char *peer;
 
     peer = shash_find_data(args, "peer");
@@ -808,6 +901,20 @@ parse_patch_config(const struct netdev_dev *dev, const struct shash *args,
 
     return 0;
 }
+
+static int
+unparse_patch_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
+                     const void *config_, struct shash *args)
+{
+    char peer[IFNAMSIZ];
+
+    ovs_strlcpy(peer, config_, MIN(sizeof peer, VPORT_CONFIG_SIZE));
+    if (peer[0]) {
+        smap_add(args, "peer", peer);
+    }
+
+    return 0;
+}
 
 #define VPORT_FUNCTIONS(GET_STATUS)                         \
     netdev_vport_init,                                      \
@@ -816,7 +923,7 @@ parse_patch_config(const struct netdev_dev *dev, const struct shash *args,
                                                             \
     netdev_vport_create,                                    \
     netdev_vport_destroy,                                   \
-    netdev_vport_reconfigure,                               \
+    netdev_vport_set_config,                                \
                                                             \
     netdev_vport_open,                                      \
     netdev_vport_close,                                     \
@@ -873,12 +980,13 @@ netdev_vport_register(void)
 {
     static const struct vport_class vport_classes[] = {
         { { "gre", VPORT_FUNCTIONS(netdev_vport_get_status) },
-            parse_tunnel_config },
+          parse_tunnel_config, unparse_tunnel_config },
         { { "ipsec_gre", VPORT_FUNCTIONS(netdev_vport_get_status) },
-            parse_tunnel_config },
+          parse_tunnel_config, unparse_tunnel_config },
         { { "capwap", VPORT_FUNCTIONS(netdev_vport_get_status) },
-            parse_tunnel_config },
-        { { "patch", VPORT_FUNCTIONS(NULL) }, parse_patch_config }
+          parse_tunnel_config, unparse_tunnel_config },
+        { { "patch", VPORT_FUNCTIONS(NULL) },
+          parse_patch_config, unparse_patch_config }
     };
 
     int i;

lib/netdev.c

@@ -240,7 +240,6 @@ netdev_open(struct netdev_options *options, struct netdev **netdevp)
             return error;
         }
         assert(netdev_dev->netdev_class == class);
-        update_device_args(netdev_dev, options->args);
 
     } else if (!shash_is_empty(options->args) &&
                !smap_equal(&netdev_dev->args, options->args)) {
@@ -279,7 +278,7 @@ netdev_open_default(const char *name, struct netdev **netdevp)
 /* Reconfigures the device 'netdev' with 'args'.  'args' may be empty
  * or NULL if none are needed. */
 int
-netdev_reconfigure(struct netdev *netdev, const struct shash *args)
+netdev_set_config(struct netdev *netdev, const struct shash *args)
 {
     struct shash empty_args = SHASH_INITIALIZER(&empty_args);
     struct netdev_dev *netdev_dev = netdev_get_dev(netdev);
@@ -288,19 +287,31 @@ netdev_reconfigure(struct netdev *netdev, const struct shash *args)
         args = &empty_args;
     }
 
-    if (netdev_dev->netdev_class->reconfigure) {
+    if (netdev_dev->netdev_class->set_config) {
         if (!smap_equal(&netdev_dev->args, args)) {
             update_device_args(netdev_dev, args);
-            return netdev_dev->netdev_class->reconfigure(netdev_dev, args);
+            return netdev_dev->netdev_class->set_config(netdev_dev, args);
         }
     } else if (!shash_is_empty(args)) {
-        VLOG_WARN("%s: arguments provided to device that does not have a "
-                  "reconfigure function", netdev_get_name(netdev));
+        VLOG_WARN("%s: arguments provided to device whose configuration "
+                  "cannot be changed", netdev_get_name(netdev));
     }
 
     return 0;
 }
 
+/* Returns the current configuration for 'netdev'.  This is either the
+ * configuration passed to netdev_open() or netdev_set_config(), or it is a
+ * configuration retrieved from the device itself if no configuration was
+ * passed to those functions.
+ *
+ * 'netdev' retains ownership of the returned configuration. */
+const struct shash *
+netdev_get_config(const struct netdev *netdev)
+{
+    return &netdev_get_dev(netdev)->args;
+}
+
 /* Closes and destroys 'netdev'. */
 void
 netdev_close(struct netdev *netdev)
@@ -1267,14 +1278,21 @@ exit:
     return netdev;
 }
 
-/* Initializes 'netdev_dev' as a netdev device named 'name' of the
- * specified 'netdev_class'.
+/* Initializes 'netdev_dev' as a netdev device named 'name' of the specified
+ * 'netdev_class'.  This function is ordinarily called from a netdev provider's
+ * 'create' function.
+ *
+ * 'args' should be the arguments that were passed to the netdev provider's
+ * 'create'.  If an empty set of arguments was passed, and 'name' is the name
+ * of a network device that existed before the 'create' call, then 'args' may
+ * instead be the configuration for that existing device.
  *
  * This function adds 'netdev_dev' to a netdev-owned shash, so it is
  * very important that 'netdev_dev' only be freed after calling
  * the refcount drops to zero.  */
 void
 netdev_dev_init(struct netdev_dev *netdev_dev, const char *name,
+                const struct shash *args,
                 const struct netdev_class *netdev_class)
 {
     assert(!shash_find(&netdev_dev_shash, name));
@@ -1283,7 +1301,7 @@ netdev_dev_init(struct netdev_dev *netdev_dev, const char *name,
     netdev_dev->netdev_class = netdev_class;
     netdev_dev->name = xstrdup(name);
     netdev_dev->node = shash_add(&netdev_dev_shash, name, netdev_dev);
-    shash_init(&netdev_dev->args);
+    smap_clone(&netdev_dev->args, args);
 }
 
 /* Undoes the results of initialization.

lib/netdev.h

@@ -98,7 +98,6 @@ void netdev_enumerate_types(struct svec *types);
 /* Open and close. */
 int netdev_open(struct netdev_options *, struct netdev **);
 int netdev_open_default(const char *name, struct netdev **);
-int netdev_reconfigure(struct netdev *, const struct shash *args);
 void netdev_close(struct netdev *);
 
 bool netdev_exists(const char *name);
@@ -106,6 +105,10 @@ bool netdev_is_open(const char *name);
 
 int netdev_enumerate(struct svec *);
 
+/* Options. */
+int netdev_set_config(struct netdev *, const struct shash *args);
+const struct shash *netdev_get_config(const struct netdev *);
+
 /* Basic properties. */
 const char *netdev_get_name(const struct netdev *);
 const char *netdev_get_type(const struct netdev *);

vswitchd/bridge.c

@@ -733,7 +733,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 shash_from_ovs_idl_map(iface->cfg->key_options,
                                        iface->cfg->value_options,
                                        iface->cfg->n_options, &args);
-                netdev_reconfigure(iface->netdev, &args);
+                netdev_set_config(iface->netdev, &args);
                 shash_destroy(&args);
             }
         }