From 0f4f4a610a24098a98de64bccaeb29ba5621e01d Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Mon, 18 Jan 2010 17:26:02 -0500 Subject: [PATCH] netdev: Compare full arguments instead of hash for reconfigure. We only reconfigure netdevs if the arguments have changed, which was previously detected based on a hash. This stores and compares the full argument list to avoid any chance of missing changes due to collisions. --- lib/netdev-provider.h | 8 ++- lib/netdev.c | 118 ++++++++++++++++++++++++++++++------------ 2 files changed, 92 insertions(+), 34 deletions(-) diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index b009c8dc7..a6c0fd894 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -25,6 +25,11 @@ #include "list.h" #include "shash.h" +struct arg { + char *key; + char *value; +}; + /* A network device (e.g. an Ethernet device). * * This structure should be treated as opaque by network device @@ -34,7 +39,8 @@ struct netdev_dev { const struct netdev_class *class; /* Functions to control this device. */ int ref_cnt; /* Times this devices was opened. */ struct shash_node *node; /* Pointer to element in global map. */ - uint32_t args_hash; /* Hash of arguments for the device. */ + struct arg *args; /* Argument list from last config. */ + int n_args; /* Number of arguments in 'args'. */ }; void netdev_dev_init(struct netdev_dev *, const char *name, diff --git a/lib/netdev.c b/lib/netdev.c index 6a95e1a4f..84efc6bc3 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -59,6 +59,7 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); static void close_all_netdevs(void *aux UNUSED); static int restore_flags(struct netdev *netdev); +void update_device_args(struct netdev_dev *, const struct shash *args); /* Attempts to initialize the netdev module. Returns 0 if successful, * otherwise a positive errno value. @@ -131,6 +132,75 @@ netdev_wait(void) } } +/* Compares 'args' to those used to those used by 'dev'. Returns true + * if the arguments are the same, false otherwise. Does not update the + * values stored in 'dev'. */ +static bool +compare_device_args(const struct netdev_dev *dev, const struct shash *args) +{ + const struct shash_node **new_args; + bool result = true; + int i; + + if (shash_count(args) != dev->n_args) { + return false; + } + + new_args = shash_sort(args); + for (i = 0; i < dev->n_args; i++) { + if (strcmp(dev->args[i].key, new_args[i]->name) || + strcmp(dev->args[i].value, new_args[i]->data)) { + result = false; + goto finish; + } + } + +finish: + free(new_args); + return result; +} + +static int +compare_args(const void *a_, const void *b_) +{ + const struct arg *a = a_; + const struct arg *b = b_; + return strcmp(a->key, b->key); +} + +void +update_device_args(struct netdev_dev *dev, const struct shash *args) +{ + struct shash_node *node; + int i; + + if (dev->n_args) { + for (i = 0; i < dev->n_args; i++) { + free(dev->args[i].key); + free(dev->args[i].value); + } + + free(dev->args); + dev->n_args = 0; + } + + if (!args || shash_is_empty(args)) { + return; + } + + dev->n_args = shash_count(args); + dev->args = xmalloc(dev->n_args * sizeof *dev->args); + + i = 0; + SHASH_FOR_EACH(node, args) { + dev->args[i].key = xstrdup(node->name); + dev->args[i].value = xstrdup(node->data); + i++; + } + + qsort(dev->args, dev->n_args, sizeof *dev->args, compare_args); +} + static int create_device(struct netdev_options *options, struct netdev_dev **netdev_devp) { @@ -161,22 +231,6 @@ create_device(struct netdev_options *options, struct netdev_dev **netdev_devp) return EINVAL; } -static uint32_t -shash_hash(const struct shash *shash) -{ - int hash = 0; - struct shash_node *node; - uint32_t entry_hash; - - SHASH_FOR_EACH(node, shash) { - entry_hash = hash_string(node->name, 0); - entry_hash ^= hash_string(node->data, 10); - hash ^= hash_int(entry_hash, 0); - } - - return hash; -} - /* Opens the network device named 'name' (e.g. "eth0") and returns zero if * successful, otherwise a positive errno value. On success, sets '*netdevp' * to the new network device, otherwise to null. @@ -218,21 +272,18 @@ netdev_open(struct netdev_options *options, struct netdev **netdevp) if (error) { return error; } - - netdev_dev->args_hash = shash_hash(options->args); + update_device_args(netdev_dev, options->args); } else if (options->may_open) { - if (!shash_is_empty(options->args)) { - uint32_t args_hash = shash_hash(options->args); + if (!shash_is_empty(options->args) && + !compare_device_args(netdev_dev, options->args)) { - if (args_hash != netdev_dev->args_hash) { - VLOG_WARN("attempted to open already created netdev with " - "different arguments: %s", options->name); - return EINVAL; - } + VLOG_WARN("%s: attempted to open already created netdev with " + "different arguments", options->name); + return EINVAL; } } else { - VLOG_WARN("attempted to create a netdev device with bound name: %s", + VLOG_WARN("%s: attempted to create a netdev device with bound name", options->name); return EEXIST; } @@ -278,12 +329,13 @@ netdev_reconfigure(struct netdev *netdev, const struct shash *args) } if (netdev_dev->class->reconfigure) { - uint32_t args_hash = shash_hash(args); - - if (netdev_dev->args_hash != args_hash) { - netdev_dev->args_hash = args_hash; + if (!compare_device_args(netdev_dev, args)) { + update_device_args(netdev_dev, args); return netdev_dev->class->reconfigure(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)); } return 0; @@ -843,8 +895,8 @@ netdev_dev_init(struct netdev_dev *netdev_dev, const char *name, { assert(!shash_find(&netdev_dev_shash, name)); + memset(netdev_dev, 0, sizeof *netdev_dev); netdev_dev->class = class; - netdev_dev->ref_cnt = 0; netdev_dev->name = xstrdup(name); netdev_dev->node = shash_add(&netdev_dev_shash, name, netdev_dev); } @@ -864,6 +916,7 @@ netdev_dev_uninit(struct netdev_dev *netdev_dev, bool destroy) assert(!netdev_dev->ref_cnt); shash_delete(&netdev_dev_shash, netdev_dev->node); + update_device_args(netdev_dev, NULL); if (destroy) { netdev_dev->class->destroy(netdev_dev); @@ -923,9 +976,8 @@ netdev_dev_get_devices(const struct netdev_class *class, void netdev_init(struct netdev *netdev, struct netdev_dev *netdev_dev) { + memset(netdev, 0, sizeof *netdev); netdev->netdev_dev = netdev_dev; - netdev->save_flags = 0; - netdev->changed_flags = 0; list_push_back(&netdev_list, &netdev->node); } -- 2.43.0