this fixes ofp_port_status delivery on a patch port removal.
Signed-off-by: YAMAMOTO Takashi
---
lib/dpif.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/dpif.c b/lib/dpif.c
index 02cc36a..ece5dc0 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -536,6 +536,9 @@ dpif_port_del(struct dpif
Hi Jesse,
Thanks for your quick reply :)
On 26 August 2013 22:16, Jesse Gross wrote:
> Please look at the implementation of the loop counter.
Sorry but I couldn't find anything interesting/tricky in implementation
of loop counter.. :(
struct loop_counter {
u8 count; /* Count. */
bool
On 26 August 2013 21:59, Jesse Gross wrote:
> This is compatibility code for older kernels so by definition it
> doesn't exist in current kernels. You can't just remove it.
My mistake :(
Will something like this makes sense? This was changed in 2.6.22 by
following patch:
b0e380b [SK_BUFF]: unio
On Tue, Aug 27, 2013 at 04:10:03PM +0900, YAMAMOTO Takashi wrote:
> this fixes ofp_port_status delivery on a patch port removal.
Can you explain further? I don't see any problems with patch port
removal. Example:
blp@sigsegv:~/ovs/_clang$ make sandbox
...
---
On Mon, Aug 26, 2013 at 01:39:37PM -0700, Jarno Rajahalme wrote:
> Remove push_vlan from an OF1.0 test, as it requires OF1.1+ support, but was
> silently discarded. A later patch will make this test to fail due to
> validation of usable OpenFlow protocol versions while parsing actions.
>
> It sho
Signed-off-by: Ben Pfaff
---
lib/classifier.c |3 +-
lib/flow.c | 126 ++
lib/flow.h | 10 +++--
lib/match.c | 31 +-
lib/match.h | 18 +---
5 files changed, 147 insertions(+), 41 deletions(-)
di
v3 no longer applied or compiled because of OVS changes since it was
sent out.
Ben Pfaff (4):
openvswitch/types.h: New macros OVS_BE16_MAX, OVS_BE32_MAX,
OVS_BE64_MAX.
match: New function minimatch_matches_flow().
tag: Reintroduce tag library.
classifier: Speed up lookup when metadata
These seem slightly nicer than e.g. htons(UINT16_MAX).
Signed-off-by: Ben Pfaff
---
include/openvswitch/types.h |4
lib/learn.c |2 +-
lib/match.c | 30 +++---
lib/meta-flow.c | 13 ++---
lib/nx-match.c
We have a controller that puts many rules with different metadata values
into the flow table, where metadata is used (by "resubmit"s) to distinguish
stages in a pipeline. Thus, any given flow only needs to be hashed into
classifier "cls_table"s that contain a match for the flow's metadata value.
T
It is needed for the classifier partitioning optimization.
Signed-off-by: Ben Pfaff
---
lib/automake.mk |2 +
lib/tag.c | 117
lib/tag.h | 134 +++
3 files changed, 253 insertio
Looks good to me, thx
On Thu, Aug 15, 2013 at 1:00 PM, Ben Pfaff wrote:
> At one time, the ofproto-dpif implementation of the 'rule_execute' member
> function required, indirectly, at least struct(ofp10_packet_in) bytes of
> headroom in the packet passed into it. (This allowed constructing an
Acked-by: Ethan Jackson
On Mon, Aug 26, 2013 at 5:10 PM, Ben Pfaff wrote:
> The OFPFF_* flags used in flow_mods are just confusing enough that it
> seems worthwhile to try to abstract them out. In particular:
>
> * OFPFF_EMERG was introduced in OF1.0, deleted in OF1.1, and then
> its
This is pretty cool !
Do you know how i can try compile for RISC?
One possible issue below:
> /* The IPv6 flow label is in the lower 20 bits of the first 32-bit word.
> */
> diff --git a/lib/unaligned.h b/lib/unaligned.h
> index 2654a27..154eb13 100644
> --- a/lib/unaligned.h
> +++ b/lib/unali
Suggested-by: pritesh
Signed-off-by: Ben Pfaff
---
ofproto/tunnel.c | 100 +-
1 file changed, 46 insertions(+), 54 deletions(-)
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 202358b..4729f5f 100644
--- a/ofproto/tunnel.c
+++ b/ofprot
On Fri, Aug 23, 2013 at 04:06:13PM -0700, Jarno Rajahalme wrote:
>
> On Aug 23, 2013, at 1:33 PM, Ben Pfaff wrote:
>
> > On Fri, Aug 23, 2013 at 12:46:47PM -0700, Jarno Rajahalme wrote:
> >> On Aug 23, 2013, at 11:26 AM, Ben Pfaff wrote:
> >>> Looking at tnl_find() in ofproto/tunnel.c, it maint
Make sure the sw_flow_key structure and valid mask boundaries are always
machine word aligned. Optimize the flow compare and mask operations
using machine word size operations. This patch improves throughput on
average by 15% when CPU is the bottleneck of forwarding packets.
This patch is inspired
On Tue, Aug 27, 2013 at 11:08:08AM -0700, Alex Wang wrote:
> This is pretty cool !
Thanks.
> Do you know how i can try compile for RISC?
You need a RISC platform. There are many, but none of them is really
popular in a desktop setting.
I do have an old SPARC machine (SunBlade 100), that belong
In add_flow, should we take the evict lock on the rule while we're
still holding the classifier readlock? It doesn't matter much if
add_flow will only be called by the main thread, however if that isn't
the case, we don't want the rule we're working on to be evicted from
under us.
There's a serio
Looks good and simple to understand. (passed tests also).
Acked-by: pritesh
On Aug 27, 2013, at 11:12 AM, Ben Pfaff wrote:
> Suggested-by: pritesh
> Signed-off-by: Ben Pfaff
> ---
> ofproto/tunnel.c | 100 +-
> 1 file changed, 46 insertions(
Thanks.
I am going to give others a day or so to look at this, since it is
only a cleanup. I'll apply it then assuming I get no negative
feedback.
On Tue, Aug 27, 2013 at 07:01:11PM +, Pritesh Kothari (pritkoth) wrote:
> Looks good and simple to understand. (passed tests also).
>
> Acked-by
On Mon, Aug 26, 2013 at 06:07:08PM -0700, Ethan Jackson wrote:
> > +/* Replaces 'dst' by 'src', destroying 'src'. The caller must eventually
> > free
> > + * 'dst' with miniflow_destroy(). */
> > +void
> > +miniflow_move(struct miniflow *dst, struct miniflow *src)
> > +{
> > +int n = miniflow
Much clearer, thanks.
Ethan
On Tue, Aug 27, 2013 at 12:25 PM, Ben Pfaff wrote:
> On Mon, Aug 26, 2013 at 06:07:08PM -0700, Ethan Jackson wrote:
>> > +/* Replaces 'dst' by 'src', destroying 'src'. The caller must eventually
>> > free
>> > + * 'dst' with miniflow_destroy(). */
>> > +void
>> > +m
On Tue, Aug 27, 2013 at 11:29:26AM -0700, Ethan Jackson wrote:
> In add_flow, should we take the evict lock on the rule while we're
> still holding the classifier readlock? It doesn't matter much if
> add_flow will only be called by the main thread, however if that isn't
> the case, we don't want
On Tue, Aug 27, 2013 at 11:21 AM, Andy Zhou wrote:
> Make sure the sw_flow_key structure and valid mask boundaries are always
> machine word aligned. Optimize the flow compare and mask operations
> using machine word size operations. This patch improves throughput on
> average by 15% when CPU is t
On Mon, Aug 26, 2013 at 03:23:16PM -0700, Alex Wang wrote:
> I found answer from here:
> http://stackoverflow.com/questions/337449/how-does-one-declare-an-array-of-constant-function-pointers-in-c
>
> so if i understand it correctly:
>
> extern int (*build_assert(void))[1];
>
> declare an extern
Thanks Ben for the explanation,
I confused myself. After running some experiment, I'm all good now.
> Do you know how i can try compile for RISC?
>
> You need a RISC platform. There are many, but none of them is really
> popular in a desktop setting.
>
> I do have an old SPARC machine (SunBlade
On Tue, Aug 27, 2013 at 01:01:18PM -0700, Alex Wang wrote:
> Thanks Ben for the explanation,
>
> I confused myself. After running some experiment, I'm all good now.
>
> > Do you know how i can try compile for RISC?
> >
> > You need a RISC platform. There are many, but none of them is really
> >
Thanks a lot for the info.
For you review, I still have one issue (buried in my emails):
"""
> +/* Index counter used to compute the moving average array's index. */
> > +static unsigned int idx_count = 0;
>
> It looks to me like idx_count should be protected by coverage_mutex.
I'm a bit conf
> It's my "platform" in the sense of a flat surface that you put things
> on, in this case my monitor. I haven't even plugged it in for a year.
>
> This will definitely get some exposure on RISC machines since the goal
> is to fix all the nasty Debian build failures seen here in red:
> htt
On Tue, Aug 27, 2013 at 01:15:11PM -0700, Alex Wang wrote:
> > It's my "platform" in the sense of a flat surface that you put things
> > on, in this case my monitor. I haven't even plugged it in for a year.
> >
> > This will definitely get some exposure on RISC machines since the goal
> > is to fi
From: Pravin B Shelar
Flow dump operation is read-only operation. There is no need to
take ovs-lock. Following patch use rcu-lock for dumping flows.
Signed-off-by: Pravin B Shelar
Signed-off-by: Jesse Gross
---
net/openvswitch/datapath.c | 17 +
1 file changed, 9 insertions(
A number of significant new features and optimizations for net-next/3.12.
Highlights are:
* "Megaflows", an optimization that allows userspace to specify which
flow fields were used to compute the results of the flow lookup.
This allows for a major reduction in flow setups (the major
perf
From: Justin Pettit
Signed-off-by: Justin Pettit
Signed-off-by: Jesse Gross
---
net/openvswitch/vport.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index d69e0c0..6f65dbe 100644
--- a/net/openvswitch/vport.c
+++ b/net/
From: Andy Zhou
Key_end is a better name describing the ending boundary than key_len.
Rename those variables to make it less confusing.
Signed-off-by: Andy Zhou
Signed-off-by: Jesse Gross
---
net/openvswitch/flow.c | 31 ---
net/openvswitch/flow.h | 2 +-
2 files
From: Cong Wang
Cc: Jesse Gross
Cc: Pravin B Shelar
Signed-off-by: Cong Wang
Signed-off-by: Jesse Gross
---
net/openvswitch/Makefile| 5 -
net/openvswitch/vport-gre.c | 3 ---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/openvswitch/Makefile b/net/openvswitch/Ma
From: Pravin B Shelar
Flow table destroy is done in rcu call-back context. Therefore
there is no need to use rcu variant of hlist_del().
Signed-off-by: Pravin B Shelar
Signed-off-by: Jesse Gross
---
net/openvswitch/flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ne
From: Pravin B Shelar
RCUfy dp-dump operation which is already read-only. This
makes all ovs dump operations lockless.
Signed-off-by: Pravin B Shelar
Signed-off-by: Jesse Gross
---
net/openvswitch/datapath.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net
From: Jiri Pirko
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
Signed-off-by: Jesse Gross
---
net/openvswitch/vport-netdev.c | 20 +++-
1 file changed, 19 inser
From: Andy Zhou
Make sure the sw_flow_key structure and valid mask boundaries are always
machine word aligned. Optimize the flow compare and mask operations
using machine word size operations. This patch improves throughput on
average by 15% when CPU is the bottleneck of forwarding packets.
This
From: Joe Stringer
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 checks
From: Joe Stringer
Signed-off-by: Joe Stringer
Signed-off-by: Jesse Gross
---
include/net/ipv6.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 5fe5649..7bdff04 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -41,6 +41,7 @@
#defin
On Mon, Aug 26, 2013 at 11:26 PM, P Balaji-B37839 wrote:
> Hi,
>
>
>
> Does the latest GIT Master has any dependency of other kernel modules like
> the listed below?
Yes, master has additional dependencies.
___
dev mailing list
dev@openvswitch.org
http:
On Tue, Aug 27, 2013 at 01:13:11PM -0700, Alex Wang wrote:
> Thanks a lot for the info.
>
> For you review, I still have one issue (buried in my emails):
>
> """
>
> > +/* Index counter used to compute the moving average array's index. */
> > > +static unsigned int idx_count = 0;
> >
> > It look
Batching reduces overheads and enables upto 4 times the upcall processing
performance in a specialized test case.
Signed-off-by: Jarno Rajahalme
---
ofproto/ofproto-dpif-upcall.c | 30 +++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/ofproto/ofproto
On Tue, Aug 27, 2013 at 03:04:54PM -0700, Jarno Rajahalme wrote:
> Batching reduces overheads and enables upto 4 times the upcall processing
> performance in a specialized test case.
>
> Signed-off-by: Jarno Rajahalme
Nice!
> +for (n = 0; n < udpif->n_handlers; ++n) {
> +handler = &
On Aug 27, 2013, at 3:13 PM, Ben Pfaff wrote:
> On Tue, Aug 27, 2013 at 03:04:54PM -0700, Jarno Rajahalme wrote:
>> Batching reduces overheads and enables upto 4 times the upcall processing
>> performance in a specialized test case.
>>
>> Signed-off-by: Jarno Rajahalme
>
> Nice!
>
>> +fo
Both 3/4 and 4/4 look good to me,
Want to ask one question:
Do we need to strip off the 2 bytes added to the ofpbuf (when sending the
packets)?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Tue, Aug 27, 2013 at 03:48:47PM -0700, Alex Wang wrote:
> Both 3/4 and 4/4 look good to me,
Thank you for the reviews.
> Do we need to strip off the 2 bytes added to the ofpbuf (when sending the
> packets)?
No, because the bytes that ofpbuf_reserve() adds aren't part of the
data in the ofpbuf
Thanks for the explanation, that what I need.
Also, one last note:
Should we also do the same thing to the "compose_rarp()" function, in
lib/packets.c?
On Tue, Aug 27, 2013 at 3:52 PM, Ben Pfaff wrote:
> On Tue, Aug 27, 2013 at 03:48:47PM -0700, Alex Wang wrote:
> > Both 3/4 and 4/4 look good
On Aug 27, 2013, at 11:12 AM, Ben Pfaff wrote:
> Suggested-by: pritesh
> Signed-off-by: Ben Pfaff
> ---
> ofproto/tunnel.c | 100 +-
> 1 file changed, 46 insertions(+), 54 deletions(-)
>
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> i
rounddown() was not available in older kenrel, such as kernel 2.6.32.61.
Add it to the compatibility code.
Signed-off-by: Andy Zhou
---
datapath/linux/compat/include/linux/kernel.h |9 +
1 file changed, 9 insertions(+)
diff --git a/datapath/linux/compat/include/linux/kernel.h
b/dat
On Tue, Aug 27, 2013 at 4:19 PM, Andy Zhou wrote:
> rounddown() was not available in older kenrel, such as kernel 2.6.32.61.
> Add it to the compatibility code.
>
> Signed-off-by: Andy Zhou
Applied, thanks.
___
dev mailing list
dev@openvswitch.org
http
>
> I missed that only one thread calls coverage_run(). Now that I
> understand the intent, I see some other issues to resolve. First,
> this intent isn't documented, but it should be (at least as a comment
> on coverage_run().)
>
Yes, I'll definitely refine the comment.
> Second, the patch
My thinking was that we need to take the lock when we do the
xpthread_cond_signal() because if we don't it's possible the thread
could go to sleep immediately after we make the signal call and
essentially never wake up (or at least not until it's called again).
On Tue, Aug 27, 2013 at 3:50 PM, Ja
On Tue, Aug 27, 2013 at 3:51 AM, Viresh Kumar wrote:
> On 26 August 2013 21:59, Jesse Gross wrote:
>
>> This is compatibility code for older kernels so by definition it
>> doesn't exist in current kernels. You can't just remove it.
>
> My mistake :(
>
> Will something like this makes sense? This
We have a call chain like this:
iface_configure_qos() calls
netdev_dump_queues(), which calls
netdev_linux_dump_queues(), which calls back through 'cb' to
qos_unixctl_show_cb(), which calls
netdev_delete_queue(), which calls
On Tue, Aug 27, 2013 at 1:37 AM, Viresh Kumar wrote:
> Hi Jesse,
>
> Thanks for your quick reply :)
>
> On 26 August 2013 22:16, Jesse Gross wrote:
>
>> Please look at the implementation of the loop counter.
>
> Sorry but I couldn't find anything interesting/tricky in implementation
> of loop cou
Acked-by: Ethan Jackson
Thanks looks good.
On Tue, Aug 27, 2013 at 5:16 PM, Ben Pfaff wrote:
> We have a call chain like this:
>
> iface_configure_qos() calls
> netdev_dump_queues(), which calls
> netdev_linux_dump_queues(), which calls back through 'cb' to
>
Thanks for taking this on, it's a lot better than what I would have
come up with. Clean.
In the commit message "differen" is missing a "t".
Acked-by: Ethan Jackson
On Mon, Aug 26, 2013 at 5:10 PM, Ben Pfaff wrote:
> add_flow() in ofproto.c has a race: it adds a new flow to the flow table
>
> On Tue, Aug 27, 2013 at 04:10:03PM +0900, YAMAMOTO Takashi wrote:
>> this fixes ofp_port_status delivery on a patch port removal.
>
> Can you explain further? I don't see any problems with patch port
> removal. Example:
>
> blp@sigsegv:~/ovs/_clang$ make sandbox
> ...
> --
From: Jesse Gross
Date: Tue, 27 Aug 2013 13:20:37 -0700
> A number of significant new features and optimizations for net-next/3.12.
> Highlights are:
> * "Megaflows", an optimization that allows userspace to specify which
>flow fields were used to compute the results of the flow lookup.
>
This commit changes the "ovs-appctl coverage/show" command to show the
per-second, per-minute and per-hour rates of function invocation. More
importantly, this makes using coverage counter an easy way to monitor
the execution of specific functions.
Signed-off-by: Alex Wang
---
v1 -> v2:
- cover
On Wed, Aug 28, 2013 at 10:38:06AM +0900, YAMAMOTO Takashi wrote:
> > On Tue, Aug 27, 2013 at 04:10:03PM +0900, YAMAMOTO Takashi wrote:
> >> this fixes ofp_port_status delivery on a patch port removal.
> >
> > Can you explain further? I don't see any problems with patch port
> > removal. Example
On Tue, Aug 27, 2013 at 05:59:10PM -0700, Ethan Jackson wrote:
> Thanks for taking this on, it's a lot better than what I would have
> come up with. Clean.
>
> In the commit message "differen" is missing a "t".
>
> Acked-by: Ethan Jackson
I'm surprised that you didn't find anything else wrong.
> I'm surprised that you didn't find anything else wrong. I am a bit
> worried that the design now falls into the category of "too complex for
> anything to be obviously wrong."
The old design was way past that point. I did my best reviewing it,
but the ofproto code is a bit beyond me. We'll se
Thanks, applied.
On Tue, Aug 27, 2013 at 05:33:59PM -0700, Ethan Jackson wrote:
> Acked-by: Ethan Jackson
>
> Thanks looks good.
>
> On Tue, Aug 27, 2013 at 5:16 PM, Ben Pfaff wrote:
> > We have a call chain like this:
> >
> > iface_configure_qos() calls
> > netdev_dump_queues(), w
On Tue, Aug 27, 2013 at 04:01:44PM -0700, Alex Wang wrote:
> Thanks for the explanation, that what I need.
>
> Also, one last note:
>
> Should we also do the same thing to the "compose_rarp()" function, in
> lib/packets.c?
Yes, that is a good idea. I made that change. I'll apply these patches
On Thu, Aug 15, 2013 at 01:03:14PM -0700, Ben Pfaff wrote:
> On Wed, Aug 14, 2013 at 11:24:38AM -0700, Ben Pfaff wrote:
> > A little work with gdb shows that 501 failed due to misaligned data
> > access in a "packet out" OpenFlow command in packet_set_ipv4(). It
> > looks like handle_packet_out()
On Tue, Aug 27, 2013 at 04:03:18PM -0700, Jarno Rajahalme wrote:
>
> On Aug 27, 2013, at 11:12 AM, Ben Pfaff wrote:
>
> > Suggested-by: pritesh
> > Signed-off-by: Ben Pfaff
> > ---
> > ofproto/tunnel.c | 100
> > +-
> > 1 file changed, 46 in
fix a regression added by commit 89454bf477d1dc95357792677ccbd4d483ab42d8.
"netdev: Fix deadlock when netdev_dump_queues() callback calls into netdev."
Signed-off-by: YAMAMOTO Takashi
---
lib/netdev-bsd.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/netdev-bsd.
On Wed, Aug 28, 2013 at 02:44:07PM +0900, YAMAMOTO Takashi wrote:
> fix a regression added by commit 89454bf477d1dc95357792677ccbd4d483ab42d8.
> "netdev: Fix deadlock when netdev_dump_queues() callback calls into netdev."
>
> Signed-off-by: YAMAMOTO Takashi
Applied, thanks. Sorry for causing tr
> On Wed, Aug 28, 2013 at 10:38:06AM +0900, YAMAMOTO Takashi wrote:
>> > On Tue, Aug 27, 2013 at 04:10:03PM +0900, YAMAMOTO Takashi wrote:
>> >> this fixes ofp_port_status delivery on a patch port removal.
>> >
>> > Can you explain further? I don't see any problems with patch port
>> > removal.
On Tue, Aug 27, 2013 at 03:50:06PM -0700, Jarno Rajahalme wrote:
>
> On Aug 27, 2013, at 3:13 PM, Ben Pfaff wrote:
>
> > On Tue, Aug 27, 2013 at 03:04:54PM -0700, Jarno Rajahalme wrote:
> >> Batching reduces overheads and enables upto 4 times the upcall processing
> >> performance in a specializ
On Wed, Jul 24, 2013 at 04:59:34PM +0300, Dmitry Fleytman wrote:
> From: Dmitry Fleytman
>
> User mode logic groups incoming packets by flows and then
> process flows in arbitrary order based on flow hash keys values.
> This behavior may lead to unexpected packets reordering. For example
> two pa
Cool! Thanks Ben.
On Aug 28, 2013, at 09:06 AM, Ben Pfaff wrote:
> On Wed, Jul 24, 2013 at 04:59:34PM +0300, Dmitry Fleytman wrote:
>> From: Dmitry Fleytman
>>
>> User mode logic groups incoming packets by flows and then
>> process flows in arbitrary order based on flow hash keys values.
>> Thi
I'm happy with it as long as we fix the race.
Ethan
On Tue, Aug 27, 2013 at 11:02 PM, Ben Pfaff wrote:
> On Tue, Aug 27, 2013 at 03:50:06PM -0700, Jarno Rajahalme wrote:
>>
>> On Aug 27, 2013, at 3:13 PM, Ben Pfaff wrote:
>>
>> > On Tue, Aug 27, 2013 at 03:04:54PM -0700, Jarno Rajahalme wrote:
76 matches
Mail list logo
