Re: [ovs-dev] [PATCH 2/5] dpif-netdev: Implement the API functions to allow multiple handler threads read upcall.

> @@ -1452,32 +1507,18 @@ dpif_netdev_queue_to_priority(const struct dpif *dpif > OVS_UNUSED, > return 0; > } > > -static struct dp_netdev_queue * > -find_nonempty_queue(struct dp_netdev *dp) > -OVS_REQUIRES(dp->queue_mutex) > -{ > -int i; > - > -for (i = 0; i < N_QUEUES; i++)

Re: [ovs-dev] [PATCH] ofproto: fix interactions between flow monitors and barriers

> On Thu, Feb 27, 2014 at 02:50:47PM +0900, YAMAMOTO Takashi wrote: >> >> On Wed, Feb 05, 2014 at 07:04:05PM +0900, YAMAMOTO Takashi wrote: >> >>> Following OpenFlow 1.4 semantics, make barriers wait for >> >>> flow monitor replies. This should fix a race in >> >>> "ofproto - flow monitoring pause

Re: [ovs-dev] [branch-2.0] ofproto-dpif: Discard any flow_miss_batches before destroy a ofproto

I see, there is no revalidator thread in branch-2.0 Looks good to me, May also make sense to remove the "revalidator" from comment, diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 7ec7e8d..6adc78e 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-d

[ovs-dev] [branch-2.0] ofproto-dpif: Discard any flow_miss_batches before destroy a ofproto

Commit dd2e44f835fac8 fixed a similar race conditions w.r.t. removal of 'ofproto', but introduced this bug. While 'ofproto' is being removed, the existing flow_miss_batches may still contain references to the to be removed 'ofproto', causing access to freed memory. Bug #1202234 Signed-off-by: And

Re: [ovs-dev] [PATCHv4 7/7] ofproto-dpif: Remove the flow_dumper thread.

On 27 February 2014 15:23, Ben Pfaff wrote: > > I would appreciate xpthread_*() wrappers for the barrier functions. I > know that they shouldn't normally fail, but error checking has found > real bugs for other types of pthread objects and it could find them > for barriers too. > Sure, I can loo

[ovs-dev] [PATCH v2 2/7] nsh: userland support for network service headers

NSH service path (nsp) can be set/unset while creating the port as well nsp can be matched on incoming packets. Signed-off-by: Pritesh Kothari --- lib/flow.c |2 + lib/flow.h |2 + lib/match.c| 21 ++ lib/match.h|2 + lib/meta-flow.c| 43 ++

[ovs-dev] [PATCH 5/5] nsh: userland support for network service index

From: pritesh Support for nsh service index (nsi) is added, mainly incoming nsi in a flow can be matched and appropriate action can be taken on the flow based on it. Signed-off-by: Pritesh Kothari --- lib/flow.c |2 ++ lib/flow.h |2 ++ lib/match.c| 25 +++

Re: [ovs-dev] [PATCHv4 6/7] dpif: New function flow_dump_next_may_destroy_keys().

On Thu, Feb 27, 2014 at 02:13:10PM -0800, Joe Stringer wrote: > This new function allows callers to determine whether previously > returned keys will be modified or reallocated on the next call to > dpif_flow_dump_next(). This will be used in a future commit to allow > batched flow deletion by reva

Re: [ovs-dev] [PATCHv4 7/7] ofproto-dpif: Remove the flow_dumper thread.

On Thu, Feb 27, 2014 at 02:13:11PM -0800, Joe Stringer wrote: > From: Ethan Jackson > > Previously, we had a separate flow_dumper thread that fetched flows from > the datapath to distribute to revalidator threads. This patch takes the > logic for dumping and pushes it into the revalidator threads

Re: [ovs-dev] [PATCH] lib: simplify flow_extract() API

I like this, some comments below, otherwise: Acked-by: Jarno Rajahalme On Feb 26, 2014, at 6:22 PM, Andy Zhou wrote: > Change the flow_extract() API to accept struct pkt_metadta, > instead of individual metadata fields. It will make the API more > logical and easier to maintain when we need to

Re: [ovs-dev] [PATCH v2] openvswitch: Disable bottom half when grabbing stats lock in userspace context

On Thu, Feb 27, 2014 at 2:10 PM, Zoltan Kiss wrote: > The megaflow feature introduced per-CPU stats, and the new code did some > refactoring as well [1]. However the new functions doesn't disable bottom > halves > when locking the stat, and as they can be called from userspace context, they > can

Re: [ovs-dev] [PATCHv4 4/7] dpif: Make dpif_flow_dump_next() thread-safe.

On Thu, Feb 27, 2014 at 02:13:08PM -0800, Joe Stringer wrote: > This patch makes it the caller's responsibility to initialize a > per-thread 'state' object and pass it down to the dpif_flow_dump_next() > implementation. The implementation can expect to be called from multiple > threads with the sam

Re: [ovs-dev] [PATCHv4 2/7] netlink: Make nl_dump_next() thread-safe.

On Thu, Feb 27, 2014 at 02:13:06PM -0800, Joe Stringer wrote: > This patch modifies 'struct nl_dump' and nl_dump_next() to allow > multiple threads to share the same nl_dump. These changes are targeted > around synchronizing dump status between multiple callers, and > allowing callers to fully proc

Re: [ovs-dev] [PATCH 2/2] sparse: add defintions to inlucde/sparse/netinet/in.h

Thanks, I will fix it before push. On Thu, Feb 27, 2014 at 2:18 PM, Joe Stringer wrote: > Before you push, there's a typo in the patch name - "inlucde" > > > On 26 February 2014 20:08, Ben Pfaff wrote: > >> On Wed, Feb 26, 2014 at 07:15:38PM -0800, Andy Zhou wrote: >> > Fixed sparse undefined s

Re: [ovs-dev] [PATCHv4 3/7] dpif: Separate local and shared flow dump state.

On Thu, Feb 27, 2014 at 02:13:07PM -0800, Joe Stringer wrote: > This patch separates the structures for thread-local flow dump state > ("state") from the shared flow dump state ("iter") in dpif-linux and > dpif-netdev. Future patches will make use of this to allow multiple > threads to dump flows f

Re: [ovs-dev] [PATCH 2/2] sparse: add defintions to inlucde/sparse/netinet/in.h

Before you push, there's a typo in the patch name - "inlucde" On 26 February 2014 20:08, Ben Pfaff wrote: > On Wed, Feb 26, 2014 at 07:15:38PM -0800, Andy Zhou wrote: > > Fixed sparse undefined symbol warnings. > > > > Signed-off-by: Andy Zhou > > Acked-by: Ben Pfaff > ___

Re: [ovs-dev] [PATCH] ofproto-dpif.at: Avoid races in MPLS tests

On Thu, Feb 27, 2014 at 03:36:59PM +0900, YAMAMOTO Takashi wrote: > Signed-off-by: YAMAMOTO Takashi Applied, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCHv4 6/7] dpif: New function flow_dump_next_may_destroy_keys().

This new function allows callers to determine whether previously returned keys will be modified or reallocated on the next call to dpif_flow_dump_next(). This will be used in a future commit to allow batched flow deletion by revalidator threads. Signed-off-by: Joe Stringer --- v3: Readded patch,

[ovs-dev] [PATCHv4 5/7] dpif: Don't synchronize flow_dump_next() status.

Recent changes to the flow_dump_next() interface have made it the responsibility of the dpif implementation to track error status over a flow dump operation. This patch removes status tracking from 'struct dpif_flow_dump', allowing multiple threads to call dpif_flow_dump_next() and track their sta

[ovs-dev] [PATCHv4 7/7] ofproto-dpif: Remove the flow_dumper thread.

From: Ethan Jackson Previously, we had a separate flow_dumper thread that fetched flows from the datapath to distribute to revalidator threads. This patch takes the logic for dumping and pushes it into the revalidator threads, resulting in simpler code with similar performance to the current code

[ovs-dev] [PATCHv4 2/7] netlink: Make nl_dump_next() thread-safe.

This patch modifies 'struct nl_dump' and nl_dump_next() to allow multiple threads to share the same nl_dump. These changes are targeted around synchronizing dump status between multiple callers, and allowing callers to fully process their existing buffers before determining whether to stop fetching

[ovs-dev] [PATCHv4 0/7] Remove the flow dumper thread.

Currently, we have a single thread (the 'flow_dumper') that fetches the flows from the datapath and distributes them to revalidator threads. To clean up the code and support more flows in the datapath, we are looking at removing this flow_dumper thread, and have each revalidator fetch flows from th

[ovs-dev] [PATCHv4 3/7] dpif: Separate local and shared flow dump state.

This patch separates the structures for thread-local flow dump state ("state") from the shared flow dump state ("iter") in dpif-linux and dpif-netdev. Future patches will make use of this to allow multiple threads to dump flows from the same flow dump operation. Signed-off-by: Joe Stringer --- v3

[ovs-dev] [PATCHv4 4/7] dpif: Make dpif_flow_dump_next() thread-safe.

This patch makes it the caller's responsibility to initialize a per-thread 'state' object and pass it down to the dpif_flow_dump_next() implementation. The implementation can expect to be called from multiple threads with the same 'iter' and different 'state' objects. When flow_dump_next() returns

[ovs-dev] [PATCHv4 1/7] netlink: Remove buffer from 'struct nl_dump'.

This patch makes all of the users of 'struct nl_dump' allocate their own buffers to pass down to nl_dump_next(). This paves the way for allowing multithreaded flow dumping. Signed-off-by: Joe Stringer --- v3: Rebase. v2: Add NL_DUMP_BUFSIZE. Add buffer to dpif_linux_flow_state. Fold "netl

Re: [ovs-dev] [PATCH] rconn: Discover errors in rconn_run() even if rconn_recv() is never called.

Thanks, applied to master. On Thu, Feb 27, 2014 at 02:24:29PM -0500, Vasu Dasari wrote: > Thanks it works. Code looks good too. > > > On Thu, Feb 27, 2014 at 2:07 PM, Ben Pfaff wrote: > > > rconn_recv() calls vconn_recv(), which will report a connection error and > > cause rconn_recv() to disc

[ovs-dev] [PATCH v2] openvswitch: Disable bottom half when grabbing stats lock in userspace context

The megaflow feature introduced per-CPU stats, and the new code did some refactoring as well [1]. However the new functions doesn't disable bottom halves when locking the stat, and as they can be called from userspace context, they can be interrupted by netif_receive_skb. [1] e298e50570: openvswit

Re: [ovs-dev] [PATCH] openvswitch: Disable bottom half when grabbing stats lock in userspace context

On 27/02/14 22:04, Zoltan Kiss wrote: The megaflow feature introduced per-CPU stats, and the new code did some refactoring as well [1]. However the new functions doesn't disable bottom halves when locking the stat, and as they can be called from userspace context, they can be interrupted by netif

Re: [ovs-dev] Support for Linux kernels >= 3.12 with the out of tree kernel module

On Thu, Feb 27, 2014 at 02:41:13PM -0600, Kyle Mestery wrote: > Is the workflow such that if I click "approve" in patchwork it will > automatically merge the patch? Do I still need to send an email to > the list with the standard "Reviewed-by" or "Acked-by"? Patchwork is just a tracking tool. It

[ovs-dev] [PATCH] openvswitch: Disable bottom half when grabbing stats lock in userspace context

The megaflow feature introduced per-CPU stats, and the new code did some refactoring as well [1]. However the new functions doesn't disable bottom halves when locking the stat, and as they can be called from userspace context, they can be interrupted by netif_receive_skb. [1] e298e50570: openvswit

Re: [ovs-dev] Support for Linux kernels >= 3.12 with the out of tree kernel module

On Thu, Feb 27, 2014 at 12:41 PM, Kyle Mestery wrote: > On Wed, Feb 26, 2014 at 5:07 PM, Pravin Shelar wrote: >> >> On Wed, Feb 26, 2014 at 2:09 PM, Kyle Mestery >> wrote: >> > I see this patch [1] in patchwork now from Pravin via James Page. >> > I pulled this down and started to look at what i

Re: [ovs-dev] [PATCH] ofproto: fix interactions between flow monitors and barriers

On Thu, Feb 27, 2014 at 02:50:47PM +0900, YAMAMOTO Takashi wrote: > >> On Wed, Feb 05, 2014 at 07:04:05PM +0900, YAMAMOTO Takashi wrote: > >>> Following OpenFlow 1.4 semantics, make barriers wait for > >>> flow monitor replies. This should fix a race in > >>> "ofproto - flow monitoring pause and r

Re: [ovs-dev] Support for Linux kernels >= 3.12 with the out of tree kernel module

On Wed, Feb 26, 2014 at 5:07 PM, Pravin Shelar wrote: > On Wed, Feb 26, 2014 at 2:09 PM, Kyle Mestery > wrote: > > I see this patch [1] in patchwork now from Pravin via James Page. > > I pulled this down and started to look at what it would take to get this > > also working with 3.13. It looks l

[ovs-dev] [PATCH 4/5] dpif-linux: Implement the API functions to allow multiple handler threads read upcall.

Signed-off-by: Alex Wang --- major changes since RFC: - for cache access efficiency, in 'recv()' index the handlers array first and then port channels array. --- lib/dpif-linux.c | 551 +++--- lib/dpif-linux.h |3 +- 2 files changed, 359 ins

[ovs-dev] [PATCH 5/5] ofproto-dpif-upcall: Remove the dispatcher thread.

With the foundation laid in previous commits, this commit removes the 'dispatcher' thread by allowing 'handler' threads to read upcalls directly from dpif. This commit significantly simplifies the flow miss handling code and brings slight improvement to flow setup rate. Note: - the flow setup rat

[ovs-dev] [PATCH 1/5] dpif: Change dpif API to allow multiple handler threads read upcall.

This commit changes the API in 'dpif-provider.h' to allow multiple handler threads call dpif_recv() simultaneously. Signed-off-by: Alex Wang --- lib/dpif-linux.c | 24 ++-- lib/dpif-netdev.c | 17 ++ lib/dpif-provider.h | 36 ++

[ovs-dev] [PATCH 3/5] datapath: Allow each vport to have an array of 'port_id's.

In order to allow handlers directly read upcalls from datapath, we need to support per-handler netlink socket for each vport in datapath. This commit makes this happen. Also, it is guaranteed that the newer datapath is compatible with the branch-2.1 userspace implementation. Signed-off-by: Alex

Re: [ovs-dev] [PATCH V3] ofproto-dpif-upcall: Remove the dispatcher thread.

Thanks Han, I'm going to post them soon, gonna be in series. Alex Wang, On Wed, Feb 26, 2014 at 4:26 PM, Han Zhou wrote: > Hi Alex, > > I had a simple test with your V3 patch, and the improvements are > obvious. > > In my 32 core machine I ran 10 handler threads, comparing hping3 test > resul

[ovs-dev] [PATCH 0/5] Remove the dispatcher thread.

Pravin made me realize that the work can be organized into logically separate patches. So, here it comes. Major changes since RFC: - Make datapath change compatible with branch-2.1 kernel. - Re-design the dpif-linux indexing order for cache access benefits. - Implement the per-handler queue in dp

[ovs-dev] [PATCH 2/5] dpif-netdev: Implement the API functions to allow multiple handler threads read upcall.

Signed-off-by: Alex Wang --- lib/dpif-netdev.c | 190 ++--- lib/flow.c| 18 + lib/flow.h|1 + 3 files changed, 143 insertions(+), 66 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index ee8be43..62a775f 1

Re: [ovs-dev] [PATCH V3] ofproto-dpif-upcall: Remove the dispatcher thread.

Thanks very much for the review!~ ;D Please see my reply inline, On Thu, Feb 20, 2014 at 3:25 PM, Pravin Shelar wrote: > > upcall.cmd = OVS_PACKET_CMD_MISS; > > upcall.key = &key; > > upcall.userdata = NULL; > > - upcall.portid = p

Re: [ovs-dev] [PATCH] rconn: Discover errors in rconn_run() even if rconn_recv() is never called.

Thanks it works. Code looks good too. On Thu, Feb 27, 2014 at 2:07 PM, Ben Pfaff wrote: > rconn_recv() calls vconn_recv(), which will report a connection error and > cause rconn_recv() to disconnect the rconn. Most rconn users regularly > call rconn_recv(), so that disconnection happens prompt

Re: [ovs-dev] [PATCH] tests: Re-fix a race.

I expect Ethan to apply this, but I guess he lost track of it, so I've applied it now on his behalf. On Thu, Feb 27, 2014 at 02:49:37PM +0900, YAMAMOTO Takashi wrote: > ping! > > >> Patch bf06c4fe (tests/ofproto-dpif.at: Workaround a race.), fixed a > >> race condition which patch 0a8763f (ofpro

Re: [ovs-dev] [ovs-discuss] Errors in test-controller

On Thu, Feb 27, 2014 at 07:34:01AM -0500, Vasu Dasari wrote: > I am seeing some uncontrollable message logs generated by test-controller > under one negative test scenario. > > 1. Launch test-controller: >test-controller ptcp: -v > 2. From another shell, just do the following: > > vdasari

[ovs-dev] [PATCH] rconn: Discover errors in rconn_run() even if rconn_recv() is never called.

rconn_recv() calls vconn_recv(), which will report a connection error and cause rconn_recv() to disconnect the rconn. Most rconn users regularly call rconn_recv(), so that disconnection happens promptly. However, the lswitch code only calls rconn_recv() after the connection negotiates an OpenFlow

Re: [ovs-dev] [PATCHv5] upcall: Configure datapath max-idle through ovs-vsctl.

I think it's clear enough, since it explicitly mentions datapath flows. There's a FAQ entry that describes the different types of flows that Ben can point to, which always makes him happy. --Justin On Feb 27, 2014, at 10:55 AM, Joe Stringer wrote: > I agree, it would be helpful to have a se

Re: [ovs-dev] [PATCHv5] upcall: Configure datapath max-idle through ovs-vsctl.

I agree, it would be helpful to have a sentence like that. This could be particularly misleading given that it is relating to datapath flows, which is completely different from OpenFlow rule timeouts. Do you think that this is clear enough with the current description? On 27 February 2014 10:50,

Re: [ovs-dev] [PATCHv5] upcall: Configure datapath max-idle through ovs-vsctl.

On Feb 27, 2014, at 10:37 AM, Joe Stringer wrote: > +The maximum idle time in milliseconds for flows to be cached in > the > +datapath. A lower value may improve flow setup performance, but > +decrease the number of cached flows in the datapath. Conversely, >

Re: [ovs-dev] adding an option for GRE tunneling without an Ethernet header

On Thu, Feb 27, 2014 at 05:16:18PM +, thomas.mo...@orange.com wrote: > Currently, OVS GRE tunnels use Ethertype 6558 and the GRE packets > produced by OVS hence always are xxx-over-Ethernet-over-GRE. > Symmetrically OVS expects received GRE packets to be of the same > ethertype and carry an

Re: [ovs-dev] adding an option for GRE tunneling without an Ethernet header

Hi Thomas, I'm the original contributor of the LISP tunneling code. Since it was originally accepted, I have been working on a series of patches to enable more generic support for layer 3 packets in OVS. While I was doint it with LISP in mind, I did consider enabling IP-over-GRE at some poi

Re: [ovs-dev] adding an option for GRE tunneling without an Ethernet header

On Thu, Feb 27, 2014 at 9:16 AM, wrote: > Hello, > > Currently, OVS GRE tunnels use Ethertype 6558 and the GRE packets > produced by OVS hence always are xxx-over-Ethernet-over-GRE. > Symmetrically OVS expects received GRE packets to be of the same > ethertype and carry an Ethernet payload. > > I

[ovs-dev] [PATCHv5] upcall: Configure datapath max-idle through ovs-vsctl.

This patch adds a new configuration option, "max-idle" to the Open_vSwitch "other-config" column. This sets how long datapath flows are cached in the datapath before revalidators expire them. Signed-off-by: Joe Stringer --- v5: Rebase. v4: Remove extra "max_idle" from Flow_Table table. v3: Use '

[ovs-dev] adding an option for GRE tunneling without an Ethernet header

Hello, Currently, OVS GRE tunnels use Ethertype 6558 and the GRE packets produced by OVS hence always are xxx-over-Ethernet-over-GRE. Symmetrically OVS expects received GRE packets to be of the same ethertype and carry an Ethernet payload. I have written the included patch, which does the foll

Re: [ovs-dev] [PATCH] ovs-lib: allow non-root users to check service status

On Thu, Feb 27, 2014 at 4:16 AM, Flavio Leitner wrote: > It tries to log the status operation, so although non-root > users can see the current status, the lack of permission > to write to the log results in an error message. > > This changes to log only if the user has the permission to > write t

[ovs-dev] Errors in test-controller

Hi, I am seeing some uncontrollable message logs generated by test-controller under one negative test scenario. 1. Launch test-controller: test-controller ptcp: -v 2. From another shell, just do the following: vdasari@ovs:~/mininet$ telnet 127.0.0.1 6633 Trying 127.0.0.1... Connected to 1

[ovs-dev] [PATCH] ovs-lib: allow non-root users to check service status

It tries to log the status operation, so although non-root users can see the current status, the lack of permission to write to the log results in an error message. This changes to log only if the user has the permission to write to the log file. Signed-off-by: Flavio Leitner --- utilities/ovs-

Re: [ovs-dev] [PATCH] ovs-lib: don't log service status operation

On Wed, Feb 26, 2014 at 08:33:22PM -0800, Gurucharan Shetty wrote: > On Wed, Feb 26, 2014 at 8:28 PM, Ben Pfaff wrote: > > On Wed, Feb 26, 2014 at 11:19:15PM -0300, Flavio Leitner wrote: > >> It prevents non-root users to check service status. > >> > >> Signed-off-by: Flavio Leitner > > > > Guru,

Re: [ovs-dev] [PATCH 2/2] vswitchd: Limit fake bridge MAC selection to ports in the same VLAN

On Wed, Feb 26, 2014 at 9:54 PM, Ben Pfaff wrote: > On Wed, Feb 19, 2014 at 11:59:33AM +0100, Helmut Schaa wrote: >> Limit fake bridge MAC address selection to only consider ports >> that use the same VLAN as the fake bridge itself. >> >> This prevents OVS from selecting a MAC address that was not

Re: [ovs-dev] [PATCH] Rapid Spanning Protocol Implementation (IEEE 802.1D) + functional tests

Jarno Rajahalme wrote: On Feb 25, 2014, at 2:58 PM, Ben Pfaff > wrote: Jarno, would you mind reviewing this? Not at all, but it’ll take some days, as I haven’t looked at RSTP before :-) Jarno Feel free to contact me and my colleague Daniele (daniele.ventur...@m