Re: [ovs-dev] [sset 1/8] Convert shash users that don't use the 'data' value to sset instead.

2011-03-25 Thread Ben Pfaff
On Fri, Mar 25, 2011 at 06:56:22PM -0700, Ethan Jackson wrote: > I think you neglected to publish the patch[s] before this one that add sset. Oops. That was dumb. Here's the patch: --8<--cut here-->8-- From: Ben Pfaff Date: Fri, 25 Mar 2011 15:

Re: [ovs-dev] [sset 1/8] Convert shash users that don't use the 'data' value to sset instead.

2011-03-25 Thread Ethan Jackson
I think you neglected to publish the patch[s] before this one that add sset. Ethan On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote: > In each of the cases converted here, an shash was used simply to maintain > a set of strings, with the shash_nodes' 'data' values set to NULL.  This > commit con

Re: [ovs-dev] [PATCH] ovs-brcompatd: Delete write-only variable.

2011-03-25 Thread Justin Pettit
Looks good to me. --Justin On Mar 25, 2011, at 3:22 PM, Ben Pfaff wrote: > --- > vswitchd/ovs-brcompatd.c |4 > 1 files changed, 0 insertions(+), 4 deletions(-) > > diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c > index 4a80289..f874a4d 100644 > --- a/vswitchd/ovs-brc

Re: [ovs-dev] [bondlib 02/19] packets: Fix potential use-after-free in compose_benign_packet().

2011-03-25 Thread Ethan Jackson
Looks Good. On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff wrote: > The second call to ofpbuf_put_zeros() could cause the 'eth' pointer to > be invalidated. > > It appears that this does not fix a real bug because the existing callers > all preallocate 128 bytes of tailroom, but the interface doesn'

Re: [ovs-dev] [bondlib 01/19] packets: New function snap_compose(); rename compose_packet() for consistency.

2011-03-25 Thread Ethan Jackson
Looks Good. Ethan On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff wrote: > The following commit will introduce the first use of snap_compose(). > --- >  lib/packets.c     |   52 +--- >  lib/packets.h     |   12 +++- >  ofproto/ofproto.c |    4

[ovs-dev] [PATCH 5/6] bridge: Write CFM updates more quickly.

2011-03-25 Thread Ethan Jackson
This commit causes updates to CFM status to be written immediately. A rate limit of 1 second is introduced to avoid performance problems. --- vswitchd/bridge.c | 56 +++- 1 files changed, 50 insertions(+), 6 deletions(-) diff --git a/vswitchd/brid

[ovs-dev] [PATCH 6/6] cfm: Create new cfm/show appctl command.

2011-03-25 Thread Ethan Jackson
This will be useful for debugging CFM problems in the future. --- lib/cfm.c | 40 +++- lib/cfm.h |3 +++ vswitchd/bridge.c | 29 + vswitchd/ovs-vswitchd.8.in |3 +++ 4 files chang

[ovs-dev] [PATCH 4/6] cfm: Reduce missed CCM detection time.

2011-03-25 Thread Ethan Jackson
The specification says that a fault should be signaled when 3.5 * ccm_interval milliseconds have passed. This commit respects that requirement, possibly increasing the responsiveness of fault detection slightly. --- lib/cfm.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --

[ovs-dev] [PATCH 3/6] cfm: Don't report unexpected remote endpoints.

2011-03-25 Thread Ethan Jackson
Before this patch, CFM would report unexpected remote maintenance points in the database. This commit no longer exposes this information. Information about precisely why a link is faulty is more interesting to a system administrator debugging a problem than a controller which will generally only

[ovs-dev] [PATCH 2/6] cfm: Immediately signal fault on bad CCM reception.

2011-03-25 Thread Ethan Jackson
Commit af5739857a23c29953520e0fe9e79860d12b256c caused the CFM library to immediately signal a fault upon reception of an unexpected remote MPID. This commit does the same for MAIDs, and remote maintenance points with invalid CCM intervals. --- lib/cfm.c | 72 +++

[ovs-dev] [PATCH 1/6] packets: Move CFM related packet information to cfm header file.

2011-03-25 Thread Ethan Jackson
--- lib/cfm.h | 24 ++-- lib/packets.h | 18 -- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/cfm.h b/lib/cfm.h index 555e39b..651b1c4 100644 --- a/lib/cfm.h +++ b/lib/cfm.h @@ -20,14 +20,34 @@ #include #include "hmap.h" -#inc

Re: [ovs-dev] [cfm 1/5] packets: Move CFM related packet information to cfm header file.

2011-03-25 Thread Ethan Jackson
Please ignore this post of the series, I messed up a rebase and squashed two commits together. Ethan On Fri, Mar 25, 2011 at 3:55 PM, Ethan Jackson wrote: > --- >  lib/cfm.h     |   24 ++-- >  lib/packets.h |   18 -- >  2 files changed, 22 insertions(+), 20 de

[ovs-dev] [cfm 5/5] cfm: Create new cfm/show appctl command.

2011-03-25 Thread Ethan Jackson
This will be useful for debugging CFM problems in the future. --- lib/cfm.c | 40 +++- lib/cfm.h |3 +++ vswitchd/bridge.c | 29 + vswitchd/ovs-vswitchd.8.in |3 +++ 4 files chang

[ovs-dev] [cfm 4/5] bridge: Write CFM updates more quickly.

2011-03-25 Thread Ethan Jackson
This commit causes updates to CFM status to be written immediately. A rate limit of 1 second is introduced to avoid performance problems. --- vswitchd/bridge.c | 56 +++- 1 files changed, 50 insertions(+), 6 deletions(-) diff --git a/vswitchd/brid

[ovs-dev] [cfm 3/5] cfm: Reduce missed CCM detection time.

2011-03-25 Thread Ethan Jackson
The specification says that a fault should be signaled when 3.5 * ccm_interval milliseconds have passed. This commit respects that requirement, possibly increasing the responsiveness of fault detection slightly. --- lib/cfm.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --

[ovs-dev] [cfm 2/5] cfm: Immediately signal fault on bad CCM reception.

2011-03-25 Thread Ethan Jackson
Commit af5739857a23c29953520e0fe9e79860d12b256c caused the CFM library to immediately signal a fault upon reception of an unexpected remote MPID. This commit does the same for MAIDs, and remote maintenance points with invalid CCM intervals. --- lib/cfm.c | 111 ++

[ovs-dev] [cfm 1/5] packets: Move CFM related packet information to cfm header file.

2011-03-25 Thread Ethan Jackson
--- lib/cfm.h | 24 ++-- lib/packets.h | 18 -- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/cfm.h b/lib/cfm.h index 555e39b..651b1c4 100644 --- a/lib/cfm.h +++ b/lib/cfm.h @@ -20,14 +20,34 @@ #include #include "hmap.h" -#inc

[ovs-dev] [sset 5/8] ofproto: Change string sets in interface from svec to sset.

2011-03-25 Thread Ben Pfaff
--- ofproto/collectors.c | 20 +++- ofproto/collectors.h |7 --- ofproto/netflow.c |3 +-- ofproto/netflow.h |6 +++--- ofproto/ofproto-sflow.c | 10 +- ofproto/ofproto.c | 23 ++- ofproto/ofproto

[ovs-dev] [sset 6/8] ovs-openflowd: Use sset in place of svec.

2011-03-25 Thread Ben Pfaff
Also deletes svec_split() since this was the only user. --- lib/svec.c| 18 +-- lib/svec.h|3 +- utilities/ovs-openflowd.c | 74 ++-- 3 files changed, 46 insertions(+), 49 deletions(-) diff --git a/lib/svec.c

[ovs-dev] [sset 8/8] ovs-brcompatd: Use sset in place of svec.

2011-03-25 Thread Ben Pfaff
--- vswitchd/ovs-brcompatd.c | 54 - 1 files changed, 24 insertions(+), 30 deletions(-) diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c index f874a4d..3234529 100644 --- a/vswitchd/ovs-brcompatd.c +++ b/vswitchd/ovs-brcompatd.c @@ -51

[ovs-dev] [sset 1/8] Convert shash users that don't use the 'data' value to sset instead.

2011-03-25 Thread Ben Pfaff
In each of the cases converted here, an shash was used simply to maintain a set of strings, with the shash_nodes' 'data' values set to NULL. This commit converts them to use sset instead. --- lib/dpif-linux.c | 18 +- lib/fatal-signal.c| 24 ++ lib/netdev.c

[ovs-dev] [sset 7/8] bridge: Use sset in place of svec.

2011-03-25 Thread Ben Pfaff
--- vswitchd/bridge.c | 11 +-- 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 4815e46..72e9b61 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -367,7 +367,7 @@ static void bridge_configure_once(const struct ovsrec_op

[ovs-dev] [sset 3/8] netdev: Use sset instead of svec in netdev interface.

2011-03-25 Thread Ben Pfaff
--- lib/netdev-linux.c| 12 ++-- lib/netdev-provider.h |8 lib/netdev.c | 31 +++ lib/netdev.h | 10 +- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c inde

[ovs-dev] [sset 2/8] dpif: Use sset instead of svec in dpif interface.

2011-03-25 Thread Ben Pfaff
--- lib/dpif-linux.c |5 ++--- lib/dpif-provider.h |2 +- lib/dpif.c| 16 lib/dpif.h|6 +++--- utilities/ovs-dpctl.c | 42 -- vswitchd/bridge.c | 24 6 files chang

[ovs-dev] [sset 4/8] ovsdb-parser: Use sset instead of svec for detecting unused members.

2011-03-25 Thread Ben Pfaff
Should be slightly cheaper than sorting a list (O(n) vs. O(n lg n)). --- lib/ovsdb-parser.c | 14 ++ lib/ovsdb-parser.h |6 +++--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/ovsdb-parser.c b/lib/ovsdb-parser.c index 2a4c3d9..e1832a9 100644 --- a/lib/ovsdb

[ovs-dev] [sset 0/8] add data type for set of strings

2011-03-25 Thread Ben Pfaff
There's an apparent great need for a "set of strings" data type in the Open vSwitch source tree, since there were many places where either shash or svec was being used as one. This series of commits adds a proper data type for a set of strings and converts all of the places that needed one over to

[ovs-dev] [PATCH] ovs-brcompatd: Delete write-only variable.

2011-03-25 Thread Ben Pfaff
--- vswitchd/ovs-brcompatd.c |4 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c index 4a80289..f874a4d 100644 --- a/vswitchd/ovs-brcompatd.c +++ b/vswitchd/ovs-brcompatd.c @@ -233,14 +233,11 @@ static void do_get_bridge_

Re: [ovs-dev] [PATCH] schema: Monitor's remote_mps is not ephemeral.

2011-03-25 Thread Ethan Jackson
Thanks, merged. Ethan On Thu, Mar 24, 2011 at 8:32 PM, Ben Pfaff wrote: > On Thu, Mar 24, 2011 at 05:55:02PM -0700, Ethan Jackson wrote: >> The remote_mps column of the Monitor table is a configuration >> parameter and should not be ephemeral. > > Looks good, thank you. > ___

[ovs-dev] [bondlib 17/19] bridge: Change "struct dst" from containing a dp_ifidx to a struct iface *.

2011-03-25 Thread Ben Pfaff
The following commit will need to iterate over a set of "struct dst"s, obtaining the iface for each. It could look them up using the hash table that indexes over dp_ifidx, but it's easier if we simply store the iface pointer directly. --- vswitchd/bridge.c | 50 ++---

[ovs-dev] [bondlib 16/19] bridge: Get rid of 'n_ifaces' member of struct port.

2011-03-25 Thread Ben Pfaff
If it doesn't exist then it can't have the wrong value. --- vswitchd/bridge.c | 17 +++-- 1 files changed, 7 insertions(+), 10 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index b6f30fd..222ed41 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -145,7 +145

[ovs-dev] [bondlib 18/19] bridge: Separate mirroring logic from forwarding logic.

2011-03-25 Thread Ben Pfaff
In my opinion this is easier to understand than the way that these two logically separate steps were previously entangled. --- vswitchd/bridge.c | 41 ++--- 1 files changed, 30 insertions(+), 11 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c

[ovs-dev] [bondlib 10/19] bridge: Drop LACP configuration members from struct iface and struct port.

2011-03-25 Thread Ben Pfaff
There's no reason that I can see to maintain this information in struct port and struct iface. It's redundant, since the lacp implementation maintains the same information. --- lib/lacp.c|8 lib/lacp.h|3 + vswitchd/bridge.c | 122 +---

[ovs-dev] [bondlib 14/19] bridge: Simplify and clean up bond slave enable/disable.

2011-03-25 Thread Ben Pfaff
The code that enables and disables bond slaves was a bit of a mess: * Disabling a slave could recursively enable a different slave. * Processing a flow could enable a slave. This commit gets rid of both of those properties, which made it difficult to reason about the code paths along whi

[ovs-dev] [bondlib 01/19] packets: New function snap_compose(); rename compose_packet() for consistency.

2011-03-25 Thread Ben Pfaff
The following commit will introduce the first use of snap_compose(). --- lib/packets.c | 52 +--- lib/packets.h | 12 +++- ofproto/ofproto.c |4 ++-- vswitchd/bridge.c |4 ++-- 4 files changed, 56 insertions(+), 16 deletio

[ovs-dev] [bondlib 19/19] bridge: Avoid partitioning the dst set.

2011-03-25 Thread Ben Pfaff
Scanning the dsts twice seems may be a little more efficient than partitioning it, and it now seems more straightforward to me. --- vswitchd/bridge.c | 73 ++--- 1 files changed, 19 insertions(+), 54 deletions(-) diff --git a/vswitchd/bridge.c b/v

[ovs-dev] [bondlib 07/19] vswitch: Improve schema documentation.

2011-03-25 Thread Ben Pfaff
--- vswitchd/vswitch.xml | 17 + 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 36d08a4..0c57ae5 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -1538,6 +1538,12 @@ + +T

[ovs-dev] [bondlib 06/19] tag: New function tag_set_union().

2011-03-25 Thread Ben Pfaff
--- lib/tag.c | 12 +++- lib/tag.h |3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/tag.c b/lib/tag.c index 0fd0de1..bf2c9a5 100644 --- a/lib/tag.c +++ b/lib/tag.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010 Nicira Networks. + * Copyright (c) 200

[ovs-dev] [bondlib 05/19] list: New functions list_is_singleton(), list_is_short().

2011-03-25 Thread Ben Pfaff
--- lib/list.c| 16 +++- lib/list.h|2 ++ tests/test-list.c |4 +++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/list.c b/lib/list.c index 0f4f2f8..b5fa389 100644 --- a/lib/list.c +++ b/lib/list.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2

[ovs-dev] [bondlib 13/19] bridge: Drop obsolete comment.

2011-03-25 Thread Ben Pfaff
It's quite clear that we don't support double tagging now. --- vswitchd/bridge.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 414b59e..bc700b4 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2703,7 +2703,6 @@ static

[ovs-dev] [bondlib 02/19] packets: Fix potential use-after-free in compose_benign_packet().

2011-03-25 Thread Ben Pfaff
The second call to ofpbuf_put_zeros() could cause the 'eth' pointer to be invalidated. It appears that this does not fix a real bug because the existing callers all preallocate 128 bytes of tailroom, but the interface doesn't document that requirement. --- lib/packets.c | 29 +++

[ovs-dev] [bondlib 04/19] packets: Reserve headroom for VLAN header in eth_compose(), snap_compose().

2011-03-25 Thread Ben Pfaff
This allows callers to add a VLAN header to the composed packet and send it out on a VLAN without copying the whole payload. --- lib/packets.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/packets.c b/lib/packets.c index 4c6cc48..25c5dda 100644 --- a/lib/packet

[ovs-dev] [bondlib 11/19] lacp: Encapsulate configuration into new structs.

2011-03-25 Thread Ben Pfaff
This makes it easier to pass configuration between modules. --- lib/lacp.c| 36 lib/lacp.h| 26 +++--- vswitchd/bridge.c | 49 - 3 files changed, 63 insertions(+), 48 dele

[ovs-dev] [bondlib 12/19] bridge: Improve comment.

2011-03-25 Thread Ben Pfaff
--- vswitchd/bridge.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 1d158e8..414b59e 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -205,7 +205,7 @@ struct bridge { /* Kernel datapath information. */ str

[ovs-dev] [bondlib 08/19] lacp: Fix misleading prototype for lacp_configure().

2011-03-25 Thread Ben Pfaff
Only the first 6 bytes (ETH_ADDR_LEN) of the 'sys_id' argument are used, but the prototype declared it as an array of 8 bytes. This has no effect on the generated code--the declared size of an array parameter is irrelevant--but it is misleading. Also, add 'const' since the array is not modified.

[ovs-dev] [bondlib 09/19] lacp: Remove unneeded forward references from header file.

2011-03-25 Thread Ben Pfaff
--- lib/lacp.h |5 - 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/lib/lacp.h b/lib/lacp.h index 31ac970..8d3bc78 100644 --- a/lib/lacp.h +++ b/lib/lacp.h @@ -21,11 +21,6 @@ #include #include "packets.h" -struct ds; -struct lacp; -struct lacp_pdu; -struct ofpbuf; - /

[ovs-dev] [bondlib 03/19] packets: New function eth_set_vlan_tci(), from dpif-netdev.

2011-03-25 Thread Ben Pfaff
This will soon be used in the upcoming bond library. --- lib/dpif-netdev.c | 32 +--- lib/packets.c | 35 ++- lib/packets.h |2 ++ 3 files changed, 37 insertions(+), 32 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dp

[ovs-dev] [bondlib 00/19] break bonding into separate library

2011-03-25 Thread Ben Pfaff
This series of patches breaks bonding out of vswitchd/bridge.c into a separate library. This should make it easier to move bonding into ofproto in future work. Breaking bonding out is followed by a few patches that do additional cleanups to vswitchd/bridge.c. This patch series is essentially unt

Re: [ovs-dev] [PATCH] datapath: Avoid memory leak in odp_packet_cmd_execute().

2011-03-25 Thread Ben Pfaff
On Thu, Mar 24, 2011 at 07:22:58PM -0700, Jesse Gross wrote: > On Thu, Mar 24, 2011 at 5:07 PM, Ben Pfaff wrote: > > The error path needs to free 'packet'. > > > > Signed-off-by: Ben Pfaff > > Good catch. > > Acked-by: Jesse Gross Thanks, I pushed this. ___