Re: [ovs-dev] [PATCH] ovs-atomic: __GNUC__ == 4 is enough

[Linhaifeng]

> 
> On 9/19/20 8:08 AM, Linhaifeng wrote:
> > 1. include config.h to avoid include different atomic head file
> 
> All .c files should include config.h as their first include, so config.h 
> should always
> be already included at the time we including ovs-atomic.h.  Do you know the .c
> file that doesn't include config.h?
> If so, please, fix it instead.  Header files should not include config.h by
> themselves.
> 

OK. Thank you.

> > 2. __GNUC__ == 4 is enough
> 
> This is technically correct, but it doesn't worth the work for fixing it 
> unless
> you're fixing some real issue at the same time.
> 

OK. Thank you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovs-atomic: __GNUC__ == 4 is enough

[Linhaifeng]

1. include config.h to avoid include different atomic head file
2. __GNUC__ == 4 is enough

Fixes: 31a3fc6e3e9c ("ovs-atomic: New library for atomic operations.")
Cc: b...@nicira.com

Signed-off-by: Linhaifeng 
---
 lib/ovs-atomic.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
index 11fa19268..2aee4c608 100644
--- a/lib/ovs-atomic.h
+++ b/lib/ovs-atomic.h
@@ -311,6 +311,7 @@
  * memory_order_seq_cst for atomic_flag_clear()).
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -331,7 +332,7 @@
 #include "ovs-atomic-c11.h"
 #elif __GNUC__ >= 5 && !defined(__cplusplus)
 #error "GCC 5+ should have "
-#elif __GNUC__ >= 5 || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 7)
+#elif __GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7)
 #include "ovs-atomic-gcc4.7+.h"
 #elif __GNUC__ && defined(__x86_64__)
 #include "ovs-atomic-x86_64.h"
-- 
2.21.0.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

Hi, Ben

Will this patch apply ?

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> Sent: Wednesday, June 3, 2020 10:33 PM
> To: Linhaifeng ; Ilya Maximets
> ; Ben Pfaff 
> Cc: d...@openvswitch.org; Lilijun (Jerry) ; Lichunhe
> ; nd ; chenchanghu
> 
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> The change could look like this:
> 
> diff --git a/lib/ovs-rcu.c b/lib/ovs-rcu.c index ebc8120f0..cde1e925b 100644
> --- a/lib/ovs-rcu.c
> +++ b/lib/ovs-rcu.c
> @@ -30,6 +30,8 @@
> 
>  VLOG_DEFINE_THIS_MODULE(ovs_rcu);
> 
> +#define MIN_CBS 16
> +
>  struct ovsrcu_cb {
>  void (*function)(void *aux);
>  void *aux;
> @@ -37,7 +39,8 @@ struct ovsrcu_cb {
> 
>  struct ovsrcu_cbset {
>  struct ovs_list list_node;
> -struct ovsrcu_cb cbs[16];
> +struct ovsrcu_cb *cbs;
> +size_t n_allocated;
>  int n_cbs;
>  };
> 
> @@ -310,16 +313,19 @@ ovsrcu_postpone__(void (*function)(void *aux), void
> *aux)
>  cbset = perthread->cbset;
>  if (!cbset) {
>  cbset = perthread->cbset = xmalloc(sizeof *perthread->cbset);
> +cbset->cbs = xmalloc(MIN_CBS * sizeof *cbset->cbs);
> +cbset->n_allocated = MIN_CBS;
>  cbset->n_cbs = 0;
>  }
> 
> +if (cbset->n_cbs == cbset->n_allocated) {
> +cbset->cbs = x2nrealloc(cbset->cbs, >n_allocated,
> +sizeof *cbset->cbs);
> +}
> +
>  cb = >cbs[cbset->n_cbs++];
>  cb->function = function;
>  cb->aux = aux;
> -
> -if (cbset->n_cbs >= ARRAY_SIZE(cbset->cbs)) {
> -ovsrcu_flush_cbset(perthread);
> -}
>  }
> 
>  static bool
> @@ -341,6 +347,7 @@ ovsrcu_call_postponed(void)
>  for (cb = cbset->cbs; cb < >cbs[cbset->n_cbs]; cb++) {
>  cb->function(cb->aux);
>  }
> +free(cbset->cbs);
>  free(cbset);
>  }
> 
> ---
> 
> 
> I could submit a formal patch, if agreed.
> 
> Ben, what do you think?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> Sent: Thursday, June 4, 2020 12:01 AM
> To: Ben Pfaff ; Linhaifeng 
> Cc: d...@openvswitch.org; Lilijun (Jerry) ; Lichunhe
> ; nd ; chenchanghu
> ; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> On 6/3/20 7:26 AM, Ben Pfaff wrote:
> > On Wed, Jun 03, 2020 at 01:22:52AM +, Linhaifeng wrote:
> >>
> >>
> >> -Original Message-
> >> From: Ben Pfaff [mailto:b...@ovn.org]
> >> Sent: Wednesday, June 3, 2020 1:28 AM
> >> To: Linhaifeng 
> >> Cc: Yanqin Wei ; d...@openvswitch.org; nd
> >> ; Lilijun (Jerry) ; chenchanghu
> >> ; Lichunhe 
> >> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> >>
> >> On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> >>> We should update rcu pointer first then use ovsrcu_postpone to free
> >>> otherwise maybe cause use-after-free.
> >>> e.g.,reader indicates momentary quiescent and access old pointer
> >>> after writer postpone free old pointer and before setting new pointer.
> >>>
> >>> Signed-off-by: Linhaifeng 
> >>
> >> I don't see how that's possible, since the writer hasn't quiesced.
> >>
> >> I think the logic is as follow, Could you help me find out where is 
> >> incorrect?
> >>
> >> 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4 ->
> >> 3.3 -> 2.2(use after free)
> >>
> >> wirter:
> >> 1.1 use postone to free old pointer
> >> 1.2 flush cbsets to flushed_cbsets
> >> 1.3 update new pointer
> >> 1.4 quiesced
> >>
> >> Read:
> >> 2.1. read pointer
> >> 2.2. use pointer
> >> 2.3. quiesced
> >>
> >> Rcu:
> >> 3.1 pop flushed_cbsets
> >> 3.2 ovsrcu_synchronize
> >> 3.3 call all cb to free
> >
> > So you're saying this:
> >
> > 1.1 use postone to free old pointer (A)
> > 1.2 flush cbsets to flushed_cbsets
> >
> > 3.1 pop flushed_cbsets
> > 3.2 ovsrcu_synchronize
> >
> > 2.1. read pointer (A)
> > 2.2. use pointer (A)
> > 2.3. quiesced
> >
> > 2.1. read pointer (A)
> 
> IIUC, 'Read' thread should enter quiescent state here at least one more time 
> in
> order to trigger the issue (or, maybe, some other thread could advance
> global_seqno before step 2.3 and after reading target_seqno by RCU thread on
> step 3.2).  Otherwise, target_seqno will be equal to thread->seqno of the
> 'Read' thread and RCU thread will wait for the next quiescent state before
> calling callbacks.
> 

Yes, you are right. The reader's thread->seqno should bigger than global_seqno 
so it maybe have run one more times before someone's thread->seqno bigger than 
global_seqno.

> >
> > 1.3 update new pointer (B)
> > 1.4 quiesced
> >
> > 3.3 call all cb to free (A)
> >
> > 2.2. use pointer (A)
> >
> > Wow, you are absolutely right.  This had never occurred to me.  Thank
> > you!  I'll review your patch.
> 
> Sharing my more detailed diagram of the execution flow that should trigger the
> issue here:
> 
>  --  --  ---
>  Thread 1Thread 2RCU Thread
>  --  --  ---
>  pointer = A
> 
>  ovsrcu_quiesce():
>   thread->seqno = 30
>   global_seqno = 31
>   quiesced
> 
>  read pointer A
>  postpone(free(A)):
>flush cbset
>  pop flushed_cbsets
>  ovsrcu_synchronize:
>target_seqno = 31
>  ovsrcu_quiesce():
>   thread->seqno = 31
>   global_seqno = 32
>   quiesced
> 
>  read pointer A
>  use pointer A
> 
>  ovsrcu_quiesce():
>   thread->seqno = 32
>   global_seqno = 33
>   quiesced
> 
>  read pointer A
>  pointer = B
> 
>  ovsrcu_quiesce():
>   thread->seqno = 33
>   global_seqno = 34
>   quiesced
> 
>  target_seqno exceeded
>  by all threads
>  call cbs to free A
>  use pointer A
>  (use after free)
>  ---
> 
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@ovn.org]
> Sent: Wednesday, June 3, 2020 6:50 PM
> To: Linhaifeng ; Ben Pfaff 
> Cc: i.maxim...@ovn.org; d...@openvswitch.org; Lilijun (Jerry)
> ; Lichunhe ; nd
> ; chenchanghu 
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> On 6/3/20 9:04 AM, Linhaifeng wrote:
> >
> >
> >> -Original Message-
> >> From: Ben Pfaff [mailto:b...@ovn.org]
> >> Sent: Wednesday, June 3, 2020 1:26 PM
> >> To: Linhaifeng 
> >> Cc: Yanqin Wei ; d...@openvswitch.org; nd
> >> ; Lilijun (Jerry) ; chenchanghu
> >> ; Lichunhe 
> >> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> >>
> >> On Wed, Jun 03, 2020 at 01:22:52AM +, Linhaifeng wrote:
> >>>
> >>>
> >>> -Original Message-
> >>> From: Ben Pfaff [mailto:b...@ovn.org]
> >>> Sent: Wednesday, June 3, 2020 1:28 AM
> >>> To: Linhaifeng 
> >>> Cc: Yanqin Wei ; d...@openvswitch.org; nd
> >>> ; Lilijun (Jerry) ;
> >>> chenchanghu ; Lichunhe
> 
> >>> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> >>>
> >>> On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> >>>> We should update rcu pointer first then use ovsrcu_postpone to free
> >>>> otherwise maybe cause use-after-free.
> >>>> e.g.,reader indicates momentary quiescent and access old pointer
> >>>> after writer postpone free old pointer and before setting new pointer.
> >>>>
> >>>> Signed-off-by: Linhaifeng 
> >>>
> >>> I don't see how that's possible, since the writer hasn't quiesced.
> >>>
> >>> I think the logic is as follow, Could you help me find out where is 
> >>> incorrect?
> >>>
> >>> 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4
> >>> ->
> >>> 3.3 -> 2.2(use after free)
> >>>
> >>> wirter:
> >>> 1.1 use postone to free old pointer
> >>> 1.2 flush cbsets to flushed_cbsets
> >>> 1.3 update new pointer
> >>> 1.4 quiesced
> >>>
> >>> Read:
> >>> 2.1. read pointer
> >>> 2.2. use pointer
> >>> 2.3. quiesced
> >>>
> >>> Rcu:
> >>> 3.1 pop flushed_cbsets
> >>> 3.2 ovsrcu_synchronize
> >>> 3.3 call all cb to free
> >>
> >> So you're saying this:
> >>
> >> 1.1 use postone to free old pointer (A)
> >> 1.2 flush cbsets to flushed_cbsets
> >>
> >> 3.1 pop flushed_cbsets
> >> 3.2 ovsrcu_synchronize
> >>
> >> 2.1. read pointer (A)
> >> 2.2. use pointer (A)
> >> 2.3. quiesced
> >>
> >> 2.1. read pointer (A)
> >>
> >> 1.3 update new pointer (B)
> >> 1.4 quiesced
> >>
> >> 3.3 call all cb to free (A)
> >>
> >> 2.2. use pointer (A)
> >>
> >> Wow, you are absolutely right.  This had never occurred to me.  Thank
> you!
> >> I'll review your patch.
> >
> > Yes, it's really hard to happen. If it happened it's also hard to find the 
> > reason
> so I suggest it can be a rule for using rcu.
> 
> I agree that there is an issue here, but I think that we should not force 
> users to
> call ovsrcu_set() before ovsrcu_postpone().  Current users doesn't do
> anything illegal since pointer must not be freed before the next grace period
> from their point of view.
> 
> For me it looks like the main issue is existence of point 1.2, i.e. flushing 
> cbsets
> while writer is not quiesced yet.  And we need to fix this inside rcu library 
> itself.
> For example, we could avoid flushing inside
> ovsrcu_postpone() by making cbs[16] a dynamically allocated array and using
> x2nrealloc instead of flushing.
> 
> Thoughts?
> 
Hi, Ilya Maximets

May be this is a good idea therefor the users not need to think about call 
ovsrcu_set() first or ovsrcu_postpone().
How about you think, ben? May be you can send a patch to modify the 
ovsrcu_postpone() not to flush cbsets to
replace of my patches.
.

> 
> Regarding the patch itself:
> 
> > diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h index ecc4c9201..98c238aea
> > 100644
> > --- a/lib/ovs-rcu.h
> > +++ b/lib/ovs-rcu.h
> > @@ -118,10 +118,10 @@
> >   * void
> >   * change_flow(struct flow *new_flow)
> >   * {
> > + * struct flow *old_flow = ovsrcu_get_protected(struct flow *,
> )
> 
> ovsrcu_get_protected() can not be used outside of the critical section.
> 
> >   * ovs_mutex_lock();
> > - * ovsrcu_postpone(free,
> > - * ovsrcu_get_protected(struct flow *,
> ));
> >   * ovsrcu_set(, new_flow);
> > + * ovsrcu_postpone(free, old_flow);
> >   * ovs_mutex_unlock();
> >   * }
> >
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, June 3, 2020 1:26 PM
> To: Linhaifeng 
> Cc: Yanqin Wei ; d...@openvswitch.org; nd
> ; Lilijun (Jerry) ; chenchanghu
> ; Lichunhe 
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> On Wed, Jun 03, 2020 at 01:22:52AM +, Linhaifeng wrote:
> >
> >
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Wednesday, June 3, 2020 1:28 AM
> > To: Linhaifeng 
> > Cc: Yanqin Wei ; d...@openvswitch.org; nd
> > ; Lilijun (Jerry) ; chenchanghu
> > ; Lichunhe 
> > Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> >
> > On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> > > We should update rcu pointer first then use ovsrcu_postpone to free
> > > otherwise maybe cause use-after-free.
> > > e.g.,reader indicates momentary quiescent and access old pointer
> > > after writer postpone free old pointer and before setting new pointer.
> > >
> > > Signed-off-by: Linhaifeng 
> >
> > I don't see how that's possible, since the writer hasn't quiesced.
> >
> > I think the logic is as follow, Could you help me find out where is 
> > incorrect?
> >
> > 1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4 ->
> > 3.3 -> 2.2(use after free)
> >
> > wirter:
> > 1.1 use postone to free old pointer
> > 1.2 flush cbsets to flushed_cbsets
> > 1.3 update new pointer
> > 1.4 quiesced
> >
> > Read:
> > 2.1. read pointer
> > 2.2. use pointer
> > 2.3. quiesced
> >
> > Rcu:
> > 3.1 pop flushed_cbsets
> > 3.2 ovsrcu_synchronize
> > 3.3 call all cb to free
> 
> So you're saying this:
> 
> 1.1 use postone to free old pointer (A)
> 1.2 flush cbsets to flushed_cbsets
> 
> 3.1 pop flushed_cbsets
> 3.2 ovsrcu_synchronize
> 
> 2.1. read pointer (A)
> 2.2. use pointer (A)
> 2.3. quiesced
> 
> 2.1. read pointer (A)
> 
> 1.3 update new pointer (B)
> 1.4 quiesced
> 
> 3.3 call all cb to free (A)
> 
> 2.2. use pointer (A)
> 
> Wow, you are absolutely right.  This had never occurred to me.  Thank you!
> I'll review your patch.

Yes, it's really hard to happen. If it happened it's also hard to find the 
reason so I suggest it can be a rule for using rcu.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, June 3, 2020 8:35 AM
> To: Yanqin Wei 
> Cc: Linhaifeng ; d...@openvswitch.org; nd
> ; Lilijun (Jerry) ; chenchanghu
> ; Lichunhe 
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> This is not how RCU works in OVS.  Every thread is by default considered
> active.  They rarely quiesce except implicitly inside poll_block().
> Please read the large comment at the top of ovs-rcu.h.
> 
> Is your patch based on actual bugs that you have found, or is it just some 
> kind
> of precaution?  If it is the latter, then it is not needed.
> 
Is an actual bug for old version bug it's also suitable for the other codes in 
ovs.

Here is the debug info:
linux-mNuKFc:/Images/linhf/830/Euler_compile_env # gdb -p `pidof ovs-vswitchd`
GNU gdb (GDB) Red Hat Enterprise Linux 8.2-3.h2
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "aarch64-Huawei-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word".
Attaching to process 102706
[New LWP 109133]
[New LWP 109134]
[New LWP 109297]
[New LWP 109298]
[New LWP 109299]
[New LWP 109300]
[New LWP 109303]
[New LWP 109304]
[New LWP 109308]
[New LWP 109309]
[New LWP 109310]
[New LWP 109311]
[New LWP 109522]
[New LWP 109523]
[New LWP 109603]
[New LWP 109615]
[New LWP 109619]
[New LWP 109655]
[New LWP 109673]
[New LWP 109794]
[New LWP 109795]
[New LWP 113953]
[New LWP 114362]
[New LWP 114364]
[New LWP 114368]
[New LWP 114370]
[New LWP 114373]
[New LWP 114377]
[New LWP 115594]
[New LWP 115595]
[New LWP 115596]
[New LWP 115597]
[New LWP 115598]
[New LWP 115600]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
0x879981ac in poll () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install glib2-2.54.2-2.h1.aarch64 
glibc-2.28-9.h17.aarch64 keyutils-libs-1.5.8-3.aarch64 
krb5-libs-1.15.1-34.h2.aarch64 libcgroup-0.41-15.h3.aarch64 
libcom_err-1.44.3-1.h4.aarch64 libgcc-7.3.0-20190804.h18.aarch64 
libselinux-2.5-12.aarch64 numactl-libs-2.0.9-7.h1.aarch64 
openssl-libs-1.0.2k-16.h6.aarch64 pcre-8.32-17.h9.aarch64 
uvpkmc-1.0.1-807.aarch64 zlib-1.2.7-17.aarch64
(gdb) b dpcls_destroy_subtable
Breakpoint 1 at 0x508bcc: file lib/dpif-netdev.c, line 6919.
(gdb) b ovsrcu_call_postponed
Breakpoint 2 at 0x5b7d34: file lib/ovs-rcu.c, line 336.
(gdb) c
Continuing.
[Switching to Thread 0x83b97860 (LWP 109304)]

Thread 9 "urcu2" hit Breakpoint 2, ovsrcu_call_postponed () at lib/ovs-rcu.c:336
warning: Source file is more recent than executable.
336 {
(gdb) n
339 int wait_del = 0;
(gdb) 
340 while(wait_del);
(gdb) set wait_del = 1
(gdb) c
Continuing.
[Switching to Thread 0x51748860 (LWP 115598)]

Thread 34 "revalidator19" hit Breakpoint 1, dpcls_destroy_subtable 
(cls=0x1c00a420, subtable=0x3c009250) at lib/dpif-netdev.c:6919
6919int wait_get = 0;
(gdb) n
6920VLOG_DBG("Destroying subtable %p for in_port %d", subtable, 
cls->in_port);
(gdb) 
6921pvector_remove(>subtables, subtable);
(gdb) 
6922cmap_remove(>subtables_map, >cmap_node,
(gdb) set wait_get = 1
(gdb) n
6924cmap_destroy(>rules);
(gdb) p subtable->rules
$1 = {impl = {p = 0x30008940}}
(gdb) s
cmap_destroy (cmap=0x3c009258) at lib/cmap.c:288
288 if (cmap) {
(gdb) n
289 struct cmap_impl *impl = cmap_get_impl(cmap);
(gdb) 
290 if (impl != _cmap) {
(gdb) 
291 ovsrcu_postpone(free_cacheline, impl);
(gdb) s
ovsrcu_postpone__ (function=0x6029b0 , aux=0x30008940) at 
lib/ovs-rcu.c:315
315 struct ovsrcu_perthread *perthread = ovsrcu_perthread_get();
(gdb) n
318 int size = ARRAY_SIZE(cbset->cbs);
(gdb) 
319 cbset = perthread->cbset;
(gdb) 
320 if (!cbset) {
(gdb) 
325 cb = >cbs[cbset->n_cbs++];
(gdb) 
326 cb->function = function;
(gdb) 
327 cb->aux = aux;
(gdb) 
329 if (cbset->n_cbs >= size) {
(gdb) set size = cbset->n_cbs
(gdb) n
330 ovsrcu_flush_cbset(perthread);
(gdb) s
ovsrcu_flush_cbset (perthread=0x30001210) at lib/ovs-rcu.c:397
397 ovsrcu_flush_

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

-Original Message-
From: Yanqin Wei [mailto:yanqin@arm.com] 
Sent: Wednesday, June 3, 2020 7:23 AM
To: Ben Pfaff ; Linhaifeng 
Cc: d...@openvswitch.org; nd ; Lilijun (Jerry) 
; chenchanghu ; Lichunhe 

Subject: RE: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

Hi Ben,

If my understanding is correct, the writer could not be a rcu thread because it 
does not need report holding or not holding pointers.
So old memory will be freed after all rcu thread report quiesce.

The write also is rcu thread. If not first update pointer the reader can also 
get the old pointer after call ovsrcu_quiesced.

Best Regards,
Wei Yanqin

> -Original Message-
> From: Ben Pfaff 
> Sent: Wednesday, June 3, 2020 1:28 AM
> To: Linhaifeng 
> Cc: Yanqin Wei ; d...@openvswitch.org; nd 
> ; Lilijun (Jerry) ; chenchanghu 
> ; Lichunhe 
> Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first
> 
> On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> > We should update rcu pointer first then use ovsrcu_postpone to free 
> > otherwise maybe cause use-after-free.
> > e.g.,reader indicates momentary quiescent and access old pointer 
> > after writer postpone free old pointer and before setting new pointer.
> >
> > Signed-off-by: Linhaifeng 
> 
> I don't see how that's possible, since the writer hasn't quiesced.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Wednesday, June 3, 2020 1:28 AM
To: Linhaifeng 
Cc: Yanqin Wei ; d...@openvswitch.org; nd ; 
Lilijun (Jerry) ; chenchanghu 
; Lichunhe 
Subject: Re: [ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

On Tue, Jun 02, 2020 at 07:27:59AM +, Linhaifeng wrote:
> We should update rcu pointer first then use ovsrcu_postpone to free 
> otherwise maybe cause use-after-free.
> e.g.,reader indicates momentary quiescent and access old pointer after 
> writer postpone free old pointer and before setting new pointer.
> 
> Signed-off-by: Linhaifeng 

I don't see how that's possible, since the writer hasn't quiesced.

I think the logic is as follow, Could you help me find out where is incorrect?

1.1 -> 1.2 -> 3.1 -> 3.2 -> 2.1 -> 2.2 -> 2.3 -> 2.1 -> 1.3 -> 1.4 -> 3.3 -> 
2.2(use after free)

wirter:
1.1 use postone to free old pointer
1.2 flush cbsets to flushed_cbsets
1.3 update new pointer
1.4 quiesced

Read:
2.1. read pointer
2.2. use pointer
2.3. quiesced

Rcu:
3.1 pop flushed_cbsets
3.2 ovsrcu_synchronize
3.3 call all cb to free



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 5/5] classifier: fix conj_set use-after-free issue

[Linhaifeng]

use ovsrcu_set first then use ovsrcu_postpone

CC: Ben Pfaff 
Fixes: 18080541d276 (\classifier: Add support for conjunctive matches.\)

Acked-by: Yanqin Wei 
Signed-off-by: Linhaifeng 
---
 lib/classifier.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index f2c3497c2..6bff76e07 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr,
 unsigned int old_n = old ? old->n : 0;
 
 if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof *conj))) {
+ovsrcu_set(>conj_set,
+   cls_conjunction_set_alloc(match, conj, n));
 if (old) {
 ovsrcu_postpone(free, old);
 }
-ovsrcu_set(>conj_set,
-   cls_conjunction_set_alloc(match, conj, n));
 }
 }
 
-- 
2.21.0.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 4/5] ofproto: fix actions use-after-free issue

[Linhaifeng]

use ovsrcu_set first then use ovsrcu_postpone

CC: Eelco Chaudron 
Fixes: f82b3b6a2f4d (\ofproto-dpif-upcall: Only call ovsrcu_postpone()
on active actions\)

Acked-by: Yanqin Wei 
Signed-off-by: Linhaifeng 
---
 ofproto/ofproto-dpif-upcall.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10d..be6dafb78 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1658,11 +1658,10 @@ ukey_set_actions(struct udpif_key *ukey, const struct 
ofpbuf *actions)
 struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *,
   >actions);
 
+ovsrcu_set(>actions, ofpbuf_clone(actions));
 if (old_actions) {
 ovsrcu_postpone(ofpbuf_delete, old_actions);
 }
-
-ovsrcu_set(>actions, ofpbuf_clone(actions));
 }
 
 static struct udpif_key *
-- 
2.21.0.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 3/5] ofproto: fix vlans use-after-free issue

[Linhaifeng]

use ovsrcu_set first then use ovsrcu_postpone

CC: Jarno Rajahalme 
Fixes: 4f6780691653 (\mirror: Allow concurrent lookups.\)

Acked-by: Yanqin Wei 
Signed-off-by: Linhaifeng 
---
 ofproto/ofproto-dpif-mirror.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
index 343b75f0e..343100c08 100644
--- a/ofproto/ofproto-dpif-mirror.c
+++ b/ofproto/ofproto-dpif-mirror.c
@@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 hmapx_destroy(_map);
 
 if (vlans || src_vlans) {
+unsigned long *new_vlans = vlan_bitmap_clone(src_vlans);
+ovsrcu_set(>vlans, new_vlans);
 ovsrcu_postpone(free, vlans);
-vlans = vlan_bitmap_clone(src_vlans);
-ovsrcu_set(>vlans, vlans);
 }
 
 mirror->out = out;
-- 
2.21.0.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 2/5] pvector: fix pvector use-after-free issue

[Linhaifeng]

use ovsrcu_set first then use ovsrcu_postpone

CC: Jarno Rajahalme 
Fixes: da9cfca6e2d7 (\Revert "pvector: Expose non-concurrent priority
vector."\)

Acked-by: Yanqin Wei 
Signed-off-by: Linhaifeng 
---
 lib/pvector.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/pvector.c b/lib/pvector.c
index cc527fdc4..aa8c6cb24 100644
--- a/lib/pvector.c
+++ b/lib/pvector.c
@@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec)
 void
 pvector_destroy(struct pvector *pvec)
 {
+struct pvector_impl *old = pvector_impl_get(pvec);
 free(pvec->temp);
 pvec->temp = NULL;
-ovsrcu_postpone(free, pvector_impl_get(pvec));
 ovsrcu_set(>impl, NULL); /* Poison. */
+ovsrcu_postpone(free, old);
 }
 
 /* Iterators for callers that need the 'index' afterward. */
@@ -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void *ptr, 
int priority)
 /* Make the modified pvector available for iteration. */
 void pvector_publish__(struct pvector *pvec)
 {
-struct pvector_impl *temp = pvec->temp;
-
+struct pvector_impl *new = pvec->temp;
+struct pvector_impl *old = ovsrcu_get_protected(struct pvector_impl *,
+   >impl);
 pvec->temp = NULL;
-pvector_impl_sort(temp); /* Also removes gaps. */
-ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *,
-   >impl));
-ovsrcu_set(>impl, temp);
+pvector_impl_sort(new); /* Also removes gaps. */
+ovsrcu_set(>impl, new);
+ovsrcu_postpone(free, old);
 }
-- 
2.21.0.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 1/5] ovs-rcu: fix rcu use-after-free issue

[Linhaifeng]

We should update rcu pointer first then use ovsrcu_postpone to free
otherwise maybe cause use-after-free.
e.g.,reader indicates momentary quiescent and access old pointer after
writer postpone free old pointer and before setting new pointer.

CC: Ben Pfaff 
Fixes: 0f2ea84841e1 (\ovs-rcu: New library.\)

Acked-by: Yanqin Wei 
Signed-off-by: Linhaifeng 
---
 lib/ovs-rcu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index ecc4c9201..98c238aea 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -118,10 +118,10 @@
  * void
  * change_flow(struct flow *new_flow)
  * {
+ * struct flow *old_flow = ovsrcu_get_protected(struct flow *, )
  * ovs_mutex_lock();
- * ovsrcu_postpone(free,
- * ovsrcu_get_protected(struct flow *, ));
  * ovsrcu_set(, new_flow);
+ * ovsrcu_postpone(free, old_flow);
  * ovs_mutex_unlock();
  * }
  *
-- 
2.21.0.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovs rcu: update rcu pointer first

[Linhaifeng]

We should update rcu pointer first then use ovsrcu_postpone to free
otherwise maybe cause use-after-free.
e.g.,reader indicates momentary quiescent and access old pointer after
writer postpone free old pointer and before setting new pointer.

Signed-off-by: Linhaifeng 
---
 lib/classifier.c  |  4 ++--
 lib/ovs-rcu.h |  4 ++--
 lib/pvector.c | 15 ---
 ofproto/ofproto-dpif-mirror.c |  4 ++--
 ofproto/ofproto-dpif-upcall.c |  3 +--
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index f2c3497c2..6bff76e07 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr,
 unsigned int old_n = old ? old->n : 0;
 
 if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof *conj))) {
+ovsrcu_set(>conj_set,
+   cls_conjunction_set_alloc(match, conj, n));
 if (old) {
 ovsrcu_postpone(free, old);
 }
-ovsrcu_set(>conj_set,
-   cls_conjunction_set_alloc(match, conj, n));
 }
 }
 
diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index ecc4c9201..98c238aea 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -118,10 +118,10 @@
  * void
  * change_flow(struct flow *new_flow)
  * {
+ * struct flow *old_flow = ovsrcu_get_protected(struct flow *, )
  * ovs_mutex_lock();
- * ovsrcu_postpone(free,
- * ovsrcu_get_protected(struct flow *, ));
  * ovsrcu_set(, new_flow);
+ * ovsrcu_postpone(free, old_flow);
  * ovs_mutex_unlock();
  * }
  *
diff --git a/lib/pvector.c b/lib/pvector.c
index cc527fdc4..aa8c6cb24 100644
--- a/lib/pvector.c
+++ b/lib/pvector.c
@@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec)
 void
 pvector_destroy(struct pvector *pvec)
 {
+struct pvector_impl *old = pvector_impl_get(pvec);
 free(pvec->temp);
 pvec->temp = NULL;
-ovsrcu_postpone(free, pvector_impl_get(pvec));
 ovsrcu_set(>impl, NULL); /* Poison. */
+ovsrcu_postpone(free, old);
 }
 
 /* Iterators for callers that need the 'index' afterward. */
@@ -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void *ptr, 
int priority)
 /* Make the modified pvector available for iteration. */
 void pvector_publish__(struct pvector *pvec)
 {
-struct pvector_impl *temp = pvec->temp;
-
+struct pvector_impl *new = pvec->temp;
+struct pvector_impl *old = ovsrcu_get_protected(struct pvector_impl *,
+   >impl);
 pvec->temp = NULL;
-pvector_impl_sort(temp); /* Also removes gaps. */
-ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *,
-   >impl));
-ovsrcu_set(>impl, temp);
+pvector_impl_sort(new); /* Also removes gaps. */
+ovsrcu_set(>impl, new);
+ovsrcu_postpone(free, old);
 }
diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
index 343b75f0e..343100c08 100644
--- a/ofproto/ofproto-dpif-mirror.c
+++ b/ofproto/ofproto-dpif-mirror.c
@@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 hmapx_destroy(_map);
 
 if (vlans || src_vlans) {
+unsigned long *new_vlans = vlan_bitmap_clone(src_vlans);
+ovsrcu_set(>vlans, new_vlans);
 ovsrcu_postpone(free, vlans);
-vlans = vlan_bitmap_clone(src_vlans);
-ovsrcu_set(>vlans, vlans);
 }
 
 mirror->out = out;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10d..be6dafb78 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1658,11 +1658,10 @@ ukey_set_actions(struct udpif_key *ukey, const struct 
ofpbuf *actions)
 struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *,
   >actions);
 
+ovsrcu_set(>actions, ofpbuf_clone(actions));
 if (old_actions) {
 ovsrcu_postpone(ofpbuf_delete, old_actions);
 }
-
-ovsrcu_set(>actions, ofpbuf_clone(actions));
 }
 
 static struct udpif_key *
-- 
2.21.0.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs rcu: update rcu pointer first

[Linhaifeng]

Hi Yanqin,

Thank you for your suggestions. I will send a new patch.

-Original Message-
From: Yanqin Wei [mailto:yanqin@arm.com] 
Sent: Tuesday, June 2, 2020 11:51 AM
To: Linhaifeng ; d...@openvswitch.org
Cc: nd 
Subject: RE: [PATCH] ovs rcu: update rcu pointer first

Hi Haifeng,

It looks indeed a risk for using ovs-rcu. A few comments inline.

Best Regards,
Wei Yanqin

> -Original Message-
> From: dev  On Behalf Of Linhaifeng
> Sent: Monday, June 1, 2020 11:13 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH] ovs rcu: update rcu pointer first
> 
> We should update rcu pointer first then use ovsrcu_postpone to free 
> otherwise maybe cause use-after-free.
> 
> e.g, thead are two threads A and B:
> 
> 1. thread A call ovsrcu_postpone and flush cbset, this time have not 
> call ovsrcu_quiesce
> 
> 2. thread rcu wait all threads call ovsrcu_quiesce
> 
> 3. thread B call ovsrcu_quiesce
> 
> 4. thread B get the old pointer next round
> 
> 5. thrad A call ovsrcu_quiesce, now all threads have called 
> ovsrcu_quiesce
[Yanqin]   Thread A is a writer and does not have to call ovsrcu_quiesce. I 
think this scenario can be simplified as follows: reader indicates momentary 
quiescent and access old pointer after writer postpone free old pointer and 
before setting new pointer. 

> 
> 6. thread rcu free old pointer
> 
> 7. thread B use-after-free
> 
> Signed-off-by: Linhaifeng 
> ---
>  lib/classifier.c  |  4 ++--
>  lib/ovs-rcu.h |  2 +-
>  lib/pvector.c | 15 ---
>  ofproto/ofproto-dpif-mirror.c |  4 ++--  
> ofproto/ofproto-dpif-upcall.c |  3 +--
>  5 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/classifier.c b/lib/classifier.c index 
> f2c3497c2..6bff76e07 100644
> --- a/lib/classifier.c
> +++ b/lib/classifier.c
> @@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr,
>  unsigned int old_n = old ? old->n : 0;
> 
>  if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof 
> *conj))) {
> +ovsrcu_set(>conj_set,
> +   cls_conjunction_set_alloc(match, conj, n));
>  if (old) {
>  ovsrcu_postpone(free, old);
>  }
> -ovsrcu_set(>conj_set,
> -   cls_conjunction_set_alloc(match, conj, n));
>  }
>  }
> 
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h index ecc4c9201..a66d868ea 
> 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -119,9 +119,9 @@
>   * change_flow(struct flow *new_flow)
>   * {
>   * ovs_mutex_lock();
> + * ovsrcu_set(, new_flow);
>   * ovsrcu_postpone(free,
>   * ovsrcu_get_protected(struct flow *, ));
[Yanqin] flowp has been set to new flow pointer here. Maybe a new variable is 
needed to store old pointer.
> - * ovsrcu_set(, new_flow);
>   * ovs_mutex_unlock();
>   * }
>   *
> diff --git a/lib/pvector.c b/lib/pvector.c index cc527fdc4..aa8c6cb24 
> 100644
> --- a/lib/pvector.c
> +++ b/lib/pvector.c
> @@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec)  void 
> pvector_destroy(struct pvector *pvec)  {
> +struct pvector_impl *old = pvector_impl_get(pvec);
>  free(pvec->temp);
>  pvec->temp = NULL;
> -ovsrcu_postpone(free, pvector_impl_get(pvec));
>  ovsrcu_set(>impl, NULL); /* Poison. */
> +ovsrcu_postpone(free, old);
>  }
> 
>  /* Iterators for callers that need the 'index' afterward. */ @@ 
> -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void 
> *ptr, int priority)
>  /* Make the modified pvector available for iteration. */  void 
> pvector_publish__(struct pvector *pvec)  {
> -struct pvector_impl *temp = pvec->temp;
> -
> +struct pvector_impl *new = pvec->temp;
> +struct pvector_impl *old = ovsrcu_get_protected(struct pvector_impl *,
> +   >impl);
>  pvec->temp = NULL;
> -pvector_impl_sort(temp); /* Also removes gaps. */
> -ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *,
> -   >impl));
> -ovsrcu_set(>impl, temp);
> +pvector_impl_sort(new); /* Also removes gaps. */
> +ovsrcu_set(>impl, new);
> +ovsrcu_postpone(free, old);
>  }
> diff --git a/ofproto/ofproto-dpif-mirror.c 
> b/ofproto/ofproto-dpif-mirror.c index
> 343b75f0e..343100c08 100644
> --- a/ofproto/ofproto-dpif-mirror.c
> +++ b/ofproto/ofproto-dpif-mirror.c
> @@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, 
> const char *name,
>  hmapx_destroy(_map);
> 
>  i

[ovs-dev] [PATCH] ovs rcu: update rcu pointer first

[Linhaifeng]

We should update rcu pointer first then use ovsrcu_postpone to free
otherwise maybe cause use-after-free.

e.g, thead are two threads A and B:

1. thread A call ovsrcu_postpone and flush cbset, this time have not
call ovsrcu_quiesce

2. thread rcu wait all threads call ovsrcu_quiesce

3. thread B call ovsrcu_quiesce

4. thread B get the old pointer next round

5. thrad A call ovsrcu_quiesce, now all threads have called ovsrcu_quiesce

6. thread rcu free old pointer

7. thread B use-after-free

Signed-off-by: Linhaifeng 
---
 lib/classifier.c  |  4 ++--
 lib/ovs-rcu.h |  2 +-
 lib/pvector.c | 15 ---
 ofproto/ofproto-dpif-mirror.c |  4 ++--
 ofproto/ofproto-dpif-upcall.c |  3 +--
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index f2c3497c2..6bff76e07 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr,
 unsigned int old_n = old ? old->n : 0;
 
 if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof *conj))) {
+ovsrcu_set(>conj_set,
+   cls_conjunction_set_alloc(match, conj, n));
 if (old) {
 ovsrcu_postpone(free, old);
 }
-ovsrcu_set(>conj_set,
-   cls_conjunction_set_alloc(match, conj, n));
 }
 }
 
diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index ecc4c9201..a66d868ea 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -119,9 +119,9 @@
  * change_flow(struct flow *new_flow)
  * {
  * ovs_mutex_lock();
+ * ovsrcu_set(, new_flow);
  * ovsrcu_postpone(free,
  * ovsrcu_get_protected(struct flow *, ));
- * ovsrcu_set(, new_flow);
  * ovs_mutex_unlock();
  * }
  *
diff --git a/lib/pvector.c b/lib/pvector.c
index cc527fdc4..aa8c6cb24 100644
--- a/lib/pvector.c
+++ b/lib/pvector.c
@@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec)
 void
 pvector_destroy(struct pvector *pvec)
 {
+struct pvector_impl *old = pvector_impl_get(pvec);
 free(pvec->temp);
 pvec->temp = NULL;
-ovsrcu_postpone(free, pvector_impl_get(pvec));
 ovsrcu_set(>impl, NULL); /* Poison. */
+ovsrcu_postpone(free, old);
 }
 
 /* Iterators for callers that need the 'index' afterward. */
@@ -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void *ptr, 
int priority)
 /* Make the modified pvector available for iteration. */
 void pvector_publish__(struct pvector *pvec)
 {
-struct pvector_impl *temp = pvec->temp;
-
+struct pvector_impl *new = pvec->temp;
+struct pvector_impl *old = ovsrcu_get_protected(struct pvector_impl *,
+   >impl);
 pvec->temp = NULL;
-pvector_impl_sort(temp); /* Also removes gaps. */
-ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *,
-   >impl));
-ovsrcu_set(>impl, temp);
+pvector_impl_sort(new); /* Also removes gaps. */
+ovsrcu_set(>impl, new);
+ovsrcu_postpone(free, old);
 }
diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
index 343b75f0e..343100c08 100644
--- a/ofproto/ofproto-dpif-mirror.c
+++ b/ofproto/ofproto-dpif-mirror.c
@@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 hmapx_destroy(_map);
 
 if (vlans || src_vlans) {
+unsigned long *new_vlans = vlan_bitmap_clone(src_vlans);
+ovsrcu_set(>vlans, new_vlans);
 ovsrcu_postpone(free, vlans);
-vlans = vlan_bitmap_clone(src_vlans);
-ovsrcu_set(>vlans, vlans);
 }
 
 mirror->out = out;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10d..be6dafb78 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1658,11 +1658,10 @@ ukey_set_actions(struct udpif_key *ukey, const struct 
ofpbuf *actions)
 struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *,
   >actions);
 
+ovsrcu_set(>actions, ofpbuf_clone(actions));
 if (old_actions) {
 ovsrcu_postpone(ofpbuf_delete, old_actions);
 }
-
-ovsrcu_set(>actions, ofpbuf_clone(actions));
 }
 
 static struct udpif_key *
-- 
2.21.0.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovs rcu: update rcu pointer first

[Linhaifeng]

We should update rcu pointer first then use ovsrcu_postpone to free
otherwise maybe cause use-after-free.

e.g, thead are two threads, A and B:
1. thread A call ovsrcu_postpone and flush cbset, have not call ovsrcu_quiesce
2. thread rcu wait all threads call ovsrcu_quiesce
3. thread B call ovsrcu_quiesce
4. thread B next round get the old pointer
5. thrad A call ovsrcu_quiesce
6. thread rcu free old pointer
7. thread B use-after-free

Signed-off-by: Haifeng Lin 
---
 lib/classifier.c  |  4 ++--
 lib/ovs-rcu.h |  2 +-
 lib/pvector.c | 15 ---
 ofproto/ofproto-dpif-mirror.c |  4 ++--
 ofproto/ofproto-dpif-upcall.c |  3 +--
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/lib/classifier.c b/lib/classifier.c
index f2c3497c2..6bff76e07 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -249,11 +249,11 @@ cls_rule_set_conjunctions(struct cls_rule *cr,
 unsigned int old_n = old ? old->n : 0;
 
 if (old_n != n || (n && memcmp(old_conj, conj, n * sizeof *conj))) {
+ovsrcu_set(>conj_set,
+   cls_conjunction_set_alloc(match, conj, n));
 if (old) {
 ovsrcu_postpone(free, old);
 }
-ovsrcu_set(>conj_set,
-   cls_conjunction_set_alloc(match, conj, n));
 }
 }
 
diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index ecc4c9201..a66d868ea 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -119,9 +119,9 @@
  * change_flow(struct flow *new_flow)
  * {
  * ovs_mutex_lock();
+ * ovsrcu_set(, new_flow);
  * ovsrcu_postpone(free,
  * ovsrcu_get_protected(struct flow *, ));
- * ovsrcu_set(, new_flow);
  * ovs_mutex_unlock();
  * }
  *
diff --git a/lib/pvector.c b/lib/pvector.c
index cc527fdc4..6a295ec53 100644
--- a/lib/pvector.c
+++ b/lib/pvector.c
@@ -67,10 +67,11 @@ pvector_init(struct pvector *pvec)
 void
 pvector_destroy(struct pvector *pvec)
 {
+struct pvector_impl *old = pvector_impl_get(pvec);
 free(pvec->temp);
 pvec->temp = NULL;
-ovsrcu_postpone(free, pvector_impl_get(pvec));
 ovsrcu_set(>impl, NULL); /* Poison. */
+ovsrcu_postpone(free, old);
 }
 
 /* Iterators for callers that need the 'index' afterward. */
@@ -205,11 +206,11 @@ pvector_change_priority(struct pvector *pvec, void *ptr, 
int priority)
 /* Make the modified pvector available for iteration. */
 void pvector_publish__(struct pvector *pvec)
 {
-struct pvector_impl *temp = pvec->temp;
-
+struct pvector_impl *new = pvec->temp;
+struct pvector_impl *old = vsrcu_get_protected(struct pvector_impl *,
+   >impl);
 pvec->temp = NULL;
-pvector_impl_sort(temp); /* Also removes gaps. */
-ovsrcu_postpone(free, ovsrcu_get_protected(struct pvector_impl *,
-   >impl));
-ovsrcu_set(>impl, temp);
+pvector_impl_sort(new); /* Also removes gaps. */
+ovsrcu_set(>impl, new);
+ovsrcu_postpone(free, old);
 }
diff --git a/ofproto/ofproto-dpif-mirror.c b/ofproto/ofproto-dpif-mirror.c
index 343b75f0e..343100c08 100644
--- a/ofproto/ofproto-dpif-mirror.c
+++ b/ofproto/ofproto-dpif-mirror.c
@@ -276,9 +276,9 @@ mirror_set(struct mbridge *mbridge, void *aux, const char 
*name,
 hmapx_destroy(_map);
 
 if (vlans || src_vlans) {
+unsigned long *new_vlans = vlan_bitmap_clone(src_vlans);
+ovsrcu_set(>vlans, new_vlans);
 ovsrcu_postpone(free, vlans);
-vlans = vlan_bitmap_clone(src_vlans);
-ovsrcu_set(>vlans, vlans);
 }
 
 mirror->out = out;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10d..be6dafb78 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1658,11 +1658,10 @@ ukey_set_actions(struct udpif_key *ukey, const struct 
ofpbuf *actions)
 struct ofpbuf *old_actions = ovsrcu_get_protected(struct ofpbuf *,
   >actions);
 
+ovsrcu_set(>actions, ofpbuf_clone(actions));
 if (old_actions) {
 ovsrcu_postpone(ofpbuf_delete, old_actions);
 }
-
-ovsrcu_set(>actions, ofpbuf_clone(actions));
 }
 
 static struct udpif_key *
-- 
2.21.0.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow

[Linhaifeng]

Hi, Ben Pfaff
Thank you. Give a like.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, December 3, 2019 4:21 AM
> To: Numan Siddique 
> Cc: Linhaifeng ; d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
> 
> On Fri, Nov 29, 2019 at 04:15:59PM +0530, Numan Siddique wrote:
> > On Fri, Nov 29, 2019 at 11:44 AM Linhaifeng 
> wrote:
> > >
> > > Should use flow->actions not >actions.
> > >
> > > here is ASAN report:
> > >
> 
> =
> > > ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address
> 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ
> of size 196 at 0x428fa0e8 thread T150 (revalidator22)
> > > #0 0x7f61a51f in __interceptor_memcpy
> (/lib64/libasan.so.4+0xa251f)
> > > #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426
> > > #2 0xd26a30cb in ofpbuf_clone_data_with_headroom
> lib/ofpbuf.c:248
> > > #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218
> > > #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208
> > > #5 0xd23e3993 in ukey_set_actions
> ofproto/ofproto-dpif-upcall.c:1640
> > > #6 0xd23e3f03 in ukey_create__
> ofproto/ofproto-dpif-upcall.c:1696
> > > #7 0xd23e553f in ukey_create_from_dpif_flow
> ofproto/ofproto-dpif-upcall.c:1806
> > > #8 0xd23e65fb in ukey_acquire
> ofproto/ofproto-dpif-upcall.c:1984
> > > #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625
> > > #10 0xd23dee5f in udpif_revalidator
> ofproto/ofproto-dpif-upcall.c:1076
> > > #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708
> > > #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb)
> > > #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb)
> > >
> > > Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) 
> > > at
> offset 328 in frame
> > > #0 0xd23e4cab in ukey_create_from_dpif_flow
> > > ofproto/ofproto-dpif-upcall.c:1762
> > >
> > >   This frame has 4 object(s):
> > > [32, 96) 'actions'
> > > [128, 192) 'buf'
> > > [224, 328) 'full_flow'
> > > [384, 2432) 'stub' <== Memory access at offset 328 partially
> > > underflows this variable
> > > HINT: this may be a false positive if your program uses some custom stack
> unwind mechanism or swapcontext
> > >   (longjmp and C++ exceptions *are* supported) Thread T150
> (revalidator22) created by T0 here:
> > > #0 0x7f5b0f7f in __interceptor_pthread_create
> (/lib64/libasan.so.4+0x38f7f)
> > > #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792
> > > #2 0xd23dc62f in udpif_start_threads
> ofproto/ofproto-dpif-upcall.c:639
> > > #3 0xd23daf87 in ofproto_set_flow_table
> ofproto/ofproto-dpif-upcall.c:446
> > > #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134
> > > #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148
> > > #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944
> > > #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240
> > > #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf)
> > > #9 0xd230a3d3
> > > (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3)
> > >
> > > SUMMARY: AddressSanitizer: stack-buffer-overflow
> (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around
> the buggy address:
> > >   0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00
> > >   0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
> > >   0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2
> > > =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2
> > >   0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> > >   Addressabl

[ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow

[Linhaifeng]

Should use flow->actions not >actions.

here is ASAN report:
=
==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address 
0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ of 
size 196 at 0x428fa0e8 thread T150 (revalidator22)
#0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f)
#1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426
#2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248
#3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218
#4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208
#5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640
#6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696
#7 0xd23e553f in ukey_create_from_dpif_flow 
ofproto/ofproto-dpif-upcall.c:1806
#8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984
#9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625
#10 0xd23dee5f in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1076
#11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708
#12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb)
#13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb)

Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) at 
offset 328 in frame
#0 0xd23e4cab in ukey_create_from_dpif_flow 
ofproto/ofproto-dpif-upcall.c:1762

  This frame has 4 object(s):
[32, 96) 'actions'
[128, 192) 'buf'
[224, 328) 'full_flow'
[384, 2432) 'stub' <== Memory access at offset 328 partially underflows 
this variable
HINT: this may be a false positive if your program uses some custom stack 
unwind mechanism or swapcontext
  (longjmp and C++ exceptions *are* supported) Thread T150 (revalidator22) 
created by T0 here:
#0 0x7f5b0f7f in __interceptor_pthread_create 
(/lib64/libasan.so.4+0x38f7f)
#1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792
#2 0xd23dc62f in udpif_start_threads ofproto/ofproto-dpif-upcall.c:639
#3 0xd23daf87 in ofproto_set_flow_table 
ofproto/ofproto-dpif-upcall.c:446
#4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134
#5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148
#6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944
#7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240
#8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf)
#9 0xd230a3d3  (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3)

SUMMARY: AddressSanitizer: stack-buffer-overflow (/lib64/libasan.so.4+0xa251f) 
in __interceptor_memcpy Shadow bytes around the buggy address:
  0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00
  0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
  0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2
=>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2
  0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte 
legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:   fa
  Freed heap region:   fd
  Stack left redzone:  f1
  Stack mid redzone:   f2
  Stack right redzone: f3
  Stack after return:  f5
  Stack use after scope:   f8
  Global redzone:  f9
  Global init order:   f6
  Poisoned by user:f7
  Container overflow:  fc
  Array cookie:ac
  Intra object redzone:bb
  ASan internal:   fe
  Left alloca redzone: ca
  Right alloca redzone:cb
==57189==ABORTING

Signed-off-by: Linhaifeng 
---
 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 dc30824771..c2fc527a31 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1796,7 +1796,7 @@ ukey_create_from_dpif_flow(const struct udpif *udpif,
 }
 
 reval_seq = seq_read(udpif->reval_seq) - 1; /* Ensure revalidation. */
-ofpbuf_use_const(, >actions, flow->actions_len);
+ofpbuf_use_const(, flow->actions, flow->actions_len);
 *ukey = ukey_create__(flow->key, flow->key_len,
   flow->mask, flow->mask_len, flow->ufid_present,
   >ufid, flow->pmd_id, ,
--
2.19.1
___

[ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow

[Linhaifeng]

Should use flow->actions not >actions.

Here is some print info for debug:
@@@ flow->actions=0xb5e48844 len=196
@@@ stub=[0x7eefa120,0x7eefa918] actions->data=0x7eefa0a0 
actions->size=196

actions->data != flow->actions and actions->data + 196 is in the range of stub .

here is ASAN report:
=
==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address 
0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ of 
size 196 at 0x428fa0e8 thread T150 (revalidator22)
#0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f)
#1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426
#2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248
#3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218
#4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208
#5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640
#6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696
#7 0xd23e553f in ukey_create_from_dpif_flow 
ofproto/ofproto-dpif-upcall.c:1806
#8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984
#9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625
#10 0xd23dee5f in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1076
#11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708
#12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb)
#13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb)

Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) at 
offset 328 in frame
#0 0xd23e4cab in ukey_create_from_dpif_flow 
ofproto/ofproto-dpif-upcall.c:1762

  This frame has 4 object(s):
[32, 96) 'actions'
[128, 192) 'buf'
[224, 328) 'full_flow'
[384, 2432) 'stub' <== Memory access at offset 328 partially underflows 
this variable
HINT: this may be a false positive if your program uses some custom stack 
unwind mechanism or swapcontext
  (longjmp and C++ exceptions *are* supported) Thread T150 (revalidator22) 
created by T0 here:
#0 0x7f5b0f7f in __interceptor_pthread_create 
(/lib64/libasan.so.4+0x38f7f)
#1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792
#2 0xd23dc62f in udpif_start_threads ofproto/ofproto-dpif-upcall.c:639
#3 0xd23daf87 in ofproto_set_flow_table 
ofproto/ofproto-dpif-upcall.c:446
#4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134
#5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148
#6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944
#7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240
#8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf)
#9 0xd230a3d3  (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3)

SUMMARY: AddressSanitizer: stack-buffer-overflow (/lib64/libasan.so.4+0xa251f) 
in __interceptor_memcpy Shadow bytes around the buggy address:
  0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00
  0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
  0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2
=>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2
  0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte 
legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:   fa
  Freed heap region:   fd
  Stack left redzone:  f1
  Stack mid redzone:   f2
  Stack right redzone: f3
  Stack after return:  f5
  Stack use after scope:   f8
  Global redzone:  f9
  Global init order:   f6
  Poisoned by user:f7
  Container overflow:  fc
  Array cookie:ac
  Intra object redzone:bb
  ASan internal:   fe
  Left alloca redzone: ca
  Right alloca redzone:cb
==57189==ABORTING
---
 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 dc30824771..c2fc527a31 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1796,7 +1796,7 @@ ukey_create_from_dpif_flow(const struct udpif *udpif,
 }
 
 reval_seq = seq_read(udpif->reval_seq) - 1; /* Ensure revalidation. */
-ofpbuf_use_const(, >actions, flow->actions_len);
+ofpbuf_use_const(, flow->actions, flow->actions_len);
 *ukey = ukey_create__(flow->key, flow->key_len,
   

[ovs-dev] 答复: [PATCH v2] netlink-notifier: support blacklist

[Linhaifeng]

@@ -1015,6 +1016,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 rte_eth_dev_info_get(dev->port_id, );
 
+rtnetlink_blacklist_add(dev->up.name);   // add a ovs-dpdk 
port to blacklist
+


Some DPDK port even use PMD they also have linux interface (like mellanox 
ethernet)but they don't need 'if change' notification.


-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org] 
发送时间: 2018年8月4日 8:25
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: [PATCH v2] netlink-notifier: support blacklist

On Fri, Aug 03, 2018 at 02:10:06PM +0800, Haifeng Lin wrote:
> in dpdk ovs mode some ether not need rtnetlink notifier so we can use 
> blacklist remove ethernet from rtnetlink notifier
> 
> Signed-off-by: Haifeng Lin 

Can you explain the benefit of the patch?

I don't see any way for a port to be removed from the blacklist.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] 答复: 答复: [bug] ovs crash when call xlate_normal_flood

[Linhaifeng]

Sorry . send wrong message. Pls igore.

The problem is ousr code had 'use-after-free'

-邮件原件-
发件人: Linhaifeng 
发送时间: 2018年8月3日 13:55
收件人: 'Ben Pfaff' 
抄送: d...@openvswitch.org
主题: 答复: [ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

Some PMD add to ovs dpdk aslo have linux if interface . Linux if interface 
would notice ovs to reconfig and would fail too when failed to add.

-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org]
发送时间: 2018年7月11日 5:08
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: [ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

Hmm, that's interesting.  What was the problem, then?

On Fri, Jun 29, 2018 at 01:14:38AM +, Linhaifeng wrote:
> Sorry.This is not ovs‘s bug.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:54
> 收件人: 'd...@openvswitch.org' 
> 主题: 答复: [bug] ovs crash when call xlate_normal_flood
> 
> Or stop upcall thread before xlate_remove_ofproto in destruct.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:49
> 收件人: 'd...@openvswitch.org' 
> mailto:d...@openvswitch.org>>
> 主题: [bug] ovs crash when call xlate_normal_flood
> 
> Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
> main thread and read in upcall thread.
> 
> The crash stack as follow:
> #5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2509
> 2509   in ofproto/ofproto_dpif_xlate.c
> (gdb) info locals
> xbundle = 0xffe8
> (gdb) p xbundle->ofbundle
> Cannot access memory at address 0xfff8
> (gdb) p in_xbundle->ofbundle
> $5 = (struct ofbundle *) 0x0
> (gdb) p in_xbundle
> $6 = (struct xbundle *) 0x553d950
> (gdb) p *in_xbundle
> $7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
> 0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 
> 0x0, bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
>   trunks = 0x0, use_priority_tags = false, floodable = false, 
> protected = false, external_id = 0}
> 
> (gdb) bt
> #0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
> #1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
> #2  0x0065e1e9 in PAT_abort ()
> #3  0x0065b32d in patchIllInsHandler ()
> #4  
> #5  0x004a04e8 in xlate_normal_flood 
> (ctx=ctx@entry=0x7faeee7b6f30, in_xbundle=in_xbundle@entry=0x553d950, 
> vlan=vlan@entry=0) at ofproto/ofproto_dpif_xlate.c:2509
> #6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port= out>, max_len=, may_packet_in=may_packet_in@entry=true) 
> at ofproto/ofproto_dpif_xlate.c:4312
> #8  0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #9  0x0049e410 in xlate_recursively (deepens=true, 
> rule=0x6a4be00, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3587
> #10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #11 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr= out>, check_stp=check_stp@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:3317
> #12 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2113
> #14 0x004a0511 in xlate_normal_flood 
> (ctx=ctx@entry=0x7faeee7b6f30, in_xbundle=in_xbundle@entry=0x3008a40, 
> vlan=vlan@entry=0) at ofproto/ofproto_dpif_xlate.c:2517
> #15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port= out>, max_len=, may_packet_in=may_packet_in@entry=true) 
> at ofproto/ofproto_dpif_xlate.c:4312
> #17 0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #18 0x0049e410 in xlate_recursively (deepens=true, 
> rule=0x6a32500, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3587
> #19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #20 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr= out>, check_stp=check_stp@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:3317
> #21 0x000

[ovs-dev] 答复: 答复: [bug] ovs crash when call xlate_normal_flood

[Linhaifeng]

Some PMD add to ovs dpdk aslo have linux if interface . Linux if interface 
would notice ovs
to reconfig and would fail too when failed to add.

-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org] 
发送时间: 2018年7月11日 5:08
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: [ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

Hmm, that's interesting.  What was the problem, then?

On Fri, Jun 29, 2018 at 01:14:38AM +, Linhaifeng wrote:
> Sorry.This is not ovs‘s bug.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:54
> 收件人: 'd...@openvswitch.org' 
> 主题: 答复: [bug] ovs crash when call xlate_normal_flood
> 
> Or stop upcall thread before xlate_remove_ofproto in destruct.
> 
> 发件人: Linhaifeng
> 发送时间: 2018年6月21日 13:49
> 收件人: 'd...@openvswitch.org' 
> mailto:d...@openvswitch.org>>
> 主题: [bug] ovs crash when call xlate_normal_flood
> 
> Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
> main thread and read in upcall thread.
> 
> The crash stack as follow:
> #5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2509
> 2509   in ofproto/ofproto_dpif_xlate.c
> (gdb) info locals
> xbundle = 0xffe8
> (gdb) p xbundle->ofbundle
> Cannot access memory at address 0xfff8
> (gdb) p in_xbundle->ofbundle
> $5 = (struct ofbundle *) 0x0
> (gdb) p in_xbundle
> $6 = (struct xbundle *) 0x553d950
> (gdb) p *in_xbundle
> $7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
> 0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 
> 0x0, bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
>   trunks = 0x0, use_priority_tags = false, floodable = false, protected = 
> false, external_id = 0}
> 
> (gdb) bt
> #0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
> #1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
> #2  0x0065e1e9 in PAT_abort ()
> #3  0x0065b32d in patchIllInsHandler ()
> #4  
> #5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2509
> #6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
> max_len=, may_packet_in=may_packet_in@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:4312
> #8  0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #9  0x0049e410 in xlate_recursively (deepens=true, rule=0x6a4be00, 
> ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
> #10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #11 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
> check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
> #12 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2113
> #14 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
> in_xbundle=in_xbundle@entry=0x3008a40, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif_xlate.c:2517
> #15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:2736
> #16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
> max_len=, may_packet_in=may_packet_in@entry=true) at 
> ofproto/ofproto_dpif_xlate.c:4312
> #17 0x0049d240 in do_xlate_actions (ofpacts=, 
> ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:5262
> #18 0x0049e410 in xlate_recursively (deepens=true, rule=0x6a32500, 
> ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
> #19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
> table_id=, may_packet_in=, 
> honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
> #20 0x0049faf7 in compose_output_action__ 
> (ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
> check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
> #21 0x004a0126 in compose_output_action (xr=, 
> ofp_port=, ctx=0x7faeee7b6f30) at 
> ofproto/ofproto_dpif_xlate.c:3567
> #22 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
> out_xbundle=out_xbundle@entry=0x93e3e10, vlan=vlan@entry=0) at 
> ofproto/ofproto_dpif

[ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

[Linhaifeng]

Sorry.This is not ovs‘s bug.

发件人: Linhaifeng
发送时间: 2018年6月21日 13:54
收件人: 'd...@openvswitch.org' 
主题: 答复: [bug] ovs crash when call xlate_normal_flood

Or stop upcall thread before xlate_remove_ofproto in destruct.

发件人: Linhaifeng
发送时间: 2018年6月21日 13:49
收件人: 'd...@openvswitch.org' mailto:d...@openvswitch.org>>
主题: [bug] ovs crash when call xlate_normal_flood

Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
main thread and read in upcall thread.

The crash stack as follow:
#5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2509
2509   in ofproto/ofproto_dpif_xlate.c
(gdb) info locals
xbundle = 0xffe8
(gdb) p xbundle->ofbundle
Cannot access memory at address 0xfff8
(gdb) p in_xbundle->ofbundle
$5 = (struct ofbundle *) 0x0
(gdb) p in_xbundle
$6 = (struct xbundle *) 0x553d950
(gdb) p *in_xbundle
$7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 0x0, 
bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
  trunks = 0x0, use_priority_tags = false, floodable = false, protected = 
false, external_id = 0}

(gdb) bt
#0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
#1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
#2  0x0065e1e9 in PAT_abort ()
#3  0x0065b32d in patchIllInsHandler ()
#4  
#5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2509
#6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#8  0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#9  0x0049e410 in xlate_recursively (deepens=true, rule=0x6a4be00, 
ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
#10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
table_id=, may_packet_in=, 
honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
#11 0x0049faf7 in compose_output_action__ 
(ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
#12 0x004a0126 in compose_output_action (xr=, 
ofp_port=, ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:3567
#13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2113
#14 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x3008a40, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2517
#15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#17 0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#18 0x0049e410 in xlate_recursively (deepens=true, rule=0x6a32500, 
ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
#19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
table_id=, may_packet_in=, 
honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
#20 0x0049faf7 in compose_output_action__ 
(ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
#21 0x004a0126 in compose_output_action (xr=, 
ofp_port=, ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:3567
#22 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
out_xbundle=out_xbundle@entry=0x93e3e10, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2113
#23 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x894de90, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2517
#24 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#25 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#26 0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#27 0x004a27df in xlate_actions (xin=xin@entry=0x7faeee7b8550, 
xout=xout@entry=0x7faeee7dd320) at ofproto/ofproto_dpif_xlate.c:6119
#28 0x004959bb in upcall_xlate (wc=0x7faeee7dd378, 
odp_actions=0x7faeee7dd338, upcall=0x7faeee7dd2c0, udpif=0x1807190) at 
ofproto/ofproto_dpif_upcall.c:1185
#29 process_upcall (udpif=udpif@entry=

[ovs-dev] 答复: 答复: [PATCH] dpif-netdev:Delete port check in do_add_port

[Linhaifeng]

It's good. If get_port_by_name  in dpif-netdev use string hash replace of 
HMAP_FOR_EACH the performance would better.

-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org] 
发送时间: 2018年6月22日 6:56
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: 答复: [ovs-dev] [PATCH] dpif-netdev:Delete port check in do_add_port

OK.

I think that it's the dpif itself that should really be doing the check (the 
kernel datapath especially needs to do this for itself), so I sent an 
alternative patch that works that way:
https://patchwork.ozlabs.org/patch/932991/

Will you test it?

Thanks,

Ben.

On Thu, Jun 21, 2018 at 06:24:09AM +, Linhaifeng wrote:
> For the performance problem. When add 3000 or more ports it costs too more 
> time and it is not needed.
> 
> port_add function in ofproto-dpif.c have checked it used 
> dpif_port_exists so we really not need to check it again in 
> do_add_port
> 
> -邮件原件-
> 发件人: Ben Pfaff [mailto:b...@ovn.org]
> 发送时间: 2018年5月10日 5:10
> 收件人: Linhaifeng 
> 抄送: d...@openvswitch.org
> 主题: Re: [ovs-dev] [PATCH] dpif-netdev:Delete port check in do_add_port
> 
> On Thu, Apr 26, 2018 at 03:12:51PM +0800, Haifeng Lin wrote:
> > It is not need check port exist in do_add_port because it had check 
> > in port_add.
> > 
> > Change-Id: Ie66206b40e305cef5f5b20af765c3128ccec6782
> > Signed-off-by: Haifeng Lin 
> 
> Can you explain the problem and the solution in a little more detail?
> 
> Thanks,
> 
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] 答复: [PATCH] dpif-netdev:Delete port check in do_add_port

[Linhaifeng]

For the performance problem. When add 3000 or more ports it costs too more time 
and it is not needed.

port_add function in ofproto-dpif.c have checked it used dpif_port_exists so we 
really not need to check it again in do_add_port

-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org] 
发送时间: 2018年5月10日 5:10
收件人: Linhaifeng 
抄送: d...@openvswitch.org
主题: Re: [ovs-dev] [PATCH] dpif-netdev:Delete port check in do_add_port

On Thu, Apr 26, 2018 at 03:12:51PM +0800, Haifeng Lin wrote:
> It is not need check port exist in do_add_port because it had check in 
> port_add.
> 
> Change-Id: Ie66206b40e305cef5f5b20af765c3128ccec6782
> Signed-off-by: Haifeng Lin 

Can you explain the problem and the solution in a little more detail?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] 答复: [bug] ovs crash when call xlate_normal_flood

[Linhaifeng]

Or stop upcall thread before xlate_remove_ofproto in destruct.

发件人: Linhaifeng
发送时间: 2018年6月21日 13:49
收件人: 'd...@openvswitch.org' 
主题: [bug] ovs crash when call xlate_normal_flood

Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
main thread and read in upcall thread.

The crash stack as follow:
#5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2509
2509   in ofproto/ofproto_dpif_xlate.c
(gdb) info locals
xbundle = 0xffe8
(gdb) p xbundle->ofbundle
Cannot access memory at address 0xfff8
(gdb) p in_xbundle->ofbundle
$5 = (struct ofbundle *) 0x0
(gdb) p in_xbundle
$6 = (struct xbundle *) 0x553d950
(gdb) p *in_xbundle
$7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 0x0, 
bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
  trunks = 0x0, use_priority_tags = false, floodable = false, protected = 
false, external_id = 0}

(gdb) bt
#0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
#1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
#2  0x0065e1e9 in PAT_abort ()
#3  0x0065b32d in patchIllInsHandler ()
#4  
#5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2509
#6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#8  0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#9  0x0049e410 in xlate_recursively (deepens=true, rule=0x6a4be00, 
ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
#10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
table_id=, may_packet_in=, 
honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
#11 0x0049faf7 in compose_output_action__ 
(ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
#12 0x004a0126 in compose_output_action (xr=, 
ofp_port=, ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:3567
#13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2113
#14 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x3008a40, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2517
#15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#17 0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#18 0x0049e410 in xlate_recursively (deepens=true, rule=0x6a32500, 
ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
#19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
table_id=, may_packet_in=, 
honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
#20 0x0049faf7 in compose_output_action__ 
(ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
#21 0x004a0126 in compose_output_action (xr=, 
ofp_port=, ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:3567
#22 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
out_xbundle=out_xbundle@entry=0x93e3e10, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2113
#23 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x894de90, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2517
#24 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#25 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#26 0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#27 0x004a27df in xlate_actions (xin=xin@entry=0x7faeee7b8550, 
xout=xout@entry=0x7faeee7dd320) at ofproto/ofproto_dpif_xlate.c:6119
#28 0x004959bb in upcall_xlate (wc=0x7faeee7dd378, 
odp_actions=0x7faeee7dd338, upcall=0x7faeee7dd2c0, udpif=0x1807190) at 
ofproto/ofproto_dpif_upcall.c:1185
#29 process_upcall (udpif=udpif@entry=0x1807190, 
upcall=upcall@entry=0x7faeee7dd2c0, 
odp_actions=odp_actions@entry=0x7faeee7dd338, wc=wc@entry=0x7faeee7dd378) at 
ofproto/ofproto_dpif_upcall.c:1322
#30 0x0049716c in 

[ovs-dev] [bug] ovs crash when call xlate_normal_flood

[Linhaifeng]

Should we use rwlock to use xbridge->xbundles? xbridge->xbundles may write in 
main thread and read in upcall thread.

The crash stack as follow:
#5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2509
2509   in ofproto/ofproto_dpif_xlate.c
(gdb) info locals
xbundle = 0xffe8
(gdb) p xbundle->ofbundle
Cannot access memory at address 0xfff8
(gdb) p in_xbundle->ofbundle
$5 = (struct ofbundle *) 0x0
(gdb) p in_xbundle
$6 = (struct xbundle *) 0x553d950
(gdb) p *in_xbundle
$7 = {hmap_node = {hash = 0, next = 0x0}, ofbundle = 0x0, list_node = {prev = 
0x0, next = 0x0}, xbridge = 0x0, xports = {prev = 0x0, next = 0x0}, name = 0x0, 
bond = 0x0, lacp = 0x0, vlan_mode = PORT_VLAN_ACCESS, vlan = 0,
  trunks = 0x0, use_priority_tags = false, floodable = false, protected = 
false, external_id = 0}

(gdb) bt
#0  0x7fb05ea85197 in raise () from /usr/lib64/libc.so.6
#1  0x7fb05ea86888 in abort () from /usr/lib64/libc.so.6
#2  0x0065e1e9 in PAT_abort ()
#3  0x0065b32d in patchIllInsHandler ()
#4  
#5  0x004a04e8 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x553d950, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2509
#6  0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#7  xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#8  0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#9  0x0049e410 in xlate_recursively (deepens=true, rule=0x6a4be00, 
ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
#10 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
table_id=, may_packet_in=, 
honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
#11 0x0049faf7 in compose_output_action__ 
(ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
#12 0x004a0126 in compose_output_action (xr=, 
ofp_port=, ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:3567
#13 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
out_xbundle=out_xbundle@entry=0x3005770, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2113
#14 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x3008a40, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2517
#15 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#16 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#17 0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#18 0x0049e410 in xlate_recursively (deepens=true, rule=0x6a32500, 
ctx=0x7faeee7b6f30) at ofproto/ofproto_dpif_xlate.c:3587
#19 xlate_table_action (ctx=0x7faeee7b6f30, in_port=, 
table_id=, may_packet_in=, 
honor_table_miss=) at ofproto/ofproto_dpif_xlate.c:3654
#20 0x0049faf7 in compose_output_action__ 
(ctx=ctx@entry=0x7faeee7b6f30, ofp_port=, xr=, 
check_stp=check_stp@entry=true) at ofproto/ofproto_dpif_xlate.c:3317
#21 0x004a0126 in compose_output_action (xr=, 
ofp_port=, ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:3567
#22 output_normal (ctx=ctx@entry=0x7faeee7b6f30, 
out_xbundle=out_xbundle@entry=0x93e3e10, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2113
#23 0x004a0511 in xlate_normal_flood (ctx=ctx@entry=0x7faeee7b6f30, 
in_xbundle=in_xbundle@entry=0x894de90, vlan=vlan@entry=0) at 
ofproto/ofproto_dpif_xlate.c:2517
#24 0x004a0e6e in xlate_normal (ctx=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:2736
#25 xlate_output_action (ctx=ctx@entry=0x7faeee7b6f30, port=, 
max_len=, may_packet_in=may_packet_in@entry=true) at 
ofproto/ofproto_dpif_xlate.c:4312
#26 0x0049d240 in do_xlate_actions (ofpacts=, 
ofpacts_len=, ctx=ctx@entry=0x7faeee7b6f30) at 
ofproto/ofproto_dpif_xlate.c:5262
#27 0x004a27df in xlate_actions (xin=xin@entry=0x7faeee7b8550, 
xout=xout@entry=0x7faeee7dd320) at ofproto/ofproto_dpif_xlate.c:6119
#28 0x004959bb in upcall_xlate (wc=0x7faeee7dd378, 
odp_actions=0x7faeee7dd338, upcall=0x7faeee7dd2c0, udpif=0x1807190) at 
ofproto/ofproto_dpif_upcall.c:1185
#29 process_upcall (udpif=udpif@entry=0x1807190, 
upcall=upcall@entry=0x7faeee7dd2c0, 
odp_actions=odp_actions@entry=0x7faeee7dd338, wc=wc@entry=0x7faeee7dd378) at 
ofproto/ofproto_dpif_upcall.c:1322
#30 0x0049716c in recv_upcalls (handler=0x64775d0, handler=0x64775d0) 
at ofproto/ofproto_dpif_upcall.c:863
#31 0x0049764a in udpif_upcall_handler (arg=0x64775d0) at 
ofproto/ofproto_dpif_upcall.c:783
#32 

[ovs-dev] 答复: [PATCH] dpif-netdev:Delete port check in do_add_port

[Linhaifeng]

Ping...

-邮件原件-
发件人: Linhaifeng 
发送时间: 2018年4月26日 15:13
收件人: d...@openvswitch.org
抄送: Linhaifeng <haifeng@huawei.com>
主题: [PATCH] dpif-netdev:Delete port check in do_add_port

It is not need check port exist in do_add_port
because it had check in port_add.

Change-Id: Ie66206b40e305cef5f5b20af765c3128ccec6782
Signed-off-by: Haifeng Lin <haifeng@huawei.com>
---
 lib/dpif-netdev.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index be31fd0..f7976b1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1568,11 +1568,6 @@ do_add_port(struct dp_netdev *dp, const char *devname, 
const char *type,
 struct dp_netdev_port *port;
 int error;
 
-/* Reject devices already in 'dp'. */
-if (!get_port_by_name(dp, devname, )) {
-return EEXIST;
-}
-
 error = port_create(devname, type, port_no, );
 if (error) {
 return error;
-- 
1.8.5.2.msysgit.0


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] how to get packet information in ovs-dpdk?

[linhaifeng]

You can use dpdk tool 'pdump' to capture packets
http://dpdk.org/doc/guides-16.07/sample_app_ug/pdump.html

在 2017/7/12 16:21, Sam 写道:
> hi all,
>
> I'm running ovs-dpdk(ovs-2.4.9), I found the counter of bond port of br_t
> is increasing, but I want to know what's these packets, how could I do this?
>
> first, I use `ovs-ofctl snoop br_t`, but it quit quickly as below:
>
>> [root@yf-mos-test-net07 ~]# /usr/local/bin/ovs-ofctl snoop
>> /usr/local/var/run/openvswitch/br_t.mgmt
>> [root@yf-mos-test-net07 ~]# /usr/local/bin/ovs-ofctl snoop
>> /usr/local/var/run/openvswitch/br_t.mgmt
>> ovs-ofctl: /usr/local/var/run/openvswitch/ovs-ofctl.pid: already running
>> as pid 22082, aborting
>
> Then I debug ovs-vswitchd use `gdb -p 213214` and set break as below:
>
>> Breakpoint 3, netdev_rxq_recv (rx=0x7f1a5a6ff2c0, buffers=0x7f25e17f9880,
>> cnt=0x7f25e17f987c) at lib/netdev.c:695
>> 695retval = rx->netdev->netdev_class->rxq_recv(rx, buffers, cnt);
>> (gdb) finish
>> Run till exit from #0  netdev_rxq_recv (rx=0x7f1a5a6ff2c0,
>> buffers=0x7f25e17f9880, cnt=0x7f25e17f987c)
>> at lib/netdev.c:695
>> 0x0055eeec in dp_netdev_process_rxq_port (pmd=0x7f25e80d7010,
>> port=0x1059fd0, rxq=0x7f1a5a6ff2c0)
>> at lib/dpif-netdev.c:2590
>> 2590error = netdev_rxq_recv(rxq, packets, );
>> Value returned is $5 = 11
>> (gdb) p packet
>> No symbol "packet" in current context.
>> (gdb) p p^CQuit
>> (gdb) p cnt
>> $6 = 0
>> (gdb) d
>> Delete all breakpoints? (y or n) y
>> (gdb) b 2593 if cnt!=0
>> Breakpoint 4 at 0x55ef27: file lib/dpif-netdev.c, line 2593.
>> (gdb) c
> but cnt is still 0, which means no packets received.
>
> At last, I use `ovs-appctl vlog/set dpif_netdev dbg`, but the log show
> nothing.
>
> How could I do this? thank you~
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev: fix crash when ifa_netmask is null

[linhaifeng]

在 2017/7/12 12:55, Ben Pfaff 写道:
> On Tue, Jul 04, 2017 at 08:52:57AM +0800, Haifeng Lin wrote:
>> The ifa_netmask is null when failed to call ioctl
>> in getifaddrs
>>
>> Signed-off-by: Haifeng Lin 
> Thanks for figuring this out.
>
> What does it mean if ifa_netmask is null?  Does it mean that the address
> should be ignored entirely?  The manpage for getifaddrs doesn't say.
> And what about for IPv4 addresses?
The glibc code shows that the ifa_netmask maybe NULL：
...
if (__ioctl (fd, SIOCGIFNETMASK, ifr) < 0)
storage[i].ia.ifa_netmask = NULL;
else
{
  storage[i].ia.ifa_netmask = [i].netmask;
  storage[i].netmask = ifr->ifr_netmask;
}
...

> Maybe the right fix would be this:
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 26e87a2ee2ec..68003a829f27 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -1967,7 +1967,8 @@ netdev_get_addrs(const char dev[], struct in6_addr 
> **paddr,
>  for (ifa = if_addr_list; ifa; ifa = ifa->ifa_next) {
>  int family;
>  
> -if (strncmp(ifa->ifa_name, dev, IFNAMSIZ) || ifa->ifa_addr == NULL) {
> +if (strncmp(ifa->ifa_name, dev, IFNAMSIZ)
> +|| !ifa->ifa_addr || !ifa->ifa_netmask) {
>  continue;
>  }
>  
> What do you think?
I think we should check ifa_name too because we don't know what would happen in 
glibc
and we should guarantee that ovs-vswitchd not crash.

diff --git a/lib/netdev.c b/lib/netdev.c index a7840a8..12041f9 100644 --- 
a/lib/netdev.c +++ b/lib/netdev.c @@ -1942,7 +1942,7 @@ netdev_get_addrs(const 
char dev[], struct in6_addr **paddr, } for (ifa = if_addr_list; ifa; ifa = 
ifa->ifa_next) { - if (ifa->ifa_addr != NULL) { + if (ifa->ifa_addr && 
ifa->ifa_name) { int family; family = ifa->ifa_addr->sa_family; @@ -1963,7 
+1963,8 @@ netdev_get_addrs(const char dev[], struct in6_addr **paddr, for (ifa 
= if_addr_list; ifa; ifa = ifa->ifa_next) { int family; - if 
(strncmp(ifa->ifa_name, dev, IFNAMSIZ) || ifa->ifa_addr == NULL) { + if 
(!ifa->ifa_name || !ifa->ifa_addr || !ifa->ifa_netmask + || 
strncmp(ifa->ifa_name, dev, IFNAMSIZ)) { continue; }
> Thanks,
>
> Ben.
>
> .
>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev