[ovs-dev] [cmap v2 2/2] dpif-netdev: Use cmap for ports.

Ben Pfaff blp at nicira.com
Wed May 21 00:11:15 UTC 2014


On Tue, May 20, 2014 at 05:03:59PM -0700, Ben Pfaff wrote:
> On Tue, May 20, 2014 at 04:03:20PM -0700, Jarno Rajahalme wrote:
> > 
> > On May 8, 2014, at 4:50 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> > > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > > ---
> > > lib/dpif-netdev.c |  163 +++++++++++++++++++++++++++--------------------------
> > > 1 file changed, 82 insertions(+), 81 deletions(-)
> > > 
> > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > > index 55712dd..b682876 100644
> > > --- a/lib/dpif-netdev.c
> > > +++ b/lib/dpif-netdev.c
> > > @@ -32,6 +32,7 @@
> > > 
> > 
> > (snip)
> > 
> > > static void
> > > -port_unref(struct dp_netdev_port *port)
> > > +port_destroy__(struct dp_netdev_port *port)
> > > {
> > > -    if (port && ovs_refcount_unref(&port->ref_cnt) == 1) {
> > > -        int i;
> > > +    int i;
> > > 
> > > -        netdev_close(port->netdev);
> > > -        netdev_restore_flags(port->sf);
> > > +    netdev_close(port->netdev);
> > > +    netdev_restore_flags(port->sf);
> > > 
> > > -        for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
> > > -            netdev_rxq_close(port->rxq[i]);
> > > -        }
> > > -        free(port->type);
> > > -        free(port);
> > > +    for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
> > > +        netdev_rxq_close(port->rxq[i]);
> > 
> > Running with the v1, I saw a core here. It appears that the
> > port->netdev is freed before the postponed port_destroy__ is
> > executed. Maybe the netdev deletion needs also be
> > ovsrcu_postponed()?
> 
> It's not obvious to me where the problem would be.
> 
> Oh, I see it under the sandbox with valgrind.  I'll investigate:
> 
> ----------------------------------------------------------------------
> You are running in a dummy Open vSwitch environment.  You can use
> ovs-vsctl, ovs-ofctl, ovs-appctl, and other tools to work with the
> dummy switch.  
> 
> Log files, pidfiles, and the configuration database are in the
> "sandbox" subdirectory.
> 
> Exit the shell to kill the running daemons.
> blp at sigsegv:~/ovs/tutorial$ ovs-vsctl add-br br0
> blp at sigsegv:~/ovs/tutorial$ ovs-vsctl add-port br0 eth0
> blp at sigsegv:~/ovs/tutorial$ ovs-vsctl del-port br0 eth0
> blp at sigsegv:~/ovs/tutorial$ ==21030== Thread 7:
> ==21030== Invalid read of size 4
> ==21030==    at 0x80AEFE7: netdev_n_rxq (netdev.c:94)
> ==21030==    by 0x80E2AEF: ovsrcu_call_postponed (ovs-rcu.c:249)
> ==21030==    by 0x80E2DAD: ovsrcu_postpone_thread (ovs-rcu.c:265)
> ==21030==    by 0x80E304D: ovsthread_wrapper (ovs-thread.c:302)
> ==21030==    by 0x54F5C38: start_thread (pthread_create.c:304)
> ==21030==    by 0x73AFD4D: clone (clone.S:130)
> ==21030==  Address 0x782d560 is 16 bytes inside a block of size 292 free'd
> ==21030==    at 0x525B50C: free (vg_replace_malloc.c:427)
> ==21030==    by 0x80B0215: netdev_unref (netdev.c:496)
> ==21030==    by 0x80961A8: port_destroy__ (dpif-netdev.c:820)
> ==21030==    by 0x80E2AEF: ovsrcu_call_postponed (ovs-rcu.c:249)
> ==21030==    by 0x80E2DAD: ovsrcu_postpone_thread (ovs-rcu.c:265)
> ==21030==    by 0x80E304D: ovsthread_wrapper (ovs-thread.c:302)
> ==21030==    by 0x54F5C38: start_thread (pthread_create.c:304)
> ==21030==    by 0x73AFD4D: clone (clone.S:130)
> ==21030== 
> 
> blp at sigsegv:~/ovs/tutorial$ 

Found it, I think.  I'll send a series that includes fixes and then
the dpif-netdev conversion.



More information about the dev mailing list