Thanks this version is much closer. Just some minor points which
should be pretty easy to clean up. Ben, so we don't have a 12 hour
review cycle on this, would you mind looking at the next version of
this patch (which should be pretty much there) and applying it if it's
ready?
>>> +/* Al
> This is the newest version,
>
> I think I address most of issues, mentioned in your comments,
Ah ok I'll have a look at this one. Thanks.
Ethan
>
>
> On Tue, Aug 6, 2013 at 8:51 AM, Alex Wang wrote:
>>
>> When there is no incoming data traffic at the interface for a period,
>> BFD decay allo
Hey Ethan,
This is the newest version,
I think I address most of issues, mentioned in your comments,
Kind Regards,
Alex Wang,
On Tue, Aug 6, 2013 at 8:51 AM, Alex Wang wrote:
> When there is no incoming data traffic at the interface for a period,
> BFD decay allows the bfd session to increas
Sorry for such a delayed review of this patch. I have bit more time
now that multithreading is in, so let's try to get this merged this
week.
Is this v4 the most recent version? Since I was so delayed reviewing
it, it no longer applies to master.
There's one major problem with this patch which
Acked-by: Ethan Jackson
Agreed thank you.
Ethan
On Tue, Aug 13, 2013 at 6:54 AM, Ben Pfaff wrote:
> The Clang support for thread-safety annotations is much more effective
> than "sparse" support. I found that I was unable to make the annotations
> warning-free under sparse.
>
> Signed-off-by:
Acked-by: Ethan Jackson
On Tue, Aug 13, 2013 at 6:54 AM, Ben Pfaff wrote:
> CodingStyle says that this is preferred.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovs-thread.h |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> in
Acked-by: Ethan Jackson
We've really got to get sparse to work with clang, this sort of thing
is going to get annoying. C'est la vie.
Ethan
On Tue, Aug 13, 2013 at 6:54 AM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
> ---
> ofproto/ofproto-dpif-upcall.c |2 +-
> 1 file changed, 1 inser
This change looks good to me,
During my run of "fakeroot debian/rules binary", there is no warning.
Could you hint me how to reproduce it?
On Mon, Aug 12, 2013 at 3:13 PM, Ben Pfaff wrote:
> Avoids these warnings from groff:
>
> :1037: warning [p 14, 6.0i]: cannot adjust line
> :1037: warning
Looks good to me, I ran the "fakeroot debian/rules binary" and saw the
change of CFLAGS attribute in "_debian/Makefile"
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
At any rate this isn't an issue. I have simply unlocked and destroyed
the rule after it's been removed from the classifier.
Ethan
On Tue, Aug 13, 2013 at 7:48 AM, Ben Pfaff wrote:
> On Mon, Aug 12, 2013 at 01:23:38PM +0900, YAMAMOTO Takashi wrote:
>> > This patch uses a read-write lock to preve
On Mon, Aug 12, 2013 at 4:59 PM, Ben Pfaff wrote:
> On Mon, Aug 05, 2013 at 04:00:03PM -0700, Jesse Gross wrote:
>> When tracing a flow, it shows the "relevant fields" that were used
>> to determine the results. However, this currently only includes fields
>> that are used for computing the action
On Mon, Aug 05, 2013 at 04:00:03PM -0700, Jesse Gross wrote:
> When tracing a flow, it shows the "relevant fields" that were used
> to determine the results. However, this currently only includes fields
> that are used for computing the actions but not the flow lookup. This
> can be confusing so th
On Mon, Aug 12, 2013 at 11:04:39AM +0800, ZhengLingyun wrote:
> I only know 'br0' can be used as an interface to linux host. Can someone
> explain me the function of 'br0'? In fact I don't understand how 'br0' works
> and why this bug does't affect me at all.
If you're not putting an IP address on
On Mon, Aug 12, 2013 at 01:23:38PM +0900, YAMAMOTO Takashi wrote:
> > This patch uses a read-write lock to prevent rules from being evicted
> > while they're used by child threads. It also changes the prototypes
> > of the various rule lookup functions so that the thread safety
> > analysis can be
On Tue, Aug 13, 2013 at 08:00:05AM +0900, y...@mwd.biglobe.ne.jp wrote:
> From: YAMAMOTO Takashi
>
> according to the OVS_RELEASES annotation, oftable_remove_rule__ is
> expected to release rule->evict lock. make it actually do so.
>
> this fixes pthread_rwlock_destroy failures observed on NetB
Hi,
I believe I've addressed the corner case in this patch set by returning a
dict() that represents the current state of the database on connection
reset/initial sync, otherwise a list of changes. This should allow the user
to have a consistent view of the database. I've also changed the response
I am also a bit concerned by issues that might arise
from a user thinking that this is always accurate, rather
than hints. Aaron, I think you had said something
regarding this when we chatted off-list, but I don't
recall the details.
-Reid
On Fri, Aug 9, 2013 at 2:50 PM, Reid Price wrote:
>
From: YAMAMOTO Takashi
according to the OVS_RELEASES annotation, oftable_remove_rule__ is
expected to release rule->evict lock. make it actually do so.
this fixes pthread_rwlock_destroy failures observed on NetBSD,
where destroying a held lock, which is specwise undefined behaviour,
actually fa
On Mon, Aug 12, 2013 at 3:46 PM, Jesse Gross wrote:
> On Mon, Aug 12, 2013 at 1:28 PM, Pravin B Shelar wrote:
>> XEN dom0 networking assumes dev->master is bond device
>> and it tries to access bond private structure from dev->master
>> ptr on receive path. This causes panic.
>> Following patch r
CodingStyle says that this is preferred.
Signed-off-by: Ben Pfaff
---
lib/ovs-thread.h |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 5d3964a..b7bc5d1 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -467,12 +467,12 @
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif-upcall.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 55ada89..b451f1d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@
The Clang support for thread-safety annotations is much more effective
than "sparse" support. I found that I was unable to make the annotations
warning-free under sparse.
Signed-off-by: Ben Pfaff
---
include/sparse/pthread.h | 38 --
lib/compiler.h
I've been building with Clang lately and didn't notice that the sparse
annotations had bit-rotted. The first two patches bring them up to
date. The final patch is a style update.
Ben Pfaff (3):
ofproto-dpif-upcall: Fix sparse warning.
sparse: Remove support for thread-safety annotations.
o
On Mon, Aug 12, 2013 at 1:28 PM, Pravin B Shelar wrote:
> XEN dom0 networking assumes dev->master is bond device
> and it tries to access bond private structure from dev->master
> ptr on receive path. This causes panic.
> Following patch removes compat code that is setting master
> device.
>
> Sig
On Tue, Aug 13, 2013 at 07:20:22AM +0900, y...@mwd.biglobe.ne.jp wrote:
> From: YAMAMOTO Takashi
>
> Signed-off-by: YAMAMOTO Takashi
Both patch applied, thank you.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
From: YAMAMOTO Takashi
Signed-off-by: YAMAMOTO Takashi
---
lib/netdev-bsd.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index f6d066b..50fb520 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -133,7 +133,7 @@ static voi
From: YAMAMOTO Takashi
Signed-off-by: YAMAMOTO Takashi
---
ofproto/ofproto-dpif-upcall.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 54d3c21..55ada89 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-d
Debian now encourages building every program with various GCC hardening
options. This commit implements that recommendation for Open vSwitch.
See https://wiki.debian.org/Hardening for details.
Found by lintian.
Signed-off-by: Ben Pfaff
---
debian/rules |1 +
1 file changed, 1 insertion(+)
Avoids these warnings from groff:
:1037: warning [p 14, 6.0i]: cannot adjust line
:1037: warning [p 14, 6.2i]: can't break line
Found by lintian.
Signed-off-by: Ben Pfaff
---
utilities/ovs-ofctl.8.in |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utilities/ovs-ofctl.8.i
These patches fix some lintian warnings for the Debian packaging.
I intend to apply them both to master and branch-1.9 for upload
of that branch to Debian.
Thanks,
Ben.
Ben Pfaff (2):
debian: Apply hardening options to build.
ovs-ofctl: Avoid groff warning due to too-long line.
debian/rule
Dear Ernesto,
I had the same problem.
Just add this line to /etc/init.d/openvswitch-switch:
# X-Stop-After: libvirt-guests
and reconfigure boot/shutdown order:
update-rc.d -f openvswitch-switch remove
insserv openvswitch-switch
Now openvswitch-switch waits to libvirt-guests shutdown all VM
On Mon, Aug 12, 2013 at 02:06:54PM -0700, Alex Wang wrote:
> On Mon, Aug 12, 2013 at 2:04 PM, Ben Pfaff wrote:
>
> >
> > I am nervous about accessing anything in 'slave' outside the lock.
> > What do you think of this variant?
> >
> > Thanks,
> >
> > Ben.
>
>
> Thanks Ben, for pointing this out
This is pretty crude (nothing else in the IDL interface requires the
caller to understand the JSON-RPC protocol) and seems difficult for
the callers to use. The changes that it returns can also be
incomplete, at least in one corner case: if the database connection
drops and reconnects, then the re
On Mon, Aug 12, 2013 at 2:04 PM, Ben Pfaff wrote:
>
> I am nervous about accessing anything in 'slave' outside the lock.
> What do you think of this variant?
>
> Thanks,
>
> Ben.
Thanks Ben, for pointing this out,
Yours looks good and safer!
___
dev
On Mon, Aug 12, 2013 at 02:14:52PM -0700, Alex Wang wrote:
> This commit fixes the error introduced by commit 4a1b8f30e59 (bond:
> Stop using tags.). The error is caused by mistakenly returning 'slave'
> where 'slave->aux' should be returned.
>
> Signed-off-by: Alex Wang
I am nervous about acces
Make sense. This may be most reasonable solution.
On Mon, Aug 12, 2013 at 1:59 PM, Ben Pfaff wrote:
> On Mon, Aug 12, 2013 at 01:56:02PM -0700, Andy Zhou wrote:
> > Thanks for the added comments. Looks good.
>
> Thanks. I applied this to master.
>
> > Coding style says we should not use "//",
On Mon, Aug 12, 2013 at 01:56:02PM -0700, Andy Zhou wrote:
> Thanks for the added comments. Looks good.
Thanks. I applied this to master.
> Coding style says we should not use "//", but not sure if this applies to
> comments.
Well, you can't use a /**/ comment inside a comment because that ends
OpenFlow 1.3 says that the band_types member of struct ofp_meter_features
is a bitmap of OFPMBT_* values. The OFPMBT_* values are 1-based, so
until now, to avoid wasting bit 0, OVS mapped an OFPMBT_* with value 1 to
bit 0, value 2 to bit 1, and so on. However, according to
http://openvswitch.org/
Thanks for the added comments. Looks good.
Coding style says we should not use "//", but not sure if this applies to
comments.
On Mon, Aug 12, 2013 at 1:45 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
> ---
> lib/seq.c | 12 ++--
> lib/seq.h | 40
This commit fixes the error introduced by commit 4a1b8f30e59 (bond:
Stop using tags.). The error is caused by mistakenly returning 'slave'
where 'slave->aux' should be returned.
Signed-off-by: Alex Wang
---
lib/bond.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bon
Thanks, applied.
On Mon, Aug 12, 2013 at 01:46:15PM -0700, Andy Zhou wrote:
> Acked-by: Andy Zhou
>
>
>
> On Mon, Aug 12, 2013 at 12:54 PM, Ben Pfaff wrote:
>
> > Commit 86f1d0326bd0 (netdev-dummy: Use netdev_get_devices() instead of a
> > local shash.) caused netdev-dummy functions that ite
Acked-by: Andy Zhou
On Mon, Aug 12, 2013 at 12:54 PM, Ben Pfaff wrote:
> Commit 86f1d0326bd0 (netdev-dummy: Use netdev_get_devices() instead of a
> local shash.) caused netdev-dummy functions that iterate over all dummy
> devices to iterate only over the ones that have class 'dummy_class'. T
Signed-off-by: Ben Pfaff
---
lib/seq.c | 12 ++--
lib/seq.h | 40 +++-
2 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/lib/seq.c b/lib/seq.c
index abe1ad8..36e5065 100644
--- a/lib/seq.c
+++ b/lib/seq.c
@@ -106,7 +106,11 @@ seq_chang
XEN dom0 networking assumes dev->master is bond device
and it tries to access bond private structure from dev->master
ptr on receive path. This causes panic.
Following patch removes compat code that is setting master
device.
Signed-off-by: Pravin B Shelar
Bug #18920
---
v1-v2:
Fixed according to
I was thinking of the first case. You are right, it is covered by
find_nonempty_queue().
Then this patch looks good to me.
On Mon, Aug 12, 2013 at 1:06 PM, Ben Pfaff wrote:
> To be specific, I guess we are talking about
> dp_netdev_output_userspace() calling seq_change() and
> dpif_netdev_recv
To be specific, I guess we are talking about
dp_netdev_output_userspace() calling seq_change() and
dpif_netdev_recv_wait() calling seq_wait(). As you say, they are
serialized by dp_netdev_mutex, so there are two possible orders:
- If dp_netdev_output_userspace() runs first, then queue_seq doe
On Mon, Aug 12, 2013 at 12:01:46PM -0700, Alex Wang wrote:
> This commit fixes a typo in "lib/bond.c" which causes the high CPU
> utilization after adding bond.
>
> Signed-off-by: Alex Wang
Applied, thanks.
___
dev mailing list
dev@openvswitch.org
http
Found by Clang.
Signed-off-by: Ben Pfaff
---
lib/netdev-dummy.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 5c31210..32b0943 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -327,12 +327,15 @@ netdev_dummy_get_config(const st
The locks and unlocks added in this commit are superfluous but should not
hurt performance and make Clang happy.
Signed-off-by: Ben Pfaff
---
lib/netdev-dummy.c | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/lib/netdev-dummy.c b/lib/netdev
Commit 86f1d0326bd0 (netdev-dummy: Use netdev_get_devices() instead of a
local shash.) caused netdev-dummy functions that iterate over all dummy
devices to iterate only over the ones that have class 'dummy_class'. This
seemed to obviously include all the ones that we want, but in fact
when ovs-vsw
On Fri, Aug 9, 2013 at 2:50 PM, Reid Price wrote:
> Or you could keep the original function behavior the same and expose this
> as a separate function
>
> def foo(...):
>
>
> def run(...):
> return self.foo(...)[0]
>
> where foo is a better function name - update? run_details?
> run
This commit fixes a typo in "lib/bond.c" which causes the high CPU
utilization after adding bond.
Signed-off-by: Alex Wang
---
lib/bond.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bond.c b/lib/bond.c
index 06dd362..a9278db 100644
--- a/lib/bond.c
+++ b/lib/bond.c
On Wed, Aug 7, 2013 at 1:54 PM, Lori Jakab wrote:
> On 7/30/13 1:47 AM, Pravin B Shelar wrote:
>>
>> CC: Lori Jakab
>> Signed-off-by: Pravin B Shelar
>> ---
>> datapath/vport-lisp.c | 48
>> ++--
>> 1 files changed, 2 insertions(+), 46 deletions(-
Implement a per-exporter flow cache with active timeout expiration.
Add columns "cache_active_timeout" and "cache_max_flows" into table
"IPFIX" to configure each cache.
Add per-flow elements "octetDeltaSumOfSquares",
"minimumIpTotalLength", and "maximumIpTotalLength" to replace
"ethernetTotalLengt
Hi,
I updated my IPFIX caching patch to add mutex locking to the run() and
wait() functions.
Thanks,
--
Romain Lenglet
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Tue, Aug 6, 2013 at 7:44 PM, Jesse Gross wrote:
> On Thu, Aug 1, 2013 at 3:36 PM, Pravin B Shelar wrote:
>> OVS locking was recently changed to have private OVS lock which
>> simplified overall locking. Therefore there is no need to have
>> another global genl lock to protect OVS data structu
On Tue, Aug 6, 2013 at 7:42 PM, Jesse Gross wrote:
> On Fri, Aug 2, 2013 at 11:38 AM, Pravin B Shelar wrote:
>> Changes are mostly related API changes in vlan, GRE
>> restructuring.
>>
>> Signed-off-by: Pravin B Shelar
>
> Acked-by: Jesse Gross
>
> I agree with Kyle's comment about the FAQ and
XEN dom0 networking assumes dev->master is bond device
and it tries to access bond private structure from dev->master ptr
on receive path. This causes panic, Therefore do not set dev->master
for XEN.
Signed-off-by: Pravin B Shelar
Bug #18920
---
datapath/linux/compat/include/linux/netdevice.h |
On Sat, Aug 10, 2013 at 8:51 PM, Ben Pfaff wrote:
> On Sat, Aug 10, 2013 at 04:32:11PM -0700, Andy Zhou wrote:
> > Sorry for the delay in review. It took me a while to understand the
> design
> > intent to be able to understand the logic.
>
> Thanks for the review.
>
> If the design intent is un
Here is an example I can think of:
Thread A wants to do a seq_change(), Thread B wants to do seq_wait(), they
both compete for dp_netdev_mutex at the same time.
Assume Thread A wins, seq_change changed the seq->value to +1 and releases
the dp_netdev_mutex. Thread B now waits for the +1 value to c
Thanks, didn't know about that. I'll use that link instead.
On Mon, Aug 12, 2013 at 11:08 PM, Ed Maste wrote:
> On 30 July 2013 20:31, Joe Stringer wrote:
> > This implementation was derived from FreeBSD:
> > http://code.google.com/p/freebsd-head/source/browse/sys/libkern/crc32.c
>
> The canon
On 30 July 2013 20:31, Joe Stringer wrote:
> This implementation was derived from FreeBSD:
> http://code.google.com/p/freebsd-head/source/browse/sys/libkern/crc32.c
The canonical FreeBSD web repo location for this file would be:
http://svnweb.freebsd.org/base/head/sys/libkern/crc32.c
(It doesn't
62 matches
Mail list logo
