[ovs-dev] [PATCH 1/3] netdev-dummy: Include all dummy classes in iterations.

Andy Zhou azhou at nicira.com
Mon Aug 12 20:46:15 UTC 2013


Acked-by: Andy Zhou <azhou at nicira.com>



On Mon, Aug 12, 2013 at 12:54 PM, Ben Pfaff <blp at nicira.com> wrote:

> Commit 86f1d0326bd0 (netdev-dummy: Use netdev_get_devices() instead of a
> local shash.) caused netdev-dummy functions that iterate over all dummy
> devices to iterate only over the ones that have class 'dummy_class'.  This
> seemed to obviously include all the ones that we want, but in fact
> when ovs-vswitch is invoked with --enable-dummy=override, there are more
> dummy classes than just dummy_class, which this new form of iteration
> skipped over, with various negative consequences that showed up in some
> testing.
>
> This commit switches netdev-dummy back to internally tracking its own
> dummy devices.  It fixes the tests for me.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/netdev-dummy.c |   61
> ++++++++++++++++++++++++++--------------------------
>  1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index 7c43f12..5c31210 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -44,11 +44,21 @@ struct dummy_stream {
>      struct list txq;
>  };
>
> +/* Protects 'dummy_list'. */
> +static struct ovs_mutex dummy_list_mutex = OVS_MUTEX_INITIALIZER;
> +
> +/* Contains all 'struct dummy_dev's. */
> +static struct list dummy_list OVS_GUARDED_BY(dummy_list_mutex)
> +    = LIST_INITIALIZER(&dummy_list);
> +
>  struct netdev_dummy {
>      struct netdev up;
>
> +    /* In dummy_list. */
> +    struct list list_node OVS_GUARDED_BY(dummy_list_mutex);
> +
>      /* Protects all members below. */
> -    struct ovs_mutex mutex;
> +    struct ovs_mutex mutex OVS_ACQ_AFTER(dummy_list_mutex);
>
>      uint8_t hwaddr[ETH_ADDR_LEN];
>      int mtu;
> @@ -64,8 +74,6 @@ struct netdev_dummy {
>      struct list rxes;           /* List of child "netdev_rx_dummy"s. */
>  };
>
> -static const struct netdev_class dummy_class;
> -
>  /* Max 'recv_queue_len' in struct netdev_dummy. */
>  #define NETDEV_DUMMY_MAX_QUEUE 100
>
> @@ -108,13 +116,10 @@ netdev_rx_dummy_cast(const struct netdev_rx *rx)
>  static void
>  netdev_dummy_run(void)
>  {
> -    struct shash dummy_netdevs;
> -    struct shash_node *node;
> +    struct netdev_dummy *dev;
>
> -    shash_init(&dummy_netdevs);
> -    netdev_get_devices(&dummy_class, &dummy_netdevs);
> -    SHASH_FOR_EACH (node, &dummy_netdevs) {
> -        struct netdev_dummy *dev = node->data;
> +    ovs_mutex_lock(&dummy_list_mutex);
> +    LIST_FOR_EACH (dev, list_node, &dummy_list) {
>          size_t i;
>
>          ovs_mutex_lock(&dev->mutex);
> @@ -211,9 +216,8 @@ netdev_dummy_run(void)
>          }
>
>          ovs_mutex_unlock(&dev->mutex);
> -        netdev_close(&dev->up);
>      }
> -    shash_destroy(&dummy_netdevs);
> +    ovs_mutex_unlock(&dummy_list_mutex);
>  }
>
>  static void
> @@ -227,13 +231,10 @@ dummy_stream_close(struct dummy_stream *s)
>  static void
>  netdev_dummy_wait(void)
>  {
> -    struct shash dummy_netdevs;
> -    struct shash_node *node;
> +    struct netdev_dummy *dev;
>
> -    shash_init(&dummy_netdevs);
> -    netdev_get_devices(&dummy_class, &dummy_netdevs);
> -    SHASH_FOR_EACH (node, &dummy_netdevs) {
> -        struct netdev_dummy *dev = node->data;
> +    ovs_mutex_lock(&dummy_list_mutex);
> +    LIST_FOR_EACH (dev, list_node, &dummy_list) {
>          size_t i;
>
>          ovs_mutex_lock(&dev->mutex);
> @@ -250,9 +251,8 @@ netdev_dummy_wait(void)
>              stream_recv_wait(s->stream);
>          }
>          ovs_mutex_unlock(&dev->mutex);
> -        netdev_close(&dev->up);
>      }
> -    shash_destroy(&dummy_netdevs);
> +    ovs_mutex_unlock(&dummy_list_mutex);
>  }
>
>  static struct netdev *
> @@ -289,6 +289,10 @@ netdev_dummy_construct(struct netdev *netdev_)
>
>      list_init(&netdev->rxes);
>
> +    ovs_mutex_lock(&dummy_list_mutex);
> +    list_push_back(&dummy_list, &netdev->list_node);
> +    ovs_mutex_unlock(&dummy_list_mutex);
> +
>      return 0;
>  }
>
> @@ -298,6 +302,10 @@ netdev_dummy_destruct(struct netdev *netdev_)
>      struct netdev_dummy *netdev = netdev_dummy_cast(netdev_);
>      size_t i;
>
> +    ovs_mutex_lock(&dummy_list_mutex);
> +    list_remove(&netdev->list_node);
> +    ovs_mutex_unlock(&dummy_list_mutex);
> +
>      pstream_close(netdev->pstream);
>      for (i = 0; i < netdev->n_streams; i++) {
>          dummy_stream_close(&netdev->streams[i]);
> @@ -863,22 +871,15 @@ netdev_dummy_set_admin_state(struct unixctl_conn
> *conn, int argc,
>              return;
>          }
>      } else {
> -        struct shash dummy_netdevs;
> -        struct shash_node *node;
> -
> -        shash_init(&dummy_netdevs);
> -        netdev_get_devices(&dummy_class, &dummy_netdevs);
> -        SHASH_FOR_EACH (node, &dummy_netdevs) {
> -            struct netdev *netdev_ = node->data;
> -            struct netdev_dummy *netdev = netdev_dummy_cast(netdev_);
> +        struct netdev_dummy *netdev;
>
> +        ovs_mutex_lock(&dummy_list_mutex);
> +        LIST_FOR_EACH (netdev, list_node, &dummy_list) {
>              ovs_mutex_lock(&netdev->mutex);
>              netdev_dummy_set_admin_state__(netdev, up);
>              ovs_mutex_unlock(&netdev->mutex);
> -
> -            netdev_close(netdev_);
>          }
> -        shash_destroy(&dummy_netdevs);
> +        ovs_mutex_unlock(&dummy_list_mutex);
>      }
>      unixctl_command_reply(conn, "OK");
>  }
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130812/927499e1/attachment-0003.html>


More information about the dev mailing list