Re: [ovs-dev] [PATCH RFC v6 1/1] netdev-dpdk: add dpdk vhost ports
> -Original Message- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: Wednesday, January 21, 2015 11:19 AM > To: Traynor, Kevin > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH RFC v6 1/1] netdev-dpdk: add dpdk vhost ports > > On Thu, Jan 08, 2015 at 11:05:02PM +, Kevin Traynor wrote: > > This patch adds support for a new port type to userspace datapath > > called dpdkvhost. This allows KVM (QEMU) to offload the servicing > > of virtio-net devices to its associated dpdkvhost port. Instructions > > for use are in INSTALL.DPDK. > > > > This has been tested on Intel multi-core platforms and with clients > > that have virtio-net interfaces. > > > > ver 6: > >- rebased with master > >- modified to use DPDK v1.8.0 vhost library > >- reworked for review comments > > ver 5: > >- rebased against latest master > > ver 4: > >- added eventfd_link.h and eventfd_link.c to EXTRA_DIST in > > utilities/automake.mk > >- rebased with master to work with DPDK 1.7 ver 3: > >- rebased with master > > ver 2: > >- rebased with master > > > > Signed-off-by: Ciara Loftus > > Signed-off-by: Kevin Traynor > > Signed-off-by: Maryam Tahhan > > --- > > INSTALL.DPDK.md | 236 + > > Makefile.am |4 + > > lib/automake.mk |1 + > > lib/netdev-dpdk.c | 649 > > +++ > > lib/netdev.c|3 +- > > utilities/automake.mk |3 +- > > utilities/qemu-wrap.py | 389 > > vswitchd/ovs-vswitchd.c |4 +- > > 8 files changed, 1177 insertions(+), 112 deletions(-) > > mode change 100644 => 100755 lib/netdev-dpdk.c > > create mode 100755 utilities/qemu-wrap.py > > > > diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md > > index 2cc7636..da8116d 100644 > > --- a/INSTALL.DPDK.md > > +++ b/INSTALL.DPDK.md > > @@ -17,6 +17,7 @@ Building and Installing: > > > > > > Required DPDK 1.7 > > +Optional `fuse`, `fuse-devel` > > > > 1. Configure build & install DPDK: > >1. Set `$DPDK_DIR` > > @@ -264,6 +265,241 @@ A general rule of thumb for better performance is > > that the client > > application should not be assigned the same dpdk core mask "-c" as > > the vswitchd. > > > > +DPDK vHost: > > +--- > > + > > +Prerequisites: > > +1. DPDK 1.8 with vHost support enabled and recompile OVS as above. > > + > > + Update `config/common_linuxapp` so that DPDK is built with vHost > > + libraries: > > + > > + `CONFIG_RTE_LIBRTE_VHOST=y` > > + > > +2. Insert the Fuse module: > > + > > + `modprobe fuse` > > + > > +3. Build and insert the `eventfd_link` module: > > + > > + `cd $DPDK_DIR/lib/librte_vhost/eventfd_link/` > > + `make` > > + `insmod $DPDK_DIR/lib/librte_vhost/eventfd_link.ko` > > + > > +4. Remove /dev/vhost-net character device: > > + > > + `rm -rf /dev/vhost-net` > > I think it's not a good idea to tell people to do this, > best to drop this section and put "with standard vhost" > here instead. Not clear what you'd like to see dropped? This will be necessary if using the default vhost file, so can change to make that clearer. > > > + > > +Following the steps above to create a bridge, you can now add DPDK vHost > > +as a port to the vswitch. > > + > > +`ovs-vsctl add-port br0 dpdkvhost0 -- set Interface dpdkvhost0 > > type=dpdkvhost` > > + > > +Unlike DPDK ring ports, DPDK vHost ports can have arbitrary names: > > + > > +`ovs-vsctl add-port br0 port123ABC -- set Interface port123ABC > > type=dpdkvhost` > > + > > +However, please note that when attaching userspace devices to QEMU, the > > +name provided during the add-port operation must match the ifname parameter > > +on the QEMU command line. > > + > > +DPDK vHost VM configuration: > > + > > + > > +1. Configure virtio-net adaptors: > > + The guest must be configured with virtio-net adapters and offloads > > + MUST BE DISABLED. > > Any plans to address this? There's no plans at present > > > +This means the following parameters should be passed > > + to the QEMU binary: > > + > > + ``` > > + -netdev tap,id=,script=no,downscript=no,ifname=,vhost=on > > + -device virtio-net-pci,netdev=net1,mac=,csum=off,gso=off, > > + guest_tso4=off,guest_tso6=off,guest_ecn=off > > + ``` > > + > > + Repeat the above parameters for multiple devices. > > + > > +2. Configure huge pages: > > + QEMU must allocate the VM's memory on hugetlbfs. Vhost ports access a > > + virtio-net device's virtual rings and packet buffers mapping the VM's > > + physical memory on hugetlbfs. To enable vhost-ports to map the VM's > > + memory into their process address space, pass the following paramters > > + to QEMU: > > + > > + `-mem-path /dev/hugepages -mem-prealloc` > > I guess you also need to request MAP_SHARED mappings - otherwise > I think you won't be able to poke
[ovs-dev] [PATCH] meta-flow: Fix wrong parentheses.
Bitwise not operator has higher precedence than Bitwise right shift operator, so the MFF_IP_DSCP_SHIFTED case always returns true currently. Signed-off-by: Kmindg --- lib/meta-flow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 9ce4cfe..6576336 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -416,7 +416,7 @@ mf_is_value_valid(const struct mf_field *mf, const union mf_value *value) case MFF_IP_DSCP: return !(value->u8 & ~IP_DSCP_MASK); case MFF_IP_DSCP_SHIFTED: -return !(value->u8 & (~IP_DSCP_MASK >> 2)); +return !(value->u8 & ~(IP_DSCP_MASK >> 2)); case MFF_IP_ECN: return !(value->u8 & ~IP_ECN_MASK); case MFF_IP_FRAG: -- 2.3.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH RFC v6 1/1] netdev-dpdk: add dpdk vhost ports
On Thu, Feb 12, 2015 at 12:59:17PM +, Traynor, Kevin wrote: > > -Original Message- > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > Sent: Wednesday, January 21, 2015 11:19 AM > > To: Traynor, Kevin > > Cc: dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH RFC v6 1/1] netdev-dpdk: add dpdk vhost ports > > > > On Thu, Jan 08, 2015 at 11:05:02PM +, Kevin Traynor wrote: > > > This patch adds support for a new port type to userspace datapath > > > called dpdkvhost. This allows KVM (QEMU) to offload the servicing > > > of virtio-net devices to its associated dpdkvhost port. Instructions > > > for use are in INSTALL.DPDK. > > > > > > This has been tested on Intel multi-core platforms and with clients > > > that have virtio-net interfaces. > > > > > > ver 6: > > >- rebased with master > > >- modified to use DPDK v1.8.0 vhost library > > >- reworked for review comments > > > ver 5: > > >- rebased against latest master > > > ver 4: > > >- added eventfd_link.h and eventfd_link.c to EXTRA_DIST in > > > utilities/automake.mk > > >- rebased with master to work with DPDK 1.7 ver 3: > > >- rebased with master > > > ver 2: > > >- rebased with master > > > > > > Signed-off-by: Ciara Loftus > > > Signed-off-by: Kevin Traynor > > > Signed-off-by: Maryam Tahhan > > > --- > > > INSTALL.DPDK.md | 236 + > > > Makefile.am |4 + > > > lib/automake.mk |1 + > > > lib/netdev-dpdk.c | 649 > > > +++ > > > lib/netdev.c|3 +- > > > utilities/automake.mk |3 +- > > > utilities/qemu-wrap.py | 389 > > > vswitchd/ovs-vswitchd.c |4 +- > > > 8 files changed, 1177 insertions(+), 112 deletions(-) > > > mode change 100644 => 100755 lib/netdev-dpdk.c > > > create mode 100755 utilities/qemu-wrap.py > > > > > > diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md > > > index 2cc7636..da8116d 100644 > > > --- a/INSTALL.DPDK.md > > > +++ b/INSTALL.DPDK.md > > > @@ -17,6 +17,7 @@ Building and Installing: > > > > > > > > > Required DPDK 1.7 > > > +Optional `fuse`, `fuse-devel` > > > > > > 1. Configure build & install DPDK: > > >1. Set `$DPDK_DIR` > > > @@ -264,6 +265,241 @@ A general rule of thumb for better performance is > > > that the client > > > application should not be assigned the same dpdk core mask "-c" as > > > the vswitchd. > > > > > > +DPDK vHost: > > > +--- > > > + > > > +Prerequisites: > > > +1. DPDK 1.8 with vHost support enabled and recompile OVS as above. > > > + > > > + Update `config/common_linuxapp` so that DPDK is built with vHost > > > + libraries: > > > + > > > + `CONFIG_RTE_LIBRTE_VHOST=y` > > > + > > > +2. Insert the Fuse module: > > > + > > > + `modprobe fuse` > > > + > > > +3. Build and insert the `eventfd_link` module: > > > + > > > + `cd $DPDK_DIR/lib/librte_vhost/eventfd_link/` > > > + `make` > > > + `insmod $DPDK_DIR/lib/librte_vhost/eventfd_link.ko` > > > + > > > +4. Remove /dev/vhost-net character device: > > > + > > > + `rm -rf /dev/vhost-net` > > > > I think it's not a good idea to tell people to do this, > > best to drop this section and put "with standard vhost" > > here instead. > > Not clear what you'd like to see dropped? > This will be necessary > if using the default vhost file, so can change to make that clearer. Using a location that is likely to conflict with a kernel device is not a good idea. So what you are promoting here is not a good configuration: people will follow your advice, then complain that kernel device stopped working for all VMs. It will also likely conflict with distro's rules for the device, if any. There are no advantages that I can see. > > > > > + > > > +Following the steps above to create a bridge, you can now add DPDK vHost > > > +as a port to the vswitch. > > > + > > > +`ovs-vsctl add-port br0 dpdkvhost0 -- set Interface dpdkvhost0 > > > type=dpdkvhost` > > > + > > > +Unlike DPDK ring ports, DPDK vHost ports can have arbitrary names: > > > + > > > +`ovs-vsctl add-port br0 port123ABC -- set Interface port123ABC > > > type=dpdkvhost` > > > + > > > +However, please note that when attaching userspace devices to QEMU, the > > > +name provided during the add-port operation must match the ifname > > > parameter > > > +on the QEMU command line. > > > + > > > +DPDK vHost VM configuration: > > > + > > > + > > > +1. Configure virtio-net adaptors: > > > + The guest must be configured with virtio-net adapters and offloads > > > + MUST BE DISABLED. > > > > Any plans to address this? > > There's no plans at present Why not? That's pretty bad I think, devices really should report their capabilities to userspace, not rely on users to configure them just so. > > > > > +This means the following parameters should be passed > > > + to the QE
[ovs-dev] Markdown coding standard
I note that there are no coding standard for Markdown documents. Given this, I have some questions: * What's the expected format for code blocks (multi-line and single-line)? * Can we use GHFM-extensions (i.e. syntax-highlighted code fences) * Line limit? It's not necessary to wrap Markdown if you don't want to, but files seem to alternate between 72, 79 and 80+ quite a bit * Any "expected approach" to validating Markdown output (i.e. confirm that it will preview correctly on GitHub) * Any other requirements? Cheers, Stephen ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] tests: Enable running parallel unit tests for Windows.
Thanks!
Acked-by: Eitan Eliahu
-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gurucharan Shetty
Sent: Wednesday, February 11, 2015 6:00 PM
To: dev@openvswitch.org
Cc: Gurucharan Shetty
Subject: [ovs-dev] [PATCH v2] tests: Enable running parallel unit tests for
Windows.
testsuite uses mkfifo in its job dispatcher that manages parallel unit tests.
MinGW does not have a mkfifo. This results in unit tests running serially on
Windows. Right now it takes up to approximately 40 minutes to run all the unit
tests on Windows.
This commit provides a job dispatcher for MinGW that uses temporary files
instead of mkfifo to manage parallel jobs.
With this commit, on a Windows machine with 4 cores and with
8 parallel unit test sessions, it takes approximately 8 minutes to finish a
unit test run.
Signed-off-by: Gurucharan Shetty
---
INSTALL.md|2 ++
tests/automake.mk |7 +++--
tests/testsuite.patch | 76 +
3 files changed, 83 insertions(+), 2 deletions(-) create mode 100644
tests/testsuite.patch
diff --git a/INSTALL.md b/INSTALL.md
index 94c25f7..273093b 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -41,6 +41,8 @@ you will need the following software:
- Python 2.x, for x >= 4.
+ - patch (The utility that is used to patch files).
+
On Linux, you may choose to compile the kernel module that comes with the
Open vSwitch distribution or to use the kernel module built into the Linux
kernel (version 3.3 or later). See the [FAQ.md] question diff --git
a/tests/automake.mk b/tests/automake.mk index 2e8d213..50d8ad2 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -6,7 +6,8 @@ EXTRA_DIST += \
$(KMOD_TESTSUITE) \
tests/atlocal.in \
$(srcdir)/package.m4 \
- $(srcdir)/tests/testsuite
+ $(srcdir)/tests/testsuite \
+ $(srcdir)/tests/testsuite.patch
COMMON_MACROS_AT = \
tests/ovsdb-macros.at \
@@ -87,6 +88,7 @@ KMOD_TESTSUITE_AT = \
tests/kmod-traffic.at
TESTSUITE = $(srcdir)/tests/testsuite
+TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite DISTCLEANFILES +=
tests/atconfig tests/atlocal
@@ -196,8 +198,9 @@ clean-local:
test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean
AUTOTEST = $(AUTOM4TE) --language=autotest
-$(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT)
+$(TESTSUITE): package.m4 $(TESTSUITE_AT) $(COMMON_MACROS_AT)
+$(TESTSUITE_PATCH)
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
+ patch -p0 $@.tmp $(TESTSUITE_PATCH)
$(AM_V_at)mv $@.tmp $@
$(KMOD_TESTSUITE): package.m4 $(KMOD_TESTSUITE_AT) $(COMMON_MACROS_AT) diff
--git a/tests/testsuite.patch b/tests/testsuite.patch new file mode 100644
index 000..e0c6bb3
--- /dev/null
+++ b/tests/testsuite.patch
@@ -0,0 +1,76 @@
+--- testsuite 2015-02-11 17:19:21.654646439 -0800
testsuite 2015-02-11 17:15:03.810653032 -0800
+@@ -4669,6 +4669,73 @@
+ fi
+ exec 6<&-
+ wait
++elif test $at_jobs -ne 1 &&
++ test "$IS_WIN32" = "yes"; then
++ # FIFO job dispatcher.
++ trap 'at_pids=
++ for at_pid in `jobs -p`; do
++at_pids="$at_pids $at_job_group$at_pid"
++ done
++ if test -n "$at_pids"; then
++at_sig=TSTP
++test "${TMOUT+set}" = set && at_sig=STOP
++kill -$at_sig $at_pids 2>/dev/null
++ fi
++ kill -STOP $$
++ test -z "$at_pids" || kill -CONT $at_pids 2>/dev/null' TSTP
++
++ echo
++ # Turn jobs into a list of numbers, starting from 1.
++ running_jobs="`pwd`/tests/jobdispatcher"
++ mkdir -p $running_jobs
++ at_joblist=`$as_echo "$at_groups" | sed -n 1,${at_jobs}p`
++
++ set X $at_joblist
++ shift
++ for at_group in $at_groups; do
++$at_job_control_on 2>/dev/null
++(
++ # Start one test group.
++ $at_job_control_off
++ touch $running_jobs/$at_group
++ trap 'set +x; set +e
++ trap "" PIPE
++ echo stop > "$at_stop_file"
++ rm -f $running_jobs/$at_group
++ as_fn_exit 141' PIPE
++ at_fn_group_prepare
++ if cd "$at_group_dir" &&
++ at_fn_test $at_group &&
++ . "$at_test_source"
++ then :; else
++ { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: unable to parse
++test group: $at_group" >&5 $as_echo "$as_me: WARNING: unable to parse test
group: $at_group" >&2;}
++ at_failed=:
++ fi
++ rm -f $running_jobs/$at_group
++ at_fn_group_postprocess
++) &
++$at_job_control_off
++shift # Consume one token.
++if test $# -gt 0; then :; else
++ while [ "`ls -l $running_jobs 2>/dev/null | wc -l`" -gt "$at_jobs" ]; do
++sleep 0.1
++ done
++ set x $*
++fi
++test -f "$at_stop_file" && break
++ done
++ # Read back the remaining ($at_jobs - 1) tokens.
++ set X $at_joblist
++ shift
++ if test $# -gt 0; then
++shift
++while
[ovs-dev] [PATCH v2] lib/util.h: use types compatible with DWORD
_BitScanForward() and friends are part of the Windows API and
take DWORD as parameter type. DWORD is defined to be 'unsigned long'
in Windows' header files.
We call into these functions from within lib/util.h. Currently, we
pass arguments of type uint32_t which is type defined to
'unsigned int'. This incompatiblity causes failures when we compile
the code as C++ code or with warnings enabled, when compiled as C
code.
The fix is to use 'unsigned long' rather than fixed size type.
Signed-off-by: Nithin Raju
Signed-off-by: Linda Sun
Co-Authored-by: Linda Sun
---
lib/util.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/util.h b/lib/util.h
index cbaa3ac..276edb5 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -352,11 +352,11 @@ static inline int
raw_ctz(uint64_t n)
{
#ifdef _WIN64
-uint32_t r = 0;
+unsigned long r = 0;
_BitScanForward64(&r, n);
return r;
#else
-uint32_t low = n, high, r = 0;
+unsigned long low = n, high, r = 0;
if (_BitScanForward(&r, low)) {
return r;
}
@@ -370,11 +370,11 @@ static inline int
raw_clz64(uint64_t n)
{
#ifdef _WIN64
-uint32_t r = 0;
+unsigned long r = 0;
_BitScanReverse64(&r, n);
return 63 - r;
#else
-uint32_t low, high = n >> 32, r = 0;
+unsigned long low, high = n >> 32, r = 0;
if (_BitScanReverse(&r, high)) {
return 31 - r;
}
--
1.9.4.msysgit.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] mac-learning: Implement per-port MAC learning fairness.
Looks good to me,
/* A MAC learning table entry.
> - * Guarded by owning 'mac_learning''s rwlock */
> + * Guarded by owning 'mac_learning''s rwlock. */
> struct mac_entry {
> struct hmap_node hmap_node; /* Node in a mac_learning hmap. */
> time_t expires; /* Expiration time. */
> @@ -47,14 +108,30 @@ struct mac_entry {
> uint16_t vlan; /* VLAN tag. */
>
> /* The following are marked guarded to prevent users from iterating
> over or
> - * accessing a mac_entry without hodling the parent mac_learning
> rwlock. */
> + * accessing a mac_entry without holding the parent mac_learning
> rwlock. */
> struct ovs_list lru_node OVS_GUARDED; /* Element in 'lrus' list. */
>
> -/* Learned port. */
> -union {
> -void *p;
> -ofp_port_t ofp_port;
> -} port OVS_GUARDED;
> +/* Learned port.
> + *
> + * The client-specified data is mlport->port. */
> +struct mac_learning_port *mlport;
>
Simple C question, why don't we need to forward declare the struct
'mac_learning_port'?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] lib/util.h: use types compatible with DWORD
On Thu, Feb 12, 2015 at 10:53 AM, Nithin Raju wrote: > _BitScanForward() and friends are part of the Windows API and > take DWORD as parameter type. DWORD is defined to be 'unsigned long' > in Windows' header files. > > We call into these functions from within lib/util.h. Currently, we > pass arguments of type uint32_t which is type defined to > 'unsigned int'. This incompatiblity causes failures when we compile > the code as C++ code or with warnings enabled, when compiled as C > code. > > The fix is to use 'unsigned long' rather than fixed size type. > > Signed-off-by: Nithin Raju > Signed-off-by: Linda Sun > Co-Authored-by: Linda Sun Thank you. Applied! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Markdown coding standard
On 02/12/15 at 05:37pm, Finucane, Stephen wrote: > I note that there are no coding standard for Markdown documents. Given this, > I have some questions: > > * What's the expected format for code blocks (multi-line and single-line)? We haven't standardized on anything yet. Is there an exciting good guidelines doc that we could inherit? Maybe just refer to the github markdown syntax page? > * Can we use GHFM-extensions (i.e. syntax-highlighted code fences) I think that's fine as long as they are compatible with the markdown parser invoked in build-aux/dist-docs to generate the docs for the website. > * Line limit? It's not necessary to wrap Markdown if you don't want to, but > files seem to alternate between 72, 79 and 80+ quite a bit I basically did the minimum required to get quotes formatted correctly and didn't reformat everything. Cleanups are definitely welcome. > * Any "expected approach" to validating Markdown output (i.e. confirm that > it will preview correctly on GitHub) Personally I have been using Haroopad when changing the docs to verify correct rendering. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: vxlan: Only set has-GBP bit in header if any other bits would be set
On 02/11/15 at 03:58pm, Pravin Shelar wrote:
> On Mon, Feb 9, 2015 at 7:54 AM, Thomas Graf wrote:
> > vxlan: Only set has-GBP bit in header if any other bits would be set
> >
> > This allows for a VXLAN-GBP socket to talk to a Linux VXLAN socket by
> > not setting any of the bits.
> >
> > Signed-off-by: Thomas Graf
> > Signed-off-by: David S. Miller
> >
> > Upstream: db79a621835e ("vxlan: Only set has-GBP bit in header if any other
> > bits would be set")
> > Signed-off-by: Thomas Graf
>
> Acked-by: Pravin B Shelar
Pushed to master, thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] mac-learning: Implement per-port MAC learning fairness.
Solution seems clean, I'm happy with this as well.
Acked-by: Ethan Jackson
On Thu, Feb 12, 2015 at 11:02 AM, Alex Wang wrote:
> Looks good to me,
>
> /* A MAC learning table entry.
>> - * Guarded by owning 'mac_learning''s rwlock */
>> + * Guarded by owning 'mac_learning''s rwlock. */
>> struct mac_entry {
>> struct hmap_node hmap_node; /* Node in a mac_learning hmap. */
>> time_t expires; /* Expiration time. */
>> @@ -47,14 +108,30 @@ struct mac_entry {
>> uint16_t vlan; /* VLAN tag. */
>>
>> /* The following are marked guarded to prevent users from iterating
>> over or
>> - * accessing a mac_entry without hodling the parent mac_learning
>> rwlock. */
>> + * accessing a mac_entry without holding the parent mac_learning
>> rwlock. */
>> struct ovs_list lru_node OVS_GUARDED; /* Element in 'lrus' list. */
>>
>> -/* Learned port. */
>> -union {
>> -void *p;
>> -ofp_port_t ofp_port;
>> -} port OVS_GUARDED;
>> +/* Learned port.
>> + *
>> + * The client-specified data is mlport->port. */
>> +struct mac_learning_port *mlport;
>>
>
>
> Simple C question, why don't we need to forward declare the struct
> 'mac_learning_port'?
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/13] datapath: Allow building against 3.19.x
I agree. we now have the initial infrastructure to add more tests. In the meantime, the infrastructure can also use some improvements. For example, it does not deal with kernel crash very well. Any suggestions or past experiences in setting up kernel testing framework are welcome. On Wed, Feb 11, 2015 at 7:59 PM, Ben Pfaff wrote: > Vagrant sounds like a real winner. Thanks for working on the kernel > testing infrastructure. I hope that we can start to build up a library > of tests. > > On Wed, Feb 11, 2015 at 06:04:31PM -0800, Andy Zhou wrote: >> I have experimented with the user-mode linux about a month back. I was >> not able to get ovs user space to run reliably with the >> (light weight) host-fs file system. It may run better with a >> disk-image (I did not try), but then we willl have to deal with >> building >> and distributing disk-images, which Vagrant solves. >> >> On Sat, Feb 7, 2015 at 10:52 PM, Ben Pfaff wrote: >> > On Sat, Feb 07, 2015 at 01:04:57PM +0100, Thomas Graf wrote: >> >> On 02/06/15 at 10:28pm, Ben Pfaff wrote: >> >> > We already do a ton of kernel module builds in travis, do you mean that >> >> > we should do one for every released kernel, or do you mean something >> >> > else? >> >> >> >> I'm primarily thinking of Andy's work to enable runtime CI as well. >> >> I think the matrix of stable kernels is fine. >> > >> > Oh, yes, that would be very nice. >> > >> > I wonder whether user-mode Linux is usable enough to test OVS. I built >> > a related prototype back in 2008 or so, but I never managed to automate >> > it properly. >> > ___ >> > dev mailing list >> > dev@openvswitch.org >> > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/13] datapath: Allow building against 3.19.x
On 02/12/15 at 01:08pm, Andy Zhou wrote: > I agree. we now have the initial infrastructure to add more tests. > > In the meantime, the infrastructure can also use some improvements. > For example, it does not deal with kernel crash > very well. Any suggestions or past experiences in setting up kernel > testing framework are welcome. Agreed, Vagrant is an excellent start. It should be simple to maintain a set of kernel platforms with packer. We could combine it with a hosted Jenkins. From [0]: * Uses Vagrant 1.x * Supports booting a Vagrant VM for your job * Execute scripts inside the VM as the "vagrant" user * Execute scripts inside the VM as the "root" user (via sudo) * Provision the Vagrant VM via the configured provisioner(s) from the Vagrantfile [0] https://wiki.jenkins-ci.org/display/JENKINS/Vagrant+Plugin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Fwd: [ovs-discuss] OVS segfault in recirculation
Does not mean to drop the list.
-- Forwarded message --
From: Andy Zhou
Date: Thu, Feb 12, 2015 at 3:42 PM
Subject: Re: [ovs-discuss] OVS segfault in recirculation
To: Salvatore Cambria
Hi, Salvatore,
I think I found a bug: hmap_remove needs to be protected by a lock. As
you pointed out, the output_normal() path is not properly protected.
Would you please
try the attached patch to see if it helps?
Bond object is protected by reference counting. Assuming reference
counter is valid, it should be O.K. for a bundle to hold on to the
object. Notice
bundle_destroy() calls unref_bond(). At any rate, I did not spot
anything obvious there.
Thanks for providing such a detailed description. It is very helpful.
Andy
iff --git a/ofproto/bond.c b/ofproto/bond.c
index c4b3a3a..a9b7a83 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -923,8 +923,8 @@ bond_may_recirc(const struct bond *bond, uint32_t
*recirc_id,
}
}
-void
-bond_update_post_recirc_rules(struct bond* bond, const bool force)
+static void
+bond_update_post_recirc_rules__(struct bond* bond, const bool force)
{
struct bond_entry *e;
bool update_rules = force; /* Always update rules if caller forces it. */
@@ -945,6 +945,14 @@ bond_update_post_recirc_rules(struct bond* bond,
const bool force)
update_recirc_rules(bond);
}
}
+
+void
+bond_update_post_recirc_rules(struct bond* bond, const bool force)
+{
+ovs_rwlock_wrlock(&rwlock);
+bond_update_post_recirc_rules__(bond, force);
+ovs_rwlock_unlock(&rwlock);
+}
^L
/* Rebalancing. */
@@ -1203,7 +1211,7 @@ bond_rebalance(struct bond *bond)
}
if (use_recirc && rebalanced) {
-bond_update_post_recirc_rules(bond,true);
+bond_update_post_recirc_rules__(bond,true);
}
done:
On Tue, Feb 10, 2015 at 7:57 AM, Salvatore Cambria
wrote:
> Hello,
>
> I am a Software Engineer from Citrix, Cambridge UK office.
>
> We are having a problem when running our nightly test on a XenServer host
> with lacp bonds with OVS. Indeed, when deleting the bond the following
> segfault error appears (from GDB):
>
> Program terminated with signal 11, Segmentation fault.
>
> #0 0x00411d2f in hmap_remove (node=0x7fd9d402e730, hmap=0x1c3e9e8)
>
> at lib/hmap.h:236
>
> 236 while (*bucket != node) {
>
> (gdb) bt
>
> #0 0x00411843 in hmap_remove (node=0x7fd9d402e730, hmap=0x1c3e9e8)
>
> at lib/hmap.h:236
>
> #1 update_recirc_rules (bond=0x1c3e920) at ofproto/bond.c:382
>
> #2 0x1ff5 in ?? ()
>
> (gdb) p *hmap
>
> $1 = {buckets = 0x7fd9d4039b70, one = 0x0, mask = 255, n = 147}
>
> (gdb) p *node
>
> $2 = {hash = 2450845263, next = 0x0}
>
> (gdb) p *bucket
>
> Cannot access memory at address 0x8
>
> (gdb) p *(hmap->buckets)
>
> $3 = (struct hmap_node *) 0x0
>
> (gdb) p node->hash & hmap->mask
>
> $4 = 79
>
> (gdb) p *(&hmap->buckets[79])
>
> $5 = (struct hmap_node *) 0x0
>
> I managed to reproduce the error using a while loop in which I repeatedly
> create and destroy a lacp bond, waiting for 15 seconds after each of these
> operations.
>
> The error is not constant in timing and I think that it can be a race
> condition due to the recirculation process. It happens indeed that when
> calling hmap_remove() from update_recirc_rules() (in case of DEL), the
> bucket points to NULL. My idea is the following:
>
> hmap_remove() is called from two different places in ofproto/bond.c:
>
> 1. from update_recirc_rules() (in which it raises the segfault)
>
> hmap_remove(&bond->pr_rule_ops, &pr_op->hmap_node);
> *pr_op->pr_rule = NULL;
> free(pr_op);
>
> 2. and from bond_unref() (called when I try to delete the bond)
>
> HMAP_FOR_EACH_SAFE(pr_op, next_op, hmap_node, &bond->pr_rule_ops) {
> hmap_remove(&bond->pr_rule_ops, &pr_op->hmap_node);
> free(pr_op);
> }
>
> Now, if these two calls happen at the same time, a conflict on the bond
> pr_rule may happen. Looking at the code I can see that the recirculation
> hmap_remove() is executed inside an external lock
> (ovs_rwlock_wrlock(&rwlock)) if called by bond_rebalance()
>
> void bond_rebalance(struct bond *bond) {
> ...
> ovs_rwlock_wrlock(&rwlock);
> ...
> ...
> if (use_recirc && rebalanced) {
> bond_update_post_recirc_rules(bond,true); < hmap_remove()
> }
>
> done:
> ovs_rwlock_unlock(&rwlock);
> }
>
> but I can't see any lock when called by output_normal()
>
> static void output_normal(struct xlate_ctx *ctx, const struct xbundle
> *out_xbundle, uint16_t vlan) {
> ...
> if (ctx->use_recirc) {
> ...
> bond_update_post_recirc_rules(out_xbundle->bond, false); <
> hmap_remove()
> ...
> }
> ...
>
> The same lock is used in bond_unref() but only for hmap_remove(all_bonds,
> &bond->hmap_node) and not for hmap_remove(&bond->pr_rule_ops,
> &pr_op->hmap_node).
>
> ...
> ovs_rwlock_wrlock(&rwlock);
> hmap_remove(all_bonds, &bond->hmap_node);
> ovs_rwlock_unlock(&rwlock);
> ...
> HMAP_FOR_EACH_SAFE(pr_op, nex
