diff mbox

[ovs-dev,v2] Update relevant artifacts to add support for DPDK 17.05.

Message ID 1496224026-17154-1-git-send-email-michalx.weglicki@intel.com
State Changes Requested
Headers show

Commit Message

Weglicki, MichalX May 31, 2017, 9:47 a.m. UTC
Following changes are applied:
- netdev-dpdk: Changes required by DPDK API modifications.
- doc: Because of DPDK API changes, backward compatibility
  with previous DPDK releases will be broken, thus all
  relevant documentation entries are updated.
- .travis: DPDK version change from 16.11.1 to 17.05.
- rhel/openvswitch-fedora.spec.in: DPDK version change
  from 16.11 to 17.05.

v1->v2: Patch rework based on minor review comments.

Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
---
 .travis/linux-build.sh                   |   2 +-
 Documentation/faq/releases.rst           |   3 +-
 Documentation/intro/install/dpdk.rst     |   8 +--
 Documentation/topics/dpdk/vhost-user.rst |   8 +--
 lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
 rhel/openvswitch-fedora.spec.in          |   2 +-
 tests/dpdk/ring_client.c                 |   6 +-
 7 files changed, 75 insertions(+), 68 deletions(-)

Comments

Kevin Traynor May 31, 2017, 12:52 p.m. UTC | #1
On 05/31/2017 10:47 AM, mweglicx wrote:
> Following changes are applied:
> - netdev-dpdk: Changes required by DPDK API modifications.
> - doc: Because of DPDK API changes, backward compatibility
>   with previous DPDK releases will be broken, thus all
>   relevant documentation entries are updated.
> - .travis: DPDK version change from 16.11.1 to 17.05.
> - rhel/openvswitch-fedora.spec.in: DPDK version change
>   from 16.11 to 17.05.
> 
> v1->v2: Patch rework based on minor review comments.
> 
> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
> ---
>  .travis/linux-build.sh                   |   2 +-
>  Documentation/faq/releases.rst           |   3 +-
>  Documentation/intro/install/dpdk.rst     |   8 +--
>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
>  lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
>  rhel/openvswitch-fedora.spec.in          |   2 +-
>  tests/dpdk/ring_client.c                 |   6 +-
>  7 files changed, 75 insertions(+), 68 deletions(-)
> 
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 8750d68..ec15fd8 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -80,7 +80,7 @@ fi
>  
>  if [ "$DPDK" ]; then
>      if [ -z "$DPDK_VER" ]; then
> -        DPDK_VER="16.11.1"
> +        DPDK_VER="17.05"
>      fi
>      install_dpdk $DPDK_VER
>      if [ "$CC" = "clang" ]; then
> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
> index c85eff8..0455a43 100644
> --- a/Documentation/faq/releases.rst
> +++ b/Documentation/faq/releases.rst
> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch release work with?
>      2.4.x        2.0
>      2.5.x        2.2
>      2.6.x        16.07.2
> -    2.7.x        16.11.1
> +    2.7.0        16.11.1 <sip:0027016111>
> +    2.7.0+       17.05
>      ============ =======

This is about OVS releases. I think you should revert back to the
original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
likely not use DPDK 17.05.

When OVS 2.8 is close to release it can be updated with it's DPDK
version, or you could set it now and it can be changed later if required.

>  
>  Q: I get an error like this when I configure Open vSwitch:
> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
> index e83f852..536450b 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -40,7 +40,7 @@ Build requirements
>  In addition to the requirements described in :doc:`general`, building Open
>  vSwitch with DPDK will require the following:
>  
> -- DPDK 16.11
> +- DPDK 17.05
>  
>  - A `DPDK supported NIC`_
>  
> @@ -69,9 +69,9 @@ Install DPDK
>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>  
>         $ cd /usr/src/
> -       $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
> -       $ tar xf dpdk-16.11.1.tar.xz
> -       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
> +       $ wget http://fast.dpdk.org/rel/dpdk-17.05.tar.xz
> +       $ tar xf dpdk-17.05.tar.xz
> +       $ export DPDK_DIR=/usr/src/dpdk-17.05
>         $ cd $DPDK_DIR
>  
>  #. (Optional) Configure DPDK as a shared library
> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
> index ba22684..71098fd 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
>  DPDK sources to VM and build DPDK::
>  
>      $ cd /root/dpdk/
> -    $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
> -    $ tar xf dpdk-16.11.1.tar.xz
> -    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
> +    $ wget http://fast.dpdk.org/rel/dpdk-17.05.tar.xz
> +    $ tar xf dpdk-17.05.tar.xz
> +    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
>      $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
>      $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>      $ cd $DPDK_DIR
> @@ -364,7 +364,7 @@ Sample XML
>          </disk>
>          <disk type='dir' device='disk'>
>            <driver name='qemu' type='fat'/>
> -          <source dir='/usr/src/dpdk-stable-16.11.1'/>
> +          <source dir='/usr/src/dpdk-17.05'/>
>            <target dev='vdb' bus='virtio'/>
>            <readonly/>
>          </disk>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 609b8da..fc16d89 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -22,6 +22,9 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <linux/virtio_net.h>
> +#include <sys/socket.h>
> +#include <linux/if.h>
>  
>  #include <rte_config.h>
>  #include <rte_cycles.h>
> @@ -31,7 +34,7 @@
>  #include <rte_malloc.h>
>  #include <rte_mbuf.h>
>  #include <rte_meter.h>
> -#include <rte_virtio_net.h>
> +#include <rte_vhost.h>
>  
>  #include "dirs.h"
>  #include "dp-packet.h"
> @@ -56,6 +59,8 @@
>  #include "timeval.h"
>  #include "unixctl.h"
>  
> +enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
> +
>  VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>  
> @@ -164,6 +169,21 @@ static const struct rte_eth_conf port_conf = {
>      },
>  };
>  
> +/*
> + * These callbacks allow virtio-net devices to be added to vhost ports when
> + * configuration has been fully completed.
> + */
> +static int new_device(int vid);
> +static void destroy_device(int vid);
> +static int vring_state_changed(int vid, uint16_t queue_id, int enable);
> +static const struct vhost_device_ops virtio_net_device_ops =
> +{
> +    .new_device =  new_device,
> +    .destroy_device = destroy_device,
> +    .vring_state_changed = vring_state_changed,
> +    .features_changed = NULL
> +};
> +
>  enum { DPDK_RING_SIZE = 256 };
>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
>  enum { DRAIN_TSC = 200000ULL };
> @@ -402,8 +422,8 @@ struct netdev_rxq_dpdk {
>      int port_id;
>  };
>  
> -static int netdev_dpdk_class_init(void);
> -static int netdev_dpdk_vhost_class_init(void);
> +static void netdev_dpdk_destruct(struct netdev *netdev);
> +static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
>  
>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
>  
> @@ -413,8 +433,8 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
>  static bool
>  is_dpdk_class(const struct netdev_class *class)
>  {
> -    return class->init == netdev_dpdk_class_init
> -           || class->init == netdev_dpdk_vhost_class_init;
> +    return class->destruct == netdev_dpdk_destruct
> +           || class->destruct == netdev_dpdk_vhost_destruct;
>  }
>  
>  /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
> @@ -942,17 +962,46 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
>               dpdk_get_vhost_sock_dir(), name);
>  
>      dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
> -    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
> +    err = rte_vhost_driver_register(dev->vhost_id,
> +                                    dev->vhost_driver_flags);
>      if (err) {
>          VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
>                   dev->vhost_id);
> +        goto out;
>      } else {
>          fatal_signal_add_file_to_unlink(dev->vhost_id);
>          VLOG_INFO("Socket %s created for vhost-user port %s\n",
>                    dev->vhost_id, name);
>      }
> +
> +    err = rte_vhost_driver_callback_register(dev->vhost_id,
> +                                                &virtio_net_device_ops);
> +    if (err) {
> +        VLOG_ERR("vhost-user callback register failed.");
> +        goto out;
> +    }
> +
> +    err = rte_vhost_driver_disable_features(dev->vhost_id,
> +                                1ULL << VIRTIO_NET_F_HOST_TSO4
> +                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
> +                                | 1ULL << VIRTIO_NET_F_CSUM);

Don't these need to be called for dpdkvhostuserclient ports as well?
Seems you could put them in the common construct

> +    if (err) {
> +        VLOG_ERR("vhost-user disable features failed.");
> +        goto out;
> +    }
> +
>      err = vhost_common_construct(netdev);
> +    if (err) {
> +        VLOG_ERR("vhost-user common construct failed.");
> +        goto out;
> +    }
> +
> +    err = rte_vhost_driver_start(dev->vhost_id);
> +    if (err) {
> +        VLOG_ERR("vhost-user driver start failed.");
> +    }
>  
> +out:
>      ovs_mutex_unlock(&dpdk_mutex);
>      return err;
>  }
> @@ -2483,12 +2532,9 @@ static void
>  set_irq_status(int vid)
>  {
>      uint32_t i;
> -    uint64_t idx;
>  
> -    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
> -        idx = i * VIRTIO_QNUM;
> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
> +    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
> +        rte_vhost_enable_guest_notification(vid, i, 0);
>      }
>  }
>  
> @@ -2551,7 +2597,7 @@ new_device(int vid)
>      LIST_FOR_EACH(dev, list_node, &dpdk_list) {
>          ovs_mutex_lock(&dev->mutex);
>          if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
> -            uint32_t qp_num = rte_vhost_get_queue_num(vid);
> +            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
>  
>              /* Get NUMA information */
>              newnode = rte_vhost_get_numa_node(vid);
> @@ -2718,27 +2764,6 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
>      return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
>  }
>  
> -/*
> - * These callbacks allow virtio-net devices to be added to vhost ports when
> - * configuration has been fully complete.
> - */
> -static const struct virtio_net_device_ops virtio_net_device_ops =
> -{
> -    .new_device =  new_device,
> -    .destroy_device = destroy_device,
> -    .vring_state_changed = vring_state_changed
> -};
> -
> -static void *
> -start_vhost_loop(void *dummy OVS_UNUSED)
> -{
> -     pthread_detach(pthread_self());
> -     /* Put the vhost thread into quiescent state. */
> -     ovsrcu_quiesce_start();
> -     rte_vhost_driver_session_start();
> -     return NULL;
> -}
> -
>  static int
>  netdev_dpdk_class_init(void)
>  {
> @@ -2761,25 +2786,6 @@ netdev_dpdk_class_init(void)
>      return 0;
>  }
>  
> -static int
> -netdev_dpdk_vhost_class_init(void)
> -{
> -    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> -
> -    /* This function can be called for different classes.  The initialization
> -     * needs to be done only once */
> -    if (ovsthread_once_start(&once)) {
> -        rte_vhost_driver_callback_register(&virtio_net_device_ops);
> -        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
> -                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
> -                                  | 1ULL << VIRTIO_NET_F_CSUM);
> -        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
> -
> -        ovsthread_once_done(&once);
> -    }
> -
> -    return 0;
> -}
>  
>  /* Client Rings */
>  
> @@ -3351,7 +3357,7 @@ static const struct netdev_class dpdk_ring_class =
>  static const struct netdev_class dpdk_vhost_class =
>      NETDEV_DPDK_CLASS(
>          "dpdkvhostuser",
> -        netdev_dpdk_vhost_class_init,
> +        NULL,
>          netdev_dpdk_vhost_construct,
>          netdev_dpdk_vhost_destruct,
>          NULL,
> @@ -3366,7 +3372,7 @@ static const struct netdev_class dpdk_vhost_class =
>  static const struct netdev_class dpdk_vhost_client_class =
>      NETDEV_DPDK_CLASS(
>          "dpdkvhostuserclient",
> -        netdev_dpdk_vhost_class_init,
> +        NULL,
>          netdev_dpdk_vhost_client_construct,
>          netdev_dpdk_vhost_destruct,
>          netdev_dpdk_vhost_client_set_config,
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 3200040..596a4b3 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -84,7 +84,7 @@ BuildRequires: libcap-ng libcap-ng-devel
>  %endif
>  %if %{with dpdk}
>  BuildRequires: libpcap-devel numactl-devel
> -BuildRequires: dpdk-devel >= 16.11
> +BuildRequires: dpdk-devel >= 17.05
>  Provides: %{name}-dpdk = %{version}-%{release}
>  %endif
>  
> diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
> index b153713..8cc3fb5 100644
> --- a/tests/dpdk/ring_client.c
> +++ b/tests/dpdk/ring_client.c
> @@ -185,15 +185,15 @@ main(int argc, char *argv[])
>          /* Try dequeuing max possible packets first, if that fails, get the
>           * most we can. Loop body should only execute once, maximum.
>           */
> -        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
> -            rx_pkts > 0) {
> +        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
> +                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
>              rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
>          }
>  
>          if (rx_pkts > 0) {
>              /* blocking enqueue */
>              do {
> -                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
> +                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
>              } while (rslt == -ENOBUFS);
>          }
>      }
>
Mark Kavanagh May 31, 2017, 1:09 p.m. UTC | #2
>From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of
>Kevin Traynor
>Sent: Wednesday, May 31, 2017 1:53 PM
>To: Weglicki, MichalX <michalx.weglicki@intel.com>; dev@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
>
>On 05/31/2017 10:47 AM, mweglicx wrote:
>> Following changes are applied:
>> - netdev-dpdk: Changes required by DPDK API modifications.
>> - doc: Because of DPDK API changes, backward compatibility
>>   with previous DPDK releases will be broken, thus all
>>   relevant documentation entries are updated.
>> - .travis: DPDK version change from 16.11.1 to 17.05.
>> - rhel/openvswitch-fedora.spec.in: DPDK version change
>>   from 16.11 to 17.05.
>>
>> v1->v2: Patch rework based on minor review comments.


Hi Michal,

Apart from the changes suggested by Kevin, LGTM.

Thanks,
Mark


>>
>> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
>> ---
>>  .travis/linux-build.sh                   |   2 +-
>>  Documentation/faq/releases.rst           |   3 +-
>>  Documentation/intro/install/dpdk.rst     |   8 +--
>>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
>>  lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
>>  rhel/openvswitch-fedora.spec.in          |   2 +-
>>  tests/dpdk/ring_client.c                 |   6 +-
>>  7 files changed, 75 insertions(+), 68 deletions(-)
>>
>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
>> index 8750d68..ec15fd8 100755
>> --- a/.travis/linux-build.sh
>> +++ b/.travis/linux-build.sh
>> @@ -80,7 +80,7 @@ fi
>>
>>  if [ "$DPDK" ]; then
>>      if [ -z "$DPDK_VER" ]; then
>> -        DPDK_VER="16.11.1"
>> +        DPDK_VER="17.05"
>>      fi
>>      install_dpdk $DPDK_VER
>>      if [ "$CC" = "clang" ]; then
>> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
>> index c85eff8..0455a43 100644
>> --- a/Documentation/faq/releases.rst
>> +++ b/Documentation/faq/releases.rst
>> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch release work with?
>>      2.4.x        2.0
>>      2.5.x        2.2
>>      2.6.x        16.07.2
>> -    2.7.x        16.11.1
>> +    2.7.0        16.11.1 <sip:0027016111>
>> +    2.7.0+       17.05
>>      ============ =======
>
>This is about OVS releases. I think you should revert back to the
>original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
>likely not use DPDK 17.05.

That's a good point - +1 on this.

>
>When OVS 2.8 is close to release it can be updated with it's DPDK
>version, or you could set it now and it can be changed later if required.
>
>>
>>  Q: I get an error like this when I configure Open vSwitch:
>> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
>> index e83f852..536450b 100644
>> --- a/Documentation/intro/install/dpdk.rst
>> +++ b/Documentation/intro/install/dpdk.rst
>> @@ -40,7 +40,7 @@ Build requirements
>>  In addition to the requirements described in :doc:`general`, building Open
>>  vSwitch with DPDK will require the following:
>>
>> -- DPDK 16.11
>> +- DPDK 17.05
>>
>>  - A `DPDK supported NIC`_
>>
>> @@ -69,9 +69,9 @@ Install DPDK
>>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>>
>>         $ cd /usr/src/
>> -       $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
>> -       $ tar xf dpdk-16.11.1.tar.xz
>> -       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
>> +       $ wget http://fast.dpdk.org/rel/dpdk-17.05.tar.xz
>> +       $ tar xf dpdk-17.05.tar.xz
>> +       $ export DPDK_DIR=/usr/src/dpdk-17.05
>>         $ cd $DPDK_DIR
>>
>>  #. (Optional) Configure DPDK as a shared library
>> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-
>user.rst
>> index ba22684..71098fd 100644
>> --- a/Documentation/topics/dpdk/vhost-user.rst
>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
>>  DPDK sources to VM and build DPDK::
>>
>>      $ cd /root/dpdk/
>> -    $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
>> -    $ tar xf dpdk-16.11.1.tar.xz
>> -    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
>> +    $ wget http://fast.dpdk.org/rel/dpdk-17.05.tar.xz
>> +    $ tar xf dpdk-17.05.tar.xz
>> +    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
>>      $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
>>      $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>>      $ cd $DPDK_DIR
>> @@ -364,7 +364,7 @@ Sample XML
>>          </disk>
>>          <disk type='dir' device='disk'>
>>            <driver name='qemu' type='fat'/>
>> -          <source dir='/usr/src/dpdk-stable-16.11.1'/>
>> +          <source dir='/usr/src/dpdk-17.05'/>
>>            <target dev='vdb' bus='virtio'/>
>>            <readonly/>
>>          </disk>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 609b8da..fc16d89 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -22,6 +22,9 @@
>>  #include <stdlib.h>
>>  #include <errno.h>
>>  #include <unistd.h>
>> +#include <linux/virtio_net.h>
>> +#include <sys/socket.h>
>> +#include <linux/if.h>
>>
>>  #include <rte_config.h>
>>  #include <rte_cycles.h>
>> @@ -31,7 +34,7 @@
>>  #include <rte_malloc.h>
>>  #include <rte_mbuf.h>
>>  #include <rte_meter.h>
>> -#include <rte_virtio_net.h>
>> +#include <rte_vhost.h>
>>
>>  #include "dirs.h"
>>  #include "dp-packet.h"
>> @@ -56,6 +59,8 @@
>>  #include "timeval.h"
>>  #include "unixctl.h"
>>
>> +enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
>> +
>>  VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
>>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>>
>> @@ -164,6 +169,21 @@ static const struct rte_eth_conf port_conf = {
>>      },
>>  };
>>
>> +/*
>> + * These callbacks allow virtio-net devices to be added to vhost ports when
>> + * configuration has been fully completed.
>> + */
>> +static int new_device(int vid);
>> +static void destroy_device(int vid);
>> +static int vring_state_changed(int vid, uint16_t queue_id, int enable);
>> +static const struct vhost_device_ops virtio_net_device_ops =
>> +{
>> +    .new_device =  new_device,
>> +    .destroy_device = destroy_device,
>> +    .vring_state_changed = vring_state_changed,
>> +    .features_changed = NULL
>> +};
>> +
>>  enum { DPDK_RING_SIZE = 256 };
>>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
>>  enum { DRAIN_TSC = 200000ULL };
>> @@ -402,8 +422,8 @@ struct netdev_rxq_dpdk {
>>      int port_id;
>>  };
>>
>> -static int netdev_dpdk_class_init(void);
>> -static int netdev_dpdk_vhost_class_init(void);
>> +static void netdev_dpdk_destruct(struct netdev *netdev);
>> +static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
>>
>>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
>>
>> @@ -413,8 +433,8 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
>>  static bool
>>  is_dpdk_class(const struct netdev_class *class)
>>  {
>> -    return class->init == netdev_dpdk_class_init
>> -           || class->init == netdev_dpdk_vhost_class_init;
>> +    return class->destruct == netdev_dpdk_destruct
>> +           || class->destruct == netdev_dpdk_vhost_destruct;
>>  }
>>
>>  /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
>> @@ -942,17 +962,46 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
>>               dpdk_get_vhost_sock_dir(), name);
>>
>>      dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
>> -    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
>> +    err = rte_vhost_driver_register(dev->vhost_id,
>> +                                    dev->vhost_driver_flags);
>>      if (err) {
>>          VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
>>                   dev->vhost_id);
>> +        goto out;
>>      } else {
>>          fatal_signal_add_file_to_unlink(dev->vhost_id);
>>          VLOG_INFO("Socket %s created for vhost-user port %s\n",
>>                    dev->vhost_id, name);
>>      }
>> +
>> +    err = rte_vhost_driver_callback_register(dev->vhost_id,
>> +                                                &virtio_net_device_ops);
>> +    if (err) {
>> +        VLOG_ERR("vhost-user callback register failed.");
>> +        goto out;
>> +    }
>> +
>> +    err = rte_vhost_driver_disable_features(dev->vhost_id,
>> +                                1ULL << VIRTIO_NET_F_HOST_TSO4
>> +                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
>> +                                | 1ULL << VIRTIO_NET_F_CSUM);
>
>Don't these need to be called for dpdkvhostuserclient ports as well?
>Seems you could put them in the common construct

Good catch.

>
>> +    if (err) {
>> +        VLOG_ERR("vhost-user disable features failed.");
>> +        goto out;
>> +    }
>> +
>>      err = vhost_common_construct(netdev);
>> +    if (err) {
>> +        VLOG_ERR("vhost-user common construct failed.");
>> +        goto out;
>> +    }
>> +
>> +    err = rte_vhost_driver_start(dev->vhost_id);
>> +    if (err) {
>> +        VLOG_ERR("vhost-user driver start failed.");
>> +    }
>>
>> +out:
>>      ovs_mutex_unlock(&dpdk_mutex);
>>      return err;
>>  }
>> @@ -2483,12 +2532,9 @@ static void
>>  set_irq_status(int vid)
>>  {
>>      uint32_t i;
>> -    uint64_t idx;
>>
>> -    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
>> -        idx = i * VIRTIO_QNUM;
>> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
>> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
>> +    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
>> +        rte_vhost_enable_guest_notification(vid, i, 0);
>>      }
>>  }
>>
>> @@ -2551,7 +2597,7 @@ new_device(int vid)
>>      LIST_FOR_EACH(dev, list_node, &dpdk_list) {
>>          ovs_mutex_lock(&dev->mutex);
>>          if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
>> -            uint32_t qp_num = rte_vhost_get_queue_num(vid);
>> +            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
>>
>>              /* Get NUMA information */
>>              newnode = rte_vhost_get_numa_node(vid);
>> @@ -2718,27 +2764,6 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
>>      return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
>>  }
>>
>> -/*
>> - * These callbacks allow virtio-net devices to be added to vhost ports when
>> - * configuration has been fully complete.
>> - */
>> -static const struct virtio_net_device_ops virtio_net_device_ops =
>> -{
>> -    .new_device =  new_device,
>> -    .destroy_device = destroy_device,
>> -    .vring_state_changed = vring_state_changed
>> -};
>> -
>> -static void *
>> -start_vhost_loop(void *dummy OVS_UNUSED)
>> -{
>> -     pthread_detach(pthread_self());
>> -     /* Put the vhost thread into quiescent state. */
>> -     ovsrcu_quiesce_start();
>> -     rte_vhost_driver_session_start();
>> -     return NULL;
>> -}
>> -
>>  static int
>>  netdev_dpdk_class_init(void)
>>  {
>> @@ -2761,25 +2786,6 @@ netdev_dpdk_class_init(void)
>>      return 0;
>>  }
>>
>> -static int
>> -netdev_dpdk_vhost_class_init(void)
>> -{
>> -    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> -
>> -    /* This function can be called for different classes.  The initialization
>> -     * needs to be done only once */
>> -    if (ovsthread_once_start(&once)) {
>> -        rte_vhost_driver_callback_register(&virtio_net_device_ops);
>> -        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
>> -                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
>> -                                  | 1ULL << VIRTIO_NET_F_CSUM);
>> -        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
>> -
>> -        ovsthread_once_done(&once);
>> -    }
>> -
>> -    return 0;
>> -}
>>
>>  /* Client Rings */
>>
>> @@ -3351,7 +3357,7 @@ static const struct netdev_class dpdk_ring_class =
>>  static const struct netdev_class dpdk_vhost_class =
>>      NETDEV_DPDK_CLASS(
>>          "dpdkvhostuser",
>> -        netdev_dpdk_vhost_class_init,
>> +        NULL,
>>          netdev_dpdk_vhost_construct,
>>          netdev_dpdk_vhost_destruct,
>>          NULL,
>> @@ -3366,7 +3372,7 @@ static const struct netdev_class dpdk_vhost_class =
>>  static const struct netdev_class dpdk_vhost_client_class =
>>      NETDEV_DPDK_CLASS(
>>          "dpdkvhostuserclient",
>> -        netdev_dpdk_vhost_class_init,
>> +        NULL,
>>          netdev_dpdk_vhost_client_construct,
>>          netdev_dpdk_vhost_destruct,
>>          netdev_dpdk_vhost_client_set_config,
>> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>> index 3200040..596a4b3 100644
>> --- a/rhel/openvswitch-fedora.spec.in
>> +++ b/rhel/openvswitch-fedora.spec.in
>> @@ -84,7 +84,7 @@ BuildRequires: libcap-ng libcap-ng-devel
>>  %endif
>>  %if %{with dpdk}
>>  BuildRequires: libpcap-devel numactl-devel
>> -BuildRequires: dpdk-devel >= 16.11
>> +BuildRequires: dpdk-devel >= 17.05
>>  Provides: %{name}-dpdk = %{version}-%{release}
>>  %endif
>>
>> diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
>> index b153713..8cc3fb5 100644
>> --- a/tests/dpdk/ring_client.c
>> +++ b/tests/dpdk/ring_client.c
>> @@ -185,15 +185,15 @@ main(int argc, char *argv[])
>>          /* Try dequeuing max possible packets first, if that fails, get the
>>           * most we can. Loop body should only execute once, maximum.
>>           */
>> -        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
>> -            rx_pkts > 0) {
>> +        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
>> +                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
>>              rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
>>          }
>>
>>          if (rx_pkts > 0) {
>>              /* blocking enqueue */
>>              do {
>> -                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
>> +                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
>>              } while (rslt == -ENOBUFS);
>>          }
>>      }
>>
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Darrell Ball May 31, 2017, 11:36 p.m. UTC | #3
I did a pass through the changes.
Few comments

On 5/31/17, 6:09 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh, Mark B" <ovs-dev-bounces@openvswitch.org on behalf of mark.b.kavanagh@intel.com> wrote:

    
    >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of
    >Kevin Traynor
    >Sent: Wednesday, May 31, 2017 1:53 PM
    >To: Weglicki, MichalX <michalx.weglicki@intel.com>; dev@openvswitch.org
    >Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
    >
    >On 05/31/2017 10:47 AM, mweglicx wrote:
    >> Following changes are applied:
    >> - netdev-dpdk: Changes required by DPDK API modifications.
    >> - doc: Because of DPDK API changes, backward compatibility
    >>   with previous DPDK releases will be broken, thus all
    >>   relevant documentation entries are updated.
    >> - .travis: DPDK version change from 16.11.1 to 17.05.
    >> - rhel/openvswitch-fedora.spec.in: DPDK version change
    >>   from 16.11 to 17.05.
    >>
    >> v1->v2: Patch rework based on minor review comments.
    
    
    Hi Michal,
    
    Apart from the changes suggested by Kevin, LGTM.
    
    Thanks,
    Mark
    
    
    >>
    >> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
    >> ---
    >>  .travis/linux-build.sh                   |   2 +-
    >>  Documentation/faq/releases.rst           |   3 +-
    >>  Documentation/intro/install/dpdk.rst     |   8 +--
    >>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
    >>  lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
    >>  rhel/openvswitch-fedora.spec.in          |   2 +-
    >>  tests/dpdk/ring_client.c                 |   6 +-
    >>  7 files changed, 75 insertions(+), 68 deletions(-)
    >>
    >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
    >> index 8750d68..ec15fd8 100755
    >> --- a/.travis/linux-build.sh
    >> +++ b/.travis/linux-build.sh
    >> @@ -80,7 +80,7 @@ fi
    >>
    >>  if [ "$DPDK" ]; then
    >>      if [ -z "$DPDK_VER" ]; then
    >> -        DPDK_VER="16.11.1"
    >> +        DPDK_VER="17.05"
    >>      fi
    >>      install_dpdk $DPDK_VER
    >>      if [ "$CC" = "clang" ]; then
    >> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
    >> index c85eff8..0455a43 100644
    >> --- a/Documentation/faq/releases.rst
    >> +++ b/Documentation/faq/releases.rst
    >> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch release work with?
    >>      2.4.x        2.0
    >>      2.5.x        2.2
    >>      2.6.x        16.07.2
    >> -    2.7.x        16.11.1
    >> +    2.7.0        16.11.1 <sip:0027016111>
    >> +    2.7.0+       17.05
    >>      ============ =======
    >
    >This is about OVS releases. I think you should revert back to the
    >original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
    >likely not use DPDK 17.05.
    
    That's a good point - +1 on this.

Yep

    
    >
    >When OVS 2.8 is close to release it can be updated with it's DPDK
    >version, or you could set it now and it can be changed later if required.
    >
    >>
    >>  Q: I get an error like this when I configure Open vSwitch:
    >> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
    >> index e83f852..536450b 100644
    >> --- a/Documentation/intro/install/dpdk.rst
    >> +++ b/Documentation/intro/install/dpdk.rst
    >> @@ -40,7 +40,7 @@ Build requirements
    >>  In addition to the requirements described in :doc:`general`, building Open
    >>  vSwitch with DPDK will require the following:
    >>
    >> -- DPDK 16.11
    >> +- DPDK 17.05
    >>
    >>  - A `DPDK supported NIC`_

Why is this comment removed ?


    >>
    >> @@ -69,9 +69,9 @@ Install DPDK
    >>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
    >>
    >>         $ cd /usr/src/
    >> -       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e= 
    >> -       $ tar xf dpdk-16.11.1.tar.xz
    >> -       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
    >> +       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e= 
    >> +       $ tar xf dpdk-17.05.tar.xz
    >> +       $ export DPDK_DIR=/usr/src/dpdk-17.05
    >>         $ cd $DPDK_DIR
    >>
    >>  #. (Optional) Configure DPDK as a shared library
    >> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-
    >user.rst
    >> index ba22684..71098fd 100644
    >> --- a/Documentation/topics/dpdk/vhost-user.rst
    >> +++ b/Documentation/topics/dpdk/vhost-user.rst
    >> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
    >>  DPDK sources to VM and build DPDK::
    >>
    >>      $ cd /root/dpdk/
    >> -    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e= 
    >> -    $ tar xf dpdk-16.11.1.tar.xz
    >> -    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
    >> +    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e= 
    >> +    $ tar xf dpdk-17.05.tar.xz
    >> +    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
    >>      $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
    >>      $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
    >>      $ cd $DPDK_DIR
    >> @@ -364,7 +364,7 @@ Sample XML
    >>          </disk>
    >>          <disk type='dir' device='disk'>
    >>            <driver name='qemu' type='fat'/>
    >> -          <source dir='/usr/src/dpdk-stable-16.11.1'/>
    >> +          <source dir='/usr/src/dpdk-17.05'/>
    >>            <target dev='vdb' bus='virtio'/>
    >>            <readonly/>
    >>          </disk>
    >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
    >> index 609b8da..fc16d89 100644
    >> --- a/lib/netdev-dpdk.c
    >> +++ b/lib/netdev-dpdk.c
    >> @@ -22,6 +22,9 @@
    >>  #include <stdlib.h>
    >>  #include <errno.h>
    >>  #include <unistd.h>
    >> +#include <linux/virtio_net.h>
    >> +#include <sys/socket.h>
    >> +#include <linux/if.h>
    >>
    >>  #include <rte_config.h>
    >>  #include <rte_cycles.h>
    >> @@ -31,7 +34,7 @@
    >>  #include <rte_malloc.h>
    >>  #include <rte_mbuf.h>
    >>  #include <rte_meter.h>
    >> -#include <rte_virtio_net.h>
    >> +#include <rte_vhost.h>
    >>
    >>  #include "dirs.h"
    >>  #include "dp-packet.h"
    >> @@ -56,6 +59,8 @@
    >>  #include "timeval.h"
    >>  #include "unixctl.h"
    >>
    >> +enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
    >> +
    >>  VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
    >>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
    >>
    >> @@ -164,6 +169,21 @@ static const struct rte_eth_conf port_conf = {
    >>      },
    >>  };
    >>
    >> +/*
    >> + * These callbacks allow virtio-net devices to be added to vhost ports when
    >> + * configuration has been fully completed.
    >> + */
    >> +static int new_device(int vid);
    >> +static void destroy_device(int vid);
    >> +static int vring_state_changed(int vid, uint16_t queue_id, int enable);
    >> +static const struct vhost_device_ops virtio_net_device_ops =
    >> +{
    >> +    .new_device =  new_device,
    >> +    .destroy_device = destroy_device,
    >> +    .vring_state_changed = vring_state_changed,
    >> +    .features_changed = NULL
    >> +};
    >> +
    >>  enum { DPDK_RING_SIZE = 256 };
    >>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
    >>  enum { DRAIN_TSC = 200000ULL };
    >> @@ -402,8 +422,8 @@ struct netdev_rxq_dpdk {
    >>      int port_id;
    >>  };
    >>
    >> -static int netdev_dpdk_class_init(void);
    >> -static int netdev_dpdk_vhost_class_init(void);
    >> +static void netdev_dpdk_destruct(struct netdev *netdev);
    >> +static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
    >>
    >>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
    >>
    >> @@ -413,8 +433,8 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
    >>  static bool
    >>  is_dpdk_class(const struct netdev_class *class)
    >>  {
    >> -    return class->init == netdev_dpdk_class_init
    >> -           || class->init == netdev_dpdk_vhost_class_init;
    >> +    return class->destruct == netdev_dpdk_destruct
    >> +           || class->destruct == netdev_dpdk_vhost_destruct;
    >>  }
    >>
    >>  /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
    >> @@ -942,17 +962,46 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
    >>               dpdk_get_vhost_sock_dir(), name);
    >>
    >>      dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
    >> -    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
    >> +    err = rte_vhost_driver_register(dev->vhost_id,
    >> +                                    dev->vhost_driver_flags);

Why this change ?; the line length was conformant.


    >>      if (err) {
    >>          VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
    >>                   dev->vhost_id);
    >> +        goto out;
    >>      } else {
    >>          fatal_signal_add_file_to_unlink(dev->vhost_id);
    >>          VLOG_INFO("Socket %s created for vhost-user port %s\n",
    >>                    dev->vhost_id, name);
    >>      }
    >> +
    >> +    err = rte_vhost_driver_callback_register(dev->vhost_id,
    >> +                                                &virtio_net_device_ops);
    >> +    if (err) {
    >> +        VLOG_ERR("vhost-user callback register failed.");
    >> +        goto out;
    >> +    }
    >> +
    >> +    err = rte_vhost_driver_disable_features(dev->vhost_id,
    >> +                                1ULL << VIRTIO_NET_F_HOST_TSO4
    >> +                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
    >> +                                | 1ULL << VIRTIO_NET_F_CSUM);
    >
    >Don't these need to be called for dpdkvhostuserclient ports as well?
    >Seems you could put them in the common construct
    
    Good catch.
    
    >
    >> +    if (err) {
    >> +        VLOG_ERR("vhost-user disable features failed.");
    >> +        goto out;
    >> +    }
    >> +
    >>      err = vhost_common_construct(netdev);
    >> +    if (err) {
    >> +        VLOG_ERR("vhost-user common construct failed.");
    >> +        goto out;
    >> +    }
    >> +
    >> +    err = rte_vhost_driver_start(dev->vhost_id);
    >> +    if (err) {
    >> +        VLOG_ERR("vhost-user driver start failed.");
    >> +    }
    >>
    >> +out:
    >>      ovs_mutex_unlock(&dpdk_mutex);
    >>      return err;
    >>  }
    >> @@ -2483,12 +2532,9 @@ static void
    >>  set_irq_status(int vid)
    >>  {
    >>      uint32_t i;
    >> -    uint64_t idx;
    >>
    >> -    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
    >> -        idx = i * VIRTIO_QNUM;
    >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
    >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
    >> +    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
    >> +        rte_vhost_enable_guest_notification(vid, i, 0);
    >>      }
    >>  }
    >>
    >> @@ -2551,7 +2597,7 @@ new_device(int vid)
    >>      LIST_FOR_EACH(dev, list_node, &dpdk_list) {
    >>          ovs_mutex_lock(&dev->mutex);
    >>          if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
    >> -            uint32_t qp_num = rte_vhost_get_queue_num(vid);
    >> +            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
    >>
    >>              /* Get NUMA information */
    >>              newnode = rte_vhost_get_numa_node(vid);
    >> @@ -2718,27 +2764,6 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
    >>      return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
    >>  }
    >>
    >> -/*
    >> - * These callbacks allow virtio-net devices to be added to vhost ports when
    >> - * configuration has been fully complete.
    >> - */
    >> -static const struct virtio_net_device_ops virtio_net_device_ops =
    >> -{
    >> -    .new_device =  new_device,
    >> -    .destroy_device = destroy_device,
    >> -    .vring_state_changed = vring_state_changed
    >> -};
    >> -
    >> -static void *
    >> -start_vhost_loop(void *dummy OVS_UNUSED)
    >> -{
    >> -     pthread_detach(pthread_self());
    >> -     /* Put the vhost thread into quiescent state. */
    >> -     ovsrcu_quiesce_start();
    >> -     rte_vhost_driver_session_start();
    >> -     return NULL;
    >> -}
    >> -
    >>  static int
    >>  netdev_dpdk_class_init(void)
    >>  {
    >> @@ -2761,25 +2786,6 @@ netdev_dpdk_class_init(void)
    >>      return 0;
    >>  }
    >>
    >> -static int
    >> -netdev_dpdk_vhost_class_init(void)
    >> -{
    >> -    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
    >> -
    >> -    /* This function can be called for different classes.  The initialization
    >> -     * needs to be done only once */
    >> -    if (ovsthread_once_start(&once)) {
    >> -        rte_vhost_driver_callback_register(&virtio_net_device_ops);
    >> -        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
    >> -                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
    >> -                                  | 1ULL << VIRTIO_NET_F_CSUM);
    >> -        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
    >> -
    >> -        ovsthread_once_done(&once);
    >> -    }
    >> -
    >> -    return 0;
    >> -}
    >>
    >>  /* Client Rings */
    >>
    >> @@ -3351,7 +3357,7 @@ static const struct netdev_class dpdk_ring_class =
    >>  static const struct netdev_class dpdk_vhost_class =
    >>      NETDEV_DPDK_CLASS(
    >>          "dpdkvhostuser",
    >> -        netdev_dpdk_vhost_class_init,
    >> +        NULL,
    >>          netdev_dpdk_vhost_construct,
    >>          netdev_dpdk_vhost_destruct,
    >>          NULL,
    >> @@ -3366,7 +3372,7 @@ static const struct netdev_class dpdk_vhost_class =
    >>  static const struct netdev_class dpdk_vhost_client_class =
    >>      NETDEV_DPDK_CLASS(
    >>          "dpdkvhostuserclient",
    >> -        netdev_dpdk_vhost_class_init,
    >> +        NULL,
    >>          netdev_dpdk_vhost_client_construct,
    >>          netdev_dpdk_vhost_destruct,
    >>          netdev_dpdk_vhost_client_set_config,
    >> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
    >> index 3200040..596a4b3 100644
    >> --- a/rhel/openvswitch-fedora.spec.in
    >> +++ b/rhel/openvswitch-fedora.spec.in
    >> @@ -84,7 +84,7 @@ BuildRequires: libcap-ng libcap-ng-devel
    >>  %endif
    >>  %if %{with dpdk}
    >>  BuildRequires: libpcap-devel numactl-devel
    >> -BuildRequires: dpdk-devel >= 16.11
    >> +BuildRequires: dpdk-devel >= 17.05
    >>  Provides: %{name}-dpdk = %{version}-%{release}
    >>  %endif
    >>
    >> diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
    >> index b153713..8cc3fb5 100644
    >> --- a/tests/dpdk/ring_client.c
    >> +++ b/tests/dpdk/ring_client.c
    >> @@ -185,15 +185,15 @@ main(int argc, char *argv[])
    >>          /* Try dequeuing max possible packets first, if that fails, get the
    >>           * most we can. Loop body should only execute once, maximum.
    >>           */
    >> -        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
    >> -            rx_pkts > 0) {
    >> +        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
    >> +                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
    >>              rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
    >>          }
    >>
    >>          if (rx_pkts > 0) {
    >>              /* blocking enqueue */
    >>              do {
    >> -                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
    >> +                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
    >>              } while (rslt == -ENOBUFS);
    >>          }
    >>      }
    >>
    >
    >_______________________________________________
    >dev mailing list
    >dev@openvswitch.org
    >https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e= 
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e=
Darrell Ball June 1, 2017, 2:41 a.m. UTC | #4
On 5/31/17, 4:36 PM, "ovs-dev-bounces@openvswitch.org on behalf of Darrell Ball" <ovs-dev-bounces@openvswitch.org on behalf of dball@vmware.com> wrote:

    I did a pass through the changes.
    Few comments
    
    On 5/31/17, 6:09 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh, Mark B" <ovs-dev-bounces@openvswitch.org on behalf of mark.b.kavanagh@intel.com> wrote:
    
        
        >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of
        >Kevin Traynor
        >Sent: Wednesday, May 31, 2017 1:53 PM
        >To: Weglicki, MichalX <michalx.weglicki@intel.com>; dev@openvswitch.org
        >Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
        >
        >On 05/31/2017 10:47 AM, mweglicx wrote:
        >> Following changes are applied:
        >> - netdev-dpdk: Changes required by DPDK API modifications.
        >> - doc: Because of DPDK API changes, backward compatibility
        >>   with previous DPDK releases will be broken, thus all
        >>   relevant documentation entries are updated.
        >> - .travis: DPDK version change from 16.11.1 to 17.05.
        >> - rhel/openvswitch-fedora.spec.in: DPDK version change
        >>   from 16.11 to 17.05.
        >>
        >> v1->v2: Patch rework based on minor review comments.
        
        
        Hi Michal,
        
        Apart from the changes suggested by Kevin, LGTM.
        
        Thanks,
        Mark
        
        
        >>
        >> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
        >> ---
        >>  .travis/linux-build.sh                   |   2 +-
        >>  Documentation/faq/releases.rst           |   3 +-
        >>  Documentation/intro/install/dpdk.rst     |   8 +--
        >>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
        >>  lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
        >>  rhel/openvswitch-fedora.spec.in          |   2 +-
        >>  tests/dpdk/ring_client.c                 |   6 +-
        >>  7 files changed, 75 insertions(+), 68 deletions(-)
        >>
        >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
        >> index 8750d68..ec15fd8 100755
        >> --- a/.travis/linux-build.sh
        >> +++ b/.travis/linux-build.sh
        >> @@ -80,7 +80,7 @@ fi
        >>
        >>  if [ "$DPDK" ]; then
        >>      if [ -z "$DPDK_VER" ]; then
        >> -        DPDK_VER="16.11.1"
        >> +        DPDK_VER="17.05"
        >>      fi
        >>      install_dpdk $DPDK_VER
        >>      if [ "$CC" = "clang" ]; then
        >> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
        >> index c85eff8..0455a43 100644
        >> --- a/Documentation/faq/releases.rst
        >> +++ b/Documentation/faq/releases.rst
        >> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch release work with?
        >>      2.4.x        2.0
        >>      2.5.x        2.2
        >>      2.6.x        16.07.2
        >> -    2.7.x        16.11.1
        >> +    2.7.0        16.11.1 <sip:0027016111>
        >> +    2.7.0+       17.05
        >>      ============ =======
        >
        >This is about OVS releases. I think you should revert back to the
        >original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
        >likely not use DPDK 17.05.
        
        That's a good point - +1 on this.
    
    Yep
    
        
        >
        >When OVS 2.8 is close to release it can be updated with it's DPDK
        >version, or you could set it now and it can be changed later if required.
        >
        >>
        >>  Q: I get an error like this when I configure Open vSwitch:
        >> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
        >> index e83f852..536450b 100644
        >> --- a/Documentation/intro/install/dpdk.rst
        >> +++ b/Documentation/intro/install/dpdk.rst
        >> @@ -40,7 +40,7 @@ Build requirements
        >>  In addition to the requirements described in :doc:`general`, building Open
        >>  vSwitch with DPDK will require the following:
        >>
        >> -- DPDK 16.11
        >> +- DPDK 17.05
        >>
        >>  - A `DPDK supported NIC`_
    
    Why is this comment removed ?

Oops, I forgot about the rst formatting; this is fine.
    
    
        >>
        >> @@ -69,9 +69,9 @@ Install DPDK
        >>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
        >>
        >>         $ cd /usr/src/
        >> -       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e= 
        >> -       $ tar xf dpdk-16.11.1.tar.xz
        >> -       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
        >> +       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e= 
        >> +       $ tar xf dpdk-17.05.tar.xz
        >> +       $ export DPDK_DIR=/usr/src/dpdk-17.05
        >>         $ cd $DPDK_DIR
        >>
        >>  #. (Optional) Configure DPDK as a shared library
        >> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-
        >user.rst
        >> index ba22684..71098fd 100644
        >> --- a/Documentation/topics/dpdk/vhost-user.rst
        >> +++ b/Documentation/topics/dpdk/vhost-user.rst
        >> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
        >>  DPDK sources to VM and build DPDK::
        >>
        >>      $ cd /root/dpdk/
        >> -    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e= 
        >> -    $ tar xf dpdk-16.11.1.tar.xz
        >> -    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
        >> +    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e= 
        >> +    $ tar xf dpdk-17.05.tar.xz
        >> +    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
        >>      $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
        >>      $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
        >>      $ cd $DPDK_DIR
        >> @@ -364,7 +364,7 @@ Sample XML
        >>          </disk>
        >>          <disk type='dir' device='disk'>
        >>            <driver name='qemu' type='fat'/>
        >> -          <source dir='/usr/src/dpdk-stable-16.11.1'/>
        >> +          <source dir='/usr/src/dpdk-17.05'/>
        >>            <target dev='vdb' bus='virtio'/>
        >>            <readonly/>
        >>          </disk>
        >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
        >> index 609b8da..fc16d89 100644
        >> --- a/lib/netdev-dpdk.c
        >> +++ b/lib/netdev-dpdk.c
        >> @@ -22,6 +22,9 @@
        >>  #include <stdlib.h>
        >>  #include <errno.h>
        >>  #include <unistd.h>
        >> +#include <linux/virtio_net.h>
        >> +#include <sys/socket.h>
        >> +#include <linux/if.h>
        >>
        >>  #include <rte_config.h>
        >>  #include <rte_cycles.h>
        >> @@ -31,7 +34,7 @@
        >>  #include <rte_malloc.h>
        >>  #include <rte_mbuf.h>
        >>  #include <rte_meter.h>
        >> -#include <rte_virtio_net.h>
        >> +#include <rte_vhost.h>
        >>
        >>  #include "dirs.h"
        >>  #include "dp-packet.h"
        >> @@ -56,6 +59,8 @@
        >>  #include "timeval.h"
        >>  #include "unixctl.h"
        >>
        >> +enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
        >> +
        >>  VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
        >>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
        >>
        >> @@ -164,6 +169,21 @@ static const struct rte_eth_conf port_conf = {
        >>      },
        >>  };
        >>
        >> +/*
        >> + * These callbacks allow virtio-net devices to be added to vhost ports when
        >> + * configuration has been fully completed.
        >> + */
        >> +static int new_device(int vid);
        >> +static void destroy_device(int vid);
        >> +static int vring_state_changed(int vid, uint16_t queue_id, int enable);
        >> +static const struct vhost_device_ops virtio_net_device_ops =
        >> +{
        >> +    .new_device =  new_device,
        >> +    .destroy_device = destroy_device,
        >> +    .vring_state_changed = vring_state_changed,
        >> +    .features_changed = NULL
        >> +};
        >> +
        >>  enum { DPDK_RING_SIZE = 256 };
        >>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
        >>  enum { DRAIN_TSC = 200000ULL };
        >> @@ -402,8 +422,8 @@ struct netdev_rxq_dpdk {
        >>      int port_id;
        >>  };
        >>
        >> -static int netdev_dpdk_class_init(void);
        >> -static int netdev_dpdk_vhost_class_init(void);
        >> +static void netdev_dpdk_destruct(struct netdev *netdev);
        >> +static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
        >>
        >>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
        >>
        >> @@ -413,8 +433,8 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
        >>  static bool
        >>  is_dpdk_class(const struct netdev_class *class)
        >>  {
        >> -    return class->init == netdev_dpdk_class_init
        >> -           || class->init == netdev_dpdk_vhost_class_init;
        >> +    return class->destruct == netdev_dpdk_destruct
        >> +           || class->destruct == netdev_dpdk_vhost_destruct;
        >>  }
        >>
        >>  /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
        >> @@ -942,17 +962,46 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
        >>               dpdk_get_vhost_sock_dir(), name);
        >>
        >>      dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
        >> -    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
        >> +    err = rte_vhost_driver_register(dev->vhost_id,
        >> +                                    dev->vhost_driver_flags);
    
    Why this change ?; the line length was conformant.
    
    
        >>      if (err) {
        >>          VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
        >>                   dev->vhost_id);
        >> +        goto out;
        >>      } else {
        >>          fatal_signal_add_file_to_unlink(dev->vhost_id);
        >>          VLOG_INFO("Socket %s created for vhost-user port %s\n",
        >>                    dev->vhost_id, name);
        >>      }
        >> +
        >> +    err = rte_vhost_driver_callback_register(dev->vhost_id,
        >> +                                                &virtio_net_device_ops);
        >> +    if (err) {
        >> +        VLOG_ERR("vhost-user callback register failed.");
        >> +        goto out;
        >> +    }
        >> +
        >> +    err = rte_vhost_driver_disable_features(dev->vhost_id,
        >> +                                1ULL << VIRTIO_NET_F_HOST_TSO4
        >> +                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
        >> +                                | 1ULL << VIRTIO_NET_F_CSUM);
        >
        >Don't these need to be called for dpdkvhostuserclient ports as well?
        >Seems you could put them in the common construct
        
        Good catch.
        
        >
        >> +    if (err) {
        >> +        VLOG_ERR("vhost-user disable features failed.");
        >> +        goto out;
        >> +    }
        >> +
        >>      err = vhost_common_construct(netdev);
        >> +    if (err) {
        >> +        VLOG_ERR("vhost-user common construct failed.");
        >> +        goto out;
        >> +    }
        >> +
        >> +    err = rte_vhost_driver_start(dev->vhost_id);
        >> +    if (err) {
        >> +        VLOG_ERR("vhost-user driver start failed.");
        >> +    }
        >>
        >> +out:
        >>      ovs_mutex_unlock(&dpdk_mutex);
        >>      return err;
        >>  }
        >> @@ -2483,12 +2532,9 @@ static void
        >>  set_irq_status(int vid)
        >>  {
        >>      uint32_t i;
        >> -    uint64_t idx;
        >>
        >> -    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
        >> -        idx = i * VIRTIO_QNUM;
        >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
        >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
        >> +    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
        >> +        rte_vhost_enable_guest_notification(vid, i, 0);
        >>      }
        >>  }
        >>
        >> @@ -2551,7 +2597,7 @@ new_device(int vid)
        >>      LIST_FOR_EACH(dev, list_node, &dpdk_list) {
        >>          ovs_mutex_lock(&dev->mutex);
        >>          if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
        >> -            uint32_t qp_num = rte_vhost_get_queue_num(vid);
        >> +            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
        >>
        >>              /* Get NUMA information */
        >>              newnode = rte_vhost_get_numa_node(vid);
        >> @@ -2718,27 +2764,6 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
        >>      return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
        >>  }
        >>
        >> -/*
        >> - * These callbacks allow virtio-net devices to be added to vhost ports when
        >> - * configuration has been fully complete.
        >> - */
        >> -static const struct virtio_net_device_ops virtio_net_device_ops =
        >> -{
        >> -    .new_device =  new_device,
        >> -    .destroy_device = destroy_device,
        >> -    .vring_state_changed = vring_state_changed
        >> -};
        >> -
        >> -static void *
        >> -start_vhost_loop(void *dummy OVS_UNUSED)
        >> -{
        >> -     pthread_detach(pthread_self());
        >> -     /* Put the vhost thread into quiescent state. */
        >> -     ovsrcu_quiesce_start();
        >> -     rte_vhost_driver_session_start();
        >> -     return NULL;
        >> -}
        >> -
        >>  static int
        >>  netdev_dpdk_class_init(void)
        >>  {
        >> @@ -2761,25 +2786,6 @@ netdev_dpdk_class_init(void)
        >>      return 0;
        >>  }
        >>
        >> -static int
        >> -netdev_dpdk_vhost_class_init(void)
        >> -{
        >> -    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
        >> -
        >> -    /* This function can be called for different classes.  The initialization
        >> -     * needs to be done only once */
        >> -    if (ovsthread_once_start(&once)) {
        >> -        rte_vhost_driver_callback_register(&virtio_net_device_ops);
        >> -        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
        >> -                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
        >> -                                  | 1ULL << VIRTIO_NET_F_CSUM);
        >> -        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
        >> -
        >> -        ovsthread_once_done(&once);
        >> -    }
        >> -
        >> -    return 0;
        >> -}
        >>
        >>  /* Client Rings */
        >>
        >> @@ -3351,7 +3357,7 @@ static const struct netdev_class dpdk_ring_class =
        >>  static const struct netdev_class dpdk_vhost_class =
        >>      NETDEV_DPDK_CLASS(
        >>          "dpdkvhostuser",
        >> -        netdev_dpdk_vhost_class_init,
        >> +        NULL,
        >>          netdev_dpdk_vhost_construct,
        >>          netdev_dpdk_vhost_destruct,
        >>          NULL,
        >> @@ -3366,7 +3372,7 @@ static const struct netdev_class dpdk_vhost_class =
        >>  static const struct netdev_class dpdk_vhost_client_class =
        >>      NETDEV_DPDK_CLASS(
        >>          "dpdkvhostuserclient",
        >> -        netdev_dpdk_vhost_class_init,
        >> +        NULL,
        >>          netdev_dpdk_vhost_client_construct,
        >>          netdev_dpdk_vhost_destruct,
        >>          netdev_dpdk_vhost_client_set_config,
        >> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
        >> index 3200040..596a4b3 100644
        >> --- a/rhel/openvswitch-fedora.spec.in
        >> +++ b/rhel/openvswitch-fedora.spec.in
        >> @@ -84,7 +84,7 @@ BuildRequires: libcap-ng libcap-ng-devel
        >>  %endif
        >>  %if %{with dpdk}
        >>  BuildRequires: libpcap-devel numactl-devel
        >> -BuildRequires: dpdk-devel >= 16.11
        >> +BuildRequires: dpdk-devel >= 17.05
        >>  Provides: %{name}-dpdk = %{version}-%{release}
        >>  %endif
        >>
        >> diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
        >> index b153713..8cc3fb5 100644
        >> --- a/tests/dpdk/ring_client.c
        >> +++ b/tests/dpdk/ring_client.c
        >> @@ -185,15 +185,15 @@ main(int argc, char *argv[])
        >>          /* Try dequeuing max possible packets first, if that fails, get the
        >>           * most we can. Loop body should only execute once, maximum.
        >>           */
        >> -        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
        >> -            rx_pkts > 0) {
        >> +        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
        >> +                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
        >>              rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
        >>          }
        >>
        >>          if (rx_pkts > 0) {
        >>              /* blocking enqueue */
        >>              do {
        >> -                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
        >> +                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
        >>              } while (rslt == -ENOBUFS);
        >>          }
        >>      }
        >>
        >
        >_______________________________________________
        >dev mailing list
        >dev@openvswitch.org
        >https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e= 
        _______________________________________________
        dev mailing list
        dev@openvswitch.org
        https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e= 
        
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=dGLCETeOAdEDOABsUoGhm4r1lCee0jOipvJ86rjNTSU&s=6tZ-sFcJEVvIOE_Ap6nTK94jXC-TdqfDGKvvBFyamf4&e=
Weglicki, MichalX June 1, 2017, 7:57 a.m. UTC | #5
Hello, 

Thank all for your comments, my answers inline. 

Br, 
Michal. 

> -----Original Message-----
> From: Darrell Ball [mailto:dball@vmware.com]
> Sent: Thursday, June 1, 2017 4:41 AM
> To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Kevin Traynor <ktraynor@redhat.com>; Weglicki, MichalX
> <michalx.weglicki@intel.com>; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
> 
> 
> 
> On 5/31/17, 4:36 PM, "ovs-dev-bounces@openvswitch.org on behalf of Darrell Ball" <ovs-dev-bounces@openvswitch.org on behalf
> of dball@vmware.com> wrote:
> 
>     I did a pass through the changes.
>     Few comments
> 
>     On 5/31/17, 6:09 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh, Mark B" <ovs-dev-bounces@openvswitch.org
> on behalf of mark.b.kavanagh@intel.com> wrote:
> 
> 
>         >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of
>         >Kevin Traynor
>         >Sent: Wednesday, May 31, 2017 1:53 PM
>         >To: Weglicki, MichalX <michalx.weglicki@intel.com>; dev@openvswitch.org
>         >Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
>         >
>         >On 05/31/2017 10:47 AM, mweglicx wrote:
>         >> Following changes are applied:
>         >> - netdev-dpdk: Changes required by DPDK API modifications.
>         >> - doc: Because of DPDK API changes, backward compatibility
>         >>   with previous DPDK releases will be broken, thus all
>         >>   relevant documentation entries are updated.
>         >> - .travis: DPDK version change from 16.11.1 to 17.05.
>         >> - rhel/openvswitch-fedora.spec.in: DPDK version change
>         >>   from 16.11 to 17.05.
>         >>
>         >> v1->v2: Patch rework based on minor review comments.
> 
> 
>         Hi Michal,
> 
>         Apart from the changes suggested by Kevin, LGTM.
> 
>         Thanks,
>         Mark
> 
> 
>         >>
>         >> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
>         >> ---
>         >>  .travis/linux-build.sh                   |   2 +-
>         >>  Documentation/faq/releases.rst           |   3 +-
>         >>  Documentation/intro/install/dpdk.rst     |   8 +--
>         >>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
>         >>  lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
>         >>  rhel/openvswitch-fedora.spec.in          |   2 +-
>         >>  tests/dpdk/ring_client.c                 |   6 +-
>         >>  7 files changed, 75 insertions(+), 68 deletions(-)
>         >>
>         >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
>         >> index 8750d68..ec15fd8 100755
>         >> --- a/.travis/linux-build.sh
>         >> +++ b/.travis/linux-build.sh
>         >> @@ -80,7 +80,7 @@ fi
>         >>
>         >>  if [ "$DPDK" ]; then
>         >>      if [ -z "$DPDK_VER" ]; then
>         >> -        DPDK_VER="16.11.1"
>         >> +        DPDK_VER="17.05"
>         >>      fi
>         >>      install_dpdk $DPDK_VER
>         >>      if [ "$CC" = "clang" ]; then
>         >> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
>         >> index c85eff8..0455a43 100644
>         >> --- a/Documentation/faq/releases.rst
>         >> +++ b/Documentation/faq/releases.rst
>         >> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch release work with?
>         >>      2.4.x        2.0
>         >>      2.5.x        2.2
>         >>      2.6.x        16.07.2
>         >> -    2.7.x        16.11.1
>         >> +    2.7.0        16.11.1 <sip:0027016111>
>         >> +    2.7.0+       17.05
>         >>      ============ =======
>         >
>         >This is about OVS releases. I think you should revert back to the
>         >original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
>         >likely not use DPDK 17.05.
> 
>         That's a good point - +1 on this.
> 
>     Yep
[MW] Will do. 
> 
> 
>         >
>         >When OVS 2.8 is close to release it can be updated with it's DPDK
>         >version, or you could set it now and it can be changed later if required.
>         >
>         >>
>         >>  Q: I get an error like this when I configure Open vSwitch:
>         >> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
>         >> index e83f852..536450b 100644
>         >> --- a/Documentation/intro/install/dpdk.rst
>         >> +++ b/Documentation/intro/install/dpdk.rst
>         >> @@ -40,7 +40,7 @@ Build requirements
>         >>  In addition to the requirements described in :doc:`general`, building Open
>         >>  vSwitch with DPDK will require the following:
>         >>
>         >> -- DPDK 16.11
>         >> +- DPDK 17.05
>         >>
>         >>  - A `DPDK supported NIC`_
> 
>     Why is this comment removed ?
> 
> Oops, I forgot about the rst formatting; this is fine.
> 
> 
>         >>
>         >> @@ -69,9 +69,9 @@ Install DPDK
>         >>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>         >>
>         >>         $ cd /usr/src/
>         >> -       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
> 2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e=
>         >> -       $ tar xf dpdk-16.11.1.tar.xz
>         >> -       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
>         >> +       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
> 2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e=
>         >> +       $ tar xf dpdk-17.05.tar.xz
>         >> +       $ export DPDK_DIR=/usr/src/dpdk-17.05
>         >>         $ cd $DPDK_DIR
>         >>
>         >>  #. (Optional) Configure DPDK as a shared library
>         >> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-
>         >user.rst
>         >> index ba22684..71098fd 100644
>         >> --- a/Documentation/topics/dpdk/vhost-user.rst
>         >> +++ b/Documentation/topics/dpdk/vhost-user.rst
>         >> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
>         >>  DPDK sources to VM and build DPDK::
>         >>
>         >>      $ cd /root/dpdk/
>         >> -    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
> 2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e=
>         >> -    $ tar xf dpdk-16.11.1.tar.xz
>         >> -    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
>         >> +    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
> 2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e=
>         >> +    $ tar xf dpdk-17.05.tar.xz
>         >> +    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
>         >>      $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
>         >>      $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>         >>      $ cd $DPDK_DIR
>         >> @@ -364,7 +364,7 @@ Sample XML
>         >>          </disk>
>         >>          <disk type='dir' device='disk'>
>         >>            <driver name='qemu' type='fat'/>
>         >> -          <source dir='/usr/src/dpdk-stable-16.11.1'/>
>         >> +          <source dir='/usr/src/dpdk-17.05'/>
>         >>            <target dev='vdb' bus='virtio'/>
>         >>            <readonly/>
>         >>          </disk>
>         >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>         >> index 609b8da..fc16d89 100644
>         >> --- a/lib/netdev-dpdk.c
>         >> +++ b/lib/netdev-dpdk.c
>         >> @@ -22,6 +22,9 @@
>         >>  #include <stdlib.h>
>         >>  #include <errno.h>
>         >>  #include <unistd.h>
>         >> +#include <linux/virtio_net.h>
>         >> +#include <sys/socket.h>
>         >> +#include <linux/if.h>
>         >>
>         >>  #include <rte_config.h>
>         >>  #include <rte_cycles.h>
>         >> @@ -31,7 +34,7 @@
>         >>  #include <rte_malloc.h>
>         >>  #include <rte_mbuf.h>
>         >>  #include <rte_meter.h>
>         >> -#include <rte_virtio_net.h>
>         >> +#include <rte_vhost.h>
>         >>
>         >>  #include "dirs.h"
>         >>  #include "dp-packet.h"
>         >> @@ -56,6 +59,8 @@
>         >>  #include "timeval.h"
>         >>  #include "unixctl.h"
>         >>
>         >> +enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
>         >> +
>         >>  VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
>         >>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>         >>
>         >> @@ -164,6 +169,21 @@ static const struct rte_eth_conf port_conf = {
>         >>      },
>         >>  };
>         >>
>         >> +/*
>         >> + * These callbacks allow virtio-net devices to be added to vhost ports when
>         >> + * configuration has been fully completed.
>         >> + */
>         >> +static int new_device(int vid);
>         >> +static void destroy_device(int vid);
>         >> +static int vring_state_changed(int vid, uint16_t queue_id, int enable);
>         >> +static const struct vhost_device_ops virtio_net_device_ops =
>         >> +{
>         >> +    .new_device =  new_device,
>         >> +    .destroy_device = destroy_device,
>         >> +    .vring_state_changed = vring_state_changed,
>         >> +    .features_changed = NULL
>         >> +};
>         >> +
>         >>  enum { DPDK_RING_SIZE = 256 };
>         >>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
>         >>  enum { DRAIN_TSC = 200000ULL };
>         >> @@ -402,8 +422,8 @@ struct netdev_rxq_dpdk {
>         >>      int port_id;
>         >>  };
>         >>
>         >> -static int netdev_dpdk_class_init(void);
>         >> -static int netdev_dpdk_vhost_class_init(void);
>         >> +static void netdev_dpdk_destruct(struct netdev *netdev);
>         >> +static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
>         >>
>         >>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
>         >>
>         >> @@ -413,8 +433,8 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
>         >>  static bool
>         >>  is_dpdk_class(const struct netdev_class *class)
>         >>  {
>         >> -    return class->init == netdev_dpdk_class_init
>         >> -           || class->init == netdev_dpdk_vhost_class_init;
>         >> +    return class->destruct == netdev_dpdk_destruct
>         >> +           || class->destruct == netdev_dpdk_vhost_destruct;
>         >>  }
>         >>
>         >>  /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
>         >> @@ -942,17 +962,46 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
>         >>               dpdk_get_vhost_sock_dir(), name);
>         >>
>         >>      dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
>         >> -    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
>         >> +    err = rte_vhost_driver_register(dev->vhost_id,
>         >> +                                    dev->vhost_driver_flags);
> 
>     Why this change ?; the line length was conformant.
[MW] Yes, you are right, in fact it was because that V1 of my patch had "ret_value" instead of "err" variable,
And that caused this change. When I changed name back to "err" in V2 I forgot about it. I will switch it back. 
> 
> 
>         >>      if (err) {
>         >>          VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
>         >>                   dev->vhost_id);
>         >> +        goto out;
>         >>      } else {
>         >>          fatal_signal_add_file_to_unlink(dev->vhost_id);
>         >>          VLOG_INFO("Socket %s created for vhost-user port %s\n",
>         >>                    dev->vhost_id, name);
>         >>      }
>         >> +
>         >> +    err = rte_vhost_driver_callback_register(dev->vhost_id,
>         >> +                                                &virtio_net_device_ops);
>         >> +    if (err) {
>         >> +        VLOG_ERR("vhost-user callback register failed.");
>         >> +        goto out;
>         >> +    }
>         >> +
>         >> +    err = rte_vhost_driver_disable_features(dev->vhost_id,
>         >> +                                1ULL << VIRTIO_NET_F_HOST_TSO4
>         >> +                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
>         >> +                                | 1ULL << VIRTIO_NET_F_CSUM);
>         >
>         >Don't these need to be called for dpdkvhostuserclient ports as well?
>         >Seems you could put them in the common construct
> 
>         Good catch.
[MW] Indeed, as I understand this case, vhost client should also configure: 
* rte_vhost_driver_disable_features
* rte_vhost_driver_callback_register
However I think that it has to be done in: netdev_dpdk_vhost_client_reconfigure
As vhost_id (socket) isn't known during construct, as it is configured in runtime. 
> 
>         >
>         >> +    if (err) {
>         >> +        VLOG_ERR("vhost-user disable features failed.");
>         >> +        goto out;
>         >> +    }
>         >> +
>         >>      err = vhost_common_construct(netdev);
>         >> +    if (err) {
>         >> +        VLOG_ERR("vhost-user common construct failed.");
>         >> +        goto out;
>         >> +    }
>         >> +
>         >> +    err = rte_vhost_driver_start(dev->vhost_id);
>         >> +    if (err) {
>         >> +        VLOG_ERR("vhost-user driver start failed.");
>         >> +    }
>         >>
>         >> +out:
>         >>      ovs_mutex_unlock(&dpdk_mutex);
>         >>      return err;
>         >>  }
>         >> @@ -2483,12 +2532,9 @@ static void
>         >>  set_irq_status(int vid)
>         >>  {
>         >>      uint32_t i;
>         >> -    uint64_t idx;
>         >>
>         >> -    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
>         >> -        idx = i * VIRTIO_QNUM;
>         >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
>         >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
>         >> +    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
>         >> +        rte_vhost_enable_guest_notification(vid, i, 0);
>         >>      }
>         >>  }
>         >>
>         >> @@ -2551,7 +2597,7 @@ new_device(int vid)
>         >>      LIST_FOR_EACH(dev, list_node, &dpdk_list) {
>         >>          ovs_mutex_lock(&dev->mutex);
>         >>          if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
>         >> -            uint32_t qp_num = rte_vhost_get_queue_num(vid);
>         >> +            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
>         >>
>         >>              /* Get NUMA information */
>         >>              newnode = rte_vhost_get_numa_node(vid);
>         >> @@ -2718,27 +2764,6 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
>         >>      return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
>         >>  }
>         >>
>         >> -/*
>         >> - * These callbacks allow virtio-net devices to be added to vhost ports when
>         >> - * configuration has been fully complete.
>         >> - */
>         >> -static const struct virtio_net_device_ops virtio_net_device_ops =
>         >> -{
>         >> -    .new_device =  new_device,
>         >> -    .destroy_device = destroy_device,
>         >> -    .vring_state_changed = vring_state_changed
>         >> -};
>         >> -
>         >> -static void *
>         >> -start_vhost_loop(void *dummy OVS_UNUSED)
>         >> -{
>         >> -     pthread_detach(pthread_self());
>         >> -     /* Put the vhost thread into quiescent state. */
>         >> -     ovsrcu_quiesce_start();
>         >> -     rte_vhost_driver_session_start();
>         >> -     return NULL;
>         >> -}
>         >> -
>         >>  static int
>         >>  netdev_dpdk_class_init(void)
>         >>  {
>         >> @@ -2761,25 +2786,6 @@ netdev_dpdk_class_init(void)
>         >>      return 0;
>         >>  }
>         >>
>         >> -static int
>         >> -netdev_dpdk_vhost_class_init(void)
>         >> -{
>         >> -    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>         >> -
>         >> -    /* This function can be called for different classes.  The initialization
>         >> -     * needs to be done only once */
>         >> -    if (ovsthread_once_start(&once)) {
>         >> -        rte_vhost_driver_callback_register(&virtio_net_device_ops);
>         >> -        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
>         >> -                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
>         >> -                                  | 1ULL << VIRTIO_NET_F_CSUM);
>         >> -        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
>         >> -
>         >> -        ovsthread_once_done(&once);
>         >> -    }
>         >> -
>         >> -    return 0;
>         >> -}
>         >>
>         >>  /* Client Rings */
>         >>
>         >> @@ -3351,7 +3357,7 @@ static const struct netdev_class dpdk_ring_class =
>         >>  static const struct netdev_class dpdk_vhost_class =
>         >>      NETDEV_DPDK_CLASS(
>         >>          "dpdkvhostuser",
>         >> -        netdev_dpdk_vhost_class_init,
>         >> +        NULL,
>         >>          netdev_dpdk_vhost_construct,
>         >>          netdev_dpdk_vhost_destruct,
>         >>          NULL,
>         >> @@ -3366,7 +3372,7 @@ static const struct netdev_class dpdk_vhost_class =
>         >>  static const struct netdev_class dpdk_vhost_client_class =
>         >>      NETDEV_DPDK_CLASS(
>         >>          "dpdkvhostuserclient",
>         >> -        netdev_dpdk_vhost_class_init,
>         >> +        NULL,
>         >>          netdev_dpdk_vhost_client_construct,
>         >>          netdev_dpdk_vhost_destruct,
>         >>          netdev_dpdk_vhost_client_set_config,
>         >> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>         >> index 3200040..596a4b3 100644
>         >> --- a/rhel/openvswitch-fedora.spec.in
>         >> +++ b/rhel/openvswitch-fedora.spec.in
>         >> @@ -84,7 +84,7 @@ BuildRequires: libcap-ng libcap-ng-devel
>         >>  %endif
>         >>  %if %{with dpdk}
>         >>  BuildRequires: libpcap-devel numactl-devel
>         >> -BuildRequires: dpdk-devel >= 16.11
>         >> +BuildRequires: dpdk-devel >= 17.05
>         >>  Provides: %{name}-dpdk = %{version}-%{release}
>         >>  %endif
>         >>
>         >> diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
>         >> index b153713..8cc3fb5 100644
>         >> --- a/tests/dpdk/ring_client.c
>         >> +++ b/tests/dpdk/ring_client.c
>         >> @@ -185,15 +185,15 @@ main(int argc, char *argv[])
>         >>          /* Try dequeuing max possible packets first, if that fails, get the
>         >>           * most we can. Loop body should only execute once, maximum.
>         >>           */
>         >> -        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
>         >> -            rx_pkts > 0) {
>         >> +        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
>         >> +                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
>         >>              rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
>         >>          }
>         >>
>         >>          if (rx_pkts > 0) {
>         >>              /* blocking enqueue */
>         >>              do {
>         >> -                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
>         >> +                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
>         >>              } while (rslt == -ENOBUFS);
>         >>          }
>         >>      }
>         >>
>         >
>         >_______________________________________________
>         >dev mailing list
>         >dev@openvswitch.org
>         >https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-
> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e=
>         _______________________________________________
>         dev mailing list
>         dev@openvswitch.org
>         https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-
> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e=
> 
> 
>     _______________________________________________
>     dev mailing list
>     dev@openvswitch.org
>     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-
> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=dGLCETeOAdEDOABsUoGhm4r1lCee0jOipvJ86rjNTSU&s=6tZ-sFcJEVvIOE_Ap6nTK94jXC-TdqfDGKvvBFyamf4&e=
> 

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
Kevin Traynor June 1, 2017, 9:24 a.m. UTC | #6
On 06/01/2017 08:57 AM, Weglicki, MichalX wrote:
> Hello, 
> 
> Thank all for your comments, my answers inline. 
> 
> Br, 
> Michal. 
> 
>> -----Original Message-----
>> From: Darrell Ball [mailto:dball@vmware.com]
>> Sent: Thursday, June 1, 2017 4:41 AM
>> To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Kevin Traynor <ktraynor@redhat.com>; Weglicki, MichalX
>> <michalx.weglicki@intel.com>; dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
>>
>>
>>
>> On 5/31/17, 4:36 PM, "ovs-dev-bounces@openvswitch.org on behalf of Darrell Ball" <ovs-dev-bounces@openvswitch.org on behalf
>> of dball@vmware.com> wrote:
>>
>>     I did a pass through the changes.
>>     Few comments
>>
>>     On 5/31/17, 6:09 AM, "ovs-dev-bounces@openvswitch.org on behalf of Kavanagh, Mark B" <ovs-dev-bounces@openvswitch.org
>> on behalf of mark.b.kavanagh@intel.com> wrote:
>>
>>
>>         >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of
>>         >Kevin Traynor
>>         >Sent: Wednesday, May 31, 2017 1:53 PM
>>         >To: Weglicki, MichalX <michalx.weglicki@intel.com>; dev@openvswitch.org
>>         >Subject: Re: [ovs-dev] [PATCH v2] Update relevant artifacts to add support for DPDK 17.05.
>>         >
>>         >On 05/31/2017 10:47 AM, mweglicx wrote:
>>         >> Following changes are applied:
>>         >> - netdev-dpdk: Changes required by DPDK API modifications.
>>         >> - doc: Because of DPDK API changes, backward compatibility
>>         >>   with previous DPDK releases will be broken, thus all
>>         >>   relevant documentation entries are updated.
>>         >> - .travis: DPDK version change from 16.11.1 to 17.05.
>>         >> - rhel/openvswitch-fedora.spec.in: DPDK version change
>>         >>   from 16.11 to 17.05.
>>         >>
>>         >> v1->v2: Patch rework based on minor review comments.
>>
>>
>>         Hi Michal,
>>
>>         Apart from the changes suggested by Kevin, LGTM.
>>
>>         Thanks,
>>         Mark
>>
>>
>>         >>
>>         >> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>
>>         >> ---
>>         >>  .travis/linux-build.sh                   |   2 +-
>>         >>  Documentation/faq/releases.rst           |   3 +-
>>         >>  Documentation/intro/install/dpdk.rst     |   8 +--
>>         >>  Documentation/topics/dpdk/vhost-user.rst |   8 +--
>>         >>  lib/netdev-dpdk.c                        | 114 ++++++++++++++++---------------
>>         >>  rhel/openvswitch-fedora.spec.in          |   2 +-
>>         >>  tests/dpdk/ring_client.c                 |   6 +-
>>         >>  7 files changed, 75 insertions(+), 68 deletions(-)
>>         >>
>>         >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
>>         >> index 8750d68..ec15fd8 100755
>>         >> --- a/.travis/linux-build.sh
>>         >> +++ b/.travis/linux-build.sh
>>         >> @@ -80,7 +80,7 @@ fi
>>         >>
>>         >>  if [ "$DPDK" ]; then
>>         >>      if [ -z "$DPDK_VER" ]; then
>>         >> -        DPDK_VER="16.11.1"
>>         >> +        DPDK_VER="17.05"
>>         >>      fi
>>         >>      install_dpdk $DPDK_VER
>>         >>      if [ "$CC" = "clang" ]; then
>>         >> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
>>         >> index c85eff8..0455a43 100644
>>         >> --- a/Documentation/faq/releases.rst
>>         >> +++ b/Documentation/faq/releases.rst
>>         >> @@ -160,7 +160,8 @@ Q: What DPDK version does each Open vSwitch release work with?
>>         >>      2.4.x        2.0
>>         >>      2.5.x        2.2
>>         >>      2.6.x        16.07.2
>>         >> -    2.7.x        16.11.1
>>         >> +    2.7.0        16.11.1 <sip:0027016111> <sip:0027016111 <sip:0027016111>>
>>         >> +    2.7.0+       17.05
>>         >>      ============ =======
>>         >
>>         >This is about OVS releases. I think you should revert back to the
>>         >original text for 2.7.x. Any OVS 2.7.x release from branch-2.7 would
>>         >likely not use DPDK 17.05.
>>
>>         That's a good point - +1 on this.
>>
>>     Yep
> [MW] Will do. 
>>
>>
>>         >
>>         >When OVS 2.8 is close to release it can be updated with it's DPDK
>>         >version, or you could set it now and it can be changed later if required.
>>         >
>>         >>
>>         >>  Q: I get an error like this when I configure Open vSwitch:
>>         >> diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
>>         >> index e83f852..536450b 100644
>>         >> --- a/Documentation/intro/install/dpdk.rst
>>         >> +++ b/Documentation/intro/install/dpdk.rst
>>         >> @@ -40,7 +40,7 @@ Build requirements
>>         >>  In addition to the requirements described in :doc:`general`, building Open
>>         >>  vSwitch with DPDK will require the following:
>>         >>
>>         >> -- DPDK 16.11
>>         >> +- DPDK 17.05
>>         >>
>>         >>  - A `DPDK supported NIC`_
>>
>>     Why is this comment removed ?
>>
>> Oops, I forgot about the rst formatting; this is fine.
>>
>>
>>         >>
>>         >> @@ -69,9 +69,9 @@ Install DPDK
>>         >>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>>         >>
>>         >>         $ cd /usr/src/
>>         >> -       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
>> 2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e=
>>         >> -       $ tar xf dpdk-16.11.1.tar.xz
>>         >> -       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
>>         >> +       $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
>> 2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e=
>>         >> +       $ tar xf dpdk-17.05.tar.xz
>>         >> +       $ export DPDK_DIR=/usr/src/dpdk-17.05
>>         >>         $ cd $DPDK_DIR
>>         >>
>>         >>  #. (Optional) Configure DPDK as a shared library
>>         >> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-
>>         >user.rst
>>         >> index ba22684..71098fd 100644
>>         >> --- a/Documentation/topics/dpdk/vhost-user.rst
>>         >> +++ b/Documentation/topics/dpdk/vhost-user.rst
>>         >> @@ -278,9 +278,9 @@ To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
>>         >>  DPDK sources to VM and build DPDK::
>>         >>
>>         >>      $ cd /root/dpdk/
>>         >> -    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
>> 2D16.11.1.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=sWpg9QUWyGG5KLaCvxfmi8rGeQ_jJpujCJHS-QA3lOs&e=
>>         >> -    $ tar xf dpdk-16.11.1.tar.xz
>>         >> -    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
>>         >> +    $ wget https://urldefense.proofpoint.com/v2/url?u=http-3A__fast.dpdk.org_rel_dpdk-
>> 2D17.05.tar.xz&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=-ao3B2Subg3ih7x3NP8_Ak3lTIxYHoXtAGCtUNuaedI&e=
>>         >> +    $ tar xf dpdk-17.05.tar.xz
>>         >> +    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
>>         >>      $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
>>         >>      $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>>         >>      $ cd $DPDK_DIR
>>         >> @@ -364,7 +364,7 @@ Sample XML
>>         >>          </disk>
>>         >>          <disk type='dir' device='disk'>
>>         >>            <driver name='qemu' type='fat'/>
>>         >> -          <source dir='/usr/src/dpdk-stable-16.11.1'/>
>>         >> +          <source dir='/usr/src/dpdk-17.05'/>
>>         >>            <target dev='vdb' bus='virtio'/>
>>         >>            <readonly/>
>>         >>          </disk>
>>         >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>         >> index 609b8da..fc16d89 100644
>>         >> --- a/lib/netdev-dpdk.c
>>         >> +++ b/lib/netdev-dpdk.c
>>         >> @@ -22,6 +22,9 @@
>>         >>  #include <stdlib.h>
>>         >>  #include <errno.h>
>>         >>  #include <unistd.h>
>>         >> +#include <linux/virtio_net.h>
>>         >> +#include <sys/socket.h>
>>         >> +#include <linux/if.h>
>>         >>
>>         >>  #include <rte_config.h>
>>         >>  #include <rte_cycles.h>
>>         >> @@ -31,7 +34,7 @@
>>         >>  #include <rte_malloc.h>
>>         >>  #include <rte_mbuf.h>
>>         >>  #include <rte_meter.h>
>>         >> -#include <rte_virtio_net.h>
>>         >> +#include <rte_vhost.h>
>>         >>
>>         >>  #include "dirs.h"
>>         >>  #include "dp-packet.h"
>>         >> @@ -56,6 +59,8 @@
>>         >>  #include "timeval.h"
>>         >>  #include "unixctl.h"
>>         >>
>>         >> +enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
>>         >> +
>>         >>  VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
>>         >>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>>         >>
>>         >> @@ -164,6 +169,21 @@ static const struct rte_eth_conf port_conf = {
>>         >>      },
>>         >>  };
>>         >>
>>         >> +/*
>>         >> + * These callbacks allow virtio-net devices to be added to vhost ports when
>>         >> + * configuration has been fully completed.
>>         >> + */
>>         >> +static int new_device(int vid);
>>         >> +static void destroy_device(int vid);
>>         >> +static int vring_state_changed(int vid, uint16_t queue_id, int enable);
>>         >> +static const struct vhost_device_ops virtio_net_device_ops =
>>         >> +{
>>         >> +    .new_device =  new_device,
>>         >> +    .destroy_device = destroy_device,
>>         >> +    .vring_state_changed = vring_state_changed,
>>         >> +    .features_changed = NULL
>>         >> +};
>>         >> +
>>         >>  enum { DPDK_RING_SIZE = 256 };
>>         >>  BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
>>         >>  enum { DRAIN_TSC = 200000ULL };
>>         >> @@ -402,8 +422,8 @@ struct netdev_rxq_dpdk {
>>         >>      int port_id;
>>         >>  };
>>         >>
>>         >> -static int netdev_dpdk_class_init(void);
>>         >> -static int netdev_dpdk_vhost_class_init(void);
>>         >> +static void netdev_dpdk_destruct(struct netdev *netdev);
>>         >> +static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
>>         >>
>>         >>  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
>>         >>
>>         >> @@ -413,8 +433,8 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
>>         >>  static bool
>>         >>  is_dpdk_class(const struct netdev_class *class)
>>         >>  {
>>         >> -    return class->init == netdev_dpdk_class_init
>>         >> -           || class->init == netdev_dpdk_vhost_class_init;
>>         >> +    return class->destruct == netdev_dpdk_destruct
>>         >> +           || class->destruct == netdev_dpdk_vhost_destruct;
>>         >>  }
>>         >>
>>         >>  /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
>>         >> @@ -942,17 +962,46 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
>>         >>               dpdk_get_vhost_sock_dir(), name);
>>         >>
>>         >>      dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
>>         >> -    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
>>         >> +    err = rte_vhost_driver_register(dev->vhost_id,
>>         >> +                                    dev->vhost_driver_flags);
>>
>>     Why this change ?; the line length was conformant.
> [MW] Yes, you are right, in fact it was because that V1 of my patch had "ret_value" instead of "err" variable,
> And that caused this change. When I changed name back to "err" in V2 I forgot about it. I will switch it back. 
>>
>>
>>         >>      if (err) {
>>         >>          VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
>>         >>                   dev->vhost_id);
>>         >> +        goto out;
>>         >>      } else {
>>         >>          fatal_signal_add_file_to_unlink(dev->vhost_id);
>>         >>          VLOG_INFO("Socket %s created for vhost-user port %s\n",
>>         >>                    dev->vhost_id, name);
>>         >>      }
>>         >> +
>>         >> +    err = rte_vhost_driver_callback_register(dev->vhost_id,
>>         >> +                                                &virtio_net_device_ops);
>>         >> +    if (err) {
>>         >> +        VLOG_ERR("vhost-user callback register failed.");
>>         >> +        goto out;
>>         >> +    }
>>         >> +
>>         >> +    err = rte_vhost_driver_disable_features(dev->vhost_id,
>>         >> +                                1ULL << VIRTIO_NET_F_HOST_TSO4
>>         >> +                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
>>         >> +                                | 1ULL << VIRTIO_NET_F_CSUM);
>>         >
>>         >Don't these need to be called for dpdkvhostuserclient ports as well?
>>         >Seems you could put them in the common construct
>>
>>         Good catch.
> [MW] Indeed, as I understand this case, vhost client should also configure: 
> * rte_vhost_driver_disable_features
> * rte_vhost_driver_callback_register
> However I think that it has to be done in: netdev_dpdk_vhost_client_reconfigure
> As vhost_id (socket) isn't known during construct, as it is configured in runtime. 

Yep, that sounds right.

>>
>>         >
>>         >> +    if (err) {
>>         >> +        VLOG_ERR("vhost-user disable features failed.");
>>         >> +        goto out;
>>         >> +    }
>>         >> +
>>         >>      err = vhost_common_construct(netdev);
>>         >> +    if (err) {
>>         >> +        VLOG_ERR("vhost-user common construct failed.");
>>         >> +        goto out;
>>         >> +    }
>>         >> +
>>         >> +    err = rte_vhost_driver_start(dev->vhost_id);
>>         >> +    if (err) {
>>         >> +        VLOG_ERR("vhost-user driver start failed.");
>>         >> +    }
>>         >>
>>         >> +out:
>>         >>      ovs_mutex_unlock(&dpdk_mutex);
>>         >>      return err;
>>         >>  }
>>         >> @@ -2483,12 +2532,9 @@ static void
>>         >>  set_irq_status(int vid)
>>         >>  {
>>         >>      uint32_t i;
>>         >> -    uint64_t idx;
>>         >>
>>         >> -    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
>>         >> -        idx = i * VIRTIO_QNUM;
>>         >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
>>         >> -        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
>>         >> +    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
>>         >> +        rte_vhost_enable_guest_notification(vid, i, 0);
>>         >>      }
>>         >>  }
>>         >>
>>         >> @@ -2551,7 +2597,7 @@ new_device(int vid)
>>         >>      LIST_FOR_EACH(dev, list_node, &dpdk_list) {
>>         >>          ovs_mutex_lock(&dev->mutex);
>>         >>          if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
>>         >> -            uint32_t qp_num = rte_vhost_get_queue_num(vid);
>>         >> +            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
>>         >>
>>         >>              /* Get NUMA information */
>>         >>              newnode = rte_vhost_get_numa_node(vid);
>>         >> @@ -2718,27 +2764,6 @@ netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
>>         >>      return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
>>         >>  }
>>         >>
>>         >> -/*
>>         >> - * These callbacks allow virtio-net devices to be added to vhost ports when
>>         >> - * configuration has been fully complete.
>>         >> - */
>>         >> -static const struct virtio_net_device_ops virtio_net_device_ops =
>>         >> -{
>>         >> -    .new_device =  new_device,
>>         >> -    .destroy_device = destroy_device,
>>         >> -    .vring_state_changed = vring_state_changed
>>         >> -};
>>         >> -
>>         >> -static void *
>>         >> -start_vhost_loop(void *dummy OVS_UNUSED)
>>         >> -{
>>         >> -     pthread_detach(pthread_self());
>>         >> -     /* Put the vhost thread into quiescent state. */
>>         >> -     ovsrcu_quiesce_start();
>>         >> -     rte_vhost_driver_session_start();
>>         >> -     return NULL;
>>         >> -}
>>         >> -
>>         >>  static int
>>         >>  netdev_dpdk_class_init(void)
>>         >>  {
>>         >> @@ -2761,25 +2786,6 @@ netdev_dpdk_class_init(void)
>>         >>      return 0;
>>         >>  }
>>         >>
>>         >> -static int
>>         >> -netdev_dpdk_vhost_class_init(void)
>>         >> -{
>>         >> -    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>>         >> -
>>         >> -    /* This function can be called for different classes.  The initialization
>>         >> -     * needs to be done only once */
>>         >> -    if (ovsthread_once_start(&once)) {
>>         >> -        rte_vhost_driver_callback_register(&virtio_net_device_ops);
>>         >> -        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
>>         >> -                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
>>         >> -                                  | 1ULL << VIRTIO_NET_F_CSUM);
>>         >> -        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
>>         >> -
>>         >> -        ovsthread_once_done(&once);
>>         >> -    }
>>         >> -
>>         >> -    return 0;
>>         >> -}
>>         >>
>>         >>  /* Client Rings */
>>         >>
>>         >> @@ -3351,7 +3357,7 @@ static const struct netdev_class dpdk_ring_class =
>>         >>  static const struct netdev_class dpdk_vhost_class =
>>         >>      NETDEV_DPDK_CLASS(
>>         >>          "dpdkvhostuser",
>>         >> -        netdev_dpdk_vhost_class_init,
>>         >> +        NULL,
>>         >>          netdev_dpdk_vhost_construct,
>>         >>          netdev_dpdk_vhost_destruct,
>>         >>          NULL,
>>         >> @@ -3366,7 +3372,7 @@ static const struct netdev_class dpdk_vhost_class =
>>         >>  static const struct netdev_class dpdk_vhost_client_class =
>>         >>      NETDEV_DPDK_CLASS(
>>         >>          "dpdkvhostuserclient",
>>         >> -        netdev_dpdk_vhost_class_init,
>>         >> +        NULL,
>>         >>          netdev_dpdk_vhost_client_construct,
>>         >>          netdev_dpdk_vhost_destruct,
>>         >>          netdev_dpdk_vhost_client_set_config,
>>         >> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>>         >> index 3200040..596a4b3 100644
>>         >> --- a/rhel/openvswitch-fedora.spec.in
>>         >> +++ b/rhel/openvswitch-fedora.spec.in
>>         >> @@ -84,7 +84,7 @@ BuildRequires: libcap-ng libcap-ng-devel
>>         >>  %endif
>>         >>  %if %{with dpdk}
>>         >>  BuildRequires: libpcap-devel numactl-devel
>>         >> -BuildRequires: dpdk-devel >= 16.11
>>         >> +BuildRequires: dpdk-devel >= 17.05
>>         >>  Provides: %{name}-dpdk = %{version}-%{release}
>>         >>  %endif
>>         >>
>>         >> diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
>>         >> index b153713..8cc3fb5 100644
>>         >> --- a/tests/dpdk/ring_client.c
>>         >> +++ b/tests/dpdk/ring_client.c
>>         >> @@ -185,15 +185,15 @@ main(int argc, char *argv[])
>>         >>          /* Try dequeuing max possible packets first, if that fails, get the
>>         >>           * most we can. Loop body should only execute once, maximum.
>>         >>           */
>>         >> -        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
>>         >> -            rx_pkts > 0) {
>>         >> +        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
>>         >> +                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
>>         >>              rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
>>         >>          }
>>         >>
>>         >>          if (rx_pkts > 0) {
>>         >>              /* blocking enqueue */
>>         >>              do {
>>         >> -                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
>>         >> +                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
>>         >>              } while (rslt == -ENOBUFS);
>>         >>          }
>>         >>      }
>>         >>
>>         >
>>         >_______________________________________________
>>         >dev mailing list
>>         >dev@openvswitch.org
>>         >https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-
>> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e=
>>         _______________________________________________
>>         dev mailing list
>>         dev@openvswitch.org
>>         https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-
>> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=wOLduAA0jsEfaPaRY3ddWirtr5d67yI0rIErqV4Uv8g&s=qO7W3aJ6oqywUv2i5EOVIitS8h4kAFauFNAAH5VfiOc&e=
>>
>>
>>     _______________________________________________
>>     dev mailing list
>>     dev@openvswitch.org
>>     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-
>> 2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>> uZnsw&m=dGLCETeOAdEDOABsUoGhm4r1lCee0jOipvJ86rjNTSU&s=6tZ-sFcJEVvIOE_Ap6nTK94jXC-TdqfDGKvvBFyamf4&e=
>>
> 
> --------------------------------------------------------------
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
> 
> 
> This e-mail and any attachments may contain confidential material for the sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact the
> sender and delete all copies.
>
diff mbox

Patch

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 8750d68..ec15fd8 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -80,7 +80,7 @@  fi
 
 if [ "$DPDK" ]; then
     if [ -z "$DPDK_VER" ]; then
-        DPDK_VER="16.11.1"
+        DPDK_VER="17.05"
     fi
     install_dpdk $DPDK_VER
     if [ "$CC" = "clang" ]; then
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index c85eff8..0455a43 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -160,7 +160,8 @@  Q: What DPDK version does each Open vSwitch release work with?
     2.4.x        2.0
     2.5.x        2.2
     2.6.x        16.07.2
-    2.7.x        16.11.1
+    2.7.0        16.11.1
+    2.7.0+       17.05
     ============ =======
 
 Q: I get an error like this when I configure Open vSwitch:
diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
index e83f852..536450b 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -40,7 +40,7 @@  Build requirements
 In addition to the requirements described in :doc:`general`, building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 16.11
+- DPDK 17.05
 
 - A `DPDK supported NIC`_
 
@@ -69,9 +69,9 @@  Install DPDK
 #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
 
        $ cd /usr/src/
-       $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
-       $ tar xf dpdk-16.11.1.tar.xz
-       $ export DPDK_DIR=/usr/src/dpdk-stable-16.11.1
+       $ wget http://fast.dpdk.org/rel/dpdk-17.05.tar.xz
+       $ tar xf dpdk-17.05.tar.xz
+       $ export DPDK_DIR=/usr/src/dpdk-17.05
        $ cd $DPDK_DIR
 
 #. (Optional) Configure DPDK as a shared library
diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
index ba22684..71098fd 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -278,9 +278,9 @@  To begin, instantiate a guest as described in :ref:`dpdk-vhost-user` or
 DPDK sources to VM and build DPDK::
 
     $ cd /root/dpdk/
-    $ wget http://fast.dpdk.org/rel/dpdk-16.11.1.tar.xz
-    $ tar xf dpdk-16.11.1.tar.xz
-    $ export DPDK_DIR=/root/dpdk/dpdk-stable-16.11.1
+    $ wget http://fast.dpdk.org/rel/dpdk-17.05.tar.xz
+    $ tar xf dpdk-17.05.tar.xz
+    $ export DPDK_DIR=/root/dpdk/dpdk-17.05
     $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
     $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
     $ cd $DPDK_DIR
@@ -364,7 +364,7 @@  Sample XML
         </disk>
         <disk type='dir' device='disk'>
           <driver name='qemu' type='fat'/>
-          <source dir='/usr/src/dpdk-stable-16.11.1'/>
+          <source dir='/usr/src/dpdk-17.05'/>
           <target dev='vdb' bus='virtio'/>
           <readonly/>
         </disk>
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 609b8da..fc16d89 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -22,6 +22,9 @@ 
 #include <stdlib.h>
 #include <errno.h>
 #include <unistd.h>
+#include <linux/virtio_net.h>
+#include <sys/socket.h>
+#include <linux/if.h>
 
 #include <rte_config.h>
 #include <rte_cycles.h>
@@ -31,7 +34,7 @@ 
 #include <rte_malloc.h>
 #include <rte_mbuf.h>
 #include <rte_meter.h>
-#include <rte_virtio_net.h>
+#include <rte_vhost.h>
 
 #include "dirs.h"
 #include "dp-packet.h"
@@ -56,6 +59,8 @@ 
 #include "timeval.h"
 #include "unixctl.h"
 
+enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
+
 VLOG_DEFINE_THIS_MODULE(netdev_dpdk);
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
 
@@ -164,6 +169,21 @@  static const struct rte_eth_conf port_conf = {
     },
 };
 
+/*
+ * These callbacks allow virtio-net devices to be added to vhost ports when
+ * configuration has been fully completed.
+ */
+static int new_device(int vid);
+static void destroy_device(int vid);
+static int vring_state_changed(int vid, uint16_t queue_id, int enable);
+static const struct vhost_device_ops virtio_net_device_ops =
+{
+    .new_device =  new_device,
+    .destroy_device = destroy_device,
+    .vring_state_changed = vring_state_changed,
+    .features_changed = NULL
+};
+
 enum { DPDK_RING_SIZE = 256 };
 BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
 enum { DRAIN_TSC = 200000ULL };
@@ -402,8 +422,8 @@  struct netdev_rxq_dpdk {
     int port_id;
 };
 
-static int netdev_dpdk_class_init(void);
-static int netdev_dpdk_vhost_class_init(void);
+static void netdev_dpdk_destruct(struct netdev *netdev);
+static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
 
 int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
 
@@ -413,8 +433,8 @@  netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
 static bool
 is_dpdk_class(const struct netdev_class *class)
 {
-    return class->init == netdev_dpdk_class_init
-           || class->init == netdev_dpdk_vhost_class_init;
+    return class->destruct == netdev_dpdk_destruct
+           || class->destruct == netdev_dpdk_vhost_destruct;
 }
 
 /* DPDK NIC drivers allocate RX buffers at a particular granularity, typically
@@ -942,17 +962,46 @@  netdev_dpdk_vhost_construct(struct netdev *netdev)
              dpdk_get_vhost_sock_dir(), name);
 
     dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
-    err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
+    err = rte_vhost_driver_register(dev->vhost_id,
+                                    dev->vhost_driver_flags);
     if (err) {
         VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
                  dev->vhost_id);
+        goto out;
     } else {
         fatal_signal_add_file_to_unlink(dev->vhost_id);
         VLOG_INFO("Socket %s created for vhost-user port %s\n",
                   dev->vhost_id, name);
     }
+
+    err = rte_vhost_driver_callback_register(dev->vhost_id,
+                                                &virtio_net_device_ops);
+    if (err) {
+        VLOG_ERR("vhost-user callback register failed.");
+        goto out;
+    }
+
+    err = rte_vhost_driver_disable_features(dev->vhost_id,
+                                1ULL << VIRTIO_NET_F_HOST_TSO4
+                                | 1ULL << VIRTIO_NET_F_HOST_TSO6
+                                | 1ULL << VIRTIO_NET_F_CSUM);
+    if (err) {
+        VLOG_ERR("vhost-user disable features failed.");
+        goto out;
+    }
+
     err = vhost_common_construct(netdev);
+    if (err) {
+        VLOG_ERR("vhost-user common construct failed.");
+        goto out;
+    }
+
+    err = rte_vhost_driver_start(dev->vhost_id);
+    if (err) {
+        VLOG_ERR("vhost-user driver start failed.");
+    }
 
+out:
     ovs_mutex_unlock(&dpdk_mutex);
     return err;
 }
@@ -2483,12 +2532,9 @@  static void
 set_irq_status(int vid)
 {
     uint32_t i;
-    uint64_t idx;
 
-    for (i = 0; i < rte_vhost_get_queue_num(vid); i++) {
-        idx = i * VIRTIO_QNUM;
-        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_RXQ, 0);
-        rte_vhost_enable_guest_notification(vid, idx + VIRTIO_TXQ, 0);
+    for (i = 0; i < rte_vhost_get_vring_num(vid); i++) {
+        rte_vhost_enable_guest_notification(vid, i, 0);
     }
 }
 
@@ -2551,7 +2597,7 @@  new_device(int vid)
     LIST_FOR_EACH(dev, list_node, &dpdk_list) {
         ovs_mutex_lock(&dev->mutex);
         if (strncmp(ifname, dev->vhost_id, IF_NAME_SZ) == 0) {
-            uint32_t qp_num = rte_vhost_get_queue_num(vid);
+            uint32_t qp_num = rte_vhost_get_vring_num(vid)/2;
 
             /* Get NUMA information */
             newnode = rte_vhost_get_numa_node(vid);
@@ -2718,27 +2764,6 @@  netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev)
     return ovsrcu_get(struct ingress_policer *, &dev->ingress_policer);
 }
 
-/*
- * These callbacks allow virtio-net devices to be added to vhost ports when
- * configuration has been fully complete.
- */
-static const struct virtio_net_device_ops virtio_net_device_ops =
-{
-    .new_device =  new_device,
-    .destroy_device = destroy_device,
-    .vring_state_changed = vring_state_changed
-};
-
-static void *
-start_vhost_loop(void *dummy OVS_UNUSED)
-{
-     pthread_detach(pthread_self());
-     /* Put the vhost thread into quiescent state. */
-     ovsrcu_quiesce_start();
-     rte_vhost_driver_session_start();
-     return NULL;
-}
-
 static int
 netdev_dpdk_class_init(void)
 {
@@ -2761,25 +2786,6 @@  netdev_dpdk_class_init(void)
     return 0;
 }
 
-static int
-netdev_dpdk_vhost_class_init(void)
-{
-    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
-
-    /* This function can be called for different classes.  The initialization
-     * needs to be done only once */
-    if (ovsthread_once_start(&once)) {
-        rte_vhost_driver_callback_register(&virtio_net_device_ops);
-        rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
-                                  | 1ULL << VIRTIO_NET_F_HOST_TSO6
-                                  | 1ULL << VIRTIO_NET_F_CSUM);
-        ovs_thread_create("vhost_thread", start_vhost_loop, NULL);
-
-        ovsthread_once_done(&once);
-    }
-
-    return 0;
-}
 
 /* Client Rings */
 
@@ -3351,7 +3357,7 @@  static const struct netdev_class dpdk_ring_class =
 static const struct netdev_class dpdk_vhost_class =
     NETDEV_DPDK_CLASS(
         "dpdkvhostuser",
-        netdev_dpdk_vhost_class_init,
+        NULL,
         netdev_dpdk_vhost_construct,
         netdev_dpdk_vhost_destruct,
         NULL,
@@ -3366,7 +3372,7 @@  static const struct netdev_class dpdk_vhost_class =
 static const struct netdev_class dpdk_vhost_client_class =
     NETDEV_DPDK_CLASS(
         "dpdkvhostuserclient",
-        netdev_dpdk_vhost_class_init,
+        NULL,
         netdev_dpdk_vhost_client_construct,
         netdev_dpdk_vhost_destruct,
         netdev_dpdk_vhost_client_set_config,
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 3200040..596a4b3 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -84,7 +84,7 @@  BuildRequires: libcap-ng libcap-ng-devel
 %endif
 %if %{with dpdk}
 BuildRequires: libpcap-devel numactl-devel
-BuildRequires: dpdk-devel >= 16.11
+BuildRequires: dpdk-devel >= 17.05
 Provides: %{name}-dpdk = %{version}-%{release}
 %endif
 
diff --git a/tests/dpdk/ring_client.c b/tests/dpdk/ring_client.c
index b153713..8cc3fb5 100644
--- a/tests/dpdk/ring_client.c
+++ b/tests/dpdk/ring_client.c
@@ -185,15 +185,15 @@  main(int argc, char *argv[])
         /* Try dequeuing max possible packets first, if that fails, get the
          * most we can. Loop body should only execute once, maximum.
          */
-        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts, rx_pkts) != 0) &&
-            rx_pkts > 0) {
+        while (unlikely(rte_ring_dequeue_bulk(rx_ring, pkts,
+                        rx_pkts, NULL) != 0) && rx_pkts > 0) {
             rx_pkts = (uint16_t)RTE_MIN(rte_ring_count(rx_ring), PKT_READ_SIZE);
         }
 
         if (rx_pkts > 0) {
             /* blocking enqueue */
             do {
-                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
+                rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts, NULL);
             } while (rslt == -ENOBUFS);
         }
     }