[ovs-dev] [PATCH] bridge: Clean leaking netdevs when route is added.

Ben Pfaff blp at ovn.org
Tue Jul 10 21:17:37 UTC 2018


On Thu, Jun 21, 2018 at 06:39:16PM +0100, Tiago Lam wrote:
> When adding a route to a bridge, by executing "$appctl ovs/route/add
> $IP/$MASK $BR", a reference to the existing netdev is taken and stored
> in an instantiated ip_dev struct which is then stored in an addr_list
> list in tnl-ports.c. When OvS is signaled to exit, as a result of a
> "$appctl $OVS_PID exit --cleanup", for example, the bridge takes care of
> destroying its allocated port and iface structs. While destroying and
> freeing an iface, the netdev associated with it is also destroyed.
> However, for this to happen its ref_cnt must be 0.  Otherwise the
> destructor of the netdev (specific to each datapath) won't be called. On
> the userspace datapath this means a system interface, such as "br0",
> wouldn't get deleted upon exit of OvS (when a route happens to be
> assocaited).
> 
> This was first observed in the "ptap - triangle bridge setup with L2 and
> L3 GRE tunnels" test, which runs as part of the system userspace
> testsuite and uses the netdev datapath (as opoosed to several tests
> which use the dummy datapath, where this issue isn't seen). The test
> would pass every other time and fail the rest of the times because the
> needed system interfaces (br-p1, br-p2 and br-p3) were already present
> (from the previous successfull run which didn't clean up properly),
> leading to a failure.
> 
> To fix the leak and clean up the interfaces upon exit, on its final
> stage before destroying a netdev, in iface_destroy__(), the bridge calls
> tnl_port_map_delete_ipdev() which takes care of freeing the instatiated
> ip_dev structs that refer to a specific netdev.
> 
> An extra test is also introduced which verifies that the resources used
> by OvS netdev datapath have been correctly cleaned up between
> OVS_TRAFFIC_VSWITCHD_STOP and AT_CLEANUP.
> 
> Signed-off-by: Tiago Lam <tiago.lam at intel.com>

I don't fully understand this one, but the commit message is so detailed
and the patch is so short, so I applied it to master.  If it needs
backporting, please let me know.


More information about the dev mailing list