Re: [ovs-dev] [netdev 23/27] netdev-bsd: Make use of AF_LINK socket thread-safe in NetBSD.

2013-08-01 Thread Ben Pfaff
Thanks for this and the other review. Did you try building it? I have not build-tested any of the changes in this series outside of a GNU/Linux environment. On Fri, Aug 02, 2013 at 02:35:42AM +, YAMAMOTO Takashi wrote: > looks ok to me. > > YAMAMOTO Takashi > > > Signed-off-by: Ben Pfaff

Re: [ovs-dev] [PATCH 2/2] odp-util: Always serialize tunnel mask attributes.

2013-08-01 Thread Andy Zhou
This is a good solution. Thanks. On Thu, Aug 1, 2013 at 6:58 PM, Jesse Gross wrote: > On Thu, Aug 1, 2013 at 4:39 PM, Andy Zhou wrote: > > acked-by: Andy Zhou > > > > > > On Thu, Aug 1, 2013 at 4:17 PM, Jesse Gross wrote: > >> > >> A tunnel value attribute is not allowed to have an empty IP

Re: [ovs-dev] [PATCH v4 0/4] SCTP Support

2013-08-01 Thread Joe Stringer
Thanks Ben. On Fri, Aug 2, 2013 at 3:04 AM, Ben Pfaff wrote: > On Wed, Jul 31, 2013 at 09:31:54AM +0900, Joe Stringer wrote: > > This patchset introduces matching and rewriting support for sctp > > src,dst ports. Round four does a rebase against the megaflow > > changes, and shuffles compat co

Re: [ovs-dev] [netdev 17/27] netdev-bsd: Use xmemdup0() to simplify netdev_bsd_get_next_hop().

2013-08-01 Thread YAMAMOTO Takashi
looks ok to me. YAMAMOTO Takashi > Signed-off-by: Ben Pfaff > CC: Ed Maste > CC: YAMAMOTO Takashi > --- > lib/netdev-bsd.c |7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c > index 2e49c94..6ff6b3e 100644 > --- a/lib/netdev

Re: [ovs-dev] [netdev 23/27] netdev-bsd: Make use of AF_LINK socket thread-safe in NetBSD.

2013-08-01 Thread YAMAMOTO Takashi
looks ok to me. YAMAMOTO Takashi > Signed-off-by: Ben Pfaff > CC: Ed Maste > CC: YAMAMOTO Takashi > --- > lib/netdev-bsd.c | 91 > +- > 1 file changed, 42 insertions(+), 49 deletions(-) > > diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.

[ovs-dev] [tags 3/3] tag: Retire the venerable tag library.

2013-08-01 Thread Ethan Jackson
This patch retires a venerable library whose inception dates before the first patch of the current repository: tags. They have served us well, but their time has come for the reasons listed below. 1) They don't actually help much. In theory, tags had been used to reduce revalidation necessary whe

[ovs-dev] [tags 2/3] bond: Stop using tags.

2013-08-01 Thread Ethan Jackson
This patch transitions bonding away from using tags as required by future patches. Signed-off-by: Ethan Jackson --- lib/bond.c | 156 +- lib/bond.h | 11 ++- ofproto/ofproto-dpif-xlate.c |5 +- ofproto/ofproto-dpi

[ovs-dev] [tags 1/3] mac-learning: Stop using tags.

2013-08-01 Thread Ethan Jackson
This patch transitions mac learning away from using tags as required by future patches. Signed-off-by: Ethan Jackson --- lib/learning-switch.c|9 +++ lib/mac-learning.c | 60 +- lib/mac-learning.h | 22 --

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Unmask mark when used for tunnels.

2013-08-01 Thread Jesse Gross
Thanks, I added a comment and pushed it. On Thu, Aug 1, 2013 at 6:24 PM, Justin Pettit wrote: > Looks good. It might be nice to add a comment stating that only tunnel uses > mark right now, since it's not inherent that mark's are specific to tunnels. > It might act as a good reminder later if

Re: [ovs-dev] [PATCH 2/2] odp-util: Always serialize tunnel mask attributes.

2013-08-01 Thread Jesse Gross
On Thu, Aug 1, 2013 at 4:39 PM, Andy Zhou wrote: > acked-by: Andy Zhou > > > On Thu, Aug 1, 2013 at 4:17 PM, Jesse Gross wrote: >> >> A tunnel value attribute is not allowed to have an empty IP destination >> address but this is legal for masks. This drops both the checks for >> serializing mask

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Unmask mark when used for tunnels.

2013-08-01 Thread Justin Pettit
Looks good. It might be nice to add a comment stating that only tunnel uses mark right now, since it's not inherent that mark's are specific to tunnels. It might act as a good reminder later if/when we use mark other places. Acked-by: Justin Pettit Thanks! --Justin On Aug 1, 2013, at 1:36

Re: [ovs-dev] [PATCH 1/2] datapath: Introduce is_mask when serializing netlink attributes.

2013-08-01 Thread Andy Zhou
acked-by: Andy Zhou This change makes the code easier to read. Thanks. On Thu, Aug 1, 2013 at 4:17 PM, Jesse Gross wrote: > The intention is clearer than if we rederive it in every location. > > Signed-off-by: Jesse Gross > --- > datapath/flow.c | 9 + > 1 file changed, 5 insertions

Re: [ovs-dev] [PATCH 2/2] odp-util: Always serialize tunnel mask attributes.

2013-08-01 Thread Andy Zhou
acked-by: Andy Zhou On Thu, Aug 1, 2013 at 4:17 PM, Jesse Gross wrote: > A tunnel value attribute is not allowed to have an empty IP destination > address but this is legal for masks. This drops both the checks for > serializing masks and also the sanity checks on them. > > Signed-off-by: Jess

Re: [ovs-dev] [PATCH] ofproto-dpif: Hide rule_calculate_tag().

2013-08-01 Thread Ethan Jackson
Thanks, I've merged it. Ethan On Thu, Aug 1, 2013 at 4:19 PM, Ben Pfaff wrote: > On Thu, Aug 01, 2013 at 04:05:38PM -0700, Ethan Jackson wrote: >> No one uses it except ofproto-dpif. >> >> Signed-off-by: Ethan Jackson > > If it compiles, it's golden, thanks. > > Acked-by: Ben Pfaff X-CudaMail-

Re: [ovs-dev] [PATCH] ofproto-dpif: Hide rule_calculate_tag().

2013-08-01 Thread Ben Pfaff
On Thu, Aug 01, 2013 at 04:05:38PM -0700, Ethan Jackson wrote: > No one uses it except ofproto-dpif. > > Signed-off-by: Ethan Jackson If it compiles, it's golden, thanks. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch

[ovs-dev] [PATCH 2/2] odp-util: Always serialize tunnel mask attributes.

2013-08-01 Thread Jesse Gross
A tunnel value attribute is not allowed to have an empty IP destination address but this is legal for masks. This drops both the checks for serializing masks and also the sanity checks on them. Signed-off-by: Jesse Gross --- datapath/flow.c | 5 - lib/odp-util.c | 2 +- 2 files changed, 5 i

[ovs-dev] [PATCH 1/2] datapath: Introduce is_mask when serializing netlink attributes.

2013-08-01 Thread Jesse Gross
The intention is clearer than if we rederive it in every location. Signed-off-by: Jesse Gross --- datapath/flow.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/datapath/flow.c b/datapath/flow.c index 0a6e040..ebe8fb3 100644 --- a/datapath/flow.c +++ b/datapath/flow

[ovs-dev] [PATCH] ofproto-dpif: Hide rule_calculate_tag().

2013-08-01 Thread Ethan Jackson
No one uses it except ofproto-dpif. Signed-off-by: Ethan Jackson --- ofproto/ofproto-dpif.c |4 +++- ofproto/ofproto-dpif.h |3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 50163a5..b04ce66 100644 --- a/ofproto/o

[ovs-dev] [PATCH 3/3] ofproto-dpif: Handle learn action flow mods asynchronously.

2013-08-01 Thread Ethan Jackson
Once we have multiple threads running, having each execute flow mods created by the learn action won't be tenable. It essentially will require us to make the core ofproto module thread safe, which is not the direction we want to go. This patch punts on the problem by handing flow mods to ofproto-

[ovs-dev] [PATCH 2/3] ofproto-dpif-xlate: Take control of the qdscp map.

2013-08-01 Thread Ethan Jackson
This will make locking easier in future patches. Signed-off-by: Ethan Jackson --- ofproto/ofproto-dpif-xlate.c | 88 ++- ofproto/ofproto-dpif-xlate.h |5 +- ofproto/ofproto-dpif.c | 104 +++--- ofproto/ofproto-dpif.

[ovs-dev] [PATCH 1/3] ofproto-dpif-xlate: Pull STP xlation into ofproto-dpif-xlate.

2013-08-01 Thread Ethan Jackson
This patch pulls the STP xlation code into ofproto-dpif-xlate where it will be easier to guard. Signed-off-by: Ethan Jackson --- ofproto/ofproto-dpif-xlate.c | 91 ++ ofproto/ofproto-dpif-xlate.h | 12 +++--- ofproto/ofproto-dpif.c | 50 +--

[ovs-dev] [PATCH] datapath: Use parallel_ops genl.

2013-08-01 Thread Pravin B Shelar
OVS locking was recently changed to have private OVS lock which simplified overall locking. Therefore there is no need to have another global genl lock to protect OVS data structures. Following patch uses of parallel_ops genl family for OVS. This also allows more granual OVS locking using ovs_mu

Re: [ovs-dev] [PATCH] INSTALL, CodingStyle: Recognize that Clang is an acceptable compiler.

2013-08-01 Thread Ben Pfaff
Applied, thanks. On Thu, Aug 01, 2013 at 03:21:20PM -0700, Ethan Jackson wrote: > Acked-by: Ethan Jackson > > > On Thu, Aug 1, 2013 at 2:07 PM, Ben Pfaff wrote: > > Clang has nice static analysis and works well as an Open vSwitch compiler, > > so mention it more explicitly. > > > > Signed-off-

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't try to optimize goto table.

2013-08-01 Thread Ethan Jackson
Thanks On Thu, Aug 1, 2013 at 1:29 PM, Ben Pfaff wrote: > On Thu, Aug 01, 2013 at 01:25:57PM -0700, Ethan Jackson wrote: >> Good Idea, here's a new version. The only change is the unit tests. > > Acked-by: Ben Pfaff X-CudaMail-Whitelist-To: dev@openvswitch.org __

Re: [ovs-dev] [PATCH] INSTALL, CodingStyle: Recognize that Clang is an acceptable compiler.

2013-08-01 Thread Ethan Jackson
Acked-by: Ethan Jackson On Thu, Aug 1, 2013 at 2:07 PM, Ben Pfaff wrote: > Clang has nice static analysis and works well as an Open vSwitch compiler, > so mention it more explicitly. > > Signed-off-by: Ben Pfaff > --- > CodingStyle |9 + > INSTALL | 14 +++--- > 2 fi

[ovs-dev] [PATCH] datapath: Support for Linux kernel 3.10

2013-08-01 Thread Pravin B Shelar
Changes are mostly related API changes in vlan, GRE restructuring. Signed-off-by: Pravin B Shelar --- datapath/actions.c |4 +- datapath/datapath.c|2 +- datapath/linux/Modules.mk |1 + datapath/

[ovs-dev] [netdev 26/27] netdev-linux: Use dedicated netlink notification socket.

2013-08-01 Thread Ben Pfaff
The rtnetlink_link asynchronous netlink notifications seem somewhat troublesome in a threaded environment. It seems more straightforward to have netdev-linux fend for itself. Signed-off-by: Ben Pfaff --- lib/netdev-linux.c | 172 +++- 1 file chan

[ovs-dev] [netdev 21/27] netdev-dummy: Use netdev_get_devices() instead of a local shash.

2013-08-01 Thread Ben Pfaff
When an upcoming commit introduces thread safety into the netdev API, this allows netdev-dummy to avoid adding more internal locking by taking advantage of netdev_get_devices() refcounting. Signed-off-by: Ben Pfaff --- lib/netdev-dummy.c | 48 ++-- 1

[ovs-dev] [netdev 23/27] netdev-bsd: Make use of AF_LINK socket thread-safe in NetBSD.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff CC: Ed Maste CC: YAMAMOTO Takashi --- lib/netdev-bsd.c | 91 +- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 503207f..d365ebf 100644 --- a/lib/netdev-bsd.c

[ovs-dev] [netdev 24/27] netdev: Adopt four-step alloc/construct/destruct/dealloc lifecycle.

2013-08-01 Thread Ben Pfaff
This is the same lifecycle used in the ofproto provider interface. Compared to the previous netdev provider interface, it has the advantage that the netdev top layer can control when any given netdev becomes visible to the outside world. Signed-off-by: Ben Pfaff --- lib/netdev-bsd.c | 116

[ovs-dev] [netdev 25/27] netdev-vport: Make netdev_vport_patch_peer() return a malloc()'d string.

2013-08-01 Thread Ben Pfaff
When threading comes into the picture there arises the possibility of a race between netdev_vport_patch_peer()'s caller using the returned string and another caller changing the peer. It is safer to return a copy. Signed-off-by: Ben Pfaff --- lib/netdev-vport.c | 21 -

[ovs-dev] [netdev 22/27] netdev-linux, netdev-bsd: Make access to AF_INET socket thread-safe.

2013-08-01 Thread Ben Pfaff
The only uses of 'af_inet_sock', in both drivers, were ioctls, so it seemed like a good abstraction to write a function that just does such an ioctl, and to factor out shared code into socket-util. Signed-off-by: Ben Pfaff CC: Ed Maste --- lib/netdev-bsd.c | 84 +

[ovs-dev] [netdev 16/27] netdev-linux: Move variable declaration inward in netdev_linux_cache_cb().

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- lib/netdev-linux.c |4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 29daef8..ba0d863 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -537,7 +537,6 @@ static void netdev_linux_cache_cb(

[ovs-dev] [netdev 20/27] netdev: Make netdev_from_name() take a reference to its returned netdev.

2013-08-01 Thread Ben Pfaff
This API change is necessary for thread safety, to be added in an upcoming commit. Otherwise, the client would not be able to safely use the returned netdev because it could already have been destroyed. Signed-off-by: Ben Pfaff --- lib/netdev-linux.c |1 + lib/netdev.c | 17

[ovs-dev] [netdev 17/27] netdev-bsd: Use xmemdup0() to simplify netdev_bsd_get_next_hop().

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff CC: Ed Maste CC: YAMAMOTO Takashi --- lib/netdev-bsd.c |7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 2e49c94..6ff6b3e 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -1312,12 +1312,9 @@ ne

[ovs-dev] [netdev 19/27] netdev: Make netdev_get_devices() take a reference to each netdev.

2013-08-01 Thread Ben Pfaff
This API change is necessary for thread safety, to be added in an upcoming commit. Otherwise, the client would not be able to actually use any of the returned netdevs because they could already have been destroyed. Signed-off-by: Ben Pfaff --- lib/netdev-bsd.c | 32 ++---

[ovs-dev] [netdev 13/27] netdev-linux: Remove pointless layers of indirection for tap devices.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- lib/netdev-linux.c | 42 +- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index cc86ec6..768b4e8 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -120,10 +12

[ovs-dev] [netdev 18/27] netdev-provider: Remove unused function netdev_assert_class().

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- lib/netdev-provider.h |6 -- 1 file changed, 6 deletions(-) diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index 137b659..19230a1 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -50,12 +50,6 @@ struct netdev *netdev_from_name(con

[ovs-dev] [netdev 15/27] netdev-linux: Remove useless member 'peer', which was always zero.

2013-08-01 Thread Ben Pfaff
Always, correct a comment on netdev_linux_get_features(). Signed-off-by: Ben Pfaff --- lib/netdev-linux.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 19e39bd..29daef8 100644 --- a/lib/netdev-linux.c +++ b/lib/n

[ovs-dev] [netdev 09/27] netdev-linux, netdev-bsd: Don't assume 'struct netdev' has offset 0.

2013-08-01 Thread Ben Pfaff
The data items returned by netdev_get_devices() are "struct netdev *"s. The code fixed up by this commit used them as "struct netdev_bsd *" or "struct netdev_linux *", which happens to work because struct netdev happens to be at offset 0 in each struct but it's better to do a proper cast in case so

[ovs-dev] [netdev 14/27] netdev-linux: Remove unused struct netdev_linux member.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- lib/netdev-linux.c |1 - 1 file changed, 1 deletion(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 768b4e8..19e39bd 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -355,7 +355,6 @@ static int tc_calc_buffer(unsigned int Bps, int mtu, u

[ovs-dev] [netdev 10/27] netdev: Minor formatting improvements.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- lib/netdev-linux.c |3 +-- lib/netdev.c |6 ++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 29d8ad9..cc86ec6 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -1424,8 +1424,7 @

[ovs-dev] [netdev 06/27] netdev-linux: Fix fd leak on error path.

2013-08-01 Thread Ben Pfaff
Found by inspection. Signed-off-by: Ben Pfaff --- lib/netdev-linux.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 301a754..0baa40f 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -672,19 +672,21 @@ netdev_l

[ovs-dev] [netdev 11/27] netdev-vport: Use ovs_mutex rather than a raw pthread_mutex_t.

2013-08-01 Thread Ben Pfaff
I'd forgotten even to use the xpthread variants here. Signed-off-by: Ben Pfaff --- lib/netdev-vport.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 4214b38..14b3347 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport

[ovs-dev] [netdev 12/27] netdev-linux: Remove unneeded struct forward declarations from header.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff --- lib/netdev-linux.h |2 -- 1 file changed, 2 deletions(-) diff --git a/lib/netdev-linux.h b/lib/netdev-linux.h index e404e46..7874dd6 100644 --- a/lib/netdev-linux.h +++ b/lib/netdev-linux.h @@ -24,8 +24,6 @@ * Linux-specific code. */ struct netdev; -struct n

[ovs-dev] [netdev 08/27] netdev-dummy: Fix memory leak on error path in netdev_rx_dummy_recv().

2013-08-01 Thread Ben Pfaff
This code failed to free the packet if it was too big for the caller. Signed-off-by: Ben Pfaff --- lib/netdev-dummy.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index c4f58b7..e7dfe9f 100644 --- a/lib/netdev-dummy.

[ovs-dev] [netdev 07/27] netdev-linux: Initialize change_seq for tap devices too.

2013-08-01 Thread Ben Pfaff
change_seq is supposed to always be nonzero but tap devices got this wrong. Signed-off-by: Ben Pfaff --- lib/netdev-linux.c |1 + 1 file changed, 1 insertion(+) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 0baa40f..55f676a 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux

[ovs-dev] [netdev 05/27] netdev-bsd: Correctly handle IPv4 netmasks.

2013-08-01 Thread Ben Pfaff
netdev_bsd_get_in4() did not set anything in its 'netmask' output argument if the IPv4 address was cached, leaving it indeterminate. It would also mark the cache as valid even if there was an error retrieving the netmask. This fixes both problems. Found by inspection. Signed-off-by: Ben Pfaff C

[ovs-dev] [netdev 04/27] netdev-bsd: Fix fd leak on error path.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff CC: Ed Maste --- lib/netdev-bsd.c |1 + 1 file changed, 1 insertion(+) diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 903659f..65e1d25 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -370,6 +370,7 @@ netdev_bsd_create_tap(const struct netdev_class *c

[ovs-dev] [netdev 02/27] netdev-bsd: Fix memory leak on error path.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff CC: Ed Maste --- lib/netdev-bsd.c |1 + 1 file changed, 1 insertion(+) diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 401d03a..8605182 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -323,6 +323,7 @@ netdev_bsd_create_system(const struct netdev_class

[ovs-dev] [netdev 01/27] dpif-linux: Fix theoretical memory leak on error path.

2013-08-01 Thread Ben Pfaff
If a notification is bigger than 4 kB (I doubt it one would be), then the lack of ofpbuf_uninit() would cause a memory leak. Signed-off-by: Ben Pfaff --- lib/dpif-linux.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c

[ovs-dev] [netdev 03/27] netdev-bsd: Fix typo in label name.

2013-08-01 Thread Ben Pfaff
Signed-off-by: Ben Pfaff CC: Ed Maste --- lib/netdev-bsd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 8605182..903659f 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -363,14 +363,14 @@ netdev_bsd_create_ta

[ovs-dev] [netdev 00/27] make netdev thread-safe

2013-08-01 Thread Ben Pfaff
The point of this series is to make the netdev layer and its implementations thread-safe, but along the way I found lots of bugs to fix and minor improvements. Ben Pfaff (27): dpif-linux: Fix theoretical memory leak on error path. netdev-bsd: Fix memory leak on error path. netdev-bsd: Fix ty

[ovs-dev] [PATCH] INSTALL, CodingStyle: Recognize that Clang is an acceptable compiler.

2013-08-01 Thread Ben Pfaff
Clang has nice static analysis and works well as an Open vSwitch compiler, so mention it more explicitly. Signed-off-by: Ben Pfaff --- CodingStyle |9 + INSTALL | 14 +++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/CodingStyle b/CodingStyle index

[ovs-dev] [PATCH] ofproto-dpif-xlate: Unmask mark when used for tunnels.

2013-08-01 Thread Jesse Gross
The tunnel lookup uses the skb_mark as part of the port find process but it isn't unmasked along with the other fields. This adds it to the list of significant fields. Signed-off-by: Jesse Gross --- ofproto/ofproto-dpif-xlate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ofproto/ofproto-

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't try to optimize goto table.

2013-08-01 Thread Ben Pfaff
On Thu, Aug 01, 2013 at 01:25:57PM -0700, Ethan Jackson wrote: > Good Idea, here's a new version. The only change is the unit tests. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't try to optimize goto table.

2013-08-01 Thread Ethan Jackson
Good Idea, here's a new version. The only change is the unit tests. --- ofproto/ofproto-dpif-xlate.c | 30 ++ tests/ofproto-dpif.at|4 ++-- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpi

Re: [ovs-dev] [netlink mask exceptions v2 3/3] datapath: Accept any 802.2 eth_type mask but override to be exact match

2013-08-01 Thread Jesse Gross
On Thu, Aug 1, 2013 at 10:49 AM, Andy Zhou wrote: > When key.eth_type is absent it is interpreted to be 802.2. In this > case, the eth_type mask, if supplied, will be overridden to be exact > match, regardless of the value supplied. > > Signed-off-by: Andy Zhou > --- > datapath/flow.c | 13 +++

Re: [ovs-dev] [netlink mask exceptions v2 2/3] datapath: Accept any in_port mask but override to be exact match

2013-08-01 Thread Jesse Gross
On Thu, Aug 1, 2013 at 10:49 AM, Andy Zhou wrote: > Pre mega flow, netlink allows the in_port key attribute > to be missing. Missing in_port is interpreted as DP_MAX_PORTS. > > For backward compatibility, mega flow implementation will always allow > the mask of in_port to be specified, as if the i

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't try to optimize goto table.

2013-08-01 Thread Ben Pfaff
On Thu, Aug 01, 2013 at 01:00:11PM -0700, Ethan Jackson wrote: > > I'm OK with changing the implementation, but I don't like the idea of > > the externally visible behavior changing. What if, instead of doing > > this iteratively, we simply don't penalize goto_table actions with > > taking up a le

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't try to optimize goto table.

2013-08-01 Thread Ethan Jackson
> I'm OK with changing the implementation, but I don't like the idea of > the externally visible behavior changing. What if, instead of doing > this iteratively, we simply don't penalize goto_table actions with > taking up a level of resubmit? We limit the levels of resubmit to > avoid loops, but

Re: [ovs-dev] [PATCH] BFD: Edit the unit test time/stop command

2013-08-01 Thread Ethan Jackson
Could I have your signed-off-by? Acked-by: Ethan Jackson X-CudaMail-Whitelist-To: dev@openvswitch.org ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [thread-safety 08/11] mac-learning: Make the mac-learning module thread safe.

2013-08-01 Thread Ethan Jackson
> I actually like data structures that are locked externally, when it is > possible, because it gives additional flexibility. You can, for > example, implement multi-object transactions without additional cost, > or you can leave out the locking entirely if it isn't needed. In the > past, I've or

Re: [ovs-dev] [thread-safety 02/11 v2] stp: Make the STP module thread safe.

2013-08-01 Thread Ethan Jackson
Thanks for the review, I'll merge this soon. Ethan On Thu, Aug 1, 2013 at 11:06 AM, Ben Pfaff wrote: > On Tue, Jul 30, 2013 at 05:59:10PM -0700, Ethan Jackson wrote: >> Signed-off-by: Ethan Jackson >> --- >> >> This version makes the "all_stps" pointer const. Adds a comment >> explaining why

Re: [ovs-dev] [netlink mask exceptions v2 2/3] datapath: Accept any in_port mask but override to be exact match

2013-08-01 Thread Ethan Jackson
Thanks for the additional explanation in the commit message. Ethan On Thu, Aug 1, 2013 at 10:49 AM, Andy Zhou wrote: > Pre mega flow, netlink allows the in_port key attribute > to be missing. Missing in_port is interpreted as DP_MAX_PORTS. > > For backward compatibility, mega flow implementation

Re: [ovs-dev] [thread-safety 08/11] mac-learning: Make the mac-learning module thread safe.

2013-08-01 Thread Ben Pfaff
On Tue, Jul 30, 2013 at 02:37:38PM -0700, Ethan Jackson wrote: > > 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 w

Re: [ovs-dev] [thread-safety 07/11] bond: Make the bond module thread safe.

2013-08-01 Thread Ben Pfaff
Fair enough. Acked-by: Ben Pfaff On Tue, Jul 30, 2013 at 06:48:42PM -0700, Ethan Jackson wrote: > 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 > > O

Re: [ovs-dev] [thread-safety 05/11] bfd: Make the BFD module thread safe.

2013-08-01 Thread Ben Pfaff
On Tue, Jul 30, 2013 at 04:02:05PM -0700, Ethan Jackson wrote: > There was actually a second potential deadlock. I fixed both of them and > systematically annotated the functions, hopefully preventing the problem in > the > future. Thanks for fixing these. I only skimmed the incremental, but: A

Re: [ovs-dev] [thread-safety 03/11 v2] cfm: Make the CFM module thread safe.

2013-08-01 Thread Ben Pfaff
On Tue, Jul 30, 2013 at 06:46:37PM -0700, Ethan Jackson wrote: > Signed-off-by: Ethan Jackson I checked for some of the changes that I requested and I trust that you fixed the rest too. Thanks. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch

Re: [ovs-dev] [thread-safety 02/11 v2] stp: Make the STP module thread safe.

2013-08-01 Thread Ben Pfaff
On Tue, Jul 30, 2013 at 05:59:10PM -0700, Ethan Jackson wrote: > 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. You'll n

Re: [ovs-dev] [thread-safety 02/11] stp: Make the STP module thread safe.

2013-08-01 Thread Ben Pfaff
On Tue, Jul 30, 2013 at 05:50:02PM -0700, Ethan Jackson wrote: > > 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'

Re: [ovs-dev] [PATCH v4 0/4] SCTP Support

2013-08-01 Thread Ben Pfaff
On Wed, Jul 31, 2013 at 09:31:54AM +0900, Joe Stringer wrote: > 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 compa

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Don't try to optimize goto table.

2013-08-01 Thread Ben Pfaff
On Tue, Jul 30, 2013 at 05:29:10PM -0700, Ethan Jackson wrote: > 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.

Re: [ovs-dev] Brcompat module into Ubuntu 13.04

2013-08-01 Thread Ben Pfaff
On Thu, Aug 01, 2013 at 07:11:25PM +0200, Arturo Mart?n wrote: > Then, there is no way to use brctl like ovs-vsctl command? There is no way to use brctl to control OVS 1.10 or later. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman

[ovs-dev] [netlink mask exceptions v2 3/3] datapath: Accept any 802.2 eth_type mask but override to be exact match

2013-08-01 Thread Andy Zhou
When key.eth_type is absent it is interpreted to be 802.2. In this case, the eth_type mask, if supplied, will be overridden to be exact match, regardless of the value supplied. Signed-off-by: Andy Zhou --- datapath/flow.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) dif

[ovs-dev] [netlink mask exceptions v2 2/3] datapath: Accept any in_port mask but override to be exact match

2013-08-01 Thread Andy Zhou
Pre mega flow, netlink allows the in_port key attribute to be missing. Missing in_port is interpreted as DP_MAX_PORTS. For backward compatibility, mega flow implementation will always allow the mask of in_port to be specified, as if the in_port key attribute is always specified. To prevent accide

[ovs-dev] [netlink mask exceptions v2 1/3] datapath: Always allow tunnel mask to be specified in the netlink

2013-08-01 Thread Andy Zhou
Netlink message usually only accpets a mask when there is a corresponding key attribute. Tunnel mask and eth_type are the only two expections so far. Signed-off-by: Andy Zhou --- datapath/flow.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/datapath/flow.c b/datapath/flow.c index 62f

Re: [ovs-dev] Brcompat module into Ubuntu 13.04

2013-08-01 Thread Arturo Martín
Thanks Ben, Then, there is no way to use brctl like ovs-vsctl command? 2013/8/1 Ben Pfaff > On Thu, Aug 01, 2013 at 06:04:41PM +0200, Arturo Mart?n wrote: > > I am trying to use brcompat module like i was using into Ubuntu 12.10, > but > > it dosn't works. > > brcompat was removed from OVS 1.1

Re: [ovs-dev] [PATCH] fix compilation on FreeBSD/NetBSD

2013-08-01 Thread Ben Pfaff
On Thu, Aug 01, 2013 at 09:31:15AM +, YAMAMOTO Takashi wrote: > > On Wed, Jul 31, 2013 at 11:01:43AM +0900, YAMAMOTO Takashi wrote: > >> probably this should be an autoconf magic but > >> 1) i'm not familiar with autoconf and 2) _np functions > >> are inheretly non-portable anyway. > >> > >> S

Re: [ovs-dev] [PATCH 2/3] datapath: Accept any in_port mask but override to be exact match

2013-08-01 Thread Andy Zhou
Thanks for the review. I will send out a V2. addressing your comments. On Thu, Aug 1, 2013 at 9:19 AM, Jesse Gross wrote: > On Wed, Jul 31, 2013 at 8:39 PM, Andy Zhou wrote: > > diff --git a/datapath/flow.c b/datapath/flow.c > > index 84df4d3..a2111e7 100644 > > --- a/datapath/flow.c > > +++ b

Re: [ovs-dev] Brcompat module into Ubuntu 13.04

2013-08-01 Thread Ben Pfaff
On Thu, Aug 01, 2013 at 06:04:41PM +0200, Arturo Mart?n wrote: > I am trying to use brcompat module like i was using into Ubuntu 12.10, but > it dosn't works. brcompat was removed from OVS 1.10. If you are using 1.10 or later, you cannot use brcompat. _

Re: [ovs-dev] [PATCH 1/3] datapath: Always allow tunnel mask to be specified in the netlink

2013-08-01 Thread Jesse Gross
On Wed, Jul 31, 2013 at 8:39 PM, Andy Zhou wrote: > Netlink message usually only accpets a mask when there is a > corresponding key attribute. Tunnel mask and eth_type are the > only two expections so far. > > Signed-off-by: Andy Zhou Applied, thanks.

Re: [ovs-dev] [PATCH 3/3] datapath: Accept any 802.2 eth_type mask but override to be exact match

2013-08-01 Thread Jesse Gross
On Wed, Jul 31, 2013 at 8:39 PM, Andy Zhou wrote: > diff --git a/datapath/flow.c b/datapath/flow.c > index a2111e7..6fc09c2 100644 > --- a/datapath/flow.c > +++ b/datapath/flow.c > @@ -1401,6 +1401,9 @@ static int ovs_key_from_nlattrs(struct sw_flow_match > *match, u64 attrs, > S

Re: [ovs-dev] [PATCH 2/3] datapath: Accept any in_port mask but override to be exact match

2013-08-01 Thread Jesse Gross
On Wed, Jul 31, 2013 at 8:39 PM, Andy Zhou wrote: > diff --git a/datapath/flow.c b/datapath/flow.c > index 84df4d3..a2111e7 100644 > --- a/datapath/flow.c > +++ b/datapath/flow.c > @@ -138,8 +138,7 @@ static bool ovs_match_validate(const struct sw_flow_match > *match, > /* Tunnel mask is

[ovs-dev] Brcompat module into Ubuntu 13.04

2013-08-01 Thread Arturo Martín
Hi all, I am trying to use brcompat module like i was using into Ubuntu 12.10, but it dosn't works. I am looking several how to on the net but brcompat module never is running. Please, could you help me on this? Many thanks in advance! Kind regards. -- Arturo. ___

Re: [ovs-dev] [PATCH] fix compilation on FreeBSD/NetBSD

2013-08-01 Thread YAMAMOTO Takashi
> On Wed, Jul 31, 2013 at 11:01:43AM +0900, YAMAMOTO Takashi wrote: >> 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 > > I'd rather do it "the right way". I sent