diff mbox

[ovs-dev] netdev-dpdk : vhost-user port link state fix

Message ID 397FD63D94A70042B39B21380C6225A829BA37E0@ESESSMB305.ericsson.se
State Accepted
Headers show

Commit Message

Zoltan Balogh June 2, 2016, 12:42 p.m. UTC 
Hi Daniele,

I fixed the patch based on your comments:

OVS reports that link state of a vhost-user port (type=dpdkvhostuser) is DOWN, even when traffic is running through the port between a Virtual Machine and the vSwitch.
Changing admin state with the "ovs-ofctl mod-port <BR> <PORT> up/down" command over OpenFlow does affect neither the reported link state nor the traffic.

The patch below does the flowing:
 - Triggers link state change by altering netdev's change_seq member.
 - Controls sending/receiving of packets through vhost-user port according to the port's current admin state.
 - Sets admin state of newly created vhost-user port to UP.

Signed-off-by: Zoltán Balogh <zoltan.balogh@ericsson.com>

Co-authored-by: Jan Scheurich <jan.scheurich@ericsson.com>
Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>


---

Comments

Daniele Di Proietto June 2, 2016, 9:56 p.m. UTC | #1 
I changed 'netdev' to 'dev' in netdev_dpdk_init(), added Jan to AUTHORS and
pushed this to master.

Thanks!

Daniele

2016-06-02 5:42 GMT-07:00 Zoltán Balogh <zoltan.balogh@ericsson.com>:

> Hi Daniele,
>
> I fixed the patch based on your comments:
>
> OVS reports that link state of a vhost-user port (type=dpdkvhostuser) is
> DOWN, even when traffic is running through the port between a Virtual
> Machine and the vSwitch.
> Changing admin state with the "ovs-ofctl mod-port <BR> <PORT> up/down"
> command over OpenFlow does affect neither the reported link state nor the
> traffic.
>
> The patch below does the flowing:
>  - Triggers link state change by altering netdev's change_seq member.
>  - Controls sending/receiving of packets through vhost-user port according
> to the port's current admin state.
>  - Sets admin state of newly created vhost-user port to UP.
>
> Signed-off-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
> Co-authored-by: Jan Scheurich <jan.scheurich@ericsson.com>
> Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
>
> ---
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 6cae930..9473bdb 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -797,6 +797,8 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int
> port_no,
>          }
>      } else {
>          netdev_dpdk_alloc_txq(dev, OVS_VHOST_MAX_QUEUE_NUM);
> +        /* Enable DPDK_DEV_VHOST device and set promiscuous mode flag. */
> +        netdev->flags = NETDEV_UP | NETDEV_PROMISC;
>
     }
>
>      ovs_list_push_back(&dpdk_list, &dev->list_node);
> @@ -1256,7 +1258,8 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
>      uint16_t nb_rx = 0;
>      uint16_t dropped = 0;
>
> -    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
> +    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev)
> +                     || !(dev->flags & NETDEV_UP))) {
>          return EAGAIN;
>      }
>
> @@ -1378,7 +1381,8 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int
> qid,
>
>      qid = dev->tx_q[qid % dev->real_n_txq].map;
>
> -    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid < 0)) {
> +    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid < 0
> +                     || !(dev->flags & NETDEV_UP))) {
>          rte_spinlock_lock(&dev->stats_lock);
>          dev->stats.tx_dropped+= cnt;
>          rte_spinlock_unlock(&dev->stats_lock);
> @@ -2117,6 +2121,23 @@ netdev_dpdk_update_flags__(struct netdev_dpdk *dev,
>          if (!(dev->flags & NETDEV_UP)) {
>              rte_eth_dev_stop(dev->port_id);
>          }
> +    } else {
> +        /* If DPDK_DEV_VHOST device's NETDEV_UP flag was changed and
> vhost is
> +         * running then change netdev's change_seq to trigger link state
> +         * update. */
> +        struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(dev);
> +
> +        if ((NETDEV_UP & ((*old_flagsp ^ on) | (*old_flagsp ^ off)))
> +            && is_vhost_running(virtio_dev)) {
> +            netdev_change_seq_changed(&dev->up);
> +
> +            /* Clear statistics if device is getting up. */
> +            if (NETDEV_UP & on) {
> +                rte_spinlock_lock(&dev->stats_lock);
> +                memset(&dev->stats, 0, sizeof(dev->stats));
> +                rte_spinlock_unlock(&dev->stats_lock);
> +            }
> +        }
>      }
>
>      return 0;
> @@ -2339,6 +2360,7 @@ new_device(struct virtio_net *virtio_dev)
>              virtio_dev->flags |= VIRTIO_DEV_RUNNING;
>              /* Disable notifications. */
>              set_irq_status(virtio_dev);
> +            netdev_change_seq_changed(&dev->up);
>              ovs_mutex_unlock(&dev->mutex);
>              break;
>          }
> @@ -2390,6 +2412,7 @@ destroy_device(volatile struct virtio_net
> *virtio_dev)
>              ovsrcu_set(&dev->virtio_dev, NULL);
>              netdev_dpdk_txq_map_clear(dev);
>              exists = true;
> +            netdev_change_seq_changed(&dev->up);
>              ovs_mutex_unlock(&dev->mutex);
>              break;
>          }
>
>
Ciara Loftus July 15, 2016, 9:12 a.m. UTC | #2 
> 

> I changed 'netdev' to 'dev' in netdev_dpdk_init(), added Jan to AUTHORS

> and

> pushed this to master.


Another backport request - could this be pushed to 2.5 too?

Thanks,
Ciara

> 

> Thanks!

> 

> Daniele

> 

> 2016-06-02 5:42 GMT-07:00 Zoltán Balogh <zoltan.balogh@ericsson.com>:

> 

> > Hi Daniele,

> >

> > I fixed the patch based on your comments:

> >

> > OVS reports that link state of a vhost-user port (type=dpdkvhostuser) is

> > DOWN, even when traffic is running through the port between a Virtual

> > Machine and the vSwitch.

> > Changing admin state with the "ovs-ofctl mod-port <BR> <PORT>

> up/down"

> > command over OpenFlow does affect neither the reported link state nor

> the

> > traffic.

> >

> > The patch below does the flowing:

> >  - Triggers link state change by altering netdev's change_seq member.

> >  - Controls sending/receiving of packets through vhost-user port according

> > to the port's current admin state.

> >  - Sets admin state of newly created vhost-user port to UP.

> >

> > Signed-off-by: Zoltán Balogh <zoltan.balogh@ericsson.com>

> > Co-authored-by: Jan Scheurich <jan.scheurich@ericsson.com>

> > Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>

> >

> > ---

> >

> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c

> > index 6cae930..9473bdb 100644

> > --- a/lib/netdev-dpdk.c

> > +++ b/lib/netdev-dpdk.c

> > @@ -797,6 +797,8 @@ netdev_dpdk_init(struct netdev *netdev, unsigned

> int

> > port_no,

> >          }

> >      } else {

> >          netdev_dpdk_alloc_txq(dev, OVS_VHOST_MAX_QUEUE_NUM);

> > +        /* Enable DPDK_DEV_VHOST device and set promiscuous mode flag.

> */

> > +        netdev->flags = NETDEV_UP | NETDEV_PROMISC;

> >

>      }

> >

> >      ovs_list_push_back(&dpdk_list, &dev->list_node);

> > @@ -1256,7 +1258,8 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq

> *rxq,

> >      uint16_t nb_rx = 0;

> >      uint16_t dropped = 0;

> >

> > -    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {

> > +    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev)

> > +                     || !(dev->flags & NETDEV_UP))) {

> >          return EAGAIN;

> >      }

> >

> > @@ -1378,7 +1381,8 @@ __netdev_dpdk_vhost_send(struct netdev

> *netdev, int

> > qid,

> >

> >      qid = dev->tx_q[qid % dev->real_n_txq].map;

> >

> > -    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid < 0)) {

> > +    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid < 0

> > +                     || !(dev->flags & NETDEV_UP))) {

> >          rte_spinlock_lock(&dev->stats_lock);

> >          dev->stats.tx_dropped+= cnt;

> >          rte_spinlock_unlock(&dev->stats_lock);

> > @@ -2117,6 +2121,23 @@ netdev_dpdk_update_flags__(struct

> netdev_dpdk *dev,

> >          if (!(dev->flags & NETDEV_UP)) {

> >              rte_eth_dev_stop(dev->port_id);

> >          }

> > +    } else {

> > +        /* If DPDK_DEV_VHOST device's NETDEV_UP flag was changed and

> > vhost is

> > +         * running then change netdev's change_seq to trigger link state

> > +         * update. */

> > +        struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(dev);

> > +

> > +        if ((NETDEV_UP & ((*old_flagsp ^ on) | (*old_flagsp ^ off)))

> > +            && is_vhost_running(virtio_dev)) {

> > +            netdev_change_seq_changed(&dev->up);

> > +

> > +            /* Clear statistics if device is getting up. */

> > +            if (NETDEV_UP & on) {

> > +                rte_spinlock_lock(&dev->stats_lock);

> > +                memset(&dev->stats, 0, sizeof(dev->stats));

> > +                rte_spinlock_unlock(&dev->stats_lock);

> > +            }

> > +        }

> >      }

> >

> >      return 0;

> > @@ -2339,6 +2360,7 @@ new_device(struct virtio_net *virtio_dev)

> >              virtio_dev->flags |= VIRTIO_DEV_RUNNING;

> >              /* Disable notifications. */

> >              set_irq_status(virtio_dev);

> > +            netdev_change_seq_changed(&dev->up);

> >              ovs_mutex_unlock(&dev->mutex);

> >              break;

> >          }

> > @@ -2390,6 +2412,7 @@ destroy_device(volatile struct virtio_net

> > *virtio_dev)

> >              ovsrcu_set(&dev->virtio_dev, NULL);

> >              netdev_dpdk_txq_map_clear(dev);

> >              exists = true;

> > +            netdev_change_seq_changed(&dev->up);

> >              ovs_mutex_unlock(&dev->mutex);

> >              break;

> >          }

> >

> >

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 6cae930..9473bdb 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -797,6 +797,8 @@  netdev_dpdk_init(struct netdev *netdev, unsigned int port_no,
         }
     } else {
         netdev_dpdk_alloc_txq(dev, OVS_VHOST_MAX_QUEUE_NUM);
+        /* Enable DPDK_DEV_VHOST device and set promiscuous mode flag. */
+        netdev->flags = NETDEV_UP | NETDEV_PROMISC;
     }
 
     ovs_list_push_back(&dpdk_list, &dev->list_node);
@@ -1256,7 +1258,8 @@  netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
     uint16_t nb_rx = 0;
     uint16_t dropped = 0;
 
-    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
+    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev)
+                     || !(dev->flags & NETDEV_UP))) {
         return EAGAIN;
     }
 
@@ -1378,7 +1381,8 @@  __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 
     qid = dev->tx_q[qid % dev->real_n_txq].map;
 
-    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid < 0)) {
+    if (OVS_UNLIKELY(!is_vhost_running(virtio_dev) || qid < 0
+                     || !(dev->flags & NETDEV_UP))) {
         rte_spinlock_lock(&dev->stats_lock);
         dev->stats.tx_dropped+= cnt;
         rte_spinlock_unlock(&dev->stats_lock);
@@ -2117,6 +2121,23 @@  netdev_dpdk_update_flags__(struct netdev_dpdk *dev,
         if (!(dev->flags & NETDEV_UP)) {
             rte_eth_dev_stop(dev->port_id);
         }
+    } else {
+        /* If DPDK_DEV_VHOST device's NETDEV_UP flag was changed and vhost is
+         * running then change netdev's change_seq to trigger link state
+         * update. */
+        struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(dev);
+
+        if ((NETDEV_UP & ((*old_flagsp ^ on) | (*old_flagsp ^ off)))
+            && is_vhost_running(virtio_dev)) {
+            netdev_change_seq_changed(&dev->up);
+
+            /* Clear statistics if device is getting up. */
+            if (NETDEV_UP & on) {
+                rte_spinlock_lock(&dev->stats_lock);
+                memset(&dev->stats, 0, sizeof(dev->stats));
+                rte_spinlock_unlock(&dev->stats_lock);
+            }
+        }
     }
 
     return 0;
@@ -2339,6 +2360,7 @@  new_device(struct virtio_net *virtio_dev)
             virtio_dev->flags |= VIRTIO_DEV_RUNNING;
             /* Disable notifications. */
             set_irq_status(virtio_dev);
+            netdev_change_seq_changed(&dev->up);
             ovs_mutex_unlock(&dev->mutex);
             break;
         }
@@ -2390,6 +2412,7 @@  destroy_device(volatile struct virtio_net *virtio_dev)
             ovsrcu_set(&dev->virtio_dev, NULL);
             netdev_dpdk_txq_map_clear(dev);
             exists = true;
+            netdev_change_seq_changed(&dev->up);
             ovs_mutex_unlock(&dev->mutex);
             break;
         }