Re: [PATCH] net_namespace: fixed net_device reference leak
flat view
From: Jojy Varghese <hidden>
Date: 2016-09-08 23:33:40
Subsystem:
networking [general], the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Sorry last patch was just the delta over my original patch. Here is the complete patch: --- net/core/dst.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/core/dst.c b/net/core/dst.c
index a1656e3..f63027e 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c@@ -433,8 +433,11 @@ static void dst_ifdown(struct dst_entry *dst,struct net_device *dev,
dst->input = dst_discard;
dst->output = dst_discard_out;
} else {
- dst->dev = dev_net(dst->dev)->loopback_dev;
- dev_hold(dst->dev);
+ if (dst->dev != dev_net(dst->dev)->loopback_dev) {
+ dst->dev = dev_net(dst->dev)->loopback_dev;
+ dev_hold(dst->dev);
+ }
+
dev_put(dev);
}
}
--
1.8.3.1
On Thu, Sep 8, 2016 at 4:16 PM, Jojy Varghese [off-list ref] wrote:quoted hunk ↗ jump to hunk
The dst's do disappear but the net device does not. A better solution is to only hold reference to dst's net device ONLY if it is not a loopback device. In the case of the loopback device, we want to do only a "put" on it and skip the "hold". Updating the patch below: --- net/core/dst.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)diff --git a/net/core/dst.c b/net/core/dst.c index 7e45593..f63027e 100644 --- a/net/core/dst.c +++ b/net/core/dst.c@@ -433,12 +433,11 @@ static void dst_ifdown(struct dst_entry *dst,struct net_device *dev, dst->input = dst_discard; dst->output = dst_discard_out; } else { - if (dst->dev == dev_net(dst->dev)->loopback_dev) - dst->dev = init_net.loopback_dev; - else + if (dst->dev != dev_net(dst->dev)->loopback_dev) { dst->dev = dev_net(dst->dev)->loopback_dev; + dev_hold(dst->dev); + } - dev_hold(dst->dev); dev_put(dev); } } -- 1.8.3.1 On Thu, Sep 8, 2016 at 3:16 PM, Eric Dumazet [off-list ref] wrote:quoted
On Thu, 2016-09-08 at 15:12 -0700, Jojy Varghese wrote:quoted
Hi Eric/Lance Thanks for taking time to look at the patch. The problem in dst.c's "dst_ifdown" is that it will never do an actual "dev_put" on the loopback net device. This results in the loopback net device to be alive even when the dst goes away.Sure, but the dst themselves should disappear, and thus release their dst->dev reference. Something is missing. loopback device is only used when we remove one device from the system, but namespace (and loopback device) is still around.-- Jojy G Varghese
-- Jojy G Varghese