[ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07
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: -http://dpdk.org/doc/guides/rel_notes/release_16_0
Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07
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 *, &i->v), &ret, 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(&i->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 > stack below are the steps > mount -t hugetlbfs nodev /de
Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07
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 *, &i->v), &ret, 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(&i->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.g
Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07
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
> > 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 *, &i->v), &ret, 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(&i->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.githu
Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07
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
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 *, &i->v), &ret, 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(&i->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` >
Re: [ovs-dev] [PATCH RFC v3 1/1] netdev-dpdk: Add support for DPDK 16.07
> > 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 *, &i->v), &ret, 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(&i->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..ec1d