Acked-by: Andy Zhou <az...@nicira.com>
On Mon, Aug 12, 2013 at 12:54 PM, Ben Pfaff <b...@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 <b...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
