On Fri, May 10, 2013 at 10:51:34AM -0400, Ed Maste wrote:
> On 9 May 2013 18:35, Ben Pfaff <[email protected]> wrote:
> > This gets rid of the only per-instance data in "struct netdev", which
> > will make it possible to merge "struct netdev_dev" into "struct netdev" in
> > a later commit.
>
> The four patches generally look good to me, and when I apply the whole
> set they pass the unit tests on FreeBSD.
>
> I tried rebasing the original BSD threaded userspace datapath patch on
> top of the set, but it didn't work properly. To further that
> investigation I planned to try merging each of these patches
> incrementally, and upon testing just this first one ran into a
> use-after-free (with the default malloc diagnostics enabled for
> FreeBSD in the unit tests):
>
> (gdb) bt
> #0 netdev_dev_unref (dev=0x5a5a5a5a5a5a5a5a) at lib/netdev.c:314
> #1 0x0000000000412158 in update_port (ofproto=0x801cce010,
> name=<optimized out>) at ofproto/ofproto.c:1975
> #2 0x0000000000414405 in reinit_ports (p=<optimized out>)
> at ofproto/ofproto.c:1660
> #3 process_port_change (devname=<optimized out>, error=<optimized out>,
> ofproto=<optimized out>) at ofproto/ofproto.c:1115
> #4 ofproto_run (p=0x801cce010) at ofproto/ofproto.c:1186
> #5 0x000000000040ccd7 in bridge_run () at vswitchd/bridge.c:2327
> #6 0x000000000040dcfa in main (argc=<optimized out>, argv=<optimized out>)
> at vswitchd/ovs-vswitchd.c:125
>
> It looks like a ref_cnt increment is missing somewhere.
Thanks, I found the problem with valgrind, and here is an incremental
fix:
diff --git a/lib/netdev.c b/lib/netdev.c
index 9590f83..415cdb4 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -322,8 +322,10 @@ void
netdev_close(struct netdev *netdev)
{
if (netdev) {
+ struct netdev_dev *dev = netdev_get_dev(netdev);
+
netdev_uninit(netdev, true);
- netdev_dev_unref(netdev_get_dev(netdev));
+ netdev_dev_unref(dev);
}
}
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
