Looks good.
Ethan
On Wed, Oct 19, 2011 at 16:03, Ben Pfaff wrote:
> There's no reason to check for overlapping flows in table A if the flow
> is going to be inserted into table B.
>
> (I doubt anyone actually uses OFPFF_CHECK_OVERLAP though.)
> ---
> ofproto/ofproto.c | 17 ++---
Looks good.
Ethan
On Wed, Oct 19, 2011 at 12:37, Ben Pfaff wrote:
> Some users were still confused by its presence.
> ---
> NEWS | 1 +
> PORTING | 5 -
> tests/.gitignore | 2 -
> tests/automake.mk | 13 -
> tests/learn.at
Looks good.
Ethan
On Thu, Oct 13, 2011 at 11:10, Ben Pfaff wrote:
> This is a nontrivial cross-port of commit 823518f1f "ofproto-dpif: Fix VLAN
> and other field handling in OFPP_NORMAL" from "master" to "branch-1.2".
>
> compose_actions(), which is part of the OFPP_NORMAL implementation, had
>
Looks good.
Ethan
On Thu, Oct 6, 2011 at 11:30, Ben Pfaff wrote:
> Now output that formerly looked like ["map", [["key1", "value1"], ["key2",
> "value2"]]] is printed like {key1=value1, key2=value2}, which I find easier
> to read.
> ---
> ovsdb/ovsdb-tool.c | 48 +
Looks good.
Ethan
On Thu, Oct 6, 2011 at 11:30, Ben Pfaff wrote:
> The "show-log" command tries to give names to the rows to make it easier to
> understand what's going on, but it's still important to see at least
> partial UUIDs so that one can search the output for references to the rows
> by
Looks good.
Ethan
On Fri, Sep 30, 2011 at 11:01, Ben Pfaff wrote:
> This function was only pretty-printing "bad request" error payloads. I
> don't know why. It makes sense to pretty-print all of them except for
> "hello" messages, which already have their own special cases.
>
> Suggestion #736
I don't think you need the extra space before/after square brackets.
Otherwise looks good.
Ethan
On Wed, Sep 28, 2011 at 11:25, Ben Pfaff wrote:
> This moves the OVS log files from the "network-status" capability, which
> has a very small maximum size, to the "system-logs" capability, which is
Looks good.
Ethan
On Thu, Sep 29, 2011 at 11:34, Ben Pfaff wrote:
> Suggested-by: Justin Pettit
> Suggested-by: Michael Mao
> ---
> DESIGN | 22 ++
> 1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/DESIGN b/DESIGN
> index 2e3fced..886994b 100644
> ---
Looks good.
Ethan
On Thu, Aug 25, 2011 at 12:34, Ben Pfaff wrote:
> The coverage of the previous version of this rule was incomplete because
> $(MANS) does not include $(noinst_man_MANS). (Also, $(MANS) is
> undocumented.) Writing it out as the list of manpages variables that
> Open vSwitch u
Looks good.
Ethan
On Thu, Aug 25, 2011 at 12:34, Ben Pfaff wrote:
> The warning is:
> tests/test-openflowd.8:365: warning: macro `DD' not defined
>
> daemon.man allows the file that is including it to include extra
> text in the description of --detach by defining a macro named DD.
> Only
Looks good.
Ethan
On Thu, Aug 25, 2011 at 12:34, Ben Pfaff wrote:
> The manpage name is test-openflowd.8, not ovs-openflowd.8.
>
> This should have caused build errors, I don't know why it didn't.
> ---
> tests/automake.mk | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff -
Should sodepends.pl be copyright 2008 as well as 2011?
I think the indentation is messed up with the OUTER: loop in
sodepends, not sure what the correct style is though so it might be
correct. Quick google search was inconclusive.
Did you intentionally add manpages.mk to the commit? I would thi
On Fri, Oct 21, 2011 at 5:12 PM, Ben Pfaff wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index de2f76b..d3147eb 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -512,24 +512,29 @@ static int validate_actions(const struct nlattr *attr,
> static int validate_s
Looks good.
Ethan
On Thu, Aug 25, 2011 at 12:34, Ben Pfaff wrote:
> Scripts for the build generally go in build-aux, so move soexpand.pl.
> soexpand.pl had the "executable" bit set, but it doesn't have a #! line
> and it's not a shell script, so that didn't make sense.
> ---
> Makefile.am
Looks good.
Ethan
On Thu, Sep 8, 2011 at 12:36, Ben Pfaff wrote:
> Some invalid ports (those above the maximum port number supported by the
> datapath, including OpenFlow reserved ports that are not translated by OVS
> into some other number) will be rejected by the datapath. It's better to
> c
Looks good.
Ethan
On Thu, Sep 8, 2011 at 12:36, Ben Pfaff wrote:
> ---
> include/openflow/nicira-ext.h | 3 +++
> ofproto/ofproto.c | 3 +--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> inde
This looks fine to me.
If FOR_EACH_MATCHING_TABLE is given an invalid TABLE_ID it simply
skips looping. Would it make sense to force callers to check their
TABLE_ID's by replacing the "return NULL" of the else clause of
first_matching_table() to a NOT_REACHED()? I don't feel strongly
about it.
On Fri, Oct 21, 2011 at 4:24 PM, Ben Pfaff wrote:
> On Thu, Oct 20, 2011 at 05:11:33PM -0700, Jesse Gross wrote:
>> On Mon, Oct 17, 2011 at 3:14 PM, Ben Pfaff wrote:
>> > diff --git a/datapath/tunnel.c b/datapath/tunnel.c
>> > index 8edff06..6fde389 100644
>> > --- a/datapath/tunnel.c
>> > +++ b/
Looks good.
Ethan
On Thu, Sep 8, 2011 at 12:36, Ben Pfaff wrote:
> Until now, logging of OpenFlow error replies sent to controllers has been
> haphazard. This commit logs them centrally, ensuring that every OpenFlow
> error sent to a controller is logged.
>
> At the same time, we can eliminate
Bug #7932.
Signed-off-by: Ben Pfaff
---
Seemed like something simple I could write before I left for the day.
Not yet tested.
diff --git a/datapath/datapath.c b/datapath/datapath.c
index de2f76b..d3147eb 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -512,24 +512,29 @@ static int v
On Fri, Oct 21, 2011 at 03:37:31PM -0700, Jesse Gross wrote:
> On Fri, Oct 21, 2011 at 3:30 PM, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff
>
> Acked-by: Jesse Gross
Thank you, I pushed this.
___
dev mailing list
dev@openvswitch.org
http://openvswi
On Fri, Oct 21, 2011 at 04:20:43PM -0700, Justin Pettit wrote:
> On Oct 18, 2011, at 1:23 PM, Ben Pfaff wrote:
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 025cc5b..065acf0 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -967,6 +967,11 @@ set_stp(
On Fri, Oct 21, 2011 at 04:24:09PM -0700, Ben Pfaff wrote:
> Revised patch follows.
No, that was the original patch. Here's the *real* revised patch.
--8<--cut here-->8--
From: Ben Pfaff
Date: Fri, 21 Oct 2011 16:25:49 -0700
Subject: [PATCH] data
On Thu, Oct 20, 2011 at 05:11:33PM -0700, Jesse Gross wrote:
> On Mon, Oct 17, 2011 at 3:14 PM, Ben Pfaff wrote:
> > diff --git a/datapath/tunnel.c b/datapath/tunnel.c
> > index 8edff06..6fde389 100644
> > --- a/datapath/tunnel.c
> > +++ b/datapath/tunnel.c
> > int tnl_set_addr(struct vport *vpor
On Oct 18, 2011, at 1:23 PM, Ben Pfaff wrote:
I've dropped references to the minor, obviously correct issues you pointed out.
> I'd be tempted to use a "struct stp_port *" in place of stp_port_num.
> It might not be worth it. Up to you.
That does make the code cleaner, so I changed it.
> Some
On Fri, Oct 21, 2011 at 3:46 PM, Pravin Shelar wrote:
> On Fri, Oct 21, 2011 at 3:26 PM, Jesse Gross wrote:
>> Commit 4edb9ae90e4092f5f56b9d914d2b88783c49860d "datapath: Refactor
>> actions in terms of match fields." introduced a spurious warning
>> because the compiler thinks a value might not h
On Fri, Oct 21, 2011 at 3:26 PM, Jesse Gross wrote:
> Commit 4edb9ae90e4092f5f56b9d914d2b88783c49860d "datapath: Refactor
> actions in terms of match fields." introduced a spurious warning
> because the compiler thinks a value might not have been assigned to
> 'err'. In practice this can't happen
On Fri, Oct 21, 2011 at 3:30 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
Acked-by: Jesse Gross
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Fri, Oct 21, 2011 at 03:33:40PM -0700, Jesse Gross wrote:
> On Fri, Oct 21, 2011 at 3:28 PM, Ben Pfaff wrote:
> > I originally meant just to fix the use of kfree_skb() instead of
> > consume_skb() on the success path, but then I realized that the failure
> > path returned an skb that it had jus
On Fri, Oct 21, 2011 at 3:28 PM, Ben Pfaff wrote:
> I originally meant just to fix the use of kfree_skb() instead of
> consume_skb() on the success path, but then I realized that the failure
> path returned an skb that it had just freed.
>
> Signed-off-by: Ben Pfaff
Thanks.
Acked-by: Jesse Gros
Signed-off-by: Ben Pfaff
---
datapath/datapath.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 10bf4b9..de2f76b 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -309,7 +309,7 @@ void dp_process_received_packe
On Thu, Oct 20, 2011 at 01:21:51PM -0700, Jesse Gross wrote:
> On Wed, Oct 19, 2011 at 9:46 PM, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff
>
> Acked-by: Jesse Gross
I fixed up the one that this patch originally addressed as part of bug
#7557, so I'm dropping this one.
> > Arguably the call
I originally meant just to fix the use of kfree_skb() instead of
consume_skb() on the success path, but then I realized that the failure
path returned an skb that it had just freed.
Signed-off-by: Ben Pfaff
---
datapath/vport-capwap.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(
Commit 4edb9ae90e4092f5f56b9d914d2b88783c49860d "datapath: Refactor
actions in terms of match fields." introduced a spurious warning
because the compiler thinks a value might not have been assigned to
'err'. In practice this can't happen because we've already validated
the actions.
CC: Pravin B S
On Fri, Oct 21, 2011 at 10:37:17AM -0700, Jesse Gross wrote:
> On Fri, Oct 21, 2011 at 10:18 AM, Ben Pfaff wrote:
> > In testing, I found a bug: "later" fragments were being extracted with
> > a key_len that covered the transport ports, but the key sent to
> > and received back from userspace omit
On Fri, Oct 21, 2011 at 1:39 PM, Jesse Gross wrote:
> On Fri, Oct 21, 2011 at 1:34 PM, Pravin B Shelar wrote:
>> Fixed according to comments from Jesse.
>>
>> --8<--cut here-->8--
>>
>> Following patch cleanup hh_cache access by avoiding hh pointer
On Fri, Oct 21, 2011 at 2:19 PM, Jesse Gross wrote:
> On Fri, Oct 21, 2011 at 2:14 PM, Pravin B Shelar wrote:
>> This is incremental patch to 'Refactor actions' patch posted. Fixed
>> according to comments from Jesse.
>>
>> Signed-off-by: Pravin Shelar
>
> Looks good, thanks.
>
> The whole thing
Looks good.
Ethan
On Thu, Sep 8, 2011 at 12:36, Ben Pfaff wrote:
> ---
> lib/ofp-util.c | 70 ---
> lib/ofp-util.h | 2 +
> 2 files changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 1e95d0
Looks good to me.
I think it would be useful to explain why this was done in the commit message.
I think the MSG_CASE family of macros could benefit from a comment.
I think lining up the sizeof's in the switch statements is going to be
a bit annoying to maintain, I don't feel strongly about it t
On Fri, Oct 21, 2011 at 2:14 PM, Pravin B Shelar wrote:
> This is incremental patch to 'Refactor actions' patch posted. Fixed
> according to comments from Jesse.
>
> Signed-off-by: Pravin Shelar
Looks good, thanks.
The whole thing:
Acked-by: Jesse Gross
This is incremental patch to 'Refactor actions' patch posted. Fixed
according to comments from Jesse.
Signed-off-by: Pravin Shelar
---
datapath/datapath.c | 12 +++-
lib/dpif-netdev.c |1 +
lib/odp-util.c |2 +-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --gi
On Fri, Oct 21, 2011 at 1:34 PM, Pravin B Shelar wrote:
> Fixed according to comments from Jesse.
>
> --8<--cut here-->8--
>
> Following patch cleanup hh_cache access by avoiding hh pointer fetching
> most of time. Now hh is read and checked at begin
Fixed according to comments from Jesse.
--8<--cut here-->8--
Following patch cleanup hh_cache access by avoiding hh pointer fetching
most of time. Now hh is read and checked at beginning of function. All
hh->hh_len access are done inside hh_lock.
Th
On Thu, Oct 20, 2011 at 5:31 PM, Pravin B Shelar wrote:
> Signed-off-by: Pravin Shelar
Acked-by: Jesse Gross
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Thu, Oct 20, 2011 at 5:31 PM, Pravin B Shelar wrote:
> diff --git a/datapath/tunnel.c b/datapath/tunnel.c
> index b750bbc..274542a 100644
> --- a/datapath/tunnel.c
> +++ b/datapath/tunnel.c
> @@ -93,6 +93,19 @@ static unsigned int remote_ports __read_mostly;
> #define rt_dst(rt) (rt->u.dst)
>
On Thu, Oct 20, 2011 at 5:30 PM, Pravin B Shelar wrote:
> Following patch cleanup hh_cache access by avoiding hh
> pointer fetching most of time. Now hh is read and checked at beginning
> of function. All hh->hh_len access are done inside hh_lock.
> This is required cleanup for next patch w
On Thu, Oct 20, 2011 at 4:55 PM, Pravin B Shelar wrote:
> +static int validate_action_key(int act_type, const struct nlattr *ovs_key,
> int act_len,
> + const struct sw_flow_key *flow_key)
> +{
I think it's clearer if you just pass the nlattr for the action since
right now you are
On Fri, Oct 21, 2011 at 10:18 AM, Ben Pfaff wrote:
> In testing, I found a bug: "later" fragments were being extracted with
> a key_len that covered the transport ports, but the key sent to
> and received back from userspace omitted it, so that the flow that
> userspace set up didn't actually matc
On Thu, Oct 20, 2011 at 03:56:41PM -0700, Jesse Gross wrote:
> On Wed, Oct 19, 2011 at 10:56 PM, Ben Pfaff wrote:
> > On Wed, Oct 19, 2011 at 06:09:30PM -0700, Jesse Gross wrote:
> >> Otherwise, the incremental looks good. ??However, I realized that there
> >> is one more issue: when we pass up th
After all I decided that best option actually would be to re-implement the
iperf logic inside the ovs-vlan-test python script itself, because of
following reasons:
1. flag "--tradeoff" (bidirectional, individual test) does not work for
TCP:
http://sourceforge.net/tracker/index.php?func=d
DE Urjente Sr. USMAN ABU
Yo soy el jefe del departamento de cuentas y la intervención que este banco
(BOA BF) Banco de África, aquí en mi país, Burkina Faso África Occidental daa
Guan contect mí y el juego después de una cuidadosa reflexión que usted puede
ser capaz de gestionar esta operac
On Thu, Oct 20, 2011 at 04:55:44PM -0700, Pravin B Shelar wrote:
> changes:
> - datapath validation for SET and PUSH actions.
> - fixed comments.
> - cleanup commit_odp_* functions.
> v2:
> Fixed according to comments from Jesse.
I have no further comments on the userspace
52 matches
Mail list logo
