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
---
v16
* No change
v15
* A
[ CC Ethan Jackson.
Ethan, this makes changes to ofproto-dpif and ofproto-dpif-xlate.
We would appreciate some feedback with how these changes fit with
your ongoing refactoring in this area. ]
Recirculation is a technique to allow a frame to re-enter
frame processing. This is intended to be
This is now sane as recirculation is implemented
Signed-off-by: Simon Horman
---
v16
* First post
---
datapath/datapath.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index c8143b9..eb14b23 100644
--- a/datapath/datap
On Mon, Jul 29, 2013 at 10:47 AM, Ben Pfaff wrote:
> @@ -22,59 +22,55 @@
> #include
> #include "ovs-atomic.h"
> #include "util.h"
> +
> +/* Mutex. */
>
> struct OVS_LOCKABLE ovs_mutex {
> pthread_mutex_t lock;
> const char *where;
> };
>
> -struct OVS_LOCKABLE ovs_rwlock {
> -
On Fri, Jul 26, 2013 at 06:07:10PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
GCC says:
../ofproto/tunnel.c: In function ‘tnl_port_del’:
../ofproto/tunnel.c:191: error: unused variable ‘tnl_port’
The logging code in tnl_port_receive() is really fucking weird (not
your fau
On Mon, Jul 29, 2013 at 3:47 PM, Pravin B Shelar wrote:
> This makes ovs-module more in-sync with upstream ovs-module.
>
> Signed-off-by: Pravin B Shelar
Kyle/Lori, do you guys want to take a first look at this series?
X-CudaMail-Whitelist-To: dev@openvswitch.org
On Fri, Jul 26, 2013 at 06:07:09PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
Why does the client do the locking?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Fri, Jul 26, 2013 at 06:07:08PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
Any particular reason you chose to use a read/write lock here? I've
read the patch but I haven't yet audited the r/w choice in each case.
___
dev mailing list
CC: Lori Jakab
Signed-off-by: Pravin B Shelar
---
datapath/vport-lisp.c | 48 ++--
1 files changed, 2 insertions(+), 46 deletions(-)
diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
index 6e37b2f..847cb39 100644
--- a/datapath/vport-lisp.c
OVS already has compat functions to handle GSO packets.
Following patch get rid of GSO packet handling in lisp
and use ovs iptunnel_xmit() function for same.
CC: Lori Jakab
Signed-off-by: Pravin B Shelar
---
datapath/vport-lisp.c | 209 -
1 files
Generic tunnel rcv and send function are only used by lisp tunneling
module, so It make sense to move them to lisp module.
CC: Lori Jakab
Signed-off-by: Pravin B Shelar
---
datapath/Modules.mk|2 -
datapath/datapath.c|1 -
datapath/datapath.h|1 -
datapath/dp_notify.c
Signed-off-by: Pravin B Shelar
---
datapath/compat.h | 42 ++
datapath/tunnel.c | 38 --
datapath/tunnel.h |4
3 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/datapath/compat.h b/datapath/com
This makes ovs-module more in-sync with upstream ovs-module.
Signed-off-by: Pravin B Shelar
---
datapath/flow.h| 16
datapath/tunnel.h | 16
datapath/vport-gre.c |2 +-
datapath/vport-lisp.c |2 +-
datapath/vport-vxlan.c |2 +-
5
Thanks, I merged both of these.
Ethan
On Mon, Jul 29, 2013 at 3:32 PM, Ben Pfaff wrote:
> On Mon, Jul 29, 2013 at 03:28:09PM -0700, Ethan Jackson wrote:
>> For some reason the current suppression fails to actually suppress the
>> warning.
>>
>> Signed-off-by: Ethan Jackson
>
> The old version w
On Mon, Jul 29, 2013 at 03:28:08PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Mon, Jul 29, 2013 at 03:28:09PM -0700, Ethan Jackson wrote:
> For some reason the current suppression fails to actually suppress the
> warning.
>
> Signed-off-by: Ethan Jackson
The old version worked OK with my glibc but so does the new one so:
Acked-by: Ben Pfaff
___
Thanks, merged.
Ethan
On Mon, Jul 29, 2013 at 2:33 PM, Jesse Gross wrote:
> On Mon, Jul 29, 2013 at 2:23 PM, Ethan Jackson wrote:
>> Signed-off-by: Ethan Jackson
>> ---
>> datapath/linux/.gitignore |2 ++
>> 1 file changed, 2 insertions(+)
>
> Acked-by: Jesse Gross
X-CudaMail-Whitelist-T
On Mon, Jul 29, 2013 at 03:22:52PM -0400, Ed Maste wrote:
> On 29 July 2013 12:57, Ben Pfaff wrote:
> > In C, one can do preprocessor tricks by making a macro expansion include
> > the macro's own name. We actually used this in the tree to automatically
> > provide function arguments, e.g.:
> >
>
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index c10ca7d..21f0fc5 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -173,7 +173,9 @@ def run():
if op
For some reason the current suppression fails to actually suppress the
warning.
Signed-off-by: Ethan Jackson
---
tests/glibc.supp |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/glibc.supp b/tests/glibc.supp
index 52d17bc..948ee01 100644
--- a/tests/glibc.supp
+++
On Mon, Jul 29, 2013 at 10:52:52AM -0700, Gurucharan Shetty wrote:
> On Mon, Jul 29, 2013 at 9:57 AM, Ben Pfaff wrote:
>
> > In C, one can do preprocessor tricks by making a macro expansion include
> >
> Usually we put a : here. I am not sure whether you expect it on every
> patch.
For commits l
On Fri, Jul 26, 2013 at 06:07:07PM -0700, Ethan Jackson wrote:
> Statistics are the only part of netdev-vport which need to be
> manipulated by multiple threads. This patch makes them thread safe.
>
> Signed-off-by: Ethan Jackson
I'd rather fold this into my upcoming netdev series if that's OK
On Fri, Jul 26, 2013 at 06:07:06PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
I believe that bfd_get_status() has a deadlock because it calls
bfd_forwarding() which also takes the lock.
Otherwise:
Acked-by: Ben Pfaff
___
dev mailing l
On Fri, Jul 26, 2013 at 06:07:05PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
In lacp_unref(), I think we need to move list_remove(&lacp->node);
inside the lock/unlock. Otherwise we have a race here where
lacp_find() can grab an entry that is being destroyed.
Acked-by: Ben Pfaff
On Mon, Jul 29, 2013 at 2:23 PM, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
> ---
> datapath/linux/.gitignore |2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Jesse Gross
X-CudaMail-Whitelist-To: dev@openvswitch.org
___
dev mailing lis
On Mon, Jul 29, 2013 at 2:05 PM, Andy Zhou wrote:
> Missing VLAN netlink attribute should be interpreted as exact match
> of no VLAN tag, instead of wildcarded match for all VLAN tags.
>
> Bug #18736.
>
> Signed-off-by: Andy Zhou
Applied, thanks.
___
d
> Some of the functions take the mutex gratuitously, e.g. because they
> access immutable state. One example is stp_get_name(). I guess that
> these are not a big deal because this is not going to be performance
> critical, but at some point we'll hit some performance-critical case
> where we don
Signed-off-by: Ethan Jackson
---
datapath/linux/.gitignore |2 ++
1 file changed, 2 insertions(+)
diff --git a/datapath/linux/.gitignore b/datapath/linux/.gitignore
index e7ac6c1..8748613 100644
--- a/datapath/linux/.gitignore
+++ b/datapath/linux/.gitignore
@@ -14,6 +14,7 @@
/exthdrs_core.
I've been using them to dev for some time. Thanks for the reviews,
I'll merge soon.
Ethan
On Mon, Jul 29, 2013 at 1:50 PM, Ben Pfaff wrote:
> On Mon, Jul 29, 2013 at 01:46:42PM -0700, Ethan Jackson wrote:
>> Signed-off-by: Ethan Jackson
>
> Acked-by: Ben Pfaff
>
> (I didn't test either of the
> Yeah, I understand the rationale, I just want to know whether we can
> make the pointer const.
I'm fine with it. I used the same trick in some other patches, I'll
be sure to update them.
Ethan
X-CudaMail-Whitelist-To: dev@openvswitch.org
___
dev mail
On Mon, Jul 29, 2013 at 02:10:40PM -0700, Ethan Jackson wrote:
> > In lockfile.c, I guess that clang can only handle guarded-by on
> > pointers? Otherwise I don't see why one would introduce the new
> > lock_table variable as a level of indirection. Assuming that's true,
> > can we declare lock_t
> OVS_RELEASES applies to both read-locks and write-locks?
>
> I guess that an ordinary mutex is considered a write-lock?
Yep, I suppose we could create a second #define for that. Not sure if
it matters or not.
> In lockfile.c, I guess that clang can only handle guarded-by on
> pointers? Otherw
Missing VLAN netlink attribute should be interpreted as exact match
of no VLAN tag, instead of wildcarded match for all VLAN tags.
Bug #18736.
Signed-off-by: Andy Zhou
---
v1->v2
*Remove the fix for in_port.
*Always set VLAN_TAG_PRESENT bit in the mask.
v2->v3
*Move VLAN
This bug will cause mask values to corrupt the flow key value. So far
the bug has not showed up because we don't write mask value when
there is no mask Netlink attributes. However, it needs to be fixed for
the next and future commits where we will start to set default
values for key and mask for m
On Fri, Jul 26, 2013 at 06:07:04PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
Why does cfm_generate_maid() need the mutex? It doesn't seem to use
any static data and when it is called the new struct cfm isn't visible
outside the running thread.
Can all_cfms be a const pointer, e
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index f3a1ba9..c10ca7d 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -52,6 +52,9 @@ def uname():
On Fri, Jul 26, 2013 at 06:07:02PM -0700, Ethan Jackson wrote:
> This commit adds annotations for thread safety check. And the
> check can be conducted by using -Wthread-safety flag in clang.
>
> Co-authored-by: Alex Wang
> Signed-off-by: Alex Wang
> Signed-off-by: Ethan Jackson
Currently, the
On Mon, Jul 29, 2013 at 01:46:41PM -0700, Ethan Jackson wrote:
> Both -Wall and -Wextra are handled by autoconf, so there's no longer a
> need for ovs-dev.py to pass them through CFLAGS.
>
> Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
___
dev mai
On Mon, Jul 29, 2013 at 01:46:42PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
(I didn't test either of these but they look reasonable to me.)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/l
I will send out V3 that returns false, and move the logic back to
ovs_key_from_nlattrs(). Userspace fixes, if any, needs to have its own
patch.
On Mon, Jul 29, 2013 at 1:29 PM, Jesse Gross wrote:
> On Mon, Jul 29, 2013 at 1:27 PM, Jesse Gross wrote:
> > On Mon, Jul 29, 2013 at 12:44 PM, Andy Z
Both -Wall and -Wextra are handled by autoconf, so there's no longer a
need for ovs-dev.py to pass them through CFLAGS.
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index 8
On Mon, Jul 29, 2013 at 1:26 PM, Andy Zhou wrote:
> This bug will cause mask values to corrupt the flow key value. So far
> the bug has not showed up because we don't write mask value when
> there is no mask Netlink attributes. However, it needs to be fixed for
> the next and future commits where
On Mon, Jul 29, 2013 at 1:27 PM, Jesse Gross wrote:
> On Mon, Jul 29, 2013 at 12:44 PM, Andy Zhou wrote:
>> On Mon, Jul 29, 2013 at 10:32 AM, Jesse Gross wrote:
>>>
>>> On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
>>> > diff --git a/datapath/flow.c b/datapath/flow.c
>>> > index ba775f4..5
On Mon, Jul 29, 2013 at 12:44 PM, Andy Zhou wrote:
> On Mon, Jul 29, 2013 at 10:32 AM, Jesse Gross wrote:
>>
>> On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
>> > diff --git a/datapath/flow.c b/datapath/flow.c
>> > index ba775f4..5ec1b69 100644
>> > --- a/datapath/flow.c
>> > +++ b/datapath
Missing VLAN netlink attribute should be interpreted as exact match
of no VLAN tag, instead of wildcarded match for all VLAN tags.
Bug #18736.
Signed-off-by: Andy Zhou
---
v1->v2
*Remove the fix for in_port.
*Always set VLAN_TAG_PRESENT bit in the mask.
---
datapath/flow.c |
This bug will cause mask values to corrupt the flow key value. So far
the bug has not showed up because we don't write mask value when
there is no mask Netlink attributes. However, it needs to be fixed for
the next and future commits where we will start to set default
values for key and mask for m
When we were previously doing just exact match, 'no-port' still
existed and it obviously wasn't a wildcard so I don't think that it
inherently means one now.
When the in port value is omitted, we currently allow you to omit the
mask or have either an all zeros or all ones mask so userspace should
On Mon, Jul 29, 2013 at 10:32 AM, Jesse Gross wrote:
> On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index ba775f4..5ec1b69 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -224,6 +224,15 @@ static bool ovs_match_valida
On Mon, Jul 29, 2013 at 11:06:55AM -0700, Alex Wang wrote:
> > > +#if __has_feature(c_thread_safety_attributes)
> > > +/* "clang" annotations for thread safety check.
> > > + *
> > > + * OVS_LOCKABLE indicates that the struct contains mutex element
> > > + * which can be locked by ovs_mutex_lock().
Yes indeed. Will add to this patch.
On Mon, Jul 29, 2013 at 9:46 AM, Jesse Gross wrote:
> On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
> > This bug will cause mask values to corrupt the flow key value. So far
> > the bug has not showed up because we don't write mask value when
> > there
On 29 July 2013 12:57, Ben Pfaff wrote:
> In C, one can do preprocessor tricks by making a macro expansion include
> the macro's own name. We actually used this in the tree to automatically
> provide function arguments, e.g.:
>
> #define f(x) f(x, __FILE__, __LINE__)
> int f(int x, const
Should 'no-port' be treated as wildcarded match on in_port?
On Mon, Jul 29, 2013 at 11:31 AM, Jesse Gross wrote:
> On Mon, Jul 29, 2013 at 10:32 AM, Jesse Gross wrote:
> > On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
> >> @@ -1317,6 +1326,7 @@ static int metadata_from_nlattrs(struct
> s
On Mon, Jul 29, 2013 at 10:32 AM, Jesse Gross wrote:
> On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
>> @@ -1317,6 +1326,7 @@ static int metadata_from_nlattrs(struct sw_flow_match
>> *match, u64 *attrs,
>> *attrs &= ~(1ULL << OVS_KEY_ATTR_IN_PORT);
>> } else if (!is
On Fri, Jul 26, 2013 at 06:07:03PM -0700, Ethan Jackson wrote:
> Signed-off-by: Ethan Jackson
In stp_unref(), the list_remove() needs to be guarded by the mutex.
Some of the annotations are only on the function prototypes and not
the definitions. Although the compiler is OK with this, it would
On Mon, Jul 29, 2013 at 10:47:47AM -0700, Ben Pfaff wrote:
> On Fri, Jul 26, 2013 at 06:07:02PM -0700, Ethan Jackson wrote:
> > This commit adds annotations for thread safety check. And the
> > check can be conducted by using -Wthread-safety flag in clang.
One additional thought. In reading the a
> > +#if __has_feature(c_thread_safety_attributes)
> > +/* "clang" annotations for thread safety check.
> > + *
> > + * OVS_LOCKABLE indicates that the struct contains mutex element
> > + * which can be locked by ovs_mutex_lock().
> > + *
>
> What does the following sentence mean? I do not underst
On Mon, Jul 29, 2013 at 9:57 AM, Ben Pfaff wrote:
> In C, one can do preprocessor tricks by making a macro expansion include
>
Usually we put a : here. I am not sure whether you expect it on every
patch.
> the macro's own name. We actually used this in the tree to automatically
> provide funct
On Fri, Jul 26, 2013 at 06:07:02PM -0700, Ethan Jackson wrote:
> This commit adds annotations for thread safety check. And the
> check can be conducted by using -Wthread-safety flag in clang.
I would think that the commit should also explain where to get clang
with this support, probably by adding
On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
> diff --git a/datapath/flow.c b/datapath/flow.c
> index ba775f4..5ec1b69 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -224,6 +224,15 @@ static bool ovs_match_validate(const struct
> sw_flow_match *match,
> return f
On Mon, Jul 29, 2013 at 10:24:50AM -0700, Gurucharan Shetty wrote:
> On Mon, Jul 29, 2013 at 9:57 AM, Ben Pfaff wrote:
>
> > In C, one can do preprocessor tricks by making a macro expansion include
> > the macro's own name. We actually used this in the tree to automatically
> > provide function
On Mon, Jul 29, 2013 at 9:57 AM, Ben Pfaff wrote:
> In C, one can do preprocessor tricks by making a macro expansion include
> the macro's own name. We actually used this in the tree to automatically
> provide function arguments, e.g.:
>
> #define f(x) f(x, __FILE__, __LINE__)
> int f(in
In C, one can do preprocessor tricks by making a macro expansion include
the macro's own name. We actually used this in the tree to automatically
provide function arguments, e.g.:
#define f(x) f(x, __FILE__, __LINE__)
int f(int x, const char *file, int line);
...
f(1);/* Expands
On Sat, Jul 27, 2013 at 10:27 PM, Andy Zhou wrote:
> This bug will cause mask values to corrupt the flow key value. So far
> the bug has not showed up because we don't write mask value when
> there is no mask Netlink attributes. However, it needs to be fixed for
> the next and future commits wher
On Mon, Jul 29, 2013 at 09:26:56AM -0700, Alex Wang wrote:
> I think so. I saw new modifications in 01/11 patch.
OK, I'll review Ethan's version.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
I think so. I saw new modifications in 01/11 patch.
Thanks,
Alex Wang,
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Fri, Jul 26, 2013 at 06:07:01PM -0700, Ethan Jackson wrote:
> This series of patches adds thread safety to a bunch of modules used
> by ofproto-dpif-xlate. The first patch of the series adds support for
> clang's thread safety annotations which as Alex mentioned aren't quite
> upstreamed yet.
66 matches
Mail list logo
