Thanks Ben for your review, very glad to see this applied.
On Tue, Jul 30, 2013 at 9:40 PM, Ben Pfaff wrote:
> On Tue, Jul 30, 2013 at 03:31:48PM -0700, Alex Wang wrote:
> > From: Ethan Jackson
> >
> > This commit adds annotations for thread safety check. And the
> > check can be conducted by u
Hi Andy,
I think your plan to remove ambiguity is a good one.
On Tue, Jul 30, 2013 at 09:09:35PM -0700, Andy Zhou wrote:
> Simon, Thanks for the review.
> The intention of my patch is to always export those attributes to remove
> the ambiguity associated with interpreting missing attributes. Glad
On Tue, Jul 30, 2013 at 03:31:48PM -0700, Alex Wang wrote:
> From: Ethan Jackson
>
> This commit adds annotations for thread safety check. And the
> check can be conducted by using -Wthread-safety flag in clang.
>
> Co-authored-by: Alex Wang
> Signed-off-by: Alex Wang
> Signed-off-by: Ethan Ja
Simon, Thanks for the review.
The intention of my patch is to always export those attributes to remove
the ambiguity associated with interpreting missing attributes. Glad to hear
this patch would work well with recirculation.
On Tue, Jul 30, 2013 at 8:54 PM, Simon Horman wrote:
> On Tue, Jul 30
On Mon, Jul 22, 2013 at 11:30:14AM -0700, Jesse Gross wrote:
> On Mon, Jul 22, 2013 at 11:13 AM, Ben Pfaff wrote:
> > Jesse, I'm leaving this series alone until you give some feedback on
> > the kernel side of things. Then I'm happy to review (and commit when
> > appropriate) user space patches.
On Tue, Jul 30, 2013 at 07:49:13PM -0700, Andy Zhou wrote:
> Handling of missing attributes in netlink can be tricky and turns out
> to be error prone. The value (savings in netlink bandwidth) does not
> seem to be significant enough to justify allowing them. This patch
> series make both kernel an
On Tue, Jul 30, 2013 at 07:31:48PM -0700, Gurucharan Shetty wrote:
> Certain platforms like xenserver do not have the latest
> python libraries that are needed by ovsdb-doc (which in-turn
> creates ovs-vswitchd.conf.db.5). When we run 'make dist' and
> copy over the tar ball to xenserver ddk enviro
On Tue, Jul 30, 2013 at 07:49:12PM -0700, Andy Zhou wrote:
> Handling of missing attributes in netlink can be tricky and turns out
> to be error prone. The value (savings in netlink bandwidth) does not
> seem to be significant enough to justify allowing them. This patch
> series make both kernel an
Certain platforms like xenserver do not have the latest
python libraries that are needed by ovsdb-doc (which in-turn
creates ovs-vswitchd.conf.db.5). When we run 'make dist' and
copy over the tar ball to xenserver ddk environemt, we
already include ovs-vswitchd.conf.db.5. But the absence of
ovsdb-d
Handling of missing attributes in netlink can be tricky and turns out
to be error prone. The value (savings in netlink bandwidth) does not
seem to be significant enough to justify allowing them. This patch
series make both kernel and userspace always export priority and
skb_mark attribute. There wi
Handling of missing attributes in netlink can be tricky and turns out
to be error prone. The value (savings in netlink bandwidth) does not
seem to be significant enough to justify allowing them. This patch
series make both kernel and userspace always export priority and
skb_mark attribute. There wi
Acked-by: Ethan Jackson
On Thu, Jul 18, 2013 at 2:26 PM, Ben Pfaff wrote:
> This reverts commit 05d299e0ccca80736cd4438c3224540c5448a7d4 (test-atomic:
> Drop atomic read-modify-write tests for the moment.) because the
> test for detecting whether GCC support atomic operation built-ins has
> bee
Acked-by: Ethan Jackson
On Thu, Jul 18, 2013 at 2:26 PM, Ben Pfaff wrote:
> We found out earlier that GCC sometimes produces an error only at link time
> for atomic built-ins that are not supported on a platform. This actually
> tries the link at configure time and should thus reliably detect
Ensure that in tunnel.c:handle_offloads() we save off skb->cb before
calling __skb_gso_segment() and restore it after the return.
Signed-off-by: Pravin B Shelar
Signed-off-by: Kyle Mestery
---
Changes in v2:
* No longer save skb->sb in vport_netdev.c:netdev_send().
---
datapath/datapath.c | 4 +
> Here's an incremental which fixes a couple of deadlocks.
Ah getting tired, this doesn't actually fix deadlocks as the mutex is
recursive. Still, I think it's better style to annotate these things,
so I've folded it in.
Ethan
>
> ---
> ofproto/ofproto-dpif-sflow.c | 41 +
Here's an incremental which fixes a couple of deadlocks.
---
ofproto/ofproto-dpif-sflow.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index fc9a836..1176b30 100644
---
Thanks for the review. I'm going through and systematically
annotating the functions, in this file, and already found a deadlock.
I'll send out an incremental.
Ethan
On Tue, Jul 30, 2013 at 10:28 AM, Ben Pfaff wrote:
> On Fri, Jul 26, 2013 at 06:07:11PM -0700, Ethan Jackson wrote:
>> Signed-off
probably this should be an autoconf magic but
1) i'm not familiar with autoconf and 2) _np functions
are inheretly non-portable anyway.
Signed-off-by: YAMAMOTO Takashi
---
lib/util.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/lib/util.c b/lib/util.c
index 6a72107..d719cd3 100644
--
On Jul 30, 2013, at 7:17 PM, Jesse Gross wrote:
> On Fri, Jul 26, 2013 at 2:21 PM, Kyle Mestery wrote:
>> Add support for Linux kernel 3.9.
>>
>> Signed-off-by: Kyle Mestery
>> CC: Pravin B Shelar
>
> I think it's not strictly required to do the restoration here since
> the only user of skb
> GCC says:
> ../ofproto/tunnel.c: In function ‘tnl_port_del’:
> ../ofproto/tunnel.c:191: error: unused variable ‘tnl_port’
Thanks, due to a series of unfortunate events, for a while I had
-Wno-unused on in my compiler and missed this.
> The logging code in tnl_port_receive() is really fu
Perhaps it's premature optimization, but I think bonding is a pretty
important use case, and I didn't want to require an exclusive lock
when running bond_choose_output_slave().
Ethan
On Mon, Jul 29, 2013 at 3:50 PM, Ben Pfaff wrote:
> On Fri, Jul 26, 2013 at 06:07:08PM -0700, Ethan Jackson wrote
Signed-off-by: Ethan Jackson
---
lib/cfm.c | 210 -
lib/cfm.h |3 +-
ofproto/ofproto.c |3 +-
ofproto/ofproto.h |2 +-
vswitchd/bridge.c |2 +
5 files changed, 150 insertions(+), 70 deletions(-)
diff --git a/lib
Signed-off-by: Ethan Jackson
---
This version makes the "all_stps" pointer const. Adds a comment explaining why
we're using a recursive mutex. And systematically marks the helper functions as
requireing a lock.
---
lib/stp.c | 426 +++-
> There are definitely ways around the thread safety analysis. My
> feeling is that we should be pretty strict about it except in modules
> where contention is a real problem.
>
>> Why does stp need a recursive mutex? I don't see any natural
>> recursion here.
So looking back at this, I'd like t
Reviewed-by: Simon Horman
Signed-off-by: Joe Stringer
---
v4: Rebase against megaflow
Move openvswitch.h header changes to datapath patch
v3: Rebase
v2: Fix broken test
Use the correct packet pointer
Calculate checksums as delta from incoming checksum
---
include/sparse/netinet/in.h
Reviewed-by: Simon Horman
Signed-off-by: Joe Stringer
---
v4: No change
v3: Rebase
v2: Add flowmod parse tests
Add set-field tests
Add NEWS item
Add sctp to ovs-ofctl.8
---
NEWS |2 ++
OPENFLOW-1.1+|4 ---
lib/match.c |4 +++
l
This patch adds support for rewriting SCTP src,dst ports similar to the
functionality already available for TCP/UDP.
Rewriting SCTP ports is expensive due to double-recalculation of the
SCTP checksums; this is performed to ensure that packets traversing OVS
with invalid checksums will continue to
This implementation was derived from FreeBSD:
http://code.google.com/p/freebsd-head/source/browse/sys/libkern/crc32.c
Reviewed-by: Simon Horman
Signed-off-by: Joe Stringer
---
v4: Fold in debian license modification
Suppress checksum test sparse warning
v3: Rebase
Return crc32c checksum
This patchset introduces matching and rewriting support for sctp src,dst ports.
Round four does a rebase against the megaflow changes, and shuffles compat code
around a bit. Functionally, we no longer have any code for Xen compatibility
and the skb->ip_summed is no longer modified on port-rewrite.
I'm happy with that, I'll drop this for now.
Ethan
On Mon, Jul 29, 2013 at 2:57 PM, Ben Pfaff wrote:
> On Fri, Jul 26, 2013 at 06:07:07PM -0700, Ethan Jackson wrote:
>> Statistics are the only part of netdev-vport which need to be
>> manipulated by multiple threads. This patch makes them thread
This patch reverts commit 5559942 (ofproto-dpif: GOTO_TABLE recursion
removal.) by reintroducing the recursion through xlate_table_action().
The main reason to do this is the introduction of new rule locking in
future patches. The code before this patch was relatively difficult
to lock in a clean
On Fri, Jul 26, 2013 at 2:21 PM, Kyle Mestery wrote:
> Add support for Linux kernel 3.9.
>
> Signed-off-by: Kyle Mestery
> CC: Pravin B Shelar
I think it's not strictly required to do the restoration here since
the only user of skb->cb in this function block is vlans, which isn't
a problem on t
On Fri, Jul 26, 2013 at 3:15 AM, Thomas Graf wrote:
> On 07/25/13 at 06:39pm, Jesse Gross wrote:
>> On Thu, Jul 25, 2013 at 5:43 AM, Thomas Graf wrote:
>> > From: Thomas Graf
>
> The question is, can we move checksum completion to user space? We only
> need to complete the checksum if the packet
Doh, please ignore this version of the patch. Doesn't pass the unit tests.
Ethan
On Tue, Jul 30, 2013 at 3:31 PM, Ethan Jackson wrote:
> This optimization makes reference counting rules unnecessarily hard in
> future patches. I doubt it makes much performance difference, so this
> patch drops
On Fri, Jul 26, 2013 at 5:01 AM, Jiri Pirko wrote:
> Link upper device properly. That will make IFLA_MASTER filled up.
> Set the master to port 0 of the datapath under which the port belongs.
>
> Signed-off-by: Jiri Pirko
Applied, thanks.
Sorry about the delay - I was waiting to see if the vxla
There was actually a second potential deadlock. I fixed both of them and
systematically annotated the functions, hopefully preventing the problem in the
future.
---
lib/bfd.c | 66 +++--
1 file changed, 38 insertions(+), 28 deletions(-)
This optimization makes reference counting rules unnecessarily hard in
future patches. I doubt it makes much performance difference, so this
patch drops it for now.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-xlate.c | 30 ++
1 file changed, 2 insertions(
> In lacp_unref(), I think we need to move list_remove(&lacp->node);
> inside the lock/unlock. Otherwise we have a race here where
> lacp_find() can grab an entry that is being destroyed.
Good catch. I fixed this, and also systematically added lock
annotations to all of the function definitions
On Tue, Jul 30, 2013 at 02:37:18PM -0700, Gurucharan Shetty wrote:
> Right now, the following 2 lines are how the header and footer
> looks like for ovs-vswitchd.conf.db
>
> @VERSION@(5) Open vSwitch Manual@VERSION@(5)
> Open vSwitch Open_vSwitch @VERSION@(5)
>
> After this comm
Right now, the following 2 lines are how the header and footer
looks like for ovs-vswitchd.conf.db
@VERSION@(5) Open vSwitch Manual@VERSION@(5)
Open vSwitch Open_vSwitch @VERSION@(5)
After this commit, they look like this:
ovs-vswitchd.conf.db(5) Open vSwitch Manual ovs-vswi
On Tue, Jul 30, 2013 at 11:41 AM, Ben Pfaff wrote:
> On Tue, Jul 30, 2013 at 11:10:44AM -0700, Gurucharan Shetty wrote:
> > diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
> > index d2e3f9a..8823ecf 100644
> > --- a/ovsdb/automake.mk
> > +++ b/ovsdb/automake.mk
> > @@ -92,7 +92,7 @@ $(OVSIDL_B
> Why does the client do the locking?
In a couple of places we iterate over each mac_entry by directly
accessing its LRU node. I tried hiding the locking internally, but
believe it or not this turned out to be cleaner. If we could hide the
iteration inside the mac-learning module that would be i
Thanks a lot.
On Tue, Jul 30, 2013 at 11:42 AM, Ben Pfaff wrote:
> On Tue, Jul 30, 2013 at 11:29:19AM -0700, Justin Pettit wrote:
> >
> > On Jul 30, 2013, at 11:17 AM, Ben Pfaff wrote:
> >
> > > On Tue, Jul 30, 2013 at 10:52:34AM -0700, Andy Zhou wrote:
> > >> Remove the DPIF_FP_MODIFY flag wh
On Tue, Jul 30, 2013 at 11:29:19AM -0700, Justin Pettit wrote:
>
> On Jul 30, 2013, at 11:17 AM, Ben Pfaff wrote:
>
> > On Tue, Jul 30, 2013 at 10:52:34AM -0700, Andy Zhou wrote:
> >> Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive
> >> in
> >> a batch, userspace may
On Tue, Jul 30, 2013 at 11:10:44AM -0700, Gurucharan Shetty wrote:
> diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
> index d2e3f9a..8823ecf 100644
> --- a/ovsdb/automake.mk
> +++ b/ovsdb/automake.mk
> @@ -92,7 +92,7 @@ $(OVSIDL_BUILT): ovsdb/ovsdb-idlc.in
> EXTRA_DIST += ovsdb/ovsdb-doc.in
>
Right now, the following 2 lines are how the header and footer
looks like for ovs-vswitchd.conf.db
@VERSION@(5) Open vSwitch Manual@VERSION@(5)
Open vSwitch Open_vSwitch @VERSION@(5)
After this commit, they look like this:
ovs-vswitchd.conf.db(5) Open vSwitch Manual ovs-vswi
On Jul 30, 2013, at 11:17 AM, Ben Pfaff wrote:
> On Tue, Jul 30, 2013 at 10:52:34AM -0700, Andy Zhou wrote:
>> Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
>> a batch, userspace may push down multiple unique flow definitions that
>> overlap when wildcards are app
On Tue, Jul 30, 2013 at 10:52:34AM -0700, Andy Zhou wrote:
> Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
> a batch, userspace may push down multiple unique flow definitions that
> overlap when wildcards are applied. Kernels support flow wildcarding
> will reject th
Thanks Ben,
I'll adjust accordingly in 01/11.
On Tue, Jul 30, 2013 at 10:33 AM, Ben Pfaff wrote:
> After looking over this series, I think that we should make
> ovs_mutex_init() take the type of mutex to create. This would save
> most of the callers creating and destroying their own
> pthread
Signed-off-by: Andy Zhou
---
ofproto/ofproto-dpif.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8c7164b..839de69 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3799,15 +
Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
a batch, userspace may push down multiple unique flow definitions that
overlap when wildcards are applied. Kernels support flow wildcarding
will reject these flow as duplicates (EEXIST), which will be logged
at a lower lo
Sorry should send notice, I figured it out later yesterday.
Thanks Ben,
On Tue, Jul 30, 2013 at 10:48 AM, Ben Pfaff wrote:
> On Mon, Jul 29, 2013 at 05:19:04PM -0700, Alex Wang wrote:
> > On Mon, Jul 29, 2013 at 10:47 AM, Ben Pfaff wrote:
> >
> > > @@ -22,59 +22,55 @@
> > > #include
> > >
On Mon, Jul 29, 2013 at 05:19:04PM -0700, Alex Wang wrote:
> On Mon, Jul 29, 2013 at 10:47 AM, Ben Pfaff wrote:
>
> > @@ -22,59 +22,55 @@
> > #include
> > #include "ovs-atomic.h"
> > #include "util.h"
> > +
> > +/* Mutex. */
> >
> > struct OVS_LOCKABLE ovs_mutex {
> > pthread_mutex_t lo
On Fri, Jul 26, 2013 at 06:07:12PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
After looking over this series, I think that we should make
ovs_mutex_init() take the type of mutex to create. This would save
most of the callers creating and destroying their own
pthread_mutexattr_t.
___
dev mailing list
dev@openvswitch.org
http://open
This is a good idea. I will send out another patch implementing this.
On Tue, Jul 30, 2013 at 10:20 AM, Ben Pfaff wrote:
> On Tue, Jul 30, 2013 at 12:24:43AM -0700, Andy Zhou wrote:
> > Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows
> arrive in
> > a batch, userspace may p
On Fri, Jul 26, 2013 at 06:07:11PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
The change dpif_sflow_run() adds a gratuitous blank line.
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinf
On Tue, Jul 30, 2013 at 12:24:44AM -0700, Andy Zhou wrote:
>
> Signed-off-by: Andy Zhou
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Tue, Jul 30, 2013 at 12:24:43AM -0700, Andy Zhou wrote:
> Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
> a batch, userspace may push down multiple unique flow definitions that
> overlap when wildcards are applied. Kernels support flow wildcarding
> will reject th
They are simple changes, It would be great if you can review them soon.
Justin and Jesse have more background information about the first patch,
but Justin will be busy till this afternoon.
On Tue, Jul 30, 2013 at 9:36 AM, Ben Pfaff wrote:
> On Tue, Jul 30, 2013 at 12:24:43AM -0700, Andy Zhou
On Tue, Jul 30, 2013 at 12:24:43AM -0700, Andy Zhou wrote:
> Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
> a batch, userspace may push down multiple unique flow definitions that
> overlap when wildcards are applied. Kernels support flow wildcarding
> will reject th
Dropping CC to dev-owner, which makes no sense.
On Tue, Jul 30, 2013 at 10:11:26AM +0200, Alessandra Agosta wrote:
> I have created a new kind of message in ofp-msgs.h file. After that, if I
> would like to sent this message from openvswitch to controller, how can I
> do it? I sensed that I should
I have created a new kind of message in ofp-msgs.h file. After that, if I
would like to sent this message from openvswitch to controller, how can I
do it? I sensed that I should modify this files below:
/ lib / poll-loop
/ lib / rconn
/ lib / vconn
I should to implemented a new method in vconn.h/v
Signed-off-by: Andy Zhou
---
ofproto/ofproto-dpif.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 03393cf..f569add 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3799,15 +
Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
a batch, userspace may push down multiple unique flow definitions that
overlap when wildcards are applied. Kernels support flow wildcarding
will reject these flow as duplicates (EEXIST), which will be logged
at a lower lo
From: Jiri Pirko
Date: Fri, 26 Jul 2013 14:01:54 +0200
> Link upper device properly. That will make IFLA_MASTER filled up.
> Set the master to port 0 of the datapath under which the port belongs.
>
> Signed-off-by: Jiri Pirko
Jesse you got this?
___
66 matches
Mail list logo
