A number of use-cases weren't handled properly when determining what can
be wildcarded for megaflows. This commit both catches additional fields
that cannot be wildcarded and loosens a few other cases.
Bug #17979
Signed-off-by: Justin Pettit
---
lib/flow.c |2 +-
ofproto/
The presense or absence of IPFIX affects wildcarding, and there needs to
cause a revalidation when changed.
Found by inspecting Ethan's similar patch on NetFlow.
Signed-off-by: Justin Pettit
---
Since I just fixed IPFIX wildcarding in the previous patch, I'll
probably fold this patch into that o
Thanks very much, Ben for the comments,
On Tue, Jun 18, 2013 at 1:10 PM, Ben Pfaff wrote:
> Thanks again for working on this.
>
> It looks like your test builds are not configured to build a kernel
> module: this patch triggers tons of errors in the kernel build due to
> the change to OVSP_LOCA
On Wed, Jun 19, 2013 at 12:02:10AM -0700, Justin Pettit wrote:
> A number of use-cases weren't handled properly when determining what can
> be wildcarded for megaflows. This commit both catches additional fields
> that cannot be wildcarded and loosens a few other cases.
>
> Bug #17979
>
> Signed
On Wed, Jun 19, 2013 at 12:32:39AM -0700, Justin Pettit wrote:
> The presense or absence of IPFIX affects wildcarding, and there needs to
presense => presence
there => therefore (?)
> cause a revalidation when changed.
>
> Found by inspecting Ethan's similar patch on NetFlow.
>
> Signed-off-by:
On Wed, Jun 19, 2013 at 08:32:30AM -0700, Alex Wang wrote:
> On Tue, Jun 18, 2013 at 1:10 PM, Ben Pfaff wrote:
> > nl_attr_get_odp_port() and nl_msg_put_odp_port() functions might be
> > handy.
>
> Sure! honestly, it seems that I could not make the right decision on what
> to provide and
> what n
On Wed, Jun 19, 2013 at 9:29 AM, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 08:32:30AM -0700, Alex Wang wrote:
> > On Tue, Jun 18, 2013 at 1:10 PM, Ben Pfaff wrote:
> > > nl_attr_get_odp_port() and nl_msg_put_odp_port() functions might be
> > > handy.
> >
> > Sure! honestly, it seems that I coul
A number of use-cases weren't handled properly when determining what can
be wildcarded for megaflows. This commit both catches additional fields
that cannot be wildcarded and loosens a few other cases.
Bug #17979
Signed-off-by: Justin Pettit
---
v1 -> v2: Remove IPFIX un-wildcarding.
---
lib/f
On Jun 19, 2013, at 9:21 AM, Ben Pfaff wrote:
> Why does ipfix need to un-wildcard fields?
Right. I was thinking it was more like NetFlow than sFlow. Since we're
sampling, we don't need to match on the fields.
> I don't understand the change in compose_output_action__(). It adds
> the ECN b
On Jun 19, 2013, at 9:21 AM, Ben Pfaff wrote:
>> Since I just fixed IPFIX wildcarding in the previous patch, I'll
>> probably fold this patch into that one.
>
> Why can't IPFIX handle wildcarding?
Yeah, I'm dropping this patch.
Thanks.
--Justin
_
On Wed, Jun 19, 2013 at 10:03:56AM -0700, Justin Pettit wrote:
> On Jun 19, 2013, at 9:21 AM, Ben Pfaff wrote:
> > I don't understand the change in compose_output_action__(). It adds
> > the ECN bits to the mask but I don't see anything there that depends
> > on those bits.
> >
> > In do_xlate_a
On Wed, Jun 19, 2013 at 10:00:54AM -0700, Justin Pettit wrote:
> A number of use-cases weren't handled properly when determining what can
> be wildcarded for megaflows. This commit both catches additional fields
> that cannot be wildcarded and loosens a few other cases.
>
> Bug #17979
>
> Signed
On Tue, Jun 18, 2013 at 04:15:10PM -0700, Andy Zhou wrote:
> Added support to allow mega flow specified and displayed. ovs-dpctl tool
> is mainly used as debugging tool.
>
> This patch also implements the low level user space routines to send
> and receive mega flow netlink messages. Those netlink
The previous code had many subtleties that were easy to miss. This code
is intended to be more obviously correct.
Signed-off-by: Ben Pfaff
---
.../usr_share_openvswitch_scripts_ovs-xapi-sync| 32 ++-
1 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/xenserve
On Tue, Jun 18, 2013 at 02:49:27PM -0700, Ben Pfaff wrote:
> On Tue, Jun 18, 2013 at 02:45:46PM -0700, Gurucharan Shetty wrote:
> > On Tue, Jun 18, 2013 at 2:35 PM, Ben Pfaff wrote:
> >
> > > On Tue, Jun 18, 2013 at 02:24:47PM -0700, Gurucharan Shetty wrote:
> > > > On Tue, Jun 18, 2013 at 1:59 P
On Wed, Jun 19, 2013 at 10:26:14AM -0700, Ben Pfaff wrote:
> The previous code had many subtleties that were easy to miss. This code
> is intended to be more obviously correct.
>
> Signed-off-by: Ben Pfaff
Oops, I generated this against a bad tree. I'll send out a fixed
version in a minute.
__
The previous code had many subtleties that were easy to miss. This code
is intended to be more obviously correct.
Signed-off-by: Ben Pfaff
---
v1->v2: Regenerate against current master.
diff --git a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
b/xenserver/usr_share_openvswitch_scripts
On Wed, Jun 19, 2013 at 10:32 AM, Ben Pfaff wrote:
> The previous code had many subtleties that were easy to miss. This code
> is intended to be more obviously correct.
>
> Signed-off-by: Ben Pfaff
>
I was about to send this. Looks good to me, please apply.
Thanks,
Guru
> ---
> v1->v2: Regen
On Wed, Jun 19, 2013 at 10:38 AM, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 10:37:08AM -0700, Gurucharan Shetty wrote:
> > On Wed, Jun 19, 2013 at 10:32 AM, Ben Pfaff wrote:
> >
> > > The previous code had many subtleties that were easy to miss. This
> code
> > > is intended to be more obvious
We will continue to allow missing eth_type in the netlink attribute to
imply Ethernet II type. 802.3 frames requires a specific eth_type
attribute.
With Mega flows, we further require a missing eth_type in the key attribute
to have a exact match (ox) in the eth_type of the mask attribute (if
p
Specifically for the case, where we know that a bridge record
should exist in xapi, if we don't get any bridge records, log
the failure and retry again later.
Signed-off-by: Gurucharan Shetty
---
.../usr_share_openvswitch_scripts_ovs-xapi-sync| 18 +-
1 file changed, 17 ins
On Wed, Jun 19, 2013 at 10:37:08AM -0700, Gurucharan Shetty wrote:
> On Wed, Jun 19, 2013 at 10:32 AM, Ben Pfaff wrote:
>
> > The previous code had many subtleties that were easy to miss. This code
> > is intended to be more obviously correct.
> >
> > Signed-off-by: Ben Pfaff
> >
> I was about
On Wed, Jun 19, 2013 at 10:39:43AM -0700, Gurucharan Shetty wrote:
> On Wed, Jun 19, 2013 at 10:38 AM, Ben Pfaff wrote:
>
> > On Wed, Jun 19, 2013 at 10:37:08AM -0700, Gurucharan Shetty wrote:
> > > On Wed, Jun 19, 2013 at 10:32 AM, Ben Pfaff wrote:
> > >
> > > > The previous code had many subtl
On Wed, Jun 19, 2013 at 10:50 AM, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 10:39:43AM -0700, Gurucharan Shetty wrote:
> > On Wed, Jun 19, 2013 at 10:38 AM, Ben Pfaff wrote:
> >
> > > On Wed, Jun 19, 2013 at 10:37:08AM -0700, Gurucharan Shetty wrote:
> > > > On Wed, Jun 19, 2013 at 10:32 AM, Be
Signed-off-by: Ben Pfaff
---
configure.ac |1 +
lib/util.c | 48 +---
lib/util.h |3 ++-
3 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/configure.ac b/configure.ac
index 6db4a00..734b2ff 100644
--- a/configure.ac
+++ b/co
Signed-off-by: Ben Pfaff
---
lib/random.c | 26 +-
1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/lib/random.c b/lib/random.c
index 45d428c..2572c1e 100644
--- a/lib/random.c
+++ b/lib/random.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011
This series has two purposes:
* Add a basic thread support library to the tree.
* Get rid of all calls to C library functions that POSIX describes
as inherently unsafe in a multithreaded program, and then add a
make-time check that flags an error if any calls get reintroduced.
This library should prove useful for the threading changes coming up.
The following commit introduces one (very simple) user.
Signed-off-by: Ben Pfaff
---
configure.ac |6 +-
lib/automake.mk |7 +
lib/ovs-atomic-c11.h | 62 +++
lib/ovs-atomic-gcc4+.c
The only tricky part here is that I'm throwing in annotations to allow
"sparse" to report unbalanced locking.
Signed-off-by: Ben Pfaff
---
include/sparse/automake.mk |1 +
include/sparse/pthread.h | 61 +
lib/automake.mk|2 +
lib/compiler.h
Signed-off-by: Ben Pfaff
---
lib/command-line.c|4 ++-
lib/daemon.c | 12 +++-
lib/ofp-version-opt.c |3 ++
lib/ovs-thread.c | 68 +
lib/ovs-thread.h | 11
lib/process.c |4 +++
lib/timev
None of these test programs are threaded, but has little cost and means
that "grep" doesn't turn up any instances of these thread-unsafe functions
in our tree.
Signed-off-by: Ben Pfaff
---
tests/test-classifier.c | 25 +
tests/test-hindex.c |3 ++-
tests/test-hm
Some functions that POSIX says cannot be used safely in multithreaded
programs are not on the initial blacklist:
- getenv() should be safe in real implementations in the absence of
changes to the environment. (putenv() and setenv() are blacklisted.)
- We only use getopt() before sp
pthread_once() is portable but it does not allow passing any parameters to
the initialization function, which is often inconvenient, because it means
that the function can only access data declared at file scope. This commit
introduces an alternative with a more convenient interface.
Signed-off-b
Thanks. Is that a review?
On Tue, Jun 18, 2013 at 09:27:18PM -0700, Reid Price wrote:
> Thanks, that seems like the right amount of information
>
> -Reid
>
>
> On Tue, Jun 18, 2013 at 9:02 PM, Ben Pfaff wrote:
>
> > Previously, commands like this:
> > ovs-vsctl add-br br0
> > ovs-v
On Wed, Jun 19, 2013 at 02:03:15AM -0700, Gurucharan Shetty wrote:
> Specifically for the case, where we know that a bridge record
> should exist in xapi, if we don't get any bridge records, log
> the failure and retry again later.
>
> Signed-off-by: Gurucharan Shetty
Could we add a helper funct
I don't really care about the formatting, only about the kernel ABI.
Pre-megaflows, the ABI was:
typemask matches
---
eth_type(0x600+) specified Ethertype II Ethertype.
On Tue, Jun 18, 2013 at 1:10 PM, Ben Pfaff wrote:
> Thanks again for working on this.
>
> It looks like your test builds are not configured to build a kernel
> module: this patch triggers tons of errors in the kernel build due to
> the change to OVSP_LOCAL, because OVS_FORCE is not defined in the
On Wed, Jun 19, 2013 at 01:46:17PM -0700, Alex Wang wrote:
> On Tue, Jun 18, 2013 at 1:10 PM, Ben Pfaff wrote:
>
> > Thanks again for working on this.
> >
> > It looks like your test builds are not configured to build a kernel
> > module: this patch triggers tons of errors in the kernel build due
Thanks,
On Wed, Jun 19, 2013 at 1:48 PM, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 01:46:17PM -0700, Alex Wang wrote:
> > On Tue, Jun 18, 2013 at 1:10 PM, Ben Pfaff wrote:
> >
> > > Thanks again for working on this.
> > >
> > > It looks like your test builds are not configured to build a kern
OFPGMFC is so nice these constants said it twice.
Signed-off-by: Ben Pfaff
---
lib/ofp-errors.h |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index 593241d..fd6f03f 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -1,5
POSIX defines a portable pthread_key_t API for per-thread data. GCC and
C11 have two different forms of per-thread data that are generally faster
than the POSIX API, where they are available. This commit adds a
macro-based wrapper, DEFINE_PER_THREAD_DATA, that takes advantage of the
GCC extension
This ensures that attempts to use them cause sparse to complain.
Signed-off-by: Ben Pfaff
---
include/sparse/math.h |5 +
include/sparse/netinet/in.h |3 +--
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/sparse/math.h b/include/sparse/math.h
index f94c5
Signed-off-by: Ben Pfaff
---
lib/ofp-parse.c|4 ++--
tests/ovs-ofctl.at |4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 1c5c761..fdec1d4 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1,5 +1,5 @@
/*
- * Copyright
Jesse,
The following patch is an incremental patch on top of yours.
-- Fix a few small bugs introduced by the cleanup patch.
-- Make mask list per table.
I believe this implementation will address the rcu lock issues we discussed
off line.
Would you please review?
Thanks,
--andy
On Tue, Jun
On 19 June 2013 16:17, Ben Pfaff wrote:
> This series has two purposes:
>
> * Add a basic thread support library to the tree.
>
> * Get rid of all calls to C library functions that POSIX describes
> as inherently unsafe in a multithreaded program, and then add a
> make-time che
On Wed, Jun 19, 2013 at 05:20:11PM -0400, Ed Maste wrote:
> On 19 June 2013 16:17, Ben Pfaff wrote:
> > This series has two purposes:
> >
> > * Add a basic thread support library to the tree.
> >
> > * Get rid of all calls to C library functions that POSIX describes
> > as inherently
On 19 June 2013 16:17, Ben Pfaff wrote:
> +#if STRERROR_R_CHAR_P
> +/* GNU style strerror_r() might return an immutable static string, or it
> + * might write and return 'buffer', but in either case we can pass the
> + * returned string directly to the caller. */
> +s = strerror_r(
On 19 June 2013 17:22, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 05:20:11PM -0400, Ed Maste wrote:
>> On 19 June 2013 16:17, Ben Pfaff wrote:
>> > This series has two purposes:
>> >
>> > * Add a basic thread support library to the tree.
>> >
>> > * Get rid of all calls to C library funct
On Wed, Jun 19, 2013 at 05:42:44PM -0400, Ed Maste wrote:
> On 19 June 2013 16:17, Ben Pfaff wrote:
> > +#if STRERROR_R_CHAR_P
> > +/* GNU style strerror_r() might return an immutable static string, or
> > it
> > + * might write and return 'buffer', but in either case we can pass the
> >
It is a good idea to capture this information in a table. The following
table should be accurate:
Pre-megaflow:
typemask matches
---
eth_type(0x600+) specified Ethertype II, or val
On Wed, Jun 19, 2013 at 05:45:30PM -0400, Ed Maste wrote:
> On 19 June 2013 17:22, Ben Pfaff wrote:
> > On Wed, Jun 19, 2013 at 05:20:11PM -0400, Ed Maste wrote:
> >> On 19 June 2013 16:17, Ben Pfaff wrote:
> >> > This series has two purposes:
> >> >
> >> > * Add a basic thread support librar
OK, can we add that in the tree somewhere, maybe datapath/README?
Thanks,
Ben.
On Wed, Jun 19, 2013 at 02:52:52PM -0700, Andy Zhou wrote:
> It is a good idea to capture this information in a table. The following
> table should be accurate:
>
> Pre-megaflow:
>
> typemask
Good idea. There may be other aspects of the mega related kernel ABI needs
document as well. Wonder if Justin and Jesse have more inputs on this.
On Wed, Jun 19, 2013 at 2:55 PM, Ben Pfaff wrote:
> OK, can we add that in the tree somewhere, maybe datapath/README?
>
> Thanks,
>
> Ben.
>
> On Wed
On Wed, Jun 19, 2013 at 02:47:23PM -0700, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 05:42:44PM -0400, Ed Maste wrote:
> > On 19 June 2013 16:17, Ben Pfaff wrote:
> > > +#if STRERROR_R_CHAR_P
> > > +/* GNU style strerror_r() might return an immutable static string,
> > > or it
> > > + * m
On Wed, Jun 19, 2013 at 02:54:11PM -0700, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 05:45:30PM -0400, Ed Maste wrote:
> > On 19 June 2013 17:22, Ben Pfaff wrote:
> > > On Wed, Jun 19, 2013 at 05:20:11PM -0400, Ed Maste wrote:
> > >> On 19 June 2013 16:17, Ben Pfaff wrote:
> > >> > This series ha
What is the reason need for iterating through the mask list itself
when destroying the table instead of doing it as the flows are
deleted? Doing it that way would both avoid the need to have multiple
list deletion functions and the differences between the RCU and
non-RCU versions of ovs_flow_free()
When table is destroyed, there is really no need to keep track of the
mask's reference counting -- The masks will have to be destroyed any ways.
During run time, the mask_list deletion needs to be protected by the
ovs_lock, But we should not require ovs_lock to be held during table
destroy. right?
I think at this point, the use of a separate RCU callback for the
masks is hurting more than it is helping and it would be easier to
just piggyback off the flow/table destroy RCU. I'll try to write a
patch to see if I can get it to work.
On Wed, Jun 19, 2013 at 4:23 PM, Andy Zhou wrote:
> When ta
Patch 27a88d1373cbfcceac6d901bbf1c17051aa7845f caused the vswitchd
documentation and the code to digress. This brings them back in line.
Signed-off-by: Joe Stringer
---
vswitchd/vswitch.xml |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vsw
From: Joe Stringer
This patch introduces a python script to generate about 1500 tests for
permutations of mpls_push,mpls_pop,dec_mpls_ttl,dec_ttl where
recirculation occurs up to (and including) three times.
Signed-off-by: Joe Stringer
Signed-off-by: Simon Horman
---
v13
* Use the new force-
Recirculation is a technique to allow a frame to re-enter
frame processing. This is intended to be used after actions
have been applied to the frame with modify the frame in
some way that makes it possible for richer processing to occur.
An example is and indeed targeted use case is MPLS. If an MP
From: Joe Stringer
This adds support for specifying flow miss handling behaviour at
runtime, through a new "other-config" option in the Open_vSwitch table.
This is an extension to flow-eviction-threshold.
By default, the behaviour is the same as before. If force-miss-model is
set to 1, then flow
On Tue, Jun 18, 2013 at 04:06:49PM +0900, Simon Horman wrote:
> Allow datapath to recognize and extract MPLS labels into flow keys
> and execute actions which push, pop, and set labels on packets.
>
> Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe
> Stringer.
>
> Cc: Ravi
From: Pravin B Shelar
Date: Mon, 17 Jun 2013 17:49:21 -0700
> Following patch series adds support for gre tunneling.
> First six patches extend kernel gre and ip_tunnel modules
> api so that there is more code sharing between gre modules
> and ovs. Rest of patches adds ovs tunneling infrastructre
On 19 June 2013 17:54, Ben Pfaff wrote:
> Oh, that's odd. C11 appears to say that the parentheses are optional
> with _Atomic in this case (you can use _Atomic(...) as a type
> specifier or _Atomic by itself as a type qualifier) but I'll change it
> since that fixes the problem.
Ahh, it looks li
On 19 June 2013 18:48, Ben Pfaff wrote:
> I pushed a fix for this issue (just s/ptb.bufsize/BUFSIZE/) and the
> ovs-atomic-c11.h issue you also mentioned to the "threads" branch at
> https://github.com/blp/ovs-reviews/commits/threads
Thanks Ben, with those two changes your threads branch passes u
OK, that idea turned out to be a bad one since masks really do have a
lifetime that is separate from an individual flow. However, here's a
patch that does the unification a different way.
What do you think?
On Wed, Jun 19, 2013 at 4:32 PM, Jesse Gross wrote:
> I think at this point, the use of a
A number of use-cases weren't handled properly when determining what can
be wildcarded for megaflows. This commit both catches additional fields
that cannot be wildcarded and loosens a few other cases.
Bug #17979
Signed-off-by: Justin Pettit
---
v1 -> v2: Remove IPFIX un-wildcarding.
v2 -> v3:
I added most of the ones that seemed important during my cleanups
yesterday. The only other thing that comes to mind is a description of
how updates are handled (such as the fact that the new mask is
ignored).
On Wed, Jun 19, 2013 at 3:04 PM, Andy Zhou wrote:
> Good idea. There may be other aspec
Good to hear, thank you.
On Wed, Jun 19, 2013 at 06:42:59PM -0700, Jesse Gross wrote:
> I added most of the ones that seemed important during my cleanups
> yesterday. The only other thing that comes to mind is a description of
> how updates are handled (such as the fact that the new mask is
> igno
Yes, especially if it passed the unit test, looks correct.
On Wed, Jun 19, 2013 at 3:26 PM, Ben Pfaff wrote:
> Thanks. Is that a review?
>
> On Tue, Jun 18, 2013 at 09:27:18PM -0700, Reid Price wrote:
> > Thanks, that seems like the right amount of information
> >
> > -Reid
> >
> >
> > On Tu
On Wed, Jun 19, 2013 at 06:38:25PM -0700, Justin Pettit wrote:
> A number of use-cases weren't handled properly when determining what can
> be wildcarded for megaflows. This commit both catches additional fields
> that cannot be wildcarded and loosens a few other cases.
>
> Bug #17979
>
> Signed
On Jun 19, 2013, at 10:44 PM, Ben Pfaff wrote:
> On Wed, Jun 19, 2013 at 06:38:25PM -0700, Justin Pettit wrote:
>> A number of use-cases weren't handled properly when determining what can
>> be wildcarded for megaflows. This commit both catches additional fields
>> that cannot be wildcarded and
Good enough, thanks. Applied.
On Thu, Jun 20, 2013 at 12:31:09AM -0500, Reid Price wrote:
> Yes, especially if it passed the unit test, looks correct.
>
>
> On Wed, Jun 19, 2013 at 3:26 PM, Ben Pfaff wrote:
>
> > Thanks. Is that a review?
> >
> > On Tue, Jun 18, 2013 at 09:27:18PM -0700, Rei
This patch seems to work fine. Please feel free to push.
I have two concerns mainly on the style side:
1. In the same file flow.c, we are mixing the _deferred_ functions with the
style of passing 'deferred' as a parameter in the same file. The
correctness of caller in setting 'deferred' value
75 matches
Mail list logo
