Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next (V4) 00/11] IPv6: rt->n (neighbour in rt6_info) removal, TAKE 4.
From: YOSHIFUJI Hideaki @ 2013-01-17 22:52 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

This is "TAKE 4" of neighbour removal from rt6_info.

3->4:
- error check in ip6_finish_output2(), in 10/11.

---

YOSHIFUJI Hideaki (11):
  ndisc: Update neigh->updated with write lock.
  ndisc: Remove tbl argument for __ipv6_neigh_lookup().
  ipv6 route: Dump gateway based on RTF_GATEWAY flag and
    rt->rt6i_gateway.
  ndisc: Introduce __ipv6_neigh_lookup_noref().
  ipv6: Do not depend on rt->n in ip6_pol_route().
  ipv6: Do not depend on rt->n in rt6_check_neigh().
  ipv6: Do not depend on rt->n in rt6_probe().
  ipv6: Introduce rt6_nexthop() to select nexthop address.
  ipv6: Do not depend on rt->n in ip6_dst_lookup_tail().
  ipv6: Do not depend on rt->n in ip6_finish_output2().
  ipv6: Complete neighbour entry removal from dst_entry.

 include/net/ip6_fib.h   |    2 -
 include/net/ip6_route.h |    8 +++
 include/net/ndisc.h     |   24 ++++++---
 net/ipv6/ip6_output.c   |   26 ++++++---
 net/ipv6/route.c        |  138 ++++++++++++-----------------------------------
 net/ipv6/xfrm6_policy.c |    1 -
 6 files changed, 76 insertions(+), 123 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH net-next (V4) 01/11] ndisc: Update neigh->updated with write lock.
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

neigh->nud_state and neigh->updated are under protection of
neigh->lock.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/route.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 7c34c01..1341f68 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -499,22 +499,26 @@ static void rt6_probe(struct rt6_info *rt)
 	 * to no more than one per minute.
 	 */
 	neigh = rt ? rt->n : NULL;
-	if (!neigh || (neigh->nud_state & NUD_VALID))
+	if (!neigh)
+		return;
+	write_lock_bh(&neigh->lock);
+	if (neigh->nud_state & NUD_VALID) {
+		write_unlock_bh(&neigh->lock);
 		return;
-	read_lock_bh(&neigh->lock);
+	}
 	if (!(neigh->nud_state & NUD_VALID) &&
 	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
 		struct in6_addr mcaddr;
 		struct in6_addr *target;
 
 		neigh->updated = jiffies;
-		read_unlock_bh(&neigh->lock);
+		write_unlock_bh(&neigh->lock);
 
 		target = (struct in6_addr *)&neigh->primary_key;
 		addrconf_addr_solict_mult(target, &mcaddr);
 		ndisc_send_ns(rt->dst.dev, NULL, target, &mcaddr, NULL);
 	} else {
-		read_unlock_bh(&neigh->lock);
+		write_unlock_bh(&neigh->lock);
 	}
 }
 #else
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 02/11] ndisc: Remove tbl argument for __ipv6_neigh_lookup().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

We can refer to nd_tbl directly.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ndisc.h |    4 ++--
 net/ipv6/route.c    |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 23b3a7c..bbc938e 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -148,7 +148,7 @@ static inline u32 ndisc_hashfn(const void *pkey, const struct net_device *dev, _
 		(p32[3] * hash_rnd[3]));
 }
 
-static inline struct neighbour *__ipv6_neigh_lookup(struct neigh_table *tbl, struct net_device *dev, const void *pkey)
+static inline struct neighbour *__ipv6_neigh_lookup(struct net_device *dev, const void *pkey)
 {
 	struct neigh_hash_table *nht;
 	const u32 *p32 = pkey;
@@ -156,7 +156,7 @@ static inline struct neighbour *__ipv6_neigh_lookup(struct neigh_table *tbl, str
 	u32 hash_val;
 
 	rcu_read_lock_bh();
-	nht = rcu_dereference_bh(tbl->nht);
+	nht = rcu_dereference_bh(nd_tbl.nht);
 	hash_val = ndisc_hashfn(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
 	for (n = rcu_dereference_bh(nht->hash_buckets[hash_val]);
 	     n != NULL;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 1341f68..5d9ca27 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -145,7 +145,7 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst,
 	struct neighbour *n;
 
 	daddr = choose_neigh_daddr(rt, skb, daddr);
-	n = __ipv6_neigh_lookup(&nd_tbl, dst->dev, daddr);
+	n = __ipv6_neigh_lookup(dst->dev, daddr);
 	if (n)
 		return n;
 	return neigh_create(&nd_tbl, daddr, dst->dev);
@@ -153,7 +153,7 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst,
 
 static int rt6_bind_neighbour(struct rt6_info *rt, struct net_device *dev)
 {
-	struct neighbour *n = __ipv6_neigh_lookup(&nd_tbl, dev, &rt->rt6i_gateway);
+	struct neighbour *n = __ipv6_neigh_lookup(dev, &rt->rt6i_gateway);
 	if (!n) {
 		n = neigh_create(&nd_tbl, &rt->rt6i_gateway, dev);
 		if (IS_ERR(n))
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 03/11] ipv6 route: Dump gateway based on RTF_GATEWAY flag and rt->rt6i_gateway.
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

Do not depend on rt->n.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/route.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5d9ca27..d82eb1f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2484,7 +2484,6 @@ static int rt6_fill_node(struct net *net,
 	struct nlmsghdr *nlh;
 	long expires;
 	u32 table;
-	struct neighbour *n;
 
 	if (prefix) {	/* user wants prefix routes only */
 		if (!(rt->rt6i_flags & RTF_PREFIX_RT)) {
@@ -2597,9 +2596,8 @@ static int rt6_fill_node(struct net *net,
 	if (rtnetlink_put_metrics(skb, dst_metrics_ptr(&rt->dst)) < 0)
 		goto nla_put_failure;
 
-	n = rt->n;
-	if (n) {
-		if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0)
+	if (rt->rt6i_flags & RTF_GATEWAY) {
+		if (nla_put(skb, RTA_GATEWAY, 16, &rt->rt6i_gateway) < 0)
 			goto nla_put_failure;
 	}
 
@@ -2794,7 +2792,6 @@ struct rt6_proc_arg
 static int rt6_info_route(struct rt6_info *rt, void *p_arg)
 {
 	struct seq_file *m = p_arg;
-	struct neighbour *n;
 
 	seq_printf(m, "%pi6 %02x ", &rt->rt6i_dst.addr, rt->rt6i_dst.plen);
 
@@ -2803,9 +2800,8 @@ static int rt6_info_route(struct rt6_info *rt, void *p_arg)
 #else
 	seq_puts(m, "00000000000000000000000000000000 00 ");
 #endif
-	n = rt->n;
-	if (n) {
-		seq_printf(m, "%pi6", n->primary_key);
+	if (rt->rt6i_flags & RTF_GATEWAY) {
+		seq_printf(m, "%pi6", &rt->rt6i_gateway);
 	} else {
 		seq_puts(m, "00000000000000000000000000000000");
 	}
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next 00/11] IPv6: rt->n (neighbour in rt6_info) removal (TAKE 3)
From: David Miller @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davej; +Cc: yoshfuji, netdev, xiyou.wangcong
In-Reply-To: <20130117224222.GA29674@redhat.com>

From: Dave Jones <davej@redhat.com>
Date: Thu, 17 Jan 2013 17:42:22 -0500

> On Thu, Jan 17, 2013 at 03:37:53PM -0500, David Miller wrote:
>  > From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>  > Date: Fri, 18 Jan 2013 02:41:41 +0900
>  > 
>  > > This is "TAKE 3" of neighbour removal from rt6_info.
>  > > 
>  > > 2->3:
>  > > - Removed "icmp6_dst_alloc() removal" patch.
>  > >   This is nothing to do with this series.
>  > > - Removed several unused variables, ifdefs, BUG_ONs.
>  > > - Handled several comments from Cong Wang.
>  > > - Reordered.
>  > 
>  > This series is almost perfect, great work.
> 
> I guess if this is going in soon, the ip6 rt->n related oops I reported
> this morning is moot ? (unless anyone cares about fixing it for older kernels)

I wouldn't say it's moot for older kernels, that's for sure.

^ permalink raw reply

* [PATCH net-next (V4) 04/11] ndisc: Introduce __ipv6_neigh_lookup_noref().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

This function, which looks up neighbour entry for an IPv6 address
without touching refcnt, will be used for patches to remove
dependency on rt->n (neighbour entry in rt6_info).

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ndisc.h |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index bbc938e..ec48f42 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -148,14 +148,13 @@ static inline u32 ndisc_hashfn(const void *pkey, const struct net_device *dev, _
 		(p32[3] * hash_rnd[3]));
 }
 
-static inline struct neighbour *__ipv6_neigh_lookup(struct net_device *dev, const void *pkey)
+static inline struct neighbour *__ipv6_neigh_lookup_noref(struct net_device *dev, const void *pkey)
 {
 	struct neigh_hash_table *nht;
 	const u32 *p32 = pkey;
 	struct neighbour *n;
 	u32 hash_val;
 
-	rcu_read_lock_bh();
 	nht = rcu_dereference_bh(nd_tbl.nht);
 	hash_val = ndisc_hashfn(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
 	for (n = rcu_dereference_bh(nht->hash_buckets[hash_val]);
@@ -164,12 +163,21 @@ static inline struct neighbour *__ipv6_neigh_lookup(struct net_device *dev, cons
 		u32 *n32 = (u32 *) n->primary_key;
 		if (n->dev == dev &&
 		    ((n32[0] ^ p32[0]) | (n32[1] ^ p32[1]) |
-		     (n32[2] ^ p32[2]) | (n32[3] ^ p32[3])) == 0) {
-			if (!atomic_inc_not_zero(&n->refcnt))
-				n = NULL;
-			break;
-		}
+		     (n32[2] ^ p32[2]) | (n32[3] ^ p32[3])) == 0)
+			return n;
 	}
+
+	return NULL;
+}
+
+static inline struct neighbour *__ipv6_neigh_lookup(struct net_device *dev, const void *pkey)
+{
+	struct neighbour *n;
+
+	rcu_read_lock_bh();
+	n = __ipv6_neigh_lookup_noref(dev, pkey);
+	if (n && !atomic_inc_not_zero(&n->refcnt))
+		n = NULL;
 	rcu_read_unlock_bh();
 
 	return n;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 05/11] ipv6: Do not depend on rt->n in ip6_pol_route().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/route.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d82eb1f..2839381 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -925,7 +925,7 @@ restart:
 	dst_hold(&rt->dst);
 	read_unlock_bh(&table->tb6_lock);
 
-	if (!rt->n && !(rt->rt6i_flags & RTF_NONEXTHOP))
+	if (!(rt->rt6i_flags & (RTF_NONEXTHOP | RTF_GATEWAY)))
 		nrt = rt6_alloc_cow(rt, &fl6->daddr, &fl6->saddr);
 	else if (!(rt->dst.flags & DST_HOST))
 		nrt = rt6_alloc_clone(rt, &fl6->daddr);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 06/11] ipv6: Do not depend on rt->n in rt6_check_neigh().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

CC: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/route.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 2839381..c7bcd77 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -546,20 +546,24 @@ static inline bool rt6_check_neigh(struct rt6_info *rt)
 	struct neighbour *neigh;
 	bool ret = false;
 
-	neigh = rt->n;
 	if (rt->rt6i_flags & RTF_NONEXTHOP ||
 	    !(rt->rt6i_flags & RTF_GATEWAY))
-		ret = true;
-	else if (neigh) {
-		read_lock_bh(&neigh->lock);
+		return true;
+
+	rcu_read_lock_bh();
+	neigh = __ipv6_neigh_lookup_noref(rt->dst.dev, &rt->rt6i_gateway);
+	if (neigh) {
+		read_lock(&neigh->lock);
 		if (neigh->nud_state & NUD_VALID)
 			ret = true;
 #ifdef CONFIG_IPV6_ROUTER_PREF
 		else if (!(neigh->nud_state & NUD_FAILED))
 			ret = true;
 #endif
-		read_unlock_bh(&neigh->lock);
+		read_unlock(&neigh->lock);
 	}
+	rcu_read_unlock_bh();
+
 	return ret;
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 07/11] ipv6: Do not depend on rt->n in rt6_probe().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/route.c |   26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c7bcd77..afc8386 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -498,28 +498,34 @@ static void rt6_probe(struct rt6_info *rt)
 	 * Router Reachability Probe MUST be rate-limited
 	 * to no more than one per minute.
 	 */
-	neigh = rt ? rt->n : NULL;
-	if (!neigh)
-		return;
-	write_lock_bh(&neigh->lock);
-	if (neigh->nud_state & NUD_VALID) {
-		write_unlock_bh(&neigh->lock);
+	if (!rt || !(rt->rt6i_flags & RTF_GATEWAY))
 		return;
+	rcu_read_lock_bh();
+	neigh = __ipv6_neigh_lookup_noref(rt->dst.dev, &rt->rt6i_gateway);
+	if (neigh) {
+		write_lock(&neigh->lock);
+		if (neigh->nud_state & NUD_VALID)
+			goto out;
 	}
-	if (!(neigh->nud_state & NUD_VALID) &&
+
+	if (!neigh ||
 	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
 		struct in6_addr mcaddr;
 		struct in6_addr *target;
 
 		neigh->updated = jiffies;
-		write_unlock_bh(&neigh->lock);
 
-		target = (struct in6_addr *)&neigh->primary_key;
+		if (neigh)
+			write_unlock(&neigh->lock);
+
+		target = (struct in6_addr *)&rt->rt6i_gateway;
 		addrconf_addr_solict_mult(target, &mcaddr);
 		ndisc_send_ns(rt->dst.dev, NULL, target, &mcaddr, NULL);
 	} else {
-		write_unlock_bh(&neigh->lock);
+out:
+		write_unlock(&neigh->lock);
 	}
+	rcu_read_unlock_bh();
 }
 #else
 static inline void rt6_probe(struct rt6_info *rt)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 08/11] ipv6: Introduce rt6_nexthop() to select nexthop address.
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

For RTF_GATEWAY route, return rt->rt6i_gateway.
Otherwise, return 2nd argument (destination address).

This will be used by following patches which remove rt->n
dependency patches in ip6_dst_lookup_tail() and ip6_finish_output2().

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ip6_route.h |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 27d8318..30cbb15 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -23,6 +23,7 @@ struct route_info {
 #include <net/sock.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
+#include <linux/route.h>
 
 #define RT6_LOOKUP_F_IFACE		0x00000001
 #define RT6_LOOKUP_F_REACHABLE		0x00000002
@@ -194,4 +195,11 @@ static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
 	       skb_dst(skb)->dev->mtu : dst_mtu(skb_dst(skb));
 }
 
+static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, struct in6_addr *dest)
+{
+	if (rt->rt6i_flags & RTF_GATEWAY)
+		return &rt->rt6i_gateway;
+	return dest;
+}
+
 #endif
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 09/11] ipv6: Do not depend on rt->n in ip6_dst_lookup_tail().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:53 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/ip6_output.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 9250c69..7c00edb 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -913,8 +913,12 @@ static int ip6_dst_lookup_tail(struct sock *sk,
 	 * dst entry of the nexthop router
 	 */
 	rt = (struct rt6_info *) *dst;
-	n = rt->n;
-	if (n && !(n->nud_state & NUD_VALID)) {
+	rcu_read_lock_bh();
+	n = __ipv6_neigh_lookup_noref(rt->dst.dev, rt6_nexthop(rt, &fl6->daddr));
+	err = n && !(n->nud_state & NUD_VALID) ? -EINVAL : 0;
+	rcu_read_unlock_bh();
+
+	if (err) {
 		struct inet6_ifaddr *ifp;
 		struct flowi6 fl_gw6;
 		int redirect;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 10/11] ipv6: Do not depend on rt->n in ip6_finish_output2().
From: YOSHIFUJI Hideaki @ 2013-01-17 22:54 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

If neigh is not found, create new one.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/ip6_output.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7c00edb..b0895f5 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -88,7 +88,8 @@ static int ip6_finish_output2(struct sk_buff *skb)
 	struct dst_entry *dst = skb_dst(skb);
 	struct net_device *dev = dst->dev;
 	struct neighbour *neigh;
-	struct rt6_info *rt;
+	struct in6_addr *nexthop;
+	int ret;
 
 	skb->protocol = htons(ETH_P_IPV6);
 	skb->dev = dev;
@@ -123,10 +124,17 @@ static int ip6_finish_output2(struct sk_buff *skb)
 				skb->len);
 	}
 
-	rt = (struct rt6_info *) dst;
-	neigh = rt->n;
-	if (neigh)
-		return dst_neigh_output(dst, neigh, skb);
+	rcu_read_lock_bh();
+	nexthop = rt6_nexthop((struct rt6_info *)dst, &ipv6_hdr(skb)->daddr);
+	neigh = __ipv6_neigh_lookup_noref(dst->dev, nexthop);
+	if (unlikely(!neigh))
+		neigh = __neigh_create(&nd_tbl, nexthop, dst->dev, false);
+	if (!IS_ERR(neigh)) {
+		ret = dst_neigh_output(dst, neigh, skb);
+		rcu_read_unlock_bh();
+		return ret;
+	}
+	rcu_read_unlock_bh();
 
 	IP6_INC_STATS_BH(dev_net(dst->dev),
 			 ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next (V4) 11/11] ipv6: Complete neighbour entry removal from dst_entry.
From: YOSHIFUJI Hideaki @ 2013-01-17 22:54 UTC (permalink / raw)
  To: davem, netdev; +Cc: yoshfuji, xiyou.wangcong

CC: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ip6_fib.h   |    2 --
 net/ipv6/route.c        |   84 +----------------------------------------------
 net/ipv6/xfrm6_policy.c |    1 -
 3 files changed, 1 insertion(+), 86 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index fdc48a9..6919a50 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -89,8 +89,6 @@ struct fib6_table;
 struct rt6_info {
 	struct dst_entry		dst;
 
-	struct neighbour		*n;
-
 	/*
 	 * Tail elements of dst_entry (__refcnt etc.)
 	 * and these elements (rarely used in hot path) are in
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index afc8386..3a562a1 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -151,19 +151,6 @@ static struct neighbour *ip6_neigh_lookup(const struct dst_entry *dst,
 	return neigh_create(&nd_tbl, daddr, dst->dev);
 }
 
-static int rt6_bind_neighbour(struct rt6_info *rt, struct net_device *dev)
-{
-	struct neighbour *n = __ipv6_neigh_lookup(dev, &rt->rt6i_gateway);
-	if (!n) {
-		n = neigh_create(&nd_tbl, &rt->rt6i_gateway, dev);
-		if (IS_ERR(n))
-			return PTR_ERR(n);
-	}
-	rt->n = n;
-
-	return 0;
-}
-
 static struct dst_ops ip6_dst_ops_template = {
 	.family			=	AF_INET6,
 	.protocol		=	cpu_to_be16(ETH_P_IPV6),
@@ -301,9 +288,6 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 	struct rt6_info *rt = (struct rt6_info *)dst;
 	struct inet6_dev *idev = rt->rt6i_idev;
 
-	if (rt->n)
-		neigh_release(rt->n);
-
 	if (!(rt->dst.flags & DST_HOST))
 		dst_destroy_metrics_generic(dst);
 
@@ -354,11 +338,6 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 				in6_dev_put(idev);
 			}
 		}
-		if (rt->n && rt->n->dev == dev) {
-			rt->n->dev = loopback_dev;
-			dev_hold(loopback_dev);
-			dev_put(dev);
-		}
 	}
 }
 
@@ -845,8 +824,6 @@ static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 	rt = ip6_rt_copy(ort, daddr);
 
 	if (rt) {
-		int attempts = !in_softirq();
-
 		if (!(rt->rt6i_flags & RTF_GATEWAY)) {
 			if (ort->rt6i_dst.plen != 128 &&
 			    ipv6_addr_equal(&ort->rt6i_dst.addr, daddr))
@@ -862,32 +839,6 @@ static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 			rt->rt6i_src.plen = 128;
 		}
 #endif
-
-	retry:
-		if (rt6_bind_neighbour(rt, rt->dst.dev)) {
-			struct net *net = dev_net(rt->dst.dev);
-			int saved_rt_min_interval =
-				net->ipv6.sysctl.ip6_rt_gc_min_interval;
-			int saved_rt_elasticity =
-				net->ipv6.sysctl.ip6_rt_gc_elasticity;
-
-			if (attempts-- > 0) {
-				net->ipv6.sysctl.ip6_rt_gc_elasticity = 1;
-				net->ipv6.sysctl.ip6_rt_gc_min_interval = 0;
-
-				ip6_dst_gc(&net->ipv6.ip6_dst_ops);
-
-				net->ipv6.sysctl.ip6_rt_gc_elasticity =
-					saved_rt_elasticity;
-				net->ipv6.sysctl.ip6_rt_gc_min_interval =
-					saved_rt_min_interval;
-				goto retry;
-			}
-
-			net_warn_ratelimited("Neighbour table overflow\n");
-			dst_free(&rt->dst);
-			return NULL;
-		}
 	}
 
 	return rt;
@@ -898,10 +849,8 @@ static struct rt6_info *rt6_alloc_clone(struct rt6_info *ort,
 {
 	struct rt6_info *rt = ip6_rt_copy(ort, daddr);
 
-	if (rt) {
+	if (rt)
 		rt->rt6i_flags |= RTF_CACHE;
-		rt->n = neigh_clone(ort->n);
-	}
 	return rt;
 }
 
@@ -1272,20 +1221,8 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 		goto out;
 	}
 
-	if (neigh)
-		neigh_hold(neigh);
-	else {
-		neigh = ip6_neigh_lookup(&rt->dst, NULL, &fl6->daddr);
-		if (IS_ERR(neigh)) {
-			in6_dev_put(idev);
-			dst_free(&rt->dst);
-			return ERR_CAST(neigh);
-		}
-	}
-
 	rt->dst.flags |= DST_HOST;
 	rt->dst.output  = ip6_output;
-	rt->n = neigh;
 	atomic_set(&rt->dst.__refcnt, 1);
 	rt->rt6i_dst.addr = fl6->daddr;
 	rt->rt6i_dst.plen = 128;
@@ -1594,12 +1531,6 @@ int ip6_route_add(struct fib6_config *cfg)
 	} else
 		rt->rt6i_prefsrc.plen = 0;
 
-	if (cfg->fc_flags & (RTF_GATEWAY | RTF_NONEXTHOP)) {
-		err = rt6_bind_neighbour(rt, dev);
-		if (err)
-			goto out;
-	}
-
 	rt->rt6i_flags = cfg->fc_flags;
 
 install_route:
@@ -1713,7 +1644,6 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
 	struct netevent_redirect netevent;
 	struct rt6_info *rt, *nrt = NULL;
 	struct ndisc_options ndopts;
-	struct neighbour *old_neigh;
 	struct inet6_dev *in6_dev;
 	struct neighbour *neigh;
 	struct rd_msg *msg;
@@ -1786,11 +1716,6 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
 	if (!neigh)
 		return;
 
-	/* Duplicate redirect: silently ignore. */
-	old_neigh = rt->n;
-	if (neigh == old_neigh)
-		goto out;
-
 	/*
 	 *	We have finally decided to accept it.
 	 */
@@ -1811,7 +1736,6 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
 		nrt->rt6i_flags &= ~RTF_GATEWAY;
 
 	nrt->rt6i_gateway = *(struct in6_addr *)neigh->primary_key;
-	nrt->n = neigh_clone(neigh);
 
 	if (ip6_ins_rt(nrt))
 		goto out;
@@ -2125,7 +2049,6 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 {
 	struct net *net = dev_net(idev->dev);
 	struct rt6_info *rt = ip6_dst_alloc(net, net->loopback_dev, 0, NULL);
-	int err;
 
 	if (!rt) {
 		net_warn_ratelimited("Maximum number of routes reached, consider increasing route/max_size\n");
@@ -2144,11 +2067,6 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 		rt->rt6i_flags |= RTF_ANYCAST;
 	else
 		rt->rt6i_flags |= RTF_LOCAL;
-	err = rt6_bind_neighbour(rt, rt->dst.dev);
-	if (err) {
-		dst_free(&rt->dst);
-		return ERR_PTR(err);
-	}
 
 	rt->rt6i_dst.addr = *addr;
 	rt->rt6i_dst.plen = 128;
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index c984413..1282737 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -110,7 +110,6 @@ static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
 
 	/* Sheit... I remember I did this right. Apparently,
 	 * it was magically lost, so this code needs audit */
-	xdst->u.rt6.n = neigh_clone(rt->n);
 	xdst->u.rt6.rt6i_flags = rt->rt6i_flags & (RTF_ANYCAST |
 						   RTF_LOCAL);
 	xdst->u.rt6.rt6i_metric = rt->rt6i_metric;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
From: David Miller @ 2013-01-17 22:54 UTC (permalink / raw)
  To: jacmet; +Cc: sque, netdev, msb
In-Reply-To: <87wqvbjujl.fsf@dell.be.48ers.dk>

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: Thu, 17 Jan 2013 23:47:58 +0100

>>>>>> "Simon" == Simon Que <sque@chromium.org> writes:
> 
>  Simon> In two places, tmp is initialized implicitly by being passed as a
>  Simon> pointer during a function call.  However, this is not obvious to the
>  Simon> compiler, which logs a warning.
> 
> What warning and what compiler version are you using?

I wouldn't bother asking this, unless it's so that you personally
can reproduce this in the future.

The compiler cannot determine with %100 certainty that tmp is
initialized in all paths.  Any time there are conditional
statements guarding setting and the use, the compiler is going
to err on the side of caution and say that if any of those
paths lead to a use without a set it's going to warn.

Even if it's actually impossible.

^ permalink raw reply

* Re: [PATCH v4] netfilter: nf_conntrack_sip: Handle Cisco 7941/7945 IP phones
From: Pablo Neira Ayuso @ 2013-01-17 23:35 UTC (permalink / raw)
  To: Kevin Cernekee
  Cc: David Woodhouse, Eric Dumazet, Patrick McHardy, David S. Miller,
	Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
	Hideaki YOSHIFUJI, Gabor Juhos, netfilter-devel, netfilter,
	coreteam, linux-kernel, netdev
In-Reply-To: <1355805238-2460-1-git-send-email-cernekee@gmail.com>

On Mon, Dec 17, 2012 at 08:33:58PM -0800, Kevin Cernekee wrote:
> Most SIP devices use a source port of 5060/udp on SIP requests, so the
> response automatically comes back to port 5060:
> 
>     phone_ip:5060 -> proxy_ip:5060   REGISTER
>     proxy_ip:5060 -> phone_ip:5060   100 Trying
> 
> The newer Cisco IP phones, however, use a randomly chosen high source
> port for the SIP request but expect the response on port 5060:
> 
>     phone_ip:49173 -> proxy_ip:5060  REGISTER
>     proxy_ip:5060 -> phone_ip:5060   100 Trying
> 
> Standard Linux NAT, with or without nf_nat_sip, will send the reply back
> to port 49173, not 5060:
> 
>     phone_ip:49173 -> proxy_ip:5060  REGISTER
>     proxy_ip:5060 -> phone_ip:49173  100 Trying
> 
> But the phone is not listening on 49173, so it will never see the reply.
> 
> This patch modifies nf_*_sip to work around this quirk by extracting
> the SIP response port from the Via: header, iff the source IP in the
> packet header matches the source IP in the SIP request.

Applied, thanks Kevin.

^ permalink raw reply

* Re: [PATCH] sctp: refactor sctp_outq_teardown to insure proper re-initalization
From: David Miller @ 2013-01-17 23:40 UTC (permalink / raw)
  To: vyasevich; +Cc: nhorman, linux-sctp, Jamie.Parsons, netdev
In-Reply-To: <50F87FB4.3070604@gmail.com>

From: Vlad Yasevich <vyasevich@gmail.com>
Date: Thu, 17 Jan 2013 17:48:20 -0500

> On 01/17/2013 04:15 PM, Neil Horman wrote:
>> Jamie Parsons reported a problem recently, in which the
>> re-initalization of an
>> association (The duplicate init case), resulted in a loss of receive
>> window
>> space.  He tracked down the root cause to sctp_outq_teardown, which
>> discarded
>> all the data on an outq during a re-initalization of the corresponding
>> association, but never reset the outq->outstanding_data field to zero.
>> I wrote,
>> and he tested this fix, which does a proper full re-initalization of
>> the outq,
>> fixing this problem, and hopefully future proofing us from simmilar
>> issues down
>> the road.
>>
> 
> Good find.
> 
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>

Applied and queued up for -stable, thanks everyone.

^ permalink raw reply

* Re: [PATCH net-next (V4) 00/11] IPv6: rt->n (neighbour in rt6_info) removal, TAKE 4.
From: David Miller @ 2013-01-17 23:40 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, xiyou.wangcong
In-Reply-To: <50F880C3.2050409@linux-ipv6.org>

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Fri, 18 Jan 2013 07:52:51 +0900

> This is "TAKE 4" of neighbour removal from rt6_info.
> 
> 3->4:
> - error check in ip6_finish_output2(), in 10/11.

All applied, I'll push this out to net-next after some quick
build validations, thanks a lot!

^ permalink raw reply

* Server Alert
From: Server Support @ 2013-01-17 23:42 UTC (permalink / raw)



Server Alert!!, Our Webmail have upgraded a new Access Login Page by our
administrator,you are currently running on 0.4GB, due to much spam
mails,you may not be able to access your account anymore with the old page
from on the 19th. You are to Submit your Account correct access e-mail and
password on our Administrator Verified Admin by clicking this link
http://www.surveytool.com/s/SFDD62B68D

Submit/log on with your correct email and password to re-validate and
increase your Mailbox limits to avoid De-activated on the 19th of January
2013.

Thank You.
Administrator

^ permalink raw reply

* Wake on LAN for USB ethernet?
From: Sarah Sharp @ 2013-01-18  0:12 UTC (permalink / raw)
  To: linux-usb, netdev, Petko Manolov; +Cc: Alan Stern, Greg KH, Oliver Neukum

[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]

Is it reasonable to expect that Wake on LAN works if the target box is
connected via a USB-to-ethernet adapter?

One of my validation testers has a USB ethernet device that successfully
wakes up Windows from hibernate (S4), but it doesn't work under Linux.
Wake on LAN works fine with the hardwired ethernet port.  USB remote
wakeup from S3/S4 works fine with a USB keyboard.

So I need to figure out if it's an issue with the kernel, their test
suite, or the USB host controller.


I've experimented with USB ethernet devices with the pegasus driver with
no success.  Here's what I've tried:

1. Connect the USB to ethernet adapter to the rootport under the xHCI
(USB 3.0) host controller. (lsusb for the two devices I've tried is
attached)

2. Enable remote wakeup for all devices (including the PCI host) by
echoing 'enabled' to power/wakeup. (script for that attached)

3. Use ethtool to turn on WOL:
   sudo ethtool -s ethX wol g

4. Find the MAC address by running ifconfig and looking at the HWaddr
field.

5. Verify with wireshark that I can see the Magic WOL packet when I run
this command on another machine:
   sudo etherwake -i ethX macaddr

6. Suspend or hibernate the target machine, and then send the Magic WOL
packet from a second machine.


Am I missing any steps in testing this?  Does WOL just not work for USB
ethernet devices under Linux?  Perhaps just the pegasus driver doesn't
support WOL?

Note that I can't currently test USB remote wakeup for any devices under
the EHCI host controller on my Lenovo x230.  For some reason, whenever I
enable remote wakeup for the rate matching hub under the EHCI host, the
system will immediately resume when I attempt to suspend it.  Don't know
why yet.

Sarah Sharp

[-- Attachment #2: enable-usb-remote-wakeup.sh --]
[-- Type: application/x-sh, Size: 285 bytes --]

[-- Attachment #3: belkin-pegasus-lsusb.txt --]
[-- Type: text/plain, Size: 2418 bytes --]


Bus 003 Device 013: ID 050d:0121 Belkin Components F5D5050 100Mbps Ethernet
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0         8
  idVendor           0x050d Belkin Components
  idProduct          0x0121 F5D5050 100Mbps Ethernet
  bcdDevice            1.01
  iManufacturer           1 ADMtek
  iProduct                2 USB To LAN Converter
  iSerial                 3 0001
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           39
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              160mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass         0 (Defined at Interface level)
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               1
Device Status:     0x0000
  (Bus Powered)

[-- Attachment #4: smc-pegasus-lsusb.txt --]
[-- Type: text/plain, Size: 2391 bytes --]


Bus 003 Device 012: ID 07a6:8513 ADMtek, Inc. AN8513 Ethernet
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass          255 Vendor Specific Class
  bDeviceSubClass         0 
  bDeviceProtocol       255 
  bMaxPacketSize0         8
  idVendor           0x07a6 ADMtek, Inc.
  idProduct          0x8513 AN8513 Ethernet
  bcdDevice            2.01
  iManufacturer           1 USB
  iProduct                2 SMC2208USB/ETH
  iSerial                 3 33D0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           39
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              160mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      1 
      bInterfaceProtocol    255 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               1
Device Status:     0x0002
  (Bus Powered)
  Remote Wakeup Enabled

^ permalink raw reply

* Re: [PATCH for 3.8] iproute2: Add "ip netns pids" and "ip netns identify"
From: Eric W. Biederman @ 2013-01-18  0:23 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Stephen Hemminger, netdev, Serge E. Hallyn
In-Reply-To: <1354039239.2701.8.camel@bwh-desktop.uk.solarflarecom.com>

Ben Hutchings <bhutchings@solarflare.com> writes:

> On Mon, 2012-11-26 at 17:16 -0600, Eric W. Biederman wrote:
>> Add command that go between network namespace names and process
>> identifiers.  The code builds and runs agains older kernels but
>> only works on Linux 3.8+ kernels where I have fixed stat to work
>> properly.
>> 
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>> ---
>> 
>> I don't know if this is too soon to send this patch to iproute as the
>> kernel code that fixes stat is currently sitting in my for-next branch
>> of:
>> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git
>> 
>> and has not hit Linus's tree yet.  Still the code runs and is harmless
>> on older kernels so it should be harmless whatever happens with it.
>> 
>>  ip/ipnetns.c        |  141 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  man/man8/ip-netns.8 |    5 ++-
>>  2 files changed, 145 insertions(+), 1 deletions(-)
>> 
>> diff --git a/ip/ipnetns.c b/ip/ipnetns.c
>> index e41a598..c55fe3a 100644
>> --- a/ip/ipnetns.c
>> +++ b/ip/ipnetns.c
>> @@ -13,6 +13,7 @@
>>  #include <dirent.h>
>>  #include <errno.h>
>>  #include <unistd.h>
>> +#include <ctype.h>
>>  
>>  #include "utils.h"
>>  #include "ip_common.h"
>> @@ -48,6 +49,8 @@ static void usage(void)
>>  	fprintf(stderr, "Usage: ip netns list\n");
>>  	fprintf(stderr, "       ip netns add NAME\n");
>>  	fprintf(stderr, "       ip netns delete NAME\n");
>> +	fprintf(stderr, "       ip netns identify PID\n");
>> +	fprintf(stderr, "       ip netns pids NAME\n");
>>  	fprintf(stderr, "       ip netns exec NAME cmd ...\n");
>>  	fprintf(stderr, "       ip netns monitor\n");
>>  	exit(-1);
>> @@ -171,6 +174,138 @@ static int netns_exec(int argc, char **argv)
>>  	exit(-1);
>>  }
>>  
>> +static int is_pid(const char *str)
>> +{
>> +	int ch;
>> +	for (; (ch = *str); str++) {
>> +		if (!isdigit(ch))
>
> ch must be cast to unsigned char before passing to isdigit().

isdigit is defined to take an int.  A legacy of the implicit casts in
the K&R C days.  Casting to unsigned char would be pointless and silly.

>> +			return 0;
>> +	}
>> +	return 1;
>> +}
>> +
>> +static int netns_pids(int argc, char **argv)
>> +{
>> +	const char *name;
>> +	char net_path[MAXPATHLEN];
>> +	int netns;
>> +	struct stat netst;
>> +	DIR *dir;
>> +	struct dirent *entry;
>> +
>> +	if (argc < 1) {
>> +		fprintf(stderr, "No netns name specified\n");
>> +		return -1;
>> +	}
>> +	if (argc > 1) {
>> +		fprintf(stderr, "extra arguments specified\n");
>> +		return -1;
>> +	}
>
> These, and many other return statements in this file which set the
> process exit code, should return 1 (general failure) or 2 (user error)
> rather than -1 (likely to be interpreted as command not found).

Good point.  

>> +	name = argv[0];
>> +	snprintf(net_path, sizeof(net_path), "%s/%s", NETNS_RUN_DIR, name);
>
> No check for truncation?

Nope.  snprintf guarantees returning a '\0' terminated string, so there
is no point in supplying a bad string to get the program to crash or act
in odd ways.  It might be more friendly to say:  "Hey silly that string
you passed me was too long to use for anyting, don't do that".  But I
don't think it is worth the code complexity to maintain the extra error
message, for something that in my experience people just don't do.

>> +	netns = open(net_path, O_RDONLY);
>
> This file descriptor is leaked, though that probably doesn't really
> matter.

Good point.  Probably worth fixing in case someone figures out how
to use these commands in batch mode.

Thanks fixed.

> [...]
>> +static int netns_identify(int argc, char **argv)
>> +{
>> +	const char *pidstr;
>> +	char net_path[MAXPATHLEN];
>> +	int netns;
>> +	struct stat netst;
>> +	DIR *dir;
>> +	struct dirent *entry;
>> +
>> +	if (argc < 1) {
>> +		fprintf(stderr, "No pid specified\n");
>> +		return -1;
>> +	}
>> +	if (argc > 1) {
>> +		fprintf(stderr, "extra arguments specified\n");
>> +		return -1;
>> +	}
>> +	pidstr = argv[0];
>> +
>> +	if (!is_pid(pidstr)) {
>> +		fprintf(stderr, "Specified string '%s' is not a pid\n",
>> +			pidstr);
>> +		return -1;
>> +	}
>> +
>> +	snprintf(net_path, sizeof(net_path), "/proc/%s/ns/net", pidstr);
>> +	netns = open(net_path, O_RDONLY);
>> +	if (netns < 0) {
>> +		fprintf(stderr, "Cannot open network namespace: %s\n",
>> +			strerror(errno));
>> +		return -1;
>> +	}
>> +	if (fstat(netns, &netst) < 0) {
>> +		fprintf(stderr, "Stat of netns failed: %s\n",
>> +			strerror(errno));
>> +		return -1;
>> +	}
>> +	dir = opendir(NETNS_RUN_DIR);
>> +	if (!dir)
>> +		return 0;
>
> Shouldn't this be treated as an error?  Or, if you want it to succeed
> when the kernel does not have netns functionality, then treat it as an
> error if !dir && errno != ENOENT.

What I want is to treat as a missing directory like an empty directory
so we don't error in the case where we simply have not named any network
namespaces yet.

But yes treating it an error when errno != ENOENT makes sense.

And is fixed in my next version.

>> +	while((entry = readdir(dir))) {
>> +		char name_path[MAXPATHLEN];
>> +		struct stat st;
>> +
>> +		if (strcmp(entry->d_name, ".") == 0)
>> +			continue;
>> +		if (strcmp(entry->d_name, "..") == 0)
>> +			continue;
>> +
>> +		snprintf(name_path, sizeof(name_path), "%s/%s",	NETNS_RUN_DIR,
>> +			entry->d_name);
>> +
>> +		if (stat(name_path, &st) != 0)
>> +			continue;
>> +
>> +		if ((st.st_dev == netst.st_dev) &&
>> +		    (st.st_ino == netst.st_ino)) {
>> +			printf("%s\n", entry->d_name);
>> +		}
>> +	}
>> +	closedir(dir);
>> +	return 0;
>> +	
>> +}
> [...]
>
> It's a shame there isn't a more efficient way to do these lookups.

Well there is no index but this just takes a single pass through
either all of the processes or all of the network namespaces.  That
is a simple O(N) algorithm and really isn't inefficient.

These two new commands really are debugging aids to make it easier
to match up processes and network namespaces.  Although at some point
a more generic version of this functionality probably needs to make it's
way into lsof and fuser as well.

I will send my updated version after I have had some sleep and
can double check everything with fresh eyes.

Eric

^ permalink raw reply

* [PATCH iproute-3.8 0/6] ip netns bug fixes and enhancements
From: Eric W. Biederman @ 2013-01-18  0:44 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Serge E. Hallyn, Ben Hutchings
In-Reply-To: <1354039239.2701.8.camel@bwh-desktop.uk.solarflarecom.com>


This is a set of patches for iproute2 3.8.  Most of the patches
are just general maintenance and cleanup the final patch adds
some commands to make it possible to find the correlation between
network namespaces and running processes.

The first patch is a significant bug fix for a condition that causes
sysfs to be unmounted on systems that choose to propogate all of the
mounts between their mount namespaces by default.

Eric W. Biederman (6):
      iproute2: Don't propogate mounts out of ip netns exec.
      iproute2: Normalize return codes in "ip netns"
      iproute2: Improve "ip netns add" failure error message
      iproute2: Make "ip netns delete" more likely to succeed
      iproute2: Fill in the ip-netns.8 manpage
      iproute2: Add "ip netns pids" and "ip netns identify"

 ip/ipnetns.c        |  236 ++++++++++++++++++++++++++++++++++++++++++++-------
 man/man8/ip-netns.8 |   64 ++++++++++++++-
 2 files changed, 268 insertions(+), 32 deletions(-)

^ permalink raw reply

* [PATCH iproute2-3.8 1/6] iproute2: Don't propogate mounts out of ip
From: Eric W. Biederman @ 2013-01-18  0:45 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev, Serge E. Hallyn, Ben Hutchings, Petr Šabata
In-Reply-To: <87622v5ngt.fsf_-_@xmission.com>


Some systems are now following the advice in
linux/Documentation/sharedsubtrees.txt and running with all mount
points shared between all mount namespaces by default.

After creating the mount namespace call mount on / with
MS_SLAVE|MS_REC to modify all mounts in the new mount namespace to
slave mounts if they are shared or private mounts otherwise.
Guarnateeing that changes to the mount namespace created with
"ip netns exec" don't propgate to other namespaces.

Reported-by: Petr Šabata <contyk@redhat.com>
Tested-by: Petr Šabata <contyk@redhat.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 ip/ipnetns.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index e41a598..f2c42ba 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -152,6 +152,12 @@ static int netns_exec(int argc, char **argv)
 		fprintf(stderr, "unshare failed: %s\n", strerror(errno));
 		return -1;
 	}
+	/* Don't let any mounts propogate back to the parent */
+	if (mount("", "/", "none", MS_SLAVE | MS_REC, NULL)) {
+		fprintf(stderr, "mount --make-rslave / failed: %s\n",
+			strerror(errno));
+		return -1;
+	}
 	/* Mount a version of /sys that describes the network namespace */
 	if (umount2("/sys", MNT_DETACH) < 0) {
 		fprintf(stderr, "umount of /sys failed: %s\n", strerror(errno));
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH iproute2-3.8 2/6] iproute2: Normalize return codes in "ip netns"
From: Eric W. Biederman @ 2013-01-18  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Serge E. Hallyn, Ben Hutchings
In-Reply-To: <87622v5ngt.fsf_-_@xmission.com>


Ben Hutchings pointed out that the return value of do_netns is passed
to exit and the current convention of returning -1 for failure is
inconsitent with that reality.

Return EXIT_FAILURE instead of -1 and EXIT_SUCCESS instead of 0.  To make
it clear that the return codes are expected to be passed to exit.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 ip/ipnetns.c |   56 ++++++++++++++++++++++++++------------------------------
 1 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index f2c42ba..ae55090 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -40,17 +40,14 @@ static int setns(int fd, int nstype)
 }
 #endif /* HAVE_SETNS */
 
-
-static void usage(void) __attribute__((noreturn));
-
-static void usage(void)
+static int usage(void)
 {
 	fprintf(stderr, "Usage: ip netns list\n");
 	fprintf(stderr, "       ip netns add NAME\n");
 	fprintf(stderr, "       ip netns delete NAME\n");
 	fprintf(stderr, "       ip netns exec NAME cmd ...\n");
 	fprintf(stderr, "       ip netns monitor\n");
-	exit(-1);
+	return EXIT_FAILURE;
 }
 
 int get_netns_fd(const char *name)
@@ -75,7 +72,7 @@ static int netns_list(int argc, char **argv)
 
 	dir = opendir(NETNS_RUN_DIR);
 	if (!dir)
-		return 0;
+		return EXIT_SUCCESS;
 
 	while ((entry = readdir(dir)) != NULL) {
 		if (strcmp(entry->d_name, ".") == 0)
@@ -85,7 +82,7 @@ static int netns_list(int argc, char **argv)
 		printf("%s\n", entry->d_name);
 	}
 	closedir(dir);
-	return 0;
+	return EXIT_SUCCESS;
 }
 
 static void bind_etc(const char *name)
@@ -127,11 +124,11 @@ static int netns_exec(int argc, char **argv)
 
 	if (argc < 1) {
 		fprintf(stderr, "No netns name specified\n");
-		return -1;
+		return EXIT_FAILURE;
 	}
 	if (argc < 2) {
 		fprintf(stderr, "No cmd specified\n");
-		return -1;
+		return EXIT_FAILURE;
 	}
 	name = argv[0];
 	cmd = argv[1];
@@ -140,32 +137,32 @@ static int netns_exec(int argc, char **argv)
 	if (netns < 0) {
 		fprintf(stderr, "Cannot open network namespace: %s\n",
 			strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	if (setns(netns, CLONE_NEWNET) < 0) {
 		fprintf(stderr, "seting the network namespace failed: %s\n",
 			strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 
 	if (unshare(CLONE_NEWNS) < 0) {
 		fprintf(stderr, "unshare failed: %s\n", strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	/* Don't let any mounts propogate back to the parent */
 	if (mount("", "/", "none", MS_SLAVE | MS_REC, NULL)) {
 		fprintf(stderr, "mount --make-rslave / failed: %s\n",
 			strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	/* Mount a version of /sys that describes the network namespace */
 	if (umount2("/sys", MNT_DETACH) < 0) {
 		fprintf(stderr, "umount of /sys failed: %s\n", strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	if (mount(name, "/sys", "sysfs", 0, NULL) < 0) {
 		fprintf(stderr, "mount of /sys failed: %s\n",strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 
 	/* Setup bind mounts for config files in /etc */
@@ -174,7 +171,7 @@ static int netns_exec(int argc, char **argv)
 	if (execvp(cmd, argv + 1)  < 0)
 		fprintf(stderr, "exec of %s failed: %s\n",
 			cmd, strerror(errno));
-	exit(-1);
+	return EXIT_FAILURE;
 }
 
 static int netns_delete(int argc, char **argv)
@@ -184,7 +181,7 @@ static int netns_delete(int argc, char **argv)
 
 	if (argc < 1) {
 		fprintf(stderr, "No netns name specified\n");
-		return -1;
+		return EXIT_FAILURE;
 	}
 
 	name = argv[0];
@@ -193,9 +190,9 @@ static int netns_delete(int argc, char **argv)
 	if (unlink(netns_path) < 0) {
 		fprintf(stderr, "Cannot remove %s: %s\n",
 			netns_path, strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
-	return 0;
+	return EXIT_SUCCESS;
 }
 
 static int netns_add(int argc, char **argv)
@@ -214,7 +211,7 @@ static int netns_add(int argc, char **argv)
 
 	if (argc < 1) {
 		fprintf(stderr, "No netns name specified\n");
-		return -1;
+		return EXIT_FAILURE;
 	}
 	name = argv[0];
 
@@ -228,7 +225,7 @@ static int netns_add(int argc, char **argv)
 	if (fd < 0) {
 		fprintf(stderr, "Could not create %s: %s\n",
 			netns_path, strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	close(fd);
 	if (unshare(CLONE_NEWNET) < 0) {
@@ -243,11 +240,10 @@ static int netns_add(int argc, char **argv)
 			netns_path, strerror(errno));
 		goto out_delete;
 	}
-	return 0;
+	return EXIT_SUCCESS;
 out_delete:
 	netns_delete(argc, argv);
-	exit(-1);
-	return -1;
+	return EXIT_FAILURE;
 }
 
 
@@ -260,19 +256,19 @@ static int netns_monitor(int argc, char **argv)
 	if (fd < 0) {
 		fprintf(stderr, "inotify_init failed: %s\n",
 			strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	if (inotify_add_watch(fd, NETNS_RUN_DIR, IN_CREATE | IN_DELETE) < 0) {
 		fprintf(stderr, "inotify_add_watch failed: %s\n",
 			strerror(errno));
-		return -1;
+		return EXIT_FAILURE;
 	}
 	for(;;) {
 		ssize_t len = read(fd, buf, sizeof(buf));
 		if (len < 0) {
 			fprintf(stderr, "read failed: %s\n",
 				strerror(errno));
-			return -1;
+			return EXIT_FAILURE;
 		}
 		for (event = (struct inotify_event *)buf;
 		     (char *)event < &buf[len];
@@ -283,7 +279,7 @@ static int netns_monitor(int argc, char **argv)
 				printf("delete %s\n", event->name);
 		}
 	}
-	return 0;
+	return EXIT_SUCCESS;
 }
 
 int do_netns(int argc, char **argv)
@@ -296,7 +292,7 @@ int do_netns(int argc, char **argv)
 		return netns_list(argc-1, argv+1);
 
 	if (matches(*argv, "help") == 0)
-		usage();
+		return usage();
 
 	if (matches(*argv, "add") == 0)
 		return netns_add(argc-1, argv+1);
@@ -311,5 +307,5 @@ int do_netns(int argc, char **argv)
 		return netns_monitor(argc-1, argv+1);
 
 	fprintf(stderr, "Command \"%s\" is unknown, try \"ip netns help\".\n", *argv);
-	exit(-1);
+	return EXIT_FAILURE;
 }
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH iproute2-3.8 3/6] iproute2: Improve "ip netns add" failure error message
From: Eric W. Biederman @ 2013-01-18  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Serge E. Hallyn, Ben Hutchings
In-Reply-To: <87622v5ngt.fsf_-_@xmission.com>


Report the name of the network namespace that could not be
added.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 ip/ipnetns.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index ae55090..4fce379 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -135,8 +135,8 @@ static int netns_exec(int argc, char **argv)
 	snprintf(net_path, sizeof(net_path), "%s/%s", NETNS_RUN_DIR, name);
 	netns = open(net_path, O_RDONLY);
 	if (netns < 0) {
-		fprintf(stderr, "Cannot open network namespace: %s\n",
-			strerror(errno));
+		fprintf(stderr, "Cannot open network namespace %s: %s\n",
+			name, strerror(errno));
 		return EXIT_FAILURE;
 	}
 	if (setns(netns, CLONE_NEWNET) < 0) {
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH iproute2-3.8 4/6] iproute2: Make "ip netns delete" more likely to succeed
From: Eric W. Biederman @ 2013-01-18  0:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Serge E. Hallyn, Ben Hutchings
In-Reply-To: <87622v5ngt.fsf_-_@xmission.com>


Sometimes "ip netns delete" fails because it can not delete the file a
network namespace was mounted on.  If this only happened when a
network namespace was really in use this would be fine, but today it
is possible to pin all network namespaces by simply having a long
running process started with "ip netns exec".

Every mount is copied when a network namespace is created so it is
impossible to prevent the mounts from getting into other mount
namespaces.  Modify all mounts in the files and subdirectories of
/var/run/netns to be shared mount points so that unmount events can
propogate, making it unlikely that "ip netns delete" will fail because
a directory is mounted in another mount namespace.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 ip/ipnetns.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 4fce379..33765b5 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -208,6 +208,7 @@ static int netns_add(int argc, char **argv)
 	char netns_path[MAXPATHLEN];
 	const char *name;
 	int fd;
+	int made_netns_run_dir_mount = 0;
 
 	if (argc < 1) {
 		fprintf(stderr, "No netns name specified\n");
@@ -220,6 +221,29 @@ static int netns_add(int argc, char **argv)
 	/* Create the base netns directory if it doesn't exist */
 	mkdir(NETNS_RUN_DIR, S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
 
+	/* Make it possible for network namespace mounts to propogate between
+	 * mount namespaces.  This makes it likely that a unmounting a network
+	 * namespace file in one namespace will unmount the network namespace
+	 * file in all namespaces allowing the network namespace to be freed
+	 * sooner.
+	 */
+	while (mount("", NETNS_RUN_DIR, "none", MS_SHARED | MS_REC, NULL)) {
+		/* Fail unless we need to make the mount point */
+		if (errno != EINVAL || made_netns_run_dir_mount) {
+			fprintf(stderr, "mount --make-shared %s failed: %s\n",
+				NETNS_RUN_DIR, strerror(errno));
+			return EXIT_FAILURE;
+		}
+
+		/* Upgrade NETNS_RUN_DIR to a mount point */
+		if (mount(NETNS_RUN_DIR, NETNS_RUN_DIR, "none", MS_BIND, NULL)) {
+			fprintf(stderr, "mount --bind %s %s failed: %s\n",
+				NETNS_RUN_DIR, NETNS_RUN_DIR, strerror(errno));
+			return EXIT_FAILURE;
+		}
+		made_netns_run_dir_mount = 1;
+	}
+
 	/* Create the filesystem state */
 	fd = open(netns_path, O_RDONLY|O_CREAT|O_EXCL, 0);
 	if (fd < 0) {
-- 
1.7.5.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox