On Wed, Mar 2, 2011 at 2:31 PM, Ben Pfaff wrote:
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 486ba48..daa260d 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -944,6 +944,7 @@ dpif_netdev_flow_dump_done(const struct dpif *dpif
> OVS_UNUSED, void *state_)
>
> stati
Looks Good.
I didn't closely review the tests.
Ethan
On Mon, Mar 28, 2011 at 1:07 PM, Ben Pfaff wrote:
> When ovsdb-server reads a database file that is corrupted at the
> transaction level (that is, the transaction is valid JSON and has the
> correct SHA-1 hash, but it does not describe a vali
Didn't closely review the test. Looks good assuming it passes.
Ethan
On Mon, Mar 28, 2011 at 1:07 PM, Ben Pfaff wrote:
> When ovsdb-server reads a database that is corrupted at the log level
> (that is, when ovsdb_log detects the corruption by checking the SHA-1 hash
> of the record or JSON par
Looks Good.
On Mon, Mar 28, 2011 at 1:07 PM, Ben Pfaff wrote:
> If there's database corruption then it indicates that something went wrong,
> e.g. the machine was powered-off by power failure. It's definitely
> something that the admin should know about. This sounds like an error to
> me, so us
Looks Good.
> - The "type" specifies the type of data stored in this column. If
> - "ephemeral" is specified as true, then this column's values are
> + The "type" specifies the type of data stored in this column.
Trailing whitespace.
___
dev m
On Wed, Mar 30, 2011 at 04:14:44PM -0700, Jesse Gross wrote:
> On Wed, Mar 2, 2011 at 2:30 PM, Ben Pfaff wrote:
> > My original intent for ofpbufs initialized with ofpbuf_use_stack() was that
> > the caller was providing enough space on the stack for the common case,
> > with dynamic allocation as
On Wed, Mar 2, 2011 at 2:30 PM, Ben Pfaff wrote:
> My original intent for ofpbufs initialized with ofpbuf_use_stack() was that
> the caller was providing enough space on the stack for the common case,
> with dynamic allocation as a fallback. But in practice, none of the
> clients actually do this
On Wed, Mar 2, 2011 at 2:30 PM, Ben Pfaff wrote:
> This seems to me to better encapsulate the inherent ugliness.
Looks good.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Wed, Mar 30, 2011 at 02:08:11PM -0700, Jesse Gross wrote:
> On Wed, Mar 2, 2011 at 4:49 PM, Ben Pfaff wrote:
> > We've noticed that packets that go up to userspace and then back down to
> > the kernel and then enter an GRE tunnel that is then ESP encapsulated
> > by IPSEC end up with a bad ESP
Looks Good.
On Mon, Mar 28, 2011 at 1:19 PM, Ben Pfaff wrote:
> "urgent" probably overstates the case. It would have prevented the
> problems that you fixed with your commit that makes remote_mps
> persistent, but now that it's persistent anyway it will only prevent
> that kind of thing from rec
Looks Good
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> ---
> 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 1
Looks Good.
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> ---
> 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.
Looks Good.
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> Also deletes svec_split() since this was the only user.
> ---
> lib/svec.c | 18 +--
> lib/svec.h | 3 +-
> utilities/ovs-openflowd.c | 74 ++--
>
Looks Good.
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> ---
> ofproto/collectors.c | 20 +++-
> ofproto/collectors.h | 7 ---
> ofproto/netflow.c | 3 +--
> ofproto/netflow.h | 6 +++---
> ofproto/ofproto-sflow.c | 10 +
Looks Good.
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> 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/ovsd
Looks Good.
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> ---
> lib/netdev-linux.c | 12 ++--
> lib/netdev-provider.h | 8
> lib/netdev.c | 31 +++
> lib/netdev.h | 10 +-
> 4 files changed, 30 insertions(+)
timems_t sounds good to me. I don't think I suggested it, but I'll
take credit =).
Ethan
On Wed, Mar 30, 2011 at 2:22 PM, Ben Pfaff wrote:
> On Wed, Mar 30, 2011 at 02:11:29PM -0700, Ethan Jackson wrote:
>> > I think it's a good idea if it makes code more obviously correct and
>> > doesn't add
On Wed, Mar 30, 2011 at 02:11:29PM -0700, Ethan Jackson wrote:
> > I think it's a good idea if it makes code more obviously correct and
> > doesn't add significant overhead. ?What do you have in mind?
>
> I haven't thought through it entirely but basically a new timer_t type
> which would be typde
> I think it's a good idea if it makes code more obviously correct and
> doesn't add significant overhead. What do you have in mind?
I haven't thought through it entirely but basically a new timer_t type
which would be typdefed as a long long int. A function to set the
timer for some number of s
On Wed, Mar 2, 2011 at 4:49 PM, Ben Pfaff wrote:
> We've noticed that packets that go up to userspace and then back down to
> the kernel and then enter an GRE tunnel that is then ESP encapsulated
> by IPSEC end up with a bad ESP "next header" value: it ends up as zero
> instead of 0x2f (IPPROTO_GR
On Wed, Mar 30, 2011 at 01:55:53PM -0700, Ethan Jackson wrote:
> Nice catch, how did you notice this?
Testing the bonding library, the buggy version caused an assertion
failure in mac_learning_lookup() whenever a gratuitous ARP appeared,
because update_learning_table() would skip calling
mac_learn
Looks good to me. I don't look at it super closely as it's so straight forward.
Ethan
On Fri, Mar 25, 2011 at 3:40 PM, Ben Pfaff wrote:
> ---
> lib/dpif-linux.c | 5 ++---
> lib/dpif-provider.h | 2 +-
> lib/dpif.c | 16
> lib/dpif.h | 6
Nice catch, how did you notice this?
Incidentally, I've been thinking about if it's a good idea to
implement a timer library which may prevent errors like this in the
future.
Ethan
On Wed, Mar 30, 2011 at 1:47 PM, Ben Pfaff wrote:
> The lock is asserted if its expiration time has not arrived ye
The lock is asserted if its expiration time has not arrived yet, not the
reverse.
---
lib/mac-learning.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/mac-learning.h b/lib/mac-learning.h
index 16c92db..51a7ac7 100644
--- a/lib/mac-learning.h
+++ b/lib/mac-learning.h
On Wed, Mar 30, 2011 at 11:24:35AM -0700, Ethan Jackson wrote:
> Looks Good.
Thank you for the reviews. I'll push these two patches soon.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Mon, Mar 28, 2011 at 01:43:40PM -0700, Ben Pfaff wrote:
> When poll interval-based logging was introduced a long time, we were
> actively interested in looking at almost every long poll interval. But
> these days, with OVS working rather well, with pretty good latency, most
> of the messages ar
Looks Good.
On Mon, Mar 28, 2011 at 1:43 PM, Ben Pfaff wrote:
> When poll interval-based logging was introduced a long time, we were
> actively interested in looking at almost every long poll interval. But
> these days, with OVS working rather well, with pretty good latency, most
> of the messag
Looks Good.
On Tue, Mar 29, 2011 at 12:31 PM, Ben Pfaff wrote:
> ---
> ofproto/ofproto.c | 16 +---
> 1 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index ce8c99b..dc5c915 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/o
Looks Good
On Tue, Mar 29, 2011 at 12:31 PM, Ben Pfaff wrote:
> Nothing uses these anymore. ofputil_decode_flow_stats_reply() is a better
> alternative.
> ---
> lib/ofp-util.c | 45 -
> lib/ofp-util.h | 7 ---
> 2 files changed, 0 insertions(
Looks Good.
On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff wrote:
> 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
Looks Good.
On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff wrote:
> 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 insertio
On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff wrote:
> 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.
In
Looks Good.
On Fri, Mar 25, 2011 at 10:35 AM, Ben Pfaff wrote:
> 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.
Sounds good. Thanks for the explanation.
Ethan
On Wed, Mar 30, 2011 at 10:04 AM, Ben Pfaff wrote:
> On Tue, Mar 29, 2011 at 02:56:13PM -0700, Ethan Jackson wrote:
>> I don't have my mind completely wrapped around this. But it seems
>> like it might be a good idea to ofproto_revalidate old_acti
On Tue, Mar 29, 2011 at 02:56:13PM -0700, Ethan Jackson wrote:
> I don't have my mind completely wrapped around this. But it seems
> like it might be a good idea to ofproto_revalidate old_active_iface if
> it exists. Similarly we may want to revalidate the new active_iface.
> This may be automati
Thanks, I pushed this.
(Why oh why can't the BSDs seem to get header files right?)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
diff -ubrw a/lib/stream-ssl.c b/lib/stream-ssl.c
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -22,6 +22,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
diff -ubrw a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
--- a/utilities/ovs-ofctl.c
+++ b/util
37 matches
Mail list logo
