From: Justin Pettit Date: Tue, 14 Jun 2011 02:26:47 +0000 (-0700) Subject: netdev: Add methods to do netdev-specific argument comparisons. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aebf4235f3938b9e8865d4eb4a767d7584478d30;p=openvswitch 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 472bdb85..9cd06f19 100644 --- 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 04c6226a..16672826 100644 --- 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 67690143..7bb4eac8 100644 --- 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 28a1bfb4..bc7d0501 100644 --- 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; +} #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 1f0b764f..b8592c19 100644 --- 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