[ovs-dev] [PATCH v3 1/3] netdev-dpdk: Remove dpdk watchdog thread

Torgny Lindberg torgny.lindberg at ericsson.com
Thu May 19 07:26:15 UTC 2016


> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Traynor,
> Kevin
> Sent: den 13 maj 2016 12:47
> To: Loftus, Ciara; dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v3 1/3] netdev-dpdk: Remove dpdk watchdog
> thread
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Ciara
> > Loftus
> > Sent: Wednesday, May 11, 2016 4:31 PM
> > To: dev at openvswitch.org
> > Subject: [ovs-dev] [PATCH v3 1/3] netdev-dpdk: Remove dpdk watchdog
> > thread
> >
> > Instead of continuously polling for link status changes on 'dpdk'
> > ports, register a callback function that will be triggered when DPDK
> > detects that the link status of that port has changed.
> 
> rte_eth_link_get_nowait() returns void, so polling it in a thread won't
> indicate some kind of error in dpdk. I can't see any benefit of the thread
> - using the callback means one less thread and less locking.
> 
> Acked-by: Kevin Traynor <kevin.traynor at intel.com>
> 


With this patch a 4s delay before detecting link-down would be introduced,
which is from the viewpoint of many use cases an unacceptably long delay.
I would like to suggest that the existing poll method is kept as it detects
and acts on link failures much faster (millisecond time scale),
alternatively that both poll and interrupt methods are supported
and the one to use is selected by configuration.

The delay occurs inside the dpdk driver.
(See e.g. dpdk, ixgbe_ethdev.c, IXGBE_LINK_DOWN_CHECK_TIMEOUT)


Best regards,
Torgny Lindberg


> >
> > Signed-off-by: Ciara Loftus <ciara.loftus at intel.com>
> > Suggested-by: Kevin Traynor <kevin.traynor at intel.com>
> > ---
> >  lib/netdev-dpdk.c | 55 ++++++++++++++++++++++++++++++---------------
> -
> > ---------
> >  1 file changed, 30 insertions(+), 25 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index af86d19..89d783a 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -62,8 +62,6 @@
> >  VLOG_DEFINE_THIS_MODULE(dpdk);
> >  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> >
> > -#define DPDK_PORT_WATCHDOG_INTERVAL 5
> > -
> >  #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE
> >  #define OVS_VPORT_DPDK "ovs_dpdk"
> >
> > @@ -386,6 +384,9 @@ static int netdev_dpdk_construct(struct netdev *);
> >
> >  struct virtio_net * netdev_dpdk_get_virtio(const struct netdev_dpdk
> > *dev);
> >
> > +void link_status_changed_callback(uint8_t port_id,
> > +        enum rte_eth_event_type type OVS_UNUSED, void *param
> > OVS_UNUSED);
> > +
> >  static bool
> >  is_dpdk_class(const struct netdev_class *class)
> >  {
> > @@ -536,27 +537,6 @@ check_link_status(struct netdev_dpdk *dev)
> >      }
> >  }
> >
> > -static void *
> > -dpdk_watchdog(void *dummy OVS_UNUSED)
> > -{
> > -    struct netdev_dpdk *dev;
> > -
> > -    pthread_detach(pthread_self());
> > -
> > -    for (;;) {
> > -        ovs_mutex_lock(&dpdk_mutex);
> > -        LIST_FOR_EACH (dev, list_node, &dpdk_list) {
> > -            ovs_mutex_lock(&dev->mutex);
> > -            check_link_status(dev);
> > -            ovs_mutex_unlock(&dev->mutex);
> > -        }
> > -        ovs_mutex_unlock(&dpdk_mutex);
> > -        xsleep(DPDK_PORT_WATCHDOG_INTERVAL);
> > -    }
> > -
> > -    return NULL;
> > -}
> > -
> >  static int
> >  dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int
> > n_txq)
> >  {
> > @@ -717,6 +697,27 @@ netdev_dpdk_alloc_txq(struct netdev_dpdk *dev,
> > unsigned int n_txqs)
> >      }
> >  }
> >
> > +void
> > +link_status_changed_callback(uint8_t port_id,
> > +                              enum rte_eth_event_type type
> > OVS_UNUSED,
> > +                              void *param OVS_UNUSED)
> > +{
> > +    struct netdev_dpdk *dev;
> > +
> > +    ovs_mutex_lock(&dpdk_mutex);
> > +    LIST_FOR_EACH (dev, list_node, &dpdk_list) {
> > +        if (port_id == dev->port_id) {
> > +            ovs_mutex_lock(&dev->mutex);
> > +            check_link_status(dev);
> > +            ovs_mutex_unlock(&dev->mutex);
> > +            break;
> > +        }
> > +    }
> > +    ovs_mutex_unlock(&dpdk_mutex);
> > +
> > +    return;
> > +}
> > +
> >  static int
> >  netdev_dpdk_init(struct netdev *netdev, unsigned int port_no,
> >                   enum dpdk_dev_type type)
> > @@ -774,6 +775,12 @@ netdev_dpdk_init(struct netdev *netdev,
> unsigned
> > int port_no,
> >          netdev_dpdk_alloc_txq(dev, OVS_VHOST_MAX_QUEUE_NUM);
> >      }
> >
> > +    if (type == DPDK_DEV_ETH) {
> > +        rte_eth_dev_callback_register(port_no,
> > RTE_ETH_EVENT_INTR_LSC,
> > +
> > (void*)link_status_changed_callback,
> > +                                      NULL);
> > +    }
> > +
> >      ovs_list_push_back(&dpdk_list, &dev->list_node);
> >
> >  unlock:
> > @@ -3207,8 +3214,6 @@ dpdk_init__(const struct smap
> *ovs_other_config)
> >      /* We are called from the main thread here */
> >      RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
> >
> > -    ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
> > -
> >  #ifdef VHOST_CUSE
> >      /* Register CUSE device to handle IOCTLs.
> >       * Unless otherwise specified, cuse_dev_name is set to vhost-net.
> > --
> > 2.4.3
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev


More information about the dev mailing list