Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Loftus, Ciara]

> 
> Given that using vhost PMD doesn't seem viable in the very short term, I
> think we should stick with the vhost lib.
> I sent a patch for ovsrcu to add a new RCU protected array index.
> 
> http://openvswitch.org/pipermail/dev/2016-August/077097.html
> Thanks,
> Daniele

Thanks Daniele, I submitted a new version of this patch that uses the vhost lib 
& the new RCU index:

http://openvswitch.org/pipermail/dev/2016-August/077125.html

Thanks,
Ciara

> 
> 2016-07-28 6:26 GMT-07:00 Loftus, Ciara :
> >
> > Thanks for the patch.
> > I have another concern with this.  If we're still going to rely on RCU to
> protect
> > the vhost device (and as pointed out by Ilya, I think we should) we need to
> > use RCU-like semantics on the vid array index. I'm not sure a boolean flag 
> > is
> > going to be enough.
> > CCing Jarno:
> > We have this int, which is an index into an array of vhost devices (the 
> > array
> is
> > inside the DPDK library).  We want to make sure that when
> > ovsrcu_synchronize() returns nobody is using the old index anymore.
> > Should we introduce an RCU type for indexing into arrays?  I found some
> > negative opinions here:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> > next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-
> > 20160722#n13
> > but I think using atomics should prevent the compiler from playing tricks
> with
> > the index.
> >
> > How about something like the code below?
> > Thanks,
> > Daniele
> 
> I think the best way forward here is to avoid the RCU mechanisms by
> merging the vHost PMD first as you have previously suggested. What do you
> think?
> If we don't go with that, I think we need to make a decision ASAP on how to
> handle the RCU (ie. is below code needed?) as both DPDK and 2.6 releases
> are imminent.
> 
> Thanks,
> Ciara
> 
> >
> >
> > diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> > index dc75749..d1a57f6 100644
> > --- a/lib/ovs-rcu.h
> > +++ b/lib/ovs-rcu.h
> > @@ -130,6 +130,41 @@
> >  #include "compiler.h"
> >  #include "ovs-atomic.h"
> >
> > +typedef struct { atomic_int v; } ovsrcu_int;
> > +
> > +static inline int ovsrcu_int_get__(const ovsrcu_int *i, memory_order
> order)
> > +{
> > +    int ret;
> > +    atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> > +    return ret;
> > +}
> > +
> > +static inline int ovsrcu_int_get(const ovsrcu_int *i)
> > +{
> > +    return ovsrcu_int_get__(i, memory_order_consume);
> > +}
> > +
> > +static inline int ovsrcu_int_get_protected(const ovsrcu_int *i)
> > +{
> > +    return ovsrcu_int_get__(i, memory_order_relaxed);
> > +}
> > +
> > +static inline void ovsrcu_int_set__(ovsrcu_int *i, int value,
> > +    memory_order order)
> > +{
> > +    atomic_store_explicit(>v, value, order);
> > +}
> > +
> > +static inline void ovsrcu_int_set(ovsrcu_int *i, int value)
> > +{
> > +    ovsrcu_int_set__(i, value, memory_order_release);
> > +}
> > +
> > +static inline void ovsrcu_int_set_protected(ovsrcu_int *i, int value)
> > +{
> > +    ovsrcu_int_set__(i, value, memory_order_relaxed);
> > +}
> > +
> >  #if __GNUC__
> >  #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
> >  #define OVSRCU_INITIALIZER(VALUE) { ATOMIC_VAR_INIT(VALUE) }
> >
> >
> > 2016-07-22 8:55 GMT-07:00 Ciara Loftus :
> > This commit introduces support for DPDK 16.07 and consequently breaks
> > compatibility with DPDK 16.04.
> >
> > DPDK 16.07 introduces some changes to various APIs. These have been
> > updated in OVS, including:
> > * xstats API: changes to structure of xstats
> > * vhost API:  replace virtio-net references with 'vid'
> >
> > Signed-off-by: Ciara Loftus 
> > Tested-by: Maxime Coquelin 
> >
> > v3:
> > - fixed style issues
> > - fixed & simplified xstats frees
> > - use xcalloc & free instead of rte_mzalloc & rte_free for stats
> > - remove libnuma include
> > - fixed & simplified vHost NUMA set
> > - added flag to indicate device reconfigured at least once
> > - re-add call to rcu synchronise in destroy_device
> > - define IF_NAME_SZ and use instead of PATH_MAX
> >
> > v2:
> > - rebase with DPDK rc2
> > - rebase with OVS master
> > - fix vhost cuse compilation
> > ---
> >  .travis/linux-build.sh   |   2 +-
> >  INSTALL.DPDK-ADVANCED.md |   8 +-
> >  INSTALL.DPDK.md          |  20 ++---
> >  NEWS                     |   1 +
> >  lib/netdev-dpdk.c        | 220 +++
> ---
> > -
> >  5 files changed, 126 insertions(+), 125 deletions(-)
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> > index 065de39..1b3d43d 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -68,7 +68,7 @@ fi
> >
> >  if [ "$DPDK" ]; then
> >      if [ -z "$DPDK_VER" ]; then
> > -        DPDK_VER="16.04"
> > +        DPDK_VER="16.07"
> >      fi
> >      install_dpdk $DPDK_VER
> >      if [ "$CC" = "clang" ]; then
> 

Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Daniele Di Proietto]

Given that using vhost PMD doesn't seem viable in the very short term, I
think we should stick with the vhost lib.

I sent a patch for ovsrcu to add a new RCU protected array index.

http://openvswitch.org/pipermail/dev/2016-August/077097.html

Thanks,

Daniele

2016-07-28 6:26 GMT-07:00 Loftus, Ciara :

> >
> > Thanks for the patch.
> > I have another concern with this.  If we're still going to rely on RCU
> to protect
> > the vhost device (and as pointed out by Ilya, I think we should) we need
> to
> > use RCU-like semantics on the vid array index. I'm not sure a boolean
> flag is
> > going to be enough.
> > CCing Jarno:
> > We have this int, which is an index into an array of vhost devices (the
> array is
> > inside the DPDK library).  We want to make sure that when
> > ovsrcu_synchronize() returns nobody is using the old index anymore.
> > Should we introduce an RCU type for indexing into arrays?  I found some
> > negative opinions here:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> > next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-
> > 20160722#n13
> > but I think using atomics should prevent the compiler from playing
> tricks with
> > the index.
> >
> > How about something like the code below?
> > Thanks,
> > Daniele
>
> I think the best way forward here is to avoid the RCU mechanisms by
> merging the vHost PMD first as you have previously suggested. What do you
> think?
> If we don't go with that, I think we need to make a decision ASAP on how
> to handle the RCU (ie. is below code needed?) as both DPDK and 2.6 releases
> are imminent.
>
> Thanks,
> Ciara
>
> >
> >
> > diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> > index dc75749..d1a57f6 100644
> > --- a/lib/ovs-rcu.h
> > +++ b/lib/ovs-rcu.h
> > @@ -130,6 +130,41 @@
> >  #include "compiler.h"
> >  #include "ovs-atomic.h"
> >
> > +typedef struct { atomic_int v; } ovsrcu_int;
> > +
> > +static inline int ovsrcu_int_get__(const ovsrcu_int *i, memory_order
> order)
> > +{
> > +int ret;
> > +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> > +return ret;
> > +}
> > +
> > +static inline int ovsrcu_int_get(const ovsrcu_int *i)
> > +{
> > +return ovsrcu_int_get__(i, memory_order_consume);
> > +}
> > +
> > +static inline int ovsrcu_int_get_protected(const ovsrcu_int *i)
> > +{
> > +return ovsrcu_int_get__(i, memory_order_relaxed);
> > +}
> > +
> > +static inline void ovsrcu_int_set__(ovsrcu_int *i, int value,
> > +memory_order order)
> > +{
> > +atomic_store_explicit(>v, value, order);
> > +}
> > +
> > +static inline void ovsrcu_int_set(ovsrcu_int *i, int value)
> > +{
> > +ovsrcu_int_set__(i, value, memory_order_release);
> > +}
> > +
> > +static inline void ovsrcu_int_set_protected(ovsrcu_int *i, int value)
> > +{
> > +ovsrcu_int_set__(i, value, memory_order_relaxed);
> > +}
> > +
> >  #if __GNUC__
> >  #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
> >  #define OVSRCU_INITIALIZER(VALUE) { ATOMIC_VAR_INIT(VALUE) }
> >
> >
> > 2016-07-22 8:55 GMT-07:00 Ciara Loftus :
> > This commit introduces support for DPDK 16.07 and consequently breaks
> > compatibility with DPDK 16.04.
> >
> > DPDK 16.07 introduces some changes to various APIs. These have been
> > updated in OVS, including:
> > * xstats API: changes to structure of xstats
> > * vhost API:  replace virtio-net references with 'vid'
> >
> > Signed-off-by: Ciara Loftus 
> > Tested-by: Maxime Coquelin 
> >
> > v3:
> > - fixed style issues
> > - fixed & simplified xstats frees
> > - use xcalloc & free instead of rte_mzalloc & rte_free for stats
> > - remove libnuma include
> > - fixed & simplified vHost NUMA set
> > - added flag to indicate device reconfigured at least once
> > - re-add call to rcu synchronise in destroy_device
> > - define IF_NAME_SZ and use instead of PATH_MAX
> >
> > v2:
> > - rebase with DPDK rc2
> > - rebase with OVS master
> > - fix vhost cuse compilation
> > ---
> >  .travis/linux-build.sh   |   2 +-
> >  INSTALL.DPDK-ADVANCED.md |   8 +-
> >  INSTALL.DPDK.md  |  20 ++---
> >  NEWS |   1 +
> >  lib/netdev-dpdk.c| 220
> +++---
> > -
> >  5 files changed, 126 insertions(+), 125 deletions(-)
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> > index 065de39..1b3d43d 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -68,7 +68,7 @@ fi
> >
> >  if [ "$DPDK" ]; then
> >  if [ -z "$DPDK_VER" ]; then
> > -DPDK_VER="16.04"
> > +DPDK_VER="16.07"
> >  fi
> >  install_dpdk $DPDK_VER
> >  if [ "$CC" = "clang" ]; then
> > diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
> > index 9ae536d..ec1de29 100644
> > --- a/INSTALL.DPDK-ADVANCED.md
> > +++ b/INSTALL.DPDK-ADVANCED.md
> > @@ -43,7 +43,7 @@ for DPDK and 

Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Daniele Di Proietto]

2016-07-24 11:06 GMT-07:00 Ben Pfaff :

> On Sun, Jul 24, 2016 at 10:17:13AM -0400, Aaron Conole wrote:
> > Daniele Di Proietto  writes:
> >
> > > Thanks for the patch.
> > >
> > > I have another concern with this.  If we're still going to rely on RCU
> to
> > > protect the vhost device (and as pointed out by Ilya, I think we
> should) we
> > > need to use RCU-like semantics on the vid array index. I'm not sure a
> > > boolean flag is going to be enough.
> > >
> > > CCing Jarno:
> > >
> > > We have this int, which is an index into an array of vhost devices (the
> > > array is inside the DPDK library).  We want to make sure that when
> > > ovsrcu_synchronize() returns nobody is using the old index anymore.
> > >
> > > Should we introduce an RCU type for indexing into arrays?  I found some
> > > negative opinions here:
> > >
> > >
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-20160722#n13
> > >
> > > but I think using atomics should prevent the compiler from playing
> tricks
> > > with the index.
> >
> > Is there a reason to prefer atomics over something like a reference
> > counted array pointer (as described in the linked text)?  Are you
> > afraid of the malloc/memcpy overhead in the worst case?  I think
> > many times side effects of atomics can be difficult to debug, because
> > of the subtleties of various chip synchronization protocols.  Maybe it's
> > okay if you are only going to support Intel chips, though.  Just my
> > $0.02
>
> We definitely don't support just Intel chips.  (We're not etcd!)
>
> Thanks,
>
> Ben.
>

I think by using atomics and making sure that we follow the C11 memory
model, it should work on every architecture supported by the compiler.

I agree that using atomics is not always straightforward, but we had a
pretty good experience so far with RCU.  The approach I'm proposing uses
atomics in the same way they're used for pointers.

It'd be the same as using an RCU pointer to an malloc'd int (the vid
index), but I'd prefer avoiding the extra indirection in the fast path and
the extra allocation in the slow path.

Thanks,

Daniele
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Loftus, Ciara]

> 
> Thanks for the patch.
> I have another concern with this.  If we're still going to rely on RCU to 
> protect
> the vhost device (and as pointed out by Ilya, I think we should) we need to
> use RCU-like semantics on the vid array index. I'm not sure a boolean flag is
> going to be enough.
> CCing Jarno:
> We have this int, which is an index into an array of vhost devices (the array 
> is
> inside the DPDK library).  We want to make sure that when
> ovsrcu_synchronize() returns nobody is using the old index anymore.
> Should we introduce an RCU type for indexing into arrays?  I found some
> negative opinions here:
> 
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-
> 20160722#n13
> but I think using atomics should prevent the compiler from playing tricks with
> the index.
> 
> How about something like the code below?
> Thanks,
> Daniele

I think the best way forward here is to avoid the RCU mechanisms by merging the 
vHost PMD first as you have previously suggested. What do you think? 
If we don't go with that, I think we need to make a decision ASAP on how to 
handle the RCU (ie. is below code needed?) as both DPDK and 2.6 releases are 
imminent.

Thanks,
Ciara

> 
> 
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index dc75749..d1a57f6 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -130,6 +130,41 @@
>  #include "compiler.h"
>  #include "ovs-atomic.h"
> 
> +typedef struct { atomic_int v; } ovsrcu_int;
> +
> +static inline int ovsrcu_int_get__(const ovsrcu_int *i, memory_order order)
> +{
> +    int ret;
> +    atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> +    return ret;
> +}
> +
> +static inline int ovsrcu_int_get(const ovsrcu_int *i)
> +{
> +    return ovsrcu_int_get__(i, memory_order_consume);
> +}
> +
> +static inline int ovsrcu_int_get_protected(const ovsrcu_int *i)
> +{
> +    return ovsrcu_int_get__(i, memory_order_relaxed);
> +}
> +
> +static inline void ovsrcu_int_set__(ovsrcu_int *i, int value,
> +    memory_order order)
> +{
> +    atomic_store_explicit(>v, value, order);
> +}
> +
> +static inline void ovsrcu_int_set(ovsrcu_int *i, int value)
> +{
> +    ovsrcu_int_set__(i, value, memory_order_release);
> +}
> +
> +static inline void ovsrcu_int_set_protected(ovsrcu_int *i, int value)
> +{
> +    ovsrcu_int_set__(i, value, memory_order_relaxed);
> +}
> +
>  #if __GNUC__
>  #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
>  #define OVSRCU_INITIALIZER(VALUE) { ATOMIC_VAR_INIT(VALUE) }
> 
> 
> 2016-07-22 8:55 GMT-07:00 Ciara Loftus :
> This commit introduces support for DPDK 16.07 and consequently breaks
> compatibility with DPDK 16.04.
> 
> DPDK 16.07 introduces some changes to various APIs. These have been
> updated in OVS, including:
> * xstats API: changes to structure of xstats
> * vhost API:  replace virtio-net references with 'vid'
> 
> Signed-off-by: Ciara Loftus 
> Tested-by: Maxime Coquelin 
> 
> v3:
> - fixed style issues
> - fixed & simplified xstats frees
> - use xcalloc & free instead of rte_mzalloc & rte_free for stats
> - remove libnuma include
> - fixed & simplified vHost NUMA set
> - added flag to indicate device reconfigured at least once
> - re-add call to rcu synchronise in destroy_device
> - define IF_NAME_SZ and use instead of PATH_MAX
> 
> v2:
> - rebase with DPDK rc2
> - rebase with OVS master
> - fix vhost cuse compilation
> ---
>  .travis/linux-build.sh   |   2 +-
>  INSTALL.DPDK-ADVANCED.md |   8 +-
>  INSTALL.DPDK.md          |  20 ++---
>  NEWS                     |   1 +
>  lib/netdev-dpdk.c        | 220 +++---
> -
>  5 files changed, 126 insertions(+), 125 deletions(-)
> 
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 065de39..1b3d43d 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -68,7 +68,7 @@ fi
> 
>  if [ "$DPDK" ]; then
>      if [ -z "$DPDK_VER" ]; then
> -        DPDK_VER="16.04"
> +        DPDK_VER="16.07"
>      fi
>      install_dpdk $DPDK_VER
>      if [ "$CC" = "clang" ]; then
> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
> index 9ae536d..ec1de29 100644
> --- a/INSTALL.DPDK-ADVANCED.md
> +++ b/INSTALL.DPDK-ADVANCED.md
> @@ -43,7 +43,7 @@ for DPDK and OVS.
>      For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-
> linuxapp-gcc`
> 
>      ```
> -    export DPDK_DIR=/usr/src/dpdk-16.04
> +    export DPDK_DIR=/usr/src/dpdk-16.07
>      export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>      make install T=$DPDK_TARGET DESTDIR=install
>      ```
> @@ -339,7 +339,7 @@ For users wanting to do packet forwarding using
> kernel stack below are the steps
>         cd /usr/src/cmdline_generator
>         wget https://raw.githubusercontent.com/netgroup-polito/un-
> orchestrator/master/orchestrator/compute_controller/plugins/kvm-
> 

Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Ben Pfaff]

On Sun, Jul 24, 2016 at 10:17:13AM -0400, Aaron Conole wrote:
> Daniele Di Proietto  writes:
> 
> > Thanks for the patch.
> >
> > I have another concern with this.  If we're still going to rely on RCU to
> > protect the vhost device (and as pointed out by Ilya, I think we should) we
> > need to use RCU-like semantics on the vid array index. I'm not sure a
> > boolean flag is going to be enough.
> >
> > CCing Jarno:
> >
> > We have this int, which is an index into an array of vhost devices (the
> > array is inside the DPDK library).  We want to make sure that when
> > ovsrcu_synchronize() returns nobody is using the old index anymore.
> >
> > Should we introduce an RCU type for indexing into arrays?  I found some
> > negative opinions here:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-20160722#n13
> >
> > but I think using atomics should prevent the compiler from playing tricks
> > with the index.
> 
> Is there a reason to prefer atomics over something like a reference
> counted array pointer (as described in the linked text)?  Are you
> afraid of the malloc/memcpy overhead in the worst case?  I think
> many times side effects of atomics can be difficult to debug, because
> of the subtleties of various chip synchronization protocols.  Maybe it's
> okay if you are only going to support Intel chips, though.  Just my
> $0.02

We definitely don't support just Intel chips.  (We're not etcd!)

Thanks,

Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Aaron Conole]

Hi Daniele,

Daniele Di Proietto  writes:

> Thanks for the patch.
>
> I have another concern with this.  If we're still going to rely on RCU to
> protect the vhost device (and as pointed out by Ilya, I think we should) we
> need to use RCU-like semantics on the vid array index. I'm not sure a
> boolean flag is going to be enough.
>
> CCing Jarno:
>
> We have this int, which is an index into an array of vhost devices (the
> array is inside the DPDK library).  We want to make sure that when
> ovsrcu_synchronize() returns nobody is using the old index anymore.
>
> Should we introduce an RCU type for indexing into arrays?  I found some
> negative opinions here:
>
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-20160722#n13
>
> but I think using atomics should prevent the compiler from playing tricks
> with the index.

Is there a reason to prefer atomics over something like a reference
counted array pointer (as described in the linked text)?  Are you
afraid of the malloc/memcpy overhead in the worst case?  I think
many times side effects of atomics can be difficult to debug, because
of the subtleties of various chip synchronization protocols.  Maybe it's
okay if you are only going to support Intel chips, though.  Just my
$0.02

-Aaron

> How about something like the code below?
>
> Thanks,
>
> Daniele
>
>
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index dc75749..d1a57f6 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -130,6 +130,41 @@
>  #include "compiler.h"
>  #include "ovs-atomic.h"
>
> +typedef struct { atomic_int v; } ovsrcu_int;
> +
> +static inline int ovsrcu_int_get__(const ovsrcu_int *i, memory_order order)
> +{
> +int ret;
> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> +return ret;
> +}
> +
> +static inline int ovsrcu_int_get(const ovsrcu_int *i)
> +{
> +return ovsrcu_int_get__(i, memory_order_consume);
> +}
> +
> +static inline int ovsrcu_int_get_protected(const ovsrcu_int *i)
> +{
> +return ovsrcu_int_get__(i, memory_order_relaxed);
> +}
> +
> +static inline void ovsrcu_int_set__(ovsrcu_int *i, int value,
> +memory_order order)
> +{
> +atomic_store_explicit(>v, value, order);
> +}
> +
> +static inline void ovsrcu_int_set(ovsrcu_int *i, int value)
> +{
> +ovsrcu_int_set__(i, value, memory_order_release);
> +}
> +
> +static inline void ovsrcu_int_set_protected(ovsrcu_int *i, int value)
> +{
> +ovsrcu_int_set__(i, value, memory_order_relaxed);
> +}
> +
>  #if __GNUC__
>  #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
>  #define OVSRCU_INITIALIZER(VALUE) { ATOMIC_VAR_INIT(VALUE) }
>
>
>
> 2016-07-22 8:55 GMT-07:00 Ciara Loftus :
>
>> This commit introduces support for DPDK 16.07 and consequently breaks
>> compatibility with DPDK 16.04.
>>
>> DPDK 16.07 introduces some changes to various APIs. These have been
>> updated in OVS, including:
>> * xstats API: changes to structure of xstats
>> * vhost API:  replace virtio-net references with 'vid'
>>
>> Signed-off-by: Ciara Loftus 
>> Tested-by: Maxime Coquelin 
>>
>> v3:
>> - fixed style issues
>> - fixed & simplified xstats frees
>> - use xcalloc & free instead of rte_mzalloc & rte_free for stats
>> - remove libnuma include
>> - fixed & simplified vHost NUMA set
>> - added flag to indicate device reconfigured at least once
>> - re-add call to rcu synchronise in destroy_device
>> - define IF_NAME_SZ and use instead of PATH_MAX
>>
>> v2:
>> - rebase with DPDK rc2
>> - rebase with OVS master
>> - fix vhost cuse compilation
>> ---
>>  .travis/linux-build.sh   |   2 +-
>>  INSTALL.DPDK-ADVANCED.md |   8 +-
>>  INSTALL.DPDK.md  |  20 ++---
>>  NEWS |   1 +
>>  lib/netdev-dpdk.c| 220
>> +++
>>  5 files changed, 126 insertions(+), 125 deletions(-)
>>
>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
>> index 065de39..1b3d43d 100755
>> --- a/.travis/linux-build.sh
>> +++ b/.travis/linux-build.sh
>> @@ -68,7 +68,7 @@ fi
>>
>>  if [ "$DPDK" ]; then
>>  if [ -z "$DPDK_VER" ]; then
>> -DPDK_VER="16.04"
>> +DPDK_VER="16.07"
>>  fi
>>  install_dpdk $DPDK_VER
>>  if [ "$CC" = "clang" ]; then
>> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
>> index 9ae536d..ec1de29 100644
>> --- a/INSTALL.DPDK-ADVANCED.md
>> +++ b/INSTALL.DPDK-ADVANCED.md
>> @@ -43,7 +43,7 @@ for DPDK and OVS.
>>  For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-linuxapp-gcc`
>>
>>  ```
>> -export DPDK_DIR=/usr/src/dpdk-16.04
>> +export DPDK_DIR=/usr/src/dpdk-16.07
>>  export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>>  make install T=$DPDK_TARGET DESTDIR=install
>>  ```
>> @@ -339,7 +339,7 @@ For users wanting to do packet forwarding using kernel
>> stack below 

Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Daniele Di Proietto]

Thanks for the patch.

I have another concern with this.  If we're still going to rely on RCU to
protect the vhost device (and as pointed out by Ilya, I think we should) we
need to use RCU-like semantics on the vid array index. I'm not sure a
boolean flag is going to be enough.

CCing Jarno:

We have this int, which is an index into an array of vhost devices (the
array is inside the DPDK library).  We want to make sure that when
ovsrcu_synchronize() returns nobody is using the old index anymore.

Should we introduce an RCU type for indexing into arrays?  I found some
negative opinions here:

https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/RCU/arrayRCU.txt?id=refs/tags/next-20160722#n13

but I think using atomics should prevent the compiler from playing tricks
with the index.

How about something like the code below?

Thanks,

Daniele


diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index dc75749..d1a57f6 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -130,6 +130,41 @@
 #include "compiler.h"
 #include "ovs-atomic.h"

+typedef struct { atomic_int v; } ovsrcu_int;
+
+static inline int ovsrcu_int_get__(const ovsrcu_int *i, memory_order order)
+{
+int ret;
+atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
+return ret;
+}
+
+static inline int ovsrcu_int_get(const ovsrcu_int *i)
+{
+return ovsrcu_int_get__(i, memory_order_consume);
+}
+
+static inline int ovsrcu_int_get_protected(const ovsrcu_int *i)
+{
+return ovsrcu_int_get__(i, memory_order_relaxed);
+}
+
+static inline void ovsrcu_int_set__(ovsrcu_int *i, int value,
+memory_order order)
+{
+atomic_store_explicit(>v, value, order);
+}
+
+static inline void ovsrcu_int_set(ovsrcu_int *i, int value)
+{
+ovsrcu_int_set__(i, value, memory_order_release);
+}
+
+static inline void ovsrcu_int_set_protected(ovsrcu_int *i, int value)
+{
+ovsrcu_int_set__(i, value, memory_order_relaxed);
+}
+
 #if __GNUC__
 #define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
 #define OVSRCU_INITIALIZER(VALUE) { ATOMIC_VAR_INIT(VALUE) }



2016-07-22 8:55 GMT-07:00 Ciara Loftus :

> This commit introduces support for DPDK 16.07 and consequently breaks
> compatibility with DPDK 16.04.
>
> DPDK 16.07 introduces some changes to various APIs. These have been
> updated in OVS, including:
> * xstats API: changes to structure of xstats
> * vhost API:  replace virtio-net references with 'vid'
>
> Signed-off-by: Ciara Loftus 
> Tested-by: Maxime Coquelin 
>
> v3:
> - fixed style issues
> - fixed & simplified xstats frees
> - use xcalloc & free instead of rte_mzalloc & rte_free for stats
> - remove libnuma include
> - fixed & simplified vHost NUMA set
> - added flag to indicate device reconfigured at least once
> - re-add call to rcu synchronise in destroy_device
> - define IF_NAME_SZ and use instead of PATH_MAX
>
> v2:
> - rebase with DPDK rc2
> - rebase with OVS master
> - fix vhost cuse compilation
> ---
>  .travis/linux-build.sh   |   2 +-
>  INSTALL.DPDK-ADVANCED.md |   8 +-
>  INSTALL.DPDK.md  |  20 ++---
>  NEWS |   1 +
>  lib/netdev-dpdk.c| 220
> +++
>  5 files changed, 126 insertions(+), 125 deletions(-)
>
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 065de39..1b3d43d 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -68,7 +68,7 @@ fi
>
>  if [ "$DPDK" ]; then
>  if [ -z "$DPDK_VER" ]; then
> -DPDK_VER="16.04"
> +DPDK_VER="16.07"
>  fi
>  install_dpdk $DPDK_VER
>  if [ "$CC" = "clang" ]; then
> diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
> index 9ae536d..ec1de29 100644
> --- a/INSTALL.DPDK-ADVANCED.md
> +++ b/INSTALL.DPDK-ADVANCED.md
> @@ -43,7 +43,7 @@ for DPDK and OVS.
>  For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-linuxapp-gcc`
>
>  ```
> -export DPDK_DIR=/usr/src/dpdk-16.04
> +export DPDK_DIR=/usr/src/dpdk-16.07
>  export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>  make install T=$DPDK_TARGET DESTDIR=install
>  ```
> @@ -339,7 +339,7 @@ For users wanting to do packet forwarding using kernel
> stack below are the steps
> cd /usr/src/cmdline_generator
> wget
> https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/cmdline_generator.c
> wget
> https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/Makefile
> -   export RTE_SDK=/usr/src/dpdk-16.04
> +   export RTE_SDK=/usr/src/dpdk-16.07
> export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
> make
> ./build/cmdline_generator -m -p dpdkr0 XXX
> @@ -363,7 +363,7 @@ For users wanting to do packet forwarding using kernel

[ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07

[Ciara Loftus]

This commit introduces support for DPDK 16.07 and consequently breaks
compatibility with DPDK 16.04.

DPDK 16.07 introduces some changes to various APIs. These have been
updated in OVS, including:
* xstats API: changes to structure of xstats
* vhost API:  replace virtio-net references with 'vid'

Signed-off-by: Ciara Loftus 
Tested-by: Maxime Coquelin 

v3:
- fixed style issues
- fixed & simplified xstats frees
- use xcalloc & free instead of rte_mzalloc & rte_free for stats
- remove libnuma include
- fixed & simplified vHost NUMA set
- added flag to indicate device reconfigured at least once
- re-add call to rcu synchronise in destroy_device
- define IF_NAME_SZ and use instead of PATH_MAX

v2:
- rebase with DPDK rc2
- rebase with OVS master
- fix vhost cuse compilation
---
 .travis/linux-build.sh   |   2 +-
 INSTALL.DPDK-ADVANCED.md |   8 +-
 INSTALL.DPDK.md  |  20 ++---
 NEWS |   1 +
 lib/netdev-dpdk.c| 220 +++
 5 files changed, 126 insertions(+), 125 deletions(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 065de39..1b3d43d 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -68,7 +68,7 @@ fi
 
 if [ "$DPDK" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="16.04"
+DPDK_VER="16.07"
 fi
 install_dpdk $DPDK_VER
 if [ "$CC" = "clang" ]; then
diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
index 9ae536d..ec1de29 100644
--- a/INSTALL.DPDK-ADVANCED.md
+++ b/INSTALL.DPDK-ADVANCED.md
@@ -43,7 +43,7 @@ for DPDK and OVS.
 For IVSHMEM case, set `export DPDK_TARGET=x86_64-ivshmem-linuxapp-gcc`
 
 ```
-export DPDK_DIR=/usr/src/dpdk-16.04
+export DPDK_DIR=/usr/src/dpdk-16.07
 export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
 make install T=$DPDK_TARGET DESTDIR=install
 ```
@@ -339,7 +339,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
cd /usr/src/cmdline_generator
wget 
https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/cmdline_generator.c
wget 
https://raw.githubusercontent.com/netgroup-polito/un-orchestrator/master/orchestrator/compute_controller/plugins/kvm-libvirt/cmdline_generator/Makefile
-   export RTE_SDK=/usr/src/dpdk-16.04
+   export RTE_SDK=/usr/src/dpdk-16.07
export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
make
./build/cmdline_generator -m -p dpdkr0 XXX
@@ -363,7 +363,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
mount -t hugetlbfs nodev /dev/hugepages (if not already mounted)
 
# Build the DPDK ring application in the VM
-   export RTE_SDK=/root/dpdk-16.04
+   export RTE_SDK=/root/dpdk-16.07
export RTE_TARGET=x86_64-ivshmem-linuxapp-gcc
make
 
@@ -374,7 +374,7 @@ For users wanting to do packet forwarding using kernel 
stack below are the steps
 
 ##  6. Vhost Walkthrough
 
-DPDK 16.04 supports two types of vhost:
+DPDK 16.07 supports two types of vhost:
 
 1. vhost-user - enabled default
 
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 5407794..9022ad8 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -21,7 +21,7 @@ The DPDK support of Open vSwitch is considered 'experimental'.
 
 ### Prerequisites
 
-* Required: DPDK 16.04, libnuma
+* Required: DPDK 16.07, libnuma
 * Hardware: [DPDK Supported NICs] when physical ports in use
 
 ##  2. Building and Installation
@@ -42,10 +42,10 @@ advanced install guide [INSTALL.DPDK-ADVANCED.md]
 
  ```
  cd /usr/src/
- wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.04.zip
- unzip dpdk-16.04.zip
+ wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
+ unzip dpdk-16.07.zip
 
- export DPDK_DIR=/usr/src/dpdk-16.04
+ export DPDK_DIR=/usr/src/dpdk-16.07
  cd $DPDK_DIR
  ```
 
@@ -329,9 +329,9 @@ can be found in [Vhost Walkthrough].
 
   ```
   cd /root/dpdk/
-  wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.04.zip
-  unzip dpdk-16.04.zip
-  export DPDK_DIR=/root/dpdk/dpdk-16.04
+  wget http://dpdk.org/browse/dpdk/snapshot/dpdk-16.07.zip
+  unzip dpdk-16.07.zip
+  export DPDK_DIR=/root/dpdk/dpdk-16.07
   export DPDK_TARGET=x86_64-native-linuxapp-gcc
   export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
   cd $DPDK_DIR
@@ -487,7 +487,7 @@ can be found in [Vhost Walkthrough].


  
- 
+ 
  
  

@@ -557,9 +557,9 @@ can be found in [Vhost Walkthrough].
 DPDK. It is recommended that users update Network Interface firmware to
 match what has been validated for the DPDK release.
 
-For DPDK 16.04, the list of validated firmware versions can be found at:
+For DPDK 16.07, the list of validated firmware versions can be found at:
 
-