Before this patch, if an interface was created with an unsupported
key in its options column, ovs-vswitchd would ignore the offending
key and create the interface. Later on, if ovs-vswitchd was
restarted, it would recreate netdevs for the interfaces in the
datapath. Then it would attempt to create netdevs for interfaces
in the database, ignoring any that had equivalent configuration to
a device already in the datapath. Ovs-vswitchd would fail to
recognize the datapath device as equivalent to the database device
because the database device would have the unsupported key in its
configuration which the datapath device did not. This would render
the switch in an inconsistent state.
This patch fixes the issue by disallowing the creation of netdevs
with unsupported configuration options.
---
lib/netdev-dummy.c | 1 +
lib/netdev-linux.c | 1 +
lib/netdev-provider.h | 15 ++++++++++
lib/netdev-vport.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++-
lib/netdev.c | 30 +++++++++++++++------
5 files changed, 106 insertions(+), 10 deletions(-)
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 9cd06f1..95f18f4 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -226,6 +226,7 @@ static const struct netdev_class dummy_class = {
netdev_dummy_create,
netdev_dummy_destroy,
+ NULL, /* config_parsable */
NULL, /* set_config */
NULL, /* config_equal */
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 385c0b8..858579e 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2246,6 +2246,7 @@ netdev_linux_change_seq(const struct netdev *netdev)
\
CREATE, \
netdev_linux_destroy, \
+ NULL, /* config_parsable */ \
NULL, /* set_config */ \
NULL, /* config_equal */ \
\
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 7bb4eac..4f3d548 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -130,6 +130,21 @@ struct netdev_class {
* called. */
void (*destroy)(struct netdev_dev *netdev_dev);
+ /* Checks that 'args' is parsable by this class.
+ *
+ * Minimally, this function should check that 'args' contains no arguments
+ * which are unsupported by this class. It may do additional sanity
+ * checking if appropriate. This function will be called before the
+ * ->set_config() and ->config_equal() functions on a given set of
+ * arguments. If it returns false, neither for these functions will be
+ * called on 'args'.
+ *
+ * A NULL ->config_parsable() function indicates that 'args' should be
+ * empty. Callers will validate that 'args' is in-fact empty before
+ * continuing with processing on it. */
+ bool (*config_parsable)(const struct netdev_class *,
+ const struct shash *args);
+
/* Changes the device 'netdev_dev''s configuration to 'args'.
*
* If this netdev class does not support reconfiguring a netdev
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index b9c1bfe..4c59b91 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -44,6 +44,7 @@
#include "rtnetlink.h"
#include "shash.h"
#include "socket-util.h"
+#include "sset.h"
#include "vlog.h"
VLOG_DEFINE_THIS_MODULE(netdev_vport);
@@ -317,6 +318,71 @@ netdev_vport_set_config(struct netdev_dev *dev_, const
struct shash *args)
}
static bool
+netdev_vport_config_parsable(const struct netdev_class *netdev_class,
+ const struct shash *args_)
+{
+ const struct vport_class *vport_class = vport_class_cast(netdev_class);
+ const char *type = vport_class->netdev_class.type;
+ struct sset options = SSET_INITIALIZER(&options);
+ bool parsable = true;
+
+ struct shash args;
+ const char *arg;
+
+ smap_clone(&args, args_);
+
+ if (!strcmp(type, "patch")) {
+ sset_add(&options, "peer");
+ } else {
+ sset_add(&options, "remote_ip");
+ sset_add(&options, "local_ip");
+ sset_add(&options, "tos");
+ sset_add(&options, "df_inherit");
+ sset_add(&options, "df_default");
+ sset_add(&options, "ttl");
+ sset_add(&options, "pmtud");
+
+ if (!strcmp(type, "capwap")) {
+ sset_add(&options, "header_cache");
+ } else {
+ sset_add(&options, "in_key");
+ sset_add(&options, "out_key");
+ sset_add(&options, "key");
+ sset_add(&options, "csum");
+
+ if (!strcmp(type, "gre")) {
+ sset_add(&options, "header_cache");
+ } else if (!strcmp(type, "ipsec_gre")) {
+ sset_add(&options, "peer_cert");
+ sset_add(&options, "certificate");
+ sset_add(&options, "private_key");
+ sset_add(&options, "psk");
+ } else {
+ NOT_REACHED();
+ }
+ }
+ }
+
+ SSET_FOR_EACH (arg, &options) {
+ shash_find_and_delete(&args, arg);
+ }
+
+ if (!shash_is_empty(&args)) {
+ struct shash_node *sn;
+
+ parsable = false;
+ SHASH_FOR_EACH (sn, &args) {
+ VLOG_WARN("%s device: unsupported argument: %s",
+ netdev_class->type, sn->name);
+ }
+ }
+
+ smap_destroy(&args);
+ sset_destroy(&options);
+ return parsable;
+}
+
+static bool
netdev_vport_config_equal(const struct netdev_dev *dev_,
const struct shash *args)
{
@@ -918,7 +984,7 @@ config_equal_ipsec(const struct shash *nd_args, const
struct shash *args)
result = smap_equal(&tmp1, &tmp2);
smap_destroy(&tmp1);
smap_destroy(&tmp2);
-
+
return result;
}
@@ -929,6 +995,7 @@ config_equal_ipsec(const struct shash *nd_args, const
struct shash *args)
\
netdev_vport_create, \
netdev_vport_destroy, \
+ netdev_vport_config_parsable, \
netdev_vport_set_config, \
netdev_vport_config_equal, \
\
diff --git a/lib/netdev.c b/lib/netdev.c
index b8592c1..8fda484 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -214,6 +214,7 @@ int
netdev_open(struct netdev_options *options, struct netdev **netdevp)
{
struct shash empty_args = SHASH_INITIALIZER(&empty_args);
+ const struct netdev_class *class;
struct netdev_dev *netdev_dev;
int error;
@@ -224,17 +225,28 @@ netdev_open(struct netdev_options *options, struct netdev
**netdevp)
options->args = &empty_args;
}
+ class = netdev_lookup_provider(options->type);
+ if (!class) {
+ VLOG_WARN("could not open netdev %s of unknown type %s", options->name,
+ options->type);
+ return EAFNOSUPPORT;
+ }
+
+ if (class->config_parsable) {
+ if (!class->config_parsable(class, options->args)) {
+ VLOG_WARN("%s: arguments for %s device are unparsable",
+ options->name, options->type);
+ return EINVAL;
+ }
+ } else if (!shash_is_empty(options->args)) {
+ VLOG_WARN("%s: arguments for %s devices should be empty",
+ options->name, options->type);
+ return EINVAL;
+ }
+
netdev_dev = shash_find_data(&netdev_dev_shash, options->name);
if (!netdev_dev) {
- const struct netdev_class *class;
-
- class = netdev_lookup_provider(options->type);
- if (!class) {
- VLOG_WARN("could not create netdev %s of unknown type %s",
- options->name, options->type);
- return EAFNOSUPPORT;
- }
error = class->create(class, options->name, options->args,
&netdev_dev);
if (error) {
@@ -652,7 +664,7 @@ netdev_set_advertisements(struct netdev *netdev, uint32_t
advertise)
*
* - EOPNOTSUPP: No IPv4 network stack attached to 'netdev'.
*
- * 'address' or 'netmask' or both may be null, in which case the address or
+ * 'address' or 'netmask' or both may be null, in which case the address or
* netmask is not reported. */
int
netdev_get_in4(const struct netdev *netdev,
--
1.7.6
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev