netdev: Add methods to do netdev-specific argument comparisons.

When doing a netdev_open(), a check is first done to make sure the
arguments are equivalent for any open devices with the same name.  In
most cases, a simple shash comparison is sufficient.  However, IPsec
key configuration is handled by an external program, so it is not pushed
down into the kernel module.  Thus, when the "unparse_config" method is
called on an existing IPsec-based vport, a simple comparison with the
returned data will not match the original configuration.  This commit
adds code to allow netdev-specific argument comparisons and has
"ipsec_gre" make use of them.

Bug #5575

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 472bdb85bde2d01650dfaabb62a27aeb32f40100..9cd06f194b357be4008b49321def99e2d1a0e71c 100644 (file)
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -226,7 +226,8 @@ static const struct netdev_class dummy_class = {
 
     netdev_dummy_create,
     netdev_dummy_destroy,
-    NULL,
+    NULL,                       /* set_config */
+    NULL,                       /* config_equal */
 
     netdev_dummy_open,
     netdev_dummy_close,

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 04c6226a8f76f6d356e509fd661db02d3938fc07..166728262b81f4b6e80ac1e54ae94c4b3de7d9d2 100644 (file)
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2247,6 +2247,7 @@ netdev_linux_change_seq(const struct netdev *netdev)
     CREATE,                                                     \
     netdev_linux_destroy,                                       \
     NULL,                       /* set_config */                \
+    NULL,                       /* config_equal */              \
                                                                 \
     netdev_linux_open,                                          \
     netdev_linux_close,                                         \

diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 67690143ccad7dc015514bfde8678ba180411f26..7bb4eac8386322ed95aeda2fb4e0e39a2c38bd62 100644 (file)
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -52,6 +52,8 @@ const char *netdev_dev_get_name(const struct netdev_dev *);
 struct netdev_dev *netdev_dev_from_name(const char *name);
 void netdev_dev_get_devices(const struct netdev_class *,
                             struct shash *device_list);
+bool netdev_dev_args_equal(const struct netdev_dev *netdev_dev,
+                           const struct shash *args);
 
 static inline void netdev_dev_assert_class(const struct netdev_dev *netdev_dev,
                                            const struct netdev_class *class_)
@@ -135,6 +137,15 @@ struct netdev_class {
      */
     int (*set_config)(struct netdev_dev *netdev_dev, const struct shash *args);
 
+    /* Returns true if 'args' is equivalent to the "args" field in
+     * 'netdev_dev', otherwise false.
+     *
+     * If no special processing needs to be done beyond a simple
+     * shash comparison, this may be a null pointer.
+     */
+    bool (*config_equal)(const struct netdev_dev *netdev_dev,
+                         const struct shash *args);
+
     /* Attempts to open a network device.  On success, sets 'netdevp'
      * to the new network device.
      *

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 28a1bfb49eaf38ed350f97655f938ecb7726844d..bc7d05016dd42fc710cc25f39d103c757bcbdf38 100644 (file)
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -68,6 +68,7 @@ struct vport_class {
     int (*unparse_config)(const char *name, const char *type,
                           const struct nlattr *options, size_t options_len,
                           struct shash *args);
+    bool (*config_equal)(const struct shash *nd_args, const struct shash *args);
 };
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
@@ -315,6 +316,20 @@ netdev_vport_set_config(struct netdev_dev *dev_, const struct shash *args)
     return error;
 }
 
+static bool
+netdev_vport_config_equal(const 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);
+
+    if (vport_class->config_equal) {
+        return vport_class->config_equal(&dev_->args, args);
+    } else {
+        return smap_equal(&dev_->args, args);
+    }
+}
+
 static int
 netdev_vport_send(struct netdev *netdev, const void *data, size_t size)
 {
@@ -868,6 +883,37 @@ unparse_patch_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
     smap_add(args, "peer", nl_attr_get_string(a[ODP_PATCH_ATTR_PEER]));
     return 0;
 }
+
+/* Returns true if 'nd_args' is equivalent to 'args', otherwise false.
+ * Typically, 'nd_args' is the result of a call to unparse_tunnel_config()
+ * and 'args' is the original definition of the port.
+ *
+ * IPsec key configuration is handled by an external program, so it is not
+ * pushed down into the kernel module.  Thus, when the "unparse_config"
+ * method is called on an existing IPsec-based vport, a simple
+ * comparison with the returned data will not match the original
+ * configuration.  This function ignores configuration about keys when
+ * doing a comparison.
+ */
+static bool
+config_equal_ipsec(const struct shash *nd_args, const struct shash *args)
+{
+        struct shash tmp;
+        bool result;
+
+        smap_clone(&tmp, args);
+
+        shash_find_and_delete(&tmp, "psk");
+        shash_find_and_delete(&tmp, "peer_cert");
+        shash_find_and_delete(&tmp, "certificate");
+        shash_find_and_delete(&tmp, "private_key");
+        shash_find_and_delete(&tmp, "use_ssl_cert");
+
+        result = smap_equal(&tmp, nd_args);
+        smap_destroy(&tmp);
+ 
+        return result;
+}
 \f
 #define VPORT_FUNCTIONS(GET_STATUS)                         \
     NULL,                                                   \
@@ -877,6 +923,7 @@ unparse_patch_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
     netdev_vport_create,                                    \
     netdev_vport_destroy,                                   \
     netdev_vport_set_config,                                \
+    netdev_vport_config_equal,                              \
                                                             \
     netdev_vport_open,                                      \
     netdev_vport_close,                                     \
@@ -933,19 +980,19 @@ netdev_vport_register(void)
     static const struct vport_class vport_classes[] = {
         { ODP_VPORT_TYPE_GRE,
           { "gre", VPORT_FUNCTIONS(netdev_vport_get_status) },
-          parse_tunnel_config, unparse_tunnel_config },
+          parse_tunnel_config, unparse_tunnel_config, NULL },
 
         { ODP_VPORT_TYPE_GRE,
           { "ipsec_gre", VPORT_FUNCTIONS(netdev_vport_get_status) },
-          parse_tunnel_config, unparse_tunnel_config },
+          parse_tunnel_config, unparse_tunnel_config, config_equal_ipsec },
 
         { ODP_VPORT_TYPE_CAPWAP,
           { "capwap", VPORT_FUNCTIONS(netdev_vport_get_status) },
-          parse_tunnel_config, unparse_tunnel_config },
+          parse_tunnel_config, unparse_tunnel_config, NULL },
 
         { ODP_VPORT_TYPE_PATCH,
           { "patch", VPORT_FUNCTIONS(NULL) },
-          parse_patch_config, unparse_patch_config }
+          parse_patch_config, unparse_patch_config, NULL }
     };
 
     int i;

diff --git a/lib/netdev.c b/lib/netdev.c
index 1f0b764f99caa66927a503b3d3d8c39ec4125d20..b8592c19f9e3e1760734a593d26f933b441be380 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -243,7 +243,7 @@ netdev_open(struct netdev_options *options, struct netdev **netdevp)
         assert(netdev_dev->netdev_class == class);
 
     } else if (!shash_is_empty(options->args) &&
-               !smap_equal(&netdev_dev->args, options->args)) {
+               !netdev_dev_args_equal(netdev_dev, options->args)) {
 
         VLOG_WARN("%s: attempted to open already open netdev with "
                   "different arguments", options->name);
@@ -289,7 +289,7 @@ netdev_set_config(struct netdev *netdev, const struct shash *args)
     }
 
     if (netdev_dev->netdev_class->set_config) {
-        if (!smap_equal(&netdev_dev->args, args)) {
+        if (!netdev_dev_args_equal(netdev_dev, args)) {
             update_device_args(netdev_dev, args);
             return netdev_dev->netdev_class->set_config(netdev_dev, args);
         }
@@ -1382,6 +1382,19 @@ netdev_dev_get_devices(const struct netdev_class *netdev_class,
     }
 }
 
+/* Returns true if 'args' is equivalent to the "args" field in
+ * 'netdev_dev', otherwise false. */
+bool
+netdev_dev_args_equal(const struct netdev_dev *netdev_dev,
+                      const struct shash *args)
+{
+    if (netdev_dev->netdev_class->config_equal) {
+        return netdev_dev->netdev_class->config_equal(netdev_dev, args);
+    } else {
+        return smap_equal(&netdev_dev->args, args);
+    }
+}
+
 /* Initializes 'netdev' as a instance of the netdev_dev.
  *
  * This function adds 'netdev' to a netdev-owned linked list, so it is very