Netdev List
 help / color / mirror / Atom feed
* [PATCH] xfrm: Return dst directly from xfrm_lookup()
From: David Miller @ 2011-03-02 21:55 UTC (permalink / raw)
  To: netdev


Instead of on the stack.

Signed-off-by: David S. Miller <davem@davemloft.net>
---

Ok, this is what I was setting up for with the changes I wrote
and posted yesterday.

Next is getting the ipv4 route lookup interfaces to do this as
well.

 include/net/dst.h                |   14 ++++++++------
 net/decnet/dn_route.c            |   12 ++++++++++--
 net/ipv4/icmp.c                  |   36 ++++++++++++++----------------------
 net/ipv4/netfilter.c             |    6 ++++--
 net/ipv4/route.c                 |    7 ++++++-
 net/ipv6/icmp.c                  |   37 ++++++++++++++++++-------------------
 net/ipv6/ip6_output.c            |   10 ++--------
 net/ipv6/ip6_tunnel.c            |    8 +++++++-
 net/ipv6/mcast.c                 |   13 ++++++++++---
 net/ipv6/ndisc.c                 |    8 ++++----
 net/ipv6/netfilter.c             |    3 ++-
 net/ipv6/netfilter/ip6t_REJECT.c |    3 ++-
 net/netfilter/ipvs/ip_vs_xmit.c  |    9 +++++++--
 net/xfrm/xfrm_policy.c           |   34 +++++++++++++++++-----------------
 14 files changed, 111 insertions(+), 89 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 8948452..2a46cba 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -426,15 +426,17 @@ enum {
 
 struct flowi;
 #ifndef CONFIG_XFRM
-static inline int xfrm_lookup(struct net *net, struct dst_entry **dst_p,
-			      const struct flowi *fl, struct sock *sk,
-			      int flags)
+static inline struct dst_entry *xfrm_lookup(struct net *net,
+					    struct dst_entry *dst_orig,
+					    const struct flowi *fl, struct sock *sk,
+					    int flags)
 {
-	return 0;
+	return dst_orig;
 } 
 #else
-extern int xfrm_lookup(struct net *net, struct dst_entry **dst_p,
-		       const struct flowi *fl, struct sock *sk, int flags);
+extern struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
+				     const struct flowi *fl, struct sock *sk,
+				     int flags);
 #endif
 #endif
 
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 0877147..484fdbf 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1222,7 +1222,11 @@ static int dn_route_output_key(struct dst_entry **pprt, struct flowi *flp, int f
 
 	err = __dn_route_output_key(pprt, flp, flags);
 	if (err == 0 && flp->proto) {
-		err = xfrm_lookup(&init_net, pprt, flp, NULL, 0);
+		*pprt = xfrm_lookup(&init_net, *pprt, flp, NULL, 0);
+		if (IS_ERR(*pprt)) {
+			err = PTR_ERR(*pprt);
+			*pprt = NULL;
+		}
 	}
 	return err;
 }
@@ -1235,7 +1239,11 @@ int dn_route_output_sock(struct dst_entry **pprt, struct flowi *fl, struct sock
 	if (err == 0 && fl->proto) {
 		if (!(flags & MSG_DONTWAIT))
 			fl->flags |= FLOWI_FLAG_CAN_SLEEP;
-		err = xfrm_lookup(&init_net, pprt, fl, sk, 0);
+		*pprt = xfrm_lookup(&init_net, *pprt, fl, sk, 0);
+		if (IS_ERR(*pprt)) {
+			err = PTR_ERR(*pprt);
+			*pprt = NULL;
+		}
 	}
 	return err;
 }
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 2a86c89..c23bd8c 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -398,18 +398,14 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
 	if (!fl.fl4_src)
 		fl.fl4_src = rt->rt_src;
 
-	err = xfrm_lookup(net, (struct dst_entry **)&rt, &fl, NULL, 0);
-	switch (err) {
-	case 0:
+	rt = (struct rtable *) xfrm_lookup(net, &rt->dst, &fl, NULL, 0);
+	if (!IS_ERR(rt)) {
 		if (rt != rt2)
 			return rt;
-		break;
-	case -EPERM:
+	} else if (PTR_ERR(rt) == -EPERM) {
 		rt = NULL;
-		break;
-	default:
-		return ERR_PTR(err);
-	}
+	} else
+		return rt;
 
 	err = xfrm_decode_session_reverse(skb_in, &fl, AF_INET);
 	if (err)
@@ -438,22 +434,18 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
 	if (err)
 		goto relookup_failed;
 
-	err = xfrm_lookup(net, (struct dst_entry **)&rt2, &fl, NULL,
-			  XFRM_LOOKUP_ICMP);
-	switch (err) {
-	case 0:
+	rt2 = (struct rtable *) xfrm_lookup(net, &rt2->dst, &fl, NULL, XFRM_LOOKUP_ICMP);
+	if (!IS_ERR(rt2)) {
 		dst_release(&rt->dst);
 		rt = rt2;
-		break;
-	case -EPERM:
-		return ERR_PTR(err);
-	default:
-		if (!rt)
-			return ERR_PTR(err);
-		break;
+	} else if (PTR_ERR(rt2) == -EPERM) {
+		if (rt)
+			dst_release(&rt->dst);
+		return rt2;
+	} else {
+		err = PTR_ERR(rt2);
+		goto relookup_failed;
 	}
-
-
 	return rt;
 
 relookup_failed:
diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index 994a1f2..9770bb4 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -69,7 +69,8 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned addr_type)
 	    xfrm_decode_session(skb, &fl, AF_INET) == 0) {
 		struct dst_entry *dst = skb_dst(skb);
 		skb_dst_set(skb, NULL);
-		if (xfrm_lookup(net, &dst, &fl, skb->sk, 0))
+		dst = xfrm_lookup(net, dst, &fl, skb->sk, 0);
+		if (IS_ERR(dst))
 			return -1;
 		skb_dst_set(skb, dst);
 	}
@@ -102,7 +103,8 @@ int ip_xfrm_me_harder(struct sk_buff *skb)
 		dst = ((struct xfrm_dst *)dst)->route;
 	dst_hold(dst);
 
-	if (xfrm_lookup(dev_net(dst->dev), &dst, &fl, skb->sk, 0) < 0)
+	dst = xfrm_lookup(dev_net(dst->dev), dst, &fl, skb->sk, 0);
+	if (IS_ERR(dst))
 		return -1;
 
 	skb_dst_drop(skb);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e24e4cf..63d3700 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2730,7 +2730,12 @@ int ip_route_output_flow(struct net *net, struct rtable **rp, struct flowi *flp,
 			flp->fl4_src = (*rp)->rt_src;
 		if (!flp->fl4_dst)
 			flp->fl4_dst = (*rp)->rt_dst;
-		return xfrm_lookup(net, (struct dst_entry **)rp, flp, sk, 0);
+		*rp = (struct rtable *) xfrm_lookup(net, &(*rp)->dst, flp, sk, 0);
+		if (IS_ERR(*rp)) {
+			err = PTR_ERR(*rp);
+			*rp = NULL;
+			return err;
+		}
 	}
 
 	return 0;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index e332bae..5566595 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -324,17 +324,15 @@ static struct dst_entry *icmpv6_route_lookup(struct net *net, struct sk_buff *sk
 	/* No need to clone since we're just using its address. */
 	dst2 = dst;
 
-	err = xfrm_lookup(net, &dst, fl, sk, 0);
-	switch (err) {
-	case 0:
+	dst = xfrm_lookup(net, dst, fl, sk, 0);
+	if (!IS_ERR(dst)) {
 		if (dst != dst2)
 			return dst;
-		break;
-	case -EPERM:
-		dst = NULL;
-		break;
-	default:
-		return ERR_PTR(err);
+	} else {
+		if (PTR_ERR(dst) == -EPERM)
+			dst = NULL;
+		else
+			return dst;
 	}
 
 	err = xfrm_decode_session_reverse(skb, &fl2, AF_INET6);
@@ -345,17 +343,17 @@ static struct dst_entry *icmpv6_route_lookup(struct net *net, struct sk_buff *sk
 	if (err)
 		goto relookup_failed;
 
-	err = xfrm_lookup(net, &dst2, &fl2, sk, XFRM_LOOKUP_ICMP);
-	switch (err) {
-	case 0:
+	dst2 = xfrm_lookup(net, dst2, &fl2, sk, XFRM_LOOKUP_ICMP);
+	if (!IS_ERR(dst2)) {
 		dst_release(dst);
 		dst = dst2;
-		break;
-	case -EPERM:
-		dst_release(dst);
-		return ERR_PTR(err);
-	default:
-		goto relookup_failed;
+	} else {
+		err = PTR_ERR(dst2);
+		if (err == -EPERM) {
+			dst_release(dst);
+			return dst2;
+		} else
+			goto relookup_failed;
 	}
 
 relookup_failed:
@@ -560,7 +558,8 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 	err = ip6_dst_lookup(sk, &dst, &fl);
 	if (err)
 		goto out;
-	if ((err = xfrm_lookup(net, &dst, &fl, sk, 0)) < 0)
+	dst = xfrm_lookup(net, dst, &fl, sk, 0);
+	if (IS_ERR(dst))
 		goto out;
 
 	if (ipv6_addr_is_multicast(&fl.fl6_dst))
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 35a4ad9..adaffaf 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1028,10 +1028,7 @@ struct dst_entry *ip6_dst_lookup_flow(struct sock *sk, struct flowi *fl,
 	if (can_sleep)
 		fl->flags |= FLOWI_FLAG_CAN_SLEEP;
 
-	err = xfrm_lookup(sock_net(sk), &dst, fl, sk, 0);
-	if (err)
-		return ERR_PTR(err);
-	return dst;
+	return xfrm_lookup(sock_net(sk), dst, fl, sk, 0);
 }
 EXPORT_SYMBOL_GPL(ip6_dst_lookup_flow);
 
@@ -1067,10 +1064,7 @@ struct dst_entry *ip6_sk_dst_lookup_flow(struct sock *sk, struct flowi *fl,
 	if (can_sleep)
 		fl->flags |= FLOWI_FLAG_CAN_SLEEP;
 
-	err = xfrm_lookup(sock_net(sk), &dst, fl, sk, 0);
-	if (err)
-		return ERR_PTR(err);
-	return dst;
+	return xfrm_lookup(sock_net(sk), dst, fl, sk, 0);
 }
 EXPORT_SYMBOL_GPL(ip6_sk_dst_lookup_flow);
 
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 4f4483e..da43038 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -903,8 +903,14 @@ static int ip6_tnl_xmit2(struct sk_buff *skb,
 	else {
 		dst = ip6_route_output(net, NULL, fl);
 
-		if (dst->error || xfrm_lookup(net, &dst, fl, NULL, 0) < 0)
+		if (dst->error)
 			goto tx_err_link_failure;
+		dst = xfrm_lookup(net, dst, fl, NULL, 0);
+		if (IS_ERR(dst)) {
+			err = PTR_ERR(dst);
+			dst = NULL;
+			goto tx_err_link_failure;
+		}
 	}
 
 	tdev = dst->dev;
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 49f986d..7b27d08 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1429,7 +1429,12 @@ static void mld_sendpack(struct sk_buff *skb)
 			 &ipv6_hdr(skb)->saddr, &ipv6_hdr(skb)->daddr,
 			 skb->dev->ifindex);
 
-	err = xfrm_lookup(net, &dst, &fl, NULL, 0);
+	dst = xfrm_lookup(net, dst, &fl, NULL, 0);
+	err = 0;
+	if (IS_ERR(dst)) {
+		err = PTR_ERR(dst);
+		dst = NULL;
+	}
 	skb_dst_set(skb, dst);
 	if (err)
 		goto err_out;
@@ -1796,9 +1801,11 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type)
 			 &ipv6_hdr(skb)->saddr, &ipv6_hdr(skb)->daddr,
 			 skb->dev->ifindex);
 
-	err = xfrm_lookup(net, &dst, &fl, NULL, 0);
-	if (err)
+	dst = xfrm_lookup(net, dst, &fl, NULL, 0);
+	if (IS_ERR(dst)) {
+		err = PTR_ERR(dst);
 		goto err_out;
+	}
 
 	skb_dst_set(skb, dst);
 	err = NF_HOOK(NFPROTO_IPV6, NF_INET_LOCAL_OUT, skb, NULL, skb->dev,
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 7254ce3..9360d3b 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -529,8 +529,8 @@ void ndisc_send_skb(struct sk_buff *skb,
 		return;
 	}
 
-	err = xfrm_lookup(net, &dst, &fl, NULL, 0);
-	if (err < 0) {
+	dst = xfrm_lookup(net, dst, &fl, NULL, 0);
+	if (IS_ERR(dst)) {
 		kfree_skb(skb);
 		return;
 	}
@@ -1542,8 +1542,8 @@ void ndisc_send_redirect(struct sk_buff *skb, struct neighbour *neigh,
 	if (dst == NULL)
 		return;
 
-	err = xfrm_lookup(net, &dst, &fl, NULL, 0);
-	if (err)
+	dst = xfrm_lookup(net, dst, &fl, NULL, 0);
+	if (IS_ERR(dst))
 		return;
 
 	rt = (struct rt6_info *) dst;
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 35915e8..8d74116 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -39,7 +39,8 @@ int ip6_route_me_harder(struct sk_buff *skb)
 	if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
 	    xfrm_decode_session(skb, &fl, AF_INET6) == 0) {
 		skb_dst_set(skb, NULL);
-		if (xfrm_lookup(net, &dst, &fl, skb->sk, 0))
+		dst = xfrm_lookup(net, dst, &fl, skb->sk, 0);
+		if (IS_ERR(dst))
 			return -1;
 		skb_dst_set(skb, dst);
 	}
diff --git a/net/ipv6/netfilter/ip6t_REJECT.c b/net/ipv6/netfilter/ip6t_REJECT.c
index bf998fe..91f6a61 100644
--- a/net/ipv6/netfilter/ip6t_REJECT.c
+++ b/net/ipv6/netfilter/ip6t_REJECT.c
@@ -101,7 +101,8 @@ static void send_reset(struct net *net, struct sk_buff *oldskb)
 		dst_release(dst);
 		return;
 	}
-	if (xfrm_lookup(net, &dst, &fl, NULL, 0))
+	dst = xfrm_lookup(net, dst, &fl, NULL, 0);
+	if (IS_ERR(dst))
 		return;
 
 	hh_len = (dst->dev->hard_header_len + 15)&~15;
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index a48239a..6264219 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -218,8 +218,13 @@ __ip_vs_route_output_v6(struct net *net, struct in6_addr *daddr,
 	    ipv6_dev_get_saddr(net, ip6_dst_idev(dst)->dev,
 			       &fl.fl6_dst, 0, &fl.fl6_src) < 0)
 		goto out_err;
-	if (do_xfrm && xfrm_lookup(net, &dst, &fl, NULL, 0) < 0)
-		goto out_err;
+	if (do_xfrm) {
+		dst = xfrm_lookup(net, dst, &fl, NULL, 0);
+		if (IS_ERR(dst)) {
+			dst = NULL;
+			goto out_err;
+		}
+	}
 	ipv6_addr_copy(ret_saddr, &fl.fl6_src);
 	return dst;
 
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 0248afa..b1932a6 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1757,14 +1757,14 @@ static struct dst_entry *make_blackhole(struct net *net, u16 family,
  * At the moment we eat a raw IP route. Mostly to speed up lookups
  * on interfaces with disabled IPsec.
  */
-int xfrm_lookup(struct net *net, struct dst_entry **dst_p,
-		const struct flowi *fl,
-		struct sock *sk, int flags)
+struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
+			      const struct flowi *fl,
+			      struct sock *sk, int flags)
 {
 	struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
 	struct flow_cache_object *flo;
 	struct xfrm_dst *xdst;
-	struct dst_entry *dst, *dst_orig = *dst_p, *route;
+	struct dst_entry *dst, *route;
 	u16 family = dst_orig->ops->family;
 	u8 dir = policy_to_flow_dir(XFRM_POLICY_OUT);
 	int i, err, num_pols, num_xfrms = 0, drop_pols = 0;
@@ -1847,11 +1847,7 @@ restart:
 			xfrm_pols_put(pols, drop_pols);
 			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTNOSTATES);
 
-			dst = make_blackhole(net, family, dst_orig);
-			if (IS_ERR(dst))
-				return PTR_ERR(dst);
-			*dst_p = dst;
-			return 0;
+			return make_blackhole(net, family, dst_orig);
 		}
 		if (fl->flags & FLOWI_FLAG_CAN_SLEEP) {
 			DECLARE_WAITQUEUE(wait, current);
@@ -1895,27 +1891,28 @@ no_transform:
 		goto error;
 	} else if (num_xfrms > 0) {
 		/* Flow transformed */
-		*dst_p = dst;
 		dst_release(dst_orig);
 	} else {
 		/* Flow passes untransformed */
 		dst_release(dst);
+		dst = dst_orig;
 	}
 ok:
 	xfrm_pols_put(pols, drop_pols);
-	return 0;
+	return dst;
 
 nopol:
-	if (!(flags & XFRM_LOOKUP_ICMP))
+	if (!(flags & XFRM_LOOKUP_ICMP)) {
+		dst = dst_orig;
 		goto ok;
+	}
 	err = -ENOENT;
 error:
 	dst_release(dst);
 dropdst:
 	dst_release(dst_orig);
-	*dst_p = NULL;
 	xfrm_pols_put(pols, drop_pols);
-	return err;
+	return ERR_PTR(err);
 }
 EXPORT_SYMBOL(xfrm_lookup);
 
@@ -2175,7 +2172,7 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 	struct net *net = dev_net(skb->dev);
 	struct flowi fl;
 	struct dst_entry *dst;
-	int res;
+	int res = 0;
 
 	if (xfrm_decode_session(skb, &fl, family) < 0) {
 		XFRM_INC_STATS(net, LINUX_MIB_XFRMFWDHDRERROR);
@@ -2183,9 +2180,12 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 	}
 
 	skb_dst_force(skb);
-	dst = skb_dst(skb);
 
-	res = xfrm_lookup(net, &dst, &fl, NULL, 0) == 0;
+	dst = xfrm_lookup(net, skb_dst(skb), &fl, NULL, 0);
+	if (IS_ERR(dst)) {
+		res = 1;
+		dst = NULL;
+	}
 	skb_dst_set(skb, dst);
 	return res;
 }
-- 
1.7.4.1


^ permalink raw reply related

* [RFC LOL OMG] pfifo_lat: qdisc that limits dequeueing based on estimated link latency
From: John W. Linville @ 2011-03-02 21:54 UTC (permalink / raw)
  To: netdev; +Cc: bloat-devel, John W. Linville
In-Reply-To: <20110228132341.194975v6ojrudl18@hayate.sektori.org>

This is a qdisc based on the existing pfifo_fast code.  The difference
is that this qdisc limits the dequeue rate based on estimates of how
many packets can be in-flight at a given time while maintaining a target
link latency.

This work is based on the eBDP documented in Section IV of "Buffer
Sizing for 802.11 Based Networks" by Tianji Li, et al.

	http://www.hamilton.ie/tianji_li/buffersizing.pdf

This implementation timestamps an skb as it dequeues it, then
computes the service time when the frame is freed by the driver.
An exponentially weighted moving average of per fragment service times
is used to restrict queueing delays in hopes of achieving a target
fragment transmission latency.  The skb->deconstructor mechanism is
abused in order to obtain packet service time estimates.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
I took a whack at reimplementing my eBDP patch at the qdisc level.
Unfortunately, it doesn't seem to work very well and I'm at a loss
as to why... :-( Comments welcome -- maybe I'm doing something really
stupid in the math and just can't see it.

The skb->deconstructor abuse includes adding a union member in the skb
to record the qdisc->handle on the way out so that it can be used for
accounting in the deconstructor -- thanks to Neil Horman for the
suggestion!

The reason I think this is an idea worth exploring is that existing
qdisc code doesn't seem to account for the fact that the devices could
be doing a lot of queueing behind them.  Even Jussi's recent
sch_fifo_ewma post doesn't seem to take into account how long the device
holds-on to packets, which limits his ability to fight latency.

Anyway, all comments appreciated!

 include/linux/skbuff.h  |    2 +
 include/net/pkt_sched.h |    1 +
 net/sched/sch_api.c     |    1 +
 net/sched/sch_fifo.c    |  131 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+), 0 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index bf221d6..d99861e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -296,6 +296,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@end: End pointer
  *	@destructor: Destruct function
  *	@mark: Generic packet mark
+ *	@qdhandle: handle of leaf qdisc that handled skb
  *	@nfct: Associated connection, if any
  *	@ipvs_property: skbuff is owned by ipvs
  *	@peeked: this packet has been seen already, so stats have been
@@ -407,6 +408,7 @@ struct sk_buff {
 	union {
 		__u32		mark;
 		__u32		dropcount;
+		__u32		qdhandle;
 	};
 
 	__u16			vlan_tci;
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index d9549af..93189f6 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -72,6 +72,7 @@ extern void qdisc_watchdog_cancel(struct qdisc_watchdog *wd);
 extern struct Qdisc_ops pfifo_qdisc_ops;
 extern struct Qdisc_ops bfifo_qdisc_ops;
 extern struct Qdisc_ops pfifo_head_drop_qdisc_ops;
+extern struct Qdisc_ops pfifo_lat_qdisc_ops;
 
 extern int fifo_set_limit(struct Qdisc *q, unsigned int limit);
 extern struct Qdisc *fifo_create_dflt(struct Qdisc *sch, struct Qdisc_ops *ops,
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b22ca2d..9c9ba9a 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1769,6 +1769,7 @@ static int __init pktsched_init(void)
 	register_qdisc(&pfifo_qdisc_ops);
 	register_qdisc(&bfifo_qdisc_ops);
 	register_qdisc(&pfifo_head_drop_qdisc_ops);
+	register_qdisc(&pfifo_lat_qdisc_ops);
 	register_qdisc(&mq_qdisc_ops);
 
 	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL);
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index d468b47..0d2cb48 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/skbuff.h>
+#include <linux/average.h>
 #include <net/pkt_sched.h>
 
 /* 1 band FIFO pseudo-"scheduler" */
@@ -24,6 +25,20 @@ struct fifo_sched_data
 	u32 limit;
 };
 
+/*
+ * Private data for a pfifo_lat scheduler containing:
+ *	- embedded fifo private data
+ *	- EWMA of average skb service time for each band
+ *	- count of currently in-flight skbs for each band
+ *	- maximum in-flight skbs for each band
+ */
+struct pfifo_lat_data {
+	struct fifo_sched_data q;
+	struct ewma tserv;
+	unsigned int inflight;
+	unsigned int inflight_max;
+};
+
 static int bfifo_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 {
 	struct fifo_sched_data *q = qdisc_priv(sch);
@@ -59,6 +74,86 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc* sch)
 	return NET_XMIT_CN;
 }
 
+static int pfifo_lat_enqueue(struct sk_buff *skb, struct Qdisc* sch)
+{
+	struct pfifo_lat_data *priv = qdisc_priv(sch);
+
+	/* include inflight count when checking queue length limit */
+	if (skb_queue_len(&sch->q) + priv->inflight < priv->q.limit)
+		return qdisc_enqueue_tail(skb, sch);
+
+	return qdisc_reshape_fail(skb, sch);
+}
+
+static void pfifo_lat_skb_free(struct sk_buff *skb)
+{
+	struct Qdisc *qdisc = qdisc_lookup(skb->dev, skb->qdhandle);
+	struct pfifo_lat_data *priv = qdisc_priv(qdisc);
+	unsigned int tserv_ns, inflight_mult;
+
+	/*
+	 * grab timestamp info for buffer control estimates and factor
+	 * that into service time estimate for this queue
+	 */
+	ewma_add(&priv->tserv,
+		 ktime_to_ns(ktime_sub(ktime_get(), skb->tstamp)));
+	tserv_ns = ewma_read(&priv->tserv);
+	if (tserv_ns) {
+		/* calculate multiplier between tserv and target latency */
+		inflight_mult = 2 * NSEC_PER_MSEC / tserv_ns;
+
+		/*
+		 * use current inflight number as proxy for number of
+		 * packets inflight when this packet was sent to
+		 * hardware queue
+		 */
+		priv->inflight_max =
+			max_t(int, 2, priv->inflight * inflight_mult);
+	}
+
+	priv->inflight--;
+}
+
+static struct sk_buff *pfifo_lat_dequeue(struct Qdisc *qdisc)
+{
+	struct pfifo_lat_data *priv = qdisc_priv(qdisc);
+	struct sk_buff *skb;
+
+	if (priv->inflight >= priv->inflight_max)
+		return NULL;
+
+	skb = qdisc_dequeue_head(qdisc);
+	if (!skb)
+		return NULL;
+
+	priv->inflight++;
+
+	/* take ownership of skb and timestamp it */
+	skb_orphan(skb);
+	skb->qdhandle = qdisc->handle;
+	skb->destructor = pfifo_lat_skb_free;
+	skb->dev = qdisc_dev(qdisc); /* do I need to set this?  */
+	skb->tstamp = ktime_get();
+
+	return skb;
+}
+
+static void pfifo_lat_reset(struct Qdisc* qdisc)
+{
+	struct pfifo_lat_data *priv = qdisc_priv(qdisc);
+
+	/*
+	 * since fifo_sched_data is embedded at head of pfifo_lat_data,
+	 * this should be OK to do...
+	 */
+	qdisc_reset_queue(qdisc);
+
+	/* need to reset priv->tserv somehow? */
+
+	priv->inflight = 0;
+	priv->inflight_max = (typeof(priv->inflight_max))-1;
+}
+
 static int fifo_init(struct Qdisc *sch, struct nlattr *opt)
 {
 	struct fifo_sched_data *q = qdisc_priv(sch);
@@ -82,6 +177,30 @@ static int fifo_init(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
+static int pfifo_lat_init(struct Qdisc *qdisc, struct nlattr *opt)
+{
+	struct pfifo_lat_data *priv = qdisc_priv(qdisc);
+	int rc;
+
+	/*
+	 * since fifo_sched_data is embedded at head of pfifo_lat_data,
+	 * this should be OK to do...
+	 */
+	rc = fifo_init(qdisc, opt);
+	if (rc)
+		return rc;
+
+	/* initialize service time estimate */
+	ewma_init(&priv->tserv, 1, 64);
+
+	priv->inflight = 0; /* necessary to set this explicitly? */
+
+	/* initial inflight_max should be ??? */
+	priv->inflight_max = (typeof(priv->inflight_max))-1;
+
+	return 0;
+}
+
 static int fifo_dump(struct Qdisc *sch, struct sk_buff *skb)
 {
 	struct fifo_sched_data *q = qdisc_priv(sch);
@@ -138,6 +257,18 @@ struct Qdisc_ops pfifo_head_drop_qdisc_ops __read_mostly = {
 	.owner		=	THIS_MODULE,
 };
 
+struct Qdisc_ops pfifo_lat_qdisc_ops __read_mostly = {
+	.id		=	"pfifo_lat",
+	.priv_size	=	sizeof(struct pfifo_lat_data),
+	.enqueue	=	pfifo_lat_enqueue,
+	.dequeue	=	pfifo_lat_dequeue,
+	.peek		=	qdisc_peek_head,
+	.init		=	pfifo_lat_init,
+	.reset		=	pfifo_lat_reset,
+	.dump		=	fifo_dump,
+	.owner		=	THIS_MODULE,
+};
+
 /* Pass size change message down to embedded FIFO */
 int fifo_set_limit(struct Qdisc *q, unsigned int limit)
 {
-- 
1.7.4


^ permalink raw reply related

* Re: [GIT PULL nf-2.6] IPVS
From: Simon Horman @ 2011-03-02 22:06 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: lvs-devel, netdev, netfilter-devel, netfilter, Hans Schillstrom,
	Julian Anastasov
In-Reply-To: <4D6E22CA.2020305@trash.net>

On Wed, Mar 02, 2011 at 11:58:18AM +0100, Patrick McHardy wrote:
> Am 01.03.2011 23:59, schrieb Simon Horman:
> > Hi Patrick,
> > 
> > please consider pulling
> > git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git for-patrick
> > to get the following change from Julian. Please note that it is an nf-2.6
> > (that is 2.6.38-rc) change.
> > 
> > Julian Anastasov (1):
> >       ipvs: fix dst_lock locking on dest update
> 
> Pulled, thanks Simon.

Thanks Patrick.

This change is also needed in nf-next-2.6 but I assume that
will automatically happen when nf-2.6 is merged into nf-next-2.6,
possibly via a merge of net-2.6 into net-next-2.6.

^ permalink raw reply

* Re: [RFC LOL OMG] pfifo_lat: qdisc that limits dequeueing based on estimated link latency
From: John W. Linville @ 2011-03-02 22:08 UTC (permalink / raw)
  To: netdev; +Cc: bloat-devel
In-Reply-To: <1299102850-2883-1-git-send-email-linville@tuxdriver.com>

On Wed, Mar 02, 2011 at 04:54:10PM -0500, John W. Linville wrote:
> This is a qdisc based on the existing pfifo_fast code.  The difference

Well, it started that way.  This is obviously based on the pfifo
code instead...

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH 02/27] HFI: Add HFI adapter control structure
From: Stephen Hemminger @ 2011-03-02 22:21 UTC (permalink / raw)
  To: dykmanj
  Cc: netdev, Piyush Chaudhary, Fu-Chung Chang,  William S. Cadden,
	 Wen C. Chen, Scot Sakolish, Jian Xiao,  Carol L. Soto,
	 Sarah J. Sheppard
In-Reply-To: <1299100213-8770-2-git-send-email-dykmanj@linux.vnet.ibm.com>

On Wed,  2 Mar 2011 16:09:48 -0500
dykmanj@linux.vnet.ibm.com wrote:

> diff --git a/drivers/net/hfi/core/Makefile b/drivers/net/hfi/core/Makefile
> index 80790c6..6fe4e60 100644
> --- a/drivers/net/hfi/core/Makefile
> +++ b/drivers/net/hfi/core/Makefile
> @@ -1,5 +1,6 @@
>  #
>  # Makefile for the HFI device driver for IBM eServer System p
>  #
> -hfi_core-objs:=	hfidd_init.o
> +hfi_core-objs:=	hfidd_adpt.o \
> +		hfidd_init.o
>  obj-$(CONFIG_HFI) += hfi_core.o
> diff --git a/drivers/net/hfi/core/hfidd_adpt.c b/drivers/net/hfi/core/hfidd_adpt.c
> new file mode 100644
> index 0000000..d64fa38
> --- /dev/null
> +++ b/drivers/net/hfi/core/hfidd_adpt.c
> @@ -0,0 +1,60 @@
> +/*
> + * hfidd_adpt.c
> + *
> + * HFI device driver for IBM System p
> + *
> + *  Authors:
> + *      Fu-Chung Chang <fcchang@linux.vnet.ibm.com>
> + *      William S. Cadden <wscadden@linux.vnet.ibm.com>
> + *      Wen C. Chen <winstonc@linux.vnet.ibm.com>
> + *      Scot Sakolish <sakolish@linux.vnet.ibm.com>
> + *      Jian Xiao <jian@linux.vnet.ibm.com>
> + *      Carol L. Soto <clsoto@linux.vnet.ibm.com>
> + *      Sarah J. Sheppard <sjsheppa@linux.vnet.ibm.com>
> + *
> + *  (C) Copyright IBM Corp. 2010
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +
> +#include <linux/hfi/hfidd_internal.h>
> +#include "hfidd_proto.h"
> +
> +int hfidd_alloc_adapter(struct hfidd_acs **adpt, dev_t devno, void *uiop)
> +{
> +
> +	struct hfidd_acs	*p_acs = NULL;
> +
> +	p_acs = kzalloc(sizeof(*p_acs), GFP_KERNEL);
> +	if (p_acs == NULL)
> +		return -ENOMEM;
> +
> +	p_acs->dev_num = devno;
> +	p_acs->index  = MINOR(devno);
> +	p_acs->state  = HFI_INVALID;
> +	snprintf(p_acs->name, HFI_DEVICE_NAME_MAX - 1,
> +			"%s%d", HFIDD_DEV_NAME, p_acs->index);
> +
> +	*adpt = p_acs;
> +	return 0;
> +}
> +
> +void hfidd_free_adapter(struct hfidd_acs *p_acs)
> +{
> +	kfree(p_acs);
> +	p_acs = NULL;
> +	return;
> +}

If these were not in a separate file the could be marked as static.

Doing a return; on last line of a void function is considered poor
style since it is unnecessary.

-- 

^ permalink raw reply

* Re: [PATCH 24/27] HFI: hf network driver
From: Stephen Hemminger @ 2011-03-02 22:26 UTC (permalink / raw)
  To: dykmanj
  Cc: netdev, Piyush Chaudhary, Fu-Chung Chang,  William S. Cadden,
	 Wen C. Chen, Scot Sakolish, Jian Xiao,  Carol L. Soto,
	 Sarah J. Sheppard
In-Reply-To: <1299100213-8770-24-git-send-email-dykmanj@linux.vnet.ibm.com>

On Wed,  2 Mar 2011 16:10:10 -0500
dykmanj@linux.vnet.ibm.com wrote:

> +struct hf_if {
> +	u32			idx;			/* 0, 1, 2, 3 ...   */
> +	u32			ai;			/* 0=hfi0, 1=hfi1   */
> +	char			name[HF_MAX_NAME_LEN];
> +	u32			isr_id;
> +	u32			ip_addr;
> +	u32			state;			/* CLOSE, OPEN */
> +	spinlock_t		lock;			/* lock for state */
> +	u32			sfifo_fv_polarity;
> +	u32			sfifo_slots_per_blk;
> +	u32			sfifo_packets;
> +	void __iomem		*doorbell;		/* mapped mmio_regs */
> +	struct hf_fifo		tx_fifo;
> +	struct hf_fifo		rx_fifo;
> +	struct hfi_client_info	client;
> +	struct sk_buff		**tx_skb;		/* array to store tx
> +							   2k skb */
> +	void			*sfifo_finishvec;
> +	struct net_device_stats	net_stats;
> +};

You don't need net_stats in this structure if you use
the standard netdev->stats structure instead.

You won't need hf_get_stats then..

-- 

^ permalink raw reply

* Re: [PATCH 27/27] HFI: hf ethtool support
From: Jim Dykman @ 2011-03-02 22:28 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, Piyush Chaudhary, Fu-Chung Chang, William S. Cadden,
	Wen C. Chen, Scot Sakolish, Jian Xiao, Carol L. Soto,
	Sarah J. Sheppard
In-Reply-To: <1299102762.4277.4.camel@localhost>

On 3/2/2011 4:52 PM, Ben Hutchings wrote:
> On Wed, 2011-03-02 at 16:10 -0500, dykmanj@linux.vnet.ibm.com wrote:
> [...]
>> +static int hf_get_sset_count(struct net_device *netdev, int sset)
>> +{
>> +	switch (sset) {
>> +	case ETH_SS_STATS:
>> +		return ARRAY_SIZE(hf_ethtool_stats_keys);
>> +	default:
>> +		return -EOPNOTSUPP;
> 
> The error code should be -EINVAL, I think.

ok

> 
>> +	}
>> +}
>> +
>> +static void hf_get_ethtool_stats(struct net_device *netdev,
>> +		struct ethtool_stats *stats, u64 *data)
>> +{
>> +	struct hf_net	*net = netdev_priv(netdev);
>> +	struct hf_if	*net_if = &(net->hfif);
>> +
>> +	memcpy(data, &(net_if->eth_stats), sizeof(struct hf_ethtool_stats));
> [...]
> 
> This may result in word tearing, particularly if this driver can be
> built for a 32-bit system.  Since the stats appear to be updated
> asynchronously in the data path, you may have to declare them as
> unsigned long and then extend them to 64-bit in hf_get_ethtool_stats().
> 
> Ben.
> 

It is 64-bit only, but we forgot to mention that in Kconfig.

Thanks.

Jim Dykman


^ permalink raw reply

* Re: [PATCH 27/27] HFI: hf ethtool support
From: David Miller @ 2011-03-02 22:32 UTC (permalink / raw)
  To: dykmanj
  Cc: bhutchings, netdev, piyushc, fcchang, wscadden, winstonc,
	sakolish, jian, clsoto, sjsheppa
In-Reply-To: <4D6EC49A.9060405@linux.vnet.ibm.com>

From: Jim Dykman <dykmanj@linux.vnet.ibm.com>
Date: Wed, 02 Mar 2011 17:28:42 -0500

> It is 64-bit only, but we forgot to mention that in Kconfig.

Please do not mark this driver as 64-bit only in the Kconfig if
at all possible, as that will markedly decrease the build test
coverage of this driver.

^ permalink raw reply

* Re: [PATCH 24/27] HFI: hf network driver
From: Ben Hutchings @ 2011-03-02 22:40 UTC (permalink / raw)
  To: dykmanj
  Cc: netdev, Piyush Chaudhary, Fu-Chung Chang, William S. Cadden,
	Wen C. Chen, Scot Sakolish, Jian Xiao, Carol L. Soto,
	Sarah J. Sheppard
In-Reply-To: <1299100213-8770-24-git-send-email-dykmanj@linux.vnet.ibm.com>

On Wed, 2011-03-02 at 16:10 -0500, dykmanj@linux.vnet.ibm.com wrote:
> From: Jim Dykman <dykmanj@linux.vnet.ibm.com>
> 
> It is a separate binary because it is not strictly necessary to use the HFI.
> This patch includes module load/unload and the window open/setup with the
> hfi device driver.
[...]
> diff --git a/drivers/net/hfi/ip/Kconfig b/drivers/net/hfi/ip/Kconfig
> new file mode 100644
> index 0000000..1a2c21d
> --- /dev/null
> +++ b/drivers/net/hfi/ip/Kconfig
> @@ -0,0 +1,9 @@
> +config HFI_IP
> +	tristate "IP-over-HFI"
> +	depends on NETDEVICES && INET && HFI
> +	---help---
> +	Support for the IP over HFI. It transports IP
> +	packets over HFI.
> +
> +	To compile the driver as a module, choose M here. The module
> +	will be called hf.

You actually call it hf_if!  But why it is not called hfi_ip?

> diff --git a/drivers/net/hfi/ip/Makefile b/drivers/net/hfi/ip/Makefile
> new file mode 100644
> index 0000000..59eff9b
> --- /dev/null
> +++ b/drivers/net/hfi/ip/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for the HF IP interface for IBM eServer System p
> +#
> +obj-$(CONFIG_HFI_IP) += hf_if.o
> +
> +hf_if-objs :=	hf_if_main.o
> diff --git a/drivers/net/hfi/ip/hf_if_main.c b/drivers/net/hfi/ip/hf_if_main.c
> new file mode 100644
> index 0000000..329baa1
> --- /dev/null
> +++ b/drivers/net/hfi/ip/hf_if_main.c
[...]
> +static int hf_inet_event(struct notifier_block *this,
> +			 unsigned long event,
> +			 void *ifa)
> +{
> +	struct in_device	*in_dev;
> +	struct net_device	*netdev;
> +
> +	in_dev = ((struct in_ifaddr *)ifa)->ifa_dev;
> +
> +	netdev = in_dev->dev;
> +
> +	if (!net_eq(dev_net(netdev), &init_net))
> +		return NOTIFY_DONE;
> +
> +	if (event == NETDEV_UP) {
> +		struct hf_if	*net_if;
> +
> +		net_if = &(((struct hf_net *)(netdev_priv(netdev)))->hfif);

Try running:

# ifconfig lo down
# ifconfig lo up

and watch the explosion.

You need to check that this is actually one of your devices.  I've done
this by comparing netdev->netdev_ops pointer.

[...]
> +static int hf_alloc_tx_resource(struct hf_if *net_if)
> +{
[...]
> +	if (net_if->tx_fifo.addr == 0) {
> +		printk(KERN_ERR "%s: hf_alloc_tx_resource: "
> +			"tx_fifo fail, size=0x%x\n",
> +			net_if->name, net_if->tx_fifo.size);
[...]

The netdev_err() and netif_err() (etc.) macros are the standard way to
format messages relating to a net device.

[...]
> +static int hf_set_mac_addr(struct net_device *netdev, void *p)
> +{
> +	struct hf_net		*net = netdev_priv(netdev);
> +	struct hf_if		*net_if = &(net->hfif);
> +
> +	/* Mac address format: 02:ClusterID:ISR:ISR:HFI_WIN:WIN */
> +
> +	/* Locally administered MAC address */
> +	netdev->dev_addr[0] = 0x2; /* bit6=1, bit7=0 */
> +
> +	netdev->dev_addr[1] = 0x0; /* cluster id */
> +
> +	*(u16 *)(&(netdev->dev_addr[2])) = (u16)(net_if->isr_id);
> +
> +	*(u16 *)(&(netdev->dev_addr[4])) = (u16)
> +	(((net_if->ai) << HF_MAC_HFI_SHIFT) | (net_if->client.window));

These two assignments should perhaps include an explicit cpu_to_be16().

[...]
> +static int hf_net_close(struct net_device *netdev)
> +{
> +	struct hf_net		*net = netdev_priv(netdev);
> +	struct hf_if		*net_if = &(net->hfif);
> +	struct hfidd_acs	*p_acs = HF_ACS(net_if);
> +
> +	if (net_if->state == HF_NET_CLOSE)
> +		return 0;

I'm a bit puzzled by this.  Do you not trust the networking core to keep
track of your device state?

> +	spin_lock(&(net_if->lock));
> +	if (net_if->state == HF_NET_OPEN) {
> +		hf_close_ip_window(net_if, p_acs);
> +
> +		hf_free_resource(net_if);
> +	}
> +
> +	hf_register_hfi_ready_callback(netdev, p_acs,
> +			HFIDD_REQ_EVENT_UNREGISTER);
> +
> +	net_if->state = HF_NET_CLOSE;
> +	spin_unlock(&(net_if->lock));
> +
> +	return 0;
> +}
> +
> +struct net_device_stats *hf_get_stats(struct net_device *netdev)
> +{
> +	struct hf_net	*net = netdev_priv(netdev);
> +	struct hf_if	*net_if = &(net->hfif);
> +
> +	return &(net_if->net_stats);
> +}

Please use the stats contained in struct net_device instead.

> +static int hf_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	if ((new_mtu <= 0) || (new_mtu > HF_NET_MTU))
> +		return -ERANGE;

Since this interface apparently only passes ARP and IPv4, the minimum
MTU should be the minimum for IPv4, which is 68.  (The spec says 576 but
the Linux IPv4 implementation uses this value.)

[...]
> +static void hf_if_setup(struct net_device *netdev)
> +{
> +	netdev->type		= ARPHRD_HFI;
> +	netdev->mtu		= HF_NET_MTU;
> +	netdev->tx_queue_len	= 1000;
> +	netdev->flags		= IFF_BROADCAST;
> +	netdev->hard_header_len	= HF_HLEN;
> +	netdev->addr_len	= HF_ALEN;
> +	netdev->needed_headroom	= 0;
> +
> +	netdev->header_ops	= &hf_header_ops;
> +	netdev->netdev_ops	= &hf_netdev_ops;
> +
> +	netdev->features       |= NETIF_F_SG;

You can't provide NETIF_F_SG without checksum offload.

> +	memcpy(netdev->broadcast, hfi_bcast_addr, HF_ALEN);
> +}
> +
> +static struct hf_net *hf_init_netdev(int idx, int ai)
> +{
> +	struct net_device	*netdev;
> +	struct hf_net		*net;
> +	int			ii;
> +	int			rc;
> +	char			ifname[HF_MAX_NAME_LEN];
> +
> +	ii = (idx * MAX_HFIS) + ai;
> +	sprintf(ifname, "hf%d", ii);
> +	netdev = alloc_netdev(sizeof(struct hf_net), ifname, hf_if_setup);
> +	if (!netdev) {
> +		printk(KERN_ERR "hf_init_netdev: "
> +				"alloc_netdev for hfi%d:hf%d fail\n", ai, idx);
> +		return (struct hf_net *) -ENODEV;

Use ERR_PTR() instead of writing this sort of cast yourself.

[...]
> +static int __init hf_init_module(void)
> +{
> +	u32		idx, ai;
> +	struct hf_net	*net;
> +
> +	memset(&hf_ginfo, 0, sizeof(struct hf_global_info));
> +
> +	for (idx = 0; idx < MAX_HF_PER_HFI; idx++) {
> +		for (ai = 0; ai < MAX_HFIS; ai++) {
> +			net = hf_init_netdev(idx, ai);
> +			if (IS_ERR(net)) {
> +				printk(KERN_ERR "hf_init_module: hf_init_netdev"
> +						" for idx %d ai %d failed rc"
> +						" 0x%016llx\n",
> +						idx, ai, (u64)(PTR_ERR(net)));

Whyever are you formatting the error like this?  Use %ld and remove the
(u64) cast.


> +
> +				goto err_out;
> +			}
> +
> +			hf_ginfo.net[idx][ai] = net;
> +		}
> +	}
> +
> +	register_inetaddr_notifier(&hf_inet_notifier);
> +
> +	printk(KERN_INFO "hf module loaded\n");
> +	return 0;
> +
> +err_out:
> +	for (idx = 0; idx < MAX_HF_PER_HFI; idx++) {
> +		for (ai = 0; ai < MAX_HFIS; ai++) {
> +			net = hf_ginfo.net[idx][ai];
> +			if (net != NULL) {
> +				hf_del_netdev(net);
> +				hf_ginfo.net[idx][ai] = NULL;
> +			}
> +		}
> +	}
> +
> +	return -EINVAL;

Use the error code you were given:

	return PTR_ERR(net);

> +}
> +
> +static void __exit hf_cleanup_module(void)
> +{
> +	u32		idx, ai;
> +	struct hf_net	*net;
> +
> +	unregister_inetaddr_notifier(&hf_inet_notifier);
> +	for (idx = 0; idx < MAX_HF_PER_HFI; idx++) {
> +		for (ai = 0; ai < MAX_HFIS; ai++) {
> +			net = hf_ginfo.net[idx][ai];
> +			if (net != NULL) {
> +				hf_del_netdev(net);
> +				hf_ginfo.net[idx][ai] = NULL;
> +			}
> +		}
> +	}
> +
> +	return;

Redundant statement is redundant.

> +}
[...]
> --- /dev/null
> +++ b/include/linux/hfi/hf_if.h
[...]
> +struct hfi_ip_extended_hdr {            /* 16B */
> +	u32		immediate_len:7;/* In bytes */
> +	u32		num_desc:3;     /* number of descriptors */
> +					/* Logical Port ID: */
> +	u32		lpid_valid:1;   /* set by sending HFI */
> +	u32		lpid:4;         /* set by sending HFI */
> +	/* Ethernet Service Header is 113 bits, which is 14 bytes + 1 bit */
> +	u32		ethernet_svc_hdr_hi:1;    /* Not used by HFI */
> +	char            ethernet_svc_hdr[12];     /* Not used by HFI */
> +	__sum16         bcast_csum;
> +} __packed;

It looks like you're relying on gcc to treat a set of bitfields with
type u32 and only 16 bits assigned as having a size of 2 in a packed
structure.  This might be true now, but I wouldn't want to rely on that
being true for later versions.  Why not define the set of bitfields with
type u16?

Also the above appears to assume big-endian byte and bit order.

[...]
> +#define HF_ALEN				6
> +struct hf_hwhdr {
> +	u8				h_dest[HF_ALEN];
> +	u8				h_source[HF_ALEN];
> +	__be16				h_proto;
> +};
> +
> +#define HF_HLEN				sizeof(struct hf_hwhdr)
[...]

This looks familiar!  Maybe you should just use the existing struct
ethhdr?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 00/27] HFI minimal device driver/network driver
From: Ben Hutchings @ 2011-03-02 22:42 UTC (permalink / raw)
  To: dykmanj; +Cc: netdev
In-Reply-To: <1299100795-9028-1-git-send-email-dykmanj@linux.vnet.ibm.com>

On Wed, 2011-03-02 at 16:19 -0500, dykmanj@linux.vnet.ibm.com wrote:
[...]
> Patches are against net-2.6 (Is that the correct tree for us to use?).

net-next-2.6 would be the correct tree at the moment.  net-2.6 is
currently destined for 2.6.38 and it is too late to add a driver to
that.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 02/27] HFI: Add HFI adapter control structure
From: Ben Hutchings @ 2011-03-02 22:44 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dykmanj, netdev, Piyush Chaudhary, Fu-Chung Chang,
	William S. Cadden, Wen C. Chen, Scot Sakolish, Jian Xiao,
	Carol L. Soto, Sarah J. Sheppard
In-Reply-To: <20110302142126.193a8817@nehalam>

On Wed, 2011-03-02 at 14:21 -0800, Stephen Hemminger wrote:
> On Wed,  2 Mar 2011 16:09:48 -0500
> dykmanj@linux.vnet.ibm.com wrote:
> 
> > diff --git a/drivers/net/hfi/core/Makefile b/drivers/net/hfi/core/Makefile
> > index 80790c6..6fe4e60 100644
> > --- a/drivers/net/hfi/core/Makefile
> > +++ b/drivers/net/hfi/core/Makefile
> > @@ -1,5 +1,6 @@
> >  #
> >  # Makefile for the HFI device driver for IBM eServer System p
> >  #
> > -hfi_core-objs:=	hfidd_init.o
> > +hfi_core-objs:=	hfidd_adpt.o \
> > +		hfidd_init.o
> >  obj-$(CONFIG_HFI) += hfi_core.o
> > diff --git a/drivers/net/hfi/core/hfidd_adpt.c b/drivers/net/hfi/core/hfidd_adpt.c
> > new file mode 100644
> > index 0000000..d64fa38
> > --- /dev/null
> > +++ b/drivers/net/hfi/core/hfidd_adpt.c
[...]
> > +void hfidd_free_adapter(struct hfidd_acs *p_acs)
> > +{
> > +	kfree(p_acs);
> > +	p_acs = NULL;
> > +	return;
> > +}
> 
> If these were not in a separate file the could be marked as static.

I assume they're intending to add some more interesting code here in the
next installment.

> Doing a return; on last line of a void function is considered poor
> style since it is unnecessary.

Assigning to a local variable just before returning is also silly.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* [PATCH] ipv4: Make output route lookup return rtable directly.
From: David Miller @ 2011-03-02 22:47 UTC (permalink / raw)
  To: netdev


Instead of on the stack.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/infiniband/core/addr.c        |    7 +-
 drivers/infiniband/hw/cxgb3/iwch_cm.c |    3 +-
 drivers/infiniband/hw/cxgb4/cm.c      |    3 +-
 drivers/infiniband/hw/nes/nes_cm.c    |    3 +-
 drivers/net/bonding/bond_main.c       |    6 +-
 drivers/net/cnic.c                    |    7 ++-
 drivers/net/pptp.c                    |    8 +-
 drivers/scsi/cxgbi/libcxgbi.c         |    3 +-
 include/net/route.h                   |   58 ++++++++++----------
 net/atm/clip.c                        |    6 +-
 net/bridge/br_netfilter.c             |    9 ++-
 net/dccp/ipv4.c                       |   27 +++++----
 net/ipv4/af_inet.c                    |   30 +++++-----
 net/ipv4/arp.c                        |   19 +++---
 net/ipv4/datagram.c                   |   11 ++--
 net/ipv4/icmp.c                       |   19 ++++--
 net/ipv4/igmp.c                       |   16 +++--
 net/ipv4/inet_connection_sock.c       |    3 +-
 net/ipv4/ip_gre.c                     |   11 ++--
 net/ipv4/ip_output.c                  |    6 +-
 net/ipv4/ipip.c                       |    7 +-
 net/ipv4/ipmr.c                       |    8 +-
 net/ipv4/netfilter.c                  |   12 +++-
 net/ipv4/raw.c                        |    8 ++-
 net/ipv4/route.c                      |  100 ++++++++++++++++-----------------
 net/ipv4/syncookies.c                 |    3 +-
 net/ipv4/tcp_ipv4.c                   |   28 +++++----
 net/ipv4/udp.c                        |    5 +-
 net/ipv4/xfrm4_policy.c               |   12 ++--
 net/ipv6/ip6_tunnel.c                 |   11 ++-
 net/ipv6/sit.c                        |    8 ++-
 net/l2tp/l2tp_ip.c                    |    8 ++-
 net/netfilter/ipvs/ip_vs_xmit.c       |    9 ++-
 net/netfilter/xt_TEE.c                |    3 +-
 net/rxrpc/ar-peer.c                   |    7 +-
 net/sctp/protocol.c                   |    7 +-
 36 files changed, 267 insertions(+), 224 deletions(-)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 8aba0ba..2d74993 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -193,10 +193,11 @@ static int addr4_resolve(struct sockaddr_in *src_in,
 	fl.nl_u.ip4_u.saddr = src_ip;
 	fl.oif = addr->bound_dev_if;
 
-	ret = ip_route_output_key(&init_net, &rt, &fl);
-	if (ret)
+	rt = ip_route_output_key(&init_net, &fl);
+	if (IS_ERR(rt)) {
+		ret = PTR_ERR(rt);
 		goto out;
-
+	}
 	src_in->sin_family = AF_INET;
 	src_in->sin_addr.s_addr = rt->rt_src;
 
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index e654285..e0ccbc5 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -354,7 +354,8 @@ static struct rtable *find_route(struct t3cdev *dev, __be32 local_ip,
 			  }
 	};
 
-	if (ip_route_output_flow(&init_net, &rt, &fl, NULL))
+	rt = ip_route_output_flow(&init_net, &fl, NULL);
+	if (IS_ERR(rt))
 		return NULL;
 	return rt;
 }
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 7e0484f..77b0eef 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -331,7 +331,8 @@ static struct rtable *find_route(struct c4iw_dev *dev, __be32 local_ip,
 			  }
 	};
 
-	if (ip_route_output_flow(&init_net, &rt, &fl, NULL))
+	rt = ip_route_output_flow(&init_net, &fl, NULL);
+	if (IS_ERR(rt))
 		return NULL;
 	return rt;
 }
diff --git a/drivers/infiniband/hw/nes/nes_cm.c b/drivers/infiniband/hw/nes/nes_cm.c
index ec3aa11..e81599c 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -1112,7 +1112,8 @@ static int nes_addr_resolve_neigh(struct nes_vnic *nesvnic, u32 dst_ip, int arpi
 
 	memset(&fl, 0, sizeof fl);
 	fl.nl_u.ip4_u.daddr = htonl(dst_ip);
-	if (ip_route_output_key(&init_net, &rt, &fl)) {
+	rt = ip_route_output_key(&init_net, &fl);
+	if (IS_ERR(rt)) {
 		printk(KERN_ERR "%s: ip_route_output_key failed for 0x%08X\n",
 				__func__, dst_ip);
 		return rc;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 584f97b..0592e6d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2681,7 +2681,7 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_
 
 static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 {
-	int i, vlan_id, rv;
+	int i, vlan_id;
 	__be32 *targets = bond->params.arp_targets;
 	struct vlan_entry *vlan;
 	struct net_device *vlan_dev;
@@ -2708,8 +2708,8 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 		fl.fl4_dst = targets[i];
 		fl.fl4_tos = RTO_ONLINK;
 
-		rv = ip_route_output_key(dev_net(bond->dev), &rt, &fl);
-		if (rv) {
+		rt = ip_route_output_key(dev_net(bond->dev), &fl);
+		if (IS_ERR(rt)) {
 			if (net_ratelimit()) {
 				pr_warning("%s: no route to arp_ip_target %pI4\n",
 					   bond->dev->name, &fl.fl4_dst);
diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 5274de3..25f0888 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -3397,9 +3397,12 @@ static int cnic_get_v4_route(struct sockaddr_in *dst_addr,
 	memset(&fl, 0, sizeof(fl));
 	fl.nl_u.ip4_u.daddr = dst_addr->sin_addr.s_addr;
 
-	err = ip_route_output_key(&init_net, &rt, &fl);
-	if (!err)
+	rt = ip_route_output_key(&init_net, &fl);
+	err = 0;
+	if (!IS_ERR(rt))
 		*dst = &rt->dst;
+	else
+		err = PTR_ERR(rt);
 	return err;
 #else
 	return -ENETUNREACH;
diff --git a/drivers/net/pptp.c b/drivers/net/pptp.c
index 164cfad..1af549c 100644
--- a/drivers/net/pptp.c
+++ b/drivers/net/pptp.c
@@ -175,7 +175,6 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	struct pptp_opt *opt = &po->proto.pptp;
 	struct pptp_gre_header *hdr;
 	unsigned int header_len = sizeof(*hdr);
-	int err = 0;
 	int islcp;
 	int len;
 	unsigned char *data;
@@ -198,8 +197,8 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 					.saddr = opt->src_addr.sin_addr.s_addr,
 					.tos = RT_TOS(0) } },
 			.proto = IPPROTO_GRE };
-		err = ip_route_output_key(&init_net, &rt, &fl);
-		if (err)
+		rt = ip_route_output_key(&init_net, &fl);
+		if (IS_ERR(rt))
 			goto tx_error;
 	}
 	tdev = rt->dst.dev;
@@ -477,7 +476,8 @@ static int pptp_connect(struct socket *sock, struct sockaddr *uservaddr,
 					.tos = RT_CONN_FLAGS(sk) } },
 			.proto = IPPROTO_GRE };
 		security_sk_classify_flow(sk, &fl);
-		if (ip_route_output_key(&init_net, &rt, &fl)) {
+		rt = ip_route_output_key(&init_net, &fl);
+		if (IS_ERR(rt)) {
 			error = -EHOSTUNREACH;
 			goto end;
 		}
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 261aa81..889199a 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -470,7 +470,8 @@ static struct rtable *find_route_ipv4(__be32 saddr, __be32 daddr,
 			}
 	};
 
-	if (ip_route_output_flow(&init_net, &rt, &fl, NULL))
+	rt = ip_route_output_flow(&init_net, &fl, NULL);
+	if (IS_ERR(rt))
 		return NULL;
 
 	return rt;
diff --git a/include/net/route.h b/include/net/route.h
index 707cfc8..088a186 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -118,9 +118,10 @@ extern void		ip_rt_redirect(__be32 old_gw, __be32 dst, __be32 new_gw,
 				       __be32 src, struct net_device *dev);
 extern void		rt_cache_flush(struct net *net, int how);
 extern void		rt_cache_flush_batch(struct net *net);
-extern int		__ip_route_output_key(struct net *, struct rtable **, const struct flowi *flp);
-extern int		ip_route_output_key(struct net *, struct rtable **, struct flowi *flp);
-extern int		ip_route_output_flow(struct net *, struct rtable **rp, struct flowi *flp, struct sock *sk);
+extern struct rtable *__ip_route_output_key(struct net *, const struct flowi *flp);
+extern struct rtable *ip_route_output_key(struct net *, struct flowi *flp);
+extern struct rtable *ip_route_output_flow(struct net *, struct flowi *flp,
+					   struct sock *sk);
 extern struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_orig);
 
 extern int ip_route_input_common(struct sk_buff *skb, __be32 dst, __be32 src,
@@ -166,10 +167,10 @@ static inline char rt_tos2priority(u8 tos)
 	return ip_tos2prio[IPTOS_TOS(tos)>>1];
 }
 
-static inline int ip_route_connect(struct rtable **rp, __be32 dst,
-				   __be32 src, u32 tos, int oif, u8 protocol,
-				   __be16 sport, __be16 dport, struct sock *sk,
-				   bool can_sleep)
+static inline struct rtable *ip_route_connect(__be32 dst, __be32 src, u32 tos,
+					      int oif, u8 protocol,
+					      __be16 sport, __be16 dport,
+					      struct sock *sk, bool can_sleep)
 {
 	struct flowi fl = { .oif = oif,
 			    .mark = sk->sk_mark,
@@ -179,8 +180,8 @@ static inline int ip_route_connect(struct rtable **rp, __be32 dst,
 			    .proto = protocol,
 			    .fl_ip_sport = sport,
 			    .fl_ip_dport = dport };
-	int err;
 	struct net *net = sock_net(sk);
+	struct rtable *rt;
 
 	if (inet_sk(sk)->transparent)
 		fl.flags |= FLOWI_FLAG_ANYSRC;
@@ -190,29 +191,29 @@ static inline int ip_route_connect(struct rtable **rp, __be32 dst,
 		fl.flags |= FLOWI_FLAG_CAN_SLEEP;
 
 	if (!dst || !src) {
-		err = __ip_route_output_key(net, rp, &fl);
-		if (err)
-			return err;
-		fl.fl4_dst = (*rp)->rt_dst;
-		fl.fl4_src = (*rp)->rt_src;
-		ip_rt_put(*rp);
-		*rp = NULL;
+		rt = __ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
+			return rt;
+		fl.fl4_dst = rt->rt_dst;
+		fl.fl4_src = rt->rt_src;
+		ip_rt_put(rt);
 	}
 	security_sk_classify_flow(sk, &fl);
-	return ip_route_output_flow(net, rp, &fl, sk);
+	return ip_route_output_flow(net, &fl, sk);
 }
 
-static inline int ip_route_newports(struct rtable **rp, u8 protocol,
-				    __be16 orig_sport, __be16 orig_dport,
-				    __be16 sport, __be16 dport, struct sock *sk)
+static inline struct rtable *ip_route_newports(struct rtable *rt,
+					       u8 protocol, __be16 orig_sport,
+					       __be16 orig_dport, __be16 sport,
+					       __be16 dport, struct sock *sk)
 {
 	if (sport != orig_sport || dport != orig_dport) {
-		struct flowi fl = { .oif = (*rp)->fl.oif,
-				    .mark = (*rp)->fl.mark,
-				    .fl4_dst = (*rp)->fl.fl4_dst,
-				    .fl4_src = (*rp)->fl.fl4_src,
-				    .fl4_tos = (*rp)->fl.fl4_tos,
-				    .proto = (*rp)->fl.proto,
+		struct flowi fl = { .oif = rt->fl.oif,
+				    .mark = rt->fl.mark,
+				    .fl4_dst = rt->fl.fl4_dst,
+				    .fl4_src = rt->fl.fl4_src,
+				    .fl4_tos = rt->fl.fl4_tos,
+				    .proto = rt->fl.proto,
 				    .fl_ip_sport = sport,
 				    .fl_ip_dport = dport };
 
@@ -220,12 +221,11 @@ static inline int ip_route_newports(struct rtable **rp, u8 protocol,
 			fl.flags |= FLOWI_FLAG_ANYSRC;
 		if (protocol == IPPROTO_TCP)
 			fl.flags |= FLOWI_FLAG_PRECOW_METRICS;
-		ip_rt_put(*rp);
-		*rp = NULL;
+		ip_rt_put(rt);
 		security_sk_classify_flow(sk, &fl);
-		return ip_route_output_flow(sock_net(sk), rp, &fl, sk);
+		return ip_route_output_flow(sock_net(sk), &fl, sk);
 	}
-	return 0;
+	return rt;
 }
 
 extern void rt_bind_peer(struct rtable *rt, int create);
diff --git a/net/atm/clip.c b/net/atm/clip.c
index d257da5..810a129 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -520,9 +520,9 @@ static int clip_setentry(struct atm_vcc *vcc, __be32 ip)
 		unlink_clip_vcc(clip_vcc);
 		return 0;
 	}
-	error = ip_route_output_key(&init_net, &rt, &fl);
-	if (error)
-		return error;
+	rt = ip_route_output_key(&init_net, &fl);
+	if (IS_ERR(rt))
+		return PTR_ERR(rt);
 	neigh = __neigh_lookup(&clip_tbl, &ip, rt->dst.dev, 1);
 	ip_rt_put(rt);
 	if (!neigh)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 4b5b66d..45b57b1 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -428,14 +428,15 @@ static int br_nf_pre_routing_finish(struct sk_buff *skb)
 			if (err != -EHOSTUNREACH || !in_dev || IN_DEV_FORWARD(in_dev))
 				goto free_skb;
 
-			if (!ip_route_output_key(dev_net(dev), &rt, &fl)) {
+			rt = ip_route_output_key(dev_net(dev), &fl);
+			if (!IS_ERR(rt)) {
 				/* - Bridged-and-DNAT'ed traffic doesn't
 				 *   require ip_forwarding. */
-				if (((struct dst_entry *)rt)->dev == dev) {
-					skb_dst_set(skb, (struct dst_entry *)rt);
+				if (rt->dst.dev == dev) {
+					skb_dst_set(skb, &rt->dst);
 					goto bridged_dnat;
 				}
-				dst_release((struct dst_entry *)rt);
+				ip_rt_put(rt);
 			}
 free_skb:
 			kfree_skb(skb);
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index a8ff955..7882377 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -46,7 +46,6 @@ int dccp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	__be16 orig_sport, orig_dport;
 	struct rtable *rt;
 	__be32 daddr, nexthop;
-	int tmp;
 	int err;
 
 	dp->dccps_role = DCCP_ROLE_CLIENT;
@@ -66,12 +65,12 @@ int dccp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 
 	orig_sport = inet->inet_sport;
 	orig_dport = usin->sin_port;
-	tmp = ip_route_connect(&rt, nexthop, inet->inet_saddr,
-			       RT_CONN_FLAGS(sk), sk->sk_bound_dev_if,
-			       IPPROTO_DCCP,
-			       orig_sport, orig_dport, sk, true);
-	if (tmp < 0)
-		return tmp;
+	rt = ip_route_connect(nexthop, inet->inet_saddr,
+			      RT_CONN_FLAGS(sk), sk->sk_bound_dev_if,
+			      IPPROTO_DCCP,
+			      orig_sport, orig_dport, sk, true);
+	if (IS_ERR(rt))
+		return PTR_ERR(rt);
 
 	if (rt->rt_flags & (RTCF_MULTICAST | RTCF_BROADCAST)) {
 		ip_rt_put(rt);
@@ -102,12 +101,13 @@ int dccp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	if (err != 0)
 		goto failure;
 
-	err = ip_route_newports(&rt, IPPROTO_DCCP,
-				orig_sport, orig_dport,
-				inet->inet_sport, inet->inet_dport, sk);
-	if (err != 0)
+	rt = ip_route_newports(rt, IPPROTO_DCCP,
+			       orig_sport, orig_dport,
+			       inet->inet_sport, inet->inet_dport, sk);
+	if (IS_ERR(rt)) {
+		rt = NULL;
 		goto failure;
-
+	}
 	/* OK, now commit destination to socket.  */
 	sk_setup_caps(sk, &rt->dst);
 
@@ -475,7 +475,8 @@ static struct dst_entry* dccp_v4_route_skb(struct net *net, struct sock *sk,
 			  };
 
 	security_skb_classify_flow(skb, &fl);
-	if (ip_route_output_flow(net, &rt, &fl, sk)) {
+	rt = ip_route_output_flow(net, &fl, sk);
+	if (IS_ERR(rt)) {
 		IP_INC_STATS_BH(net, IPSTATS_MIB_OUTNOROUTES);
 		return NULL;
 	}
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 44513bb..35a5020 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1101,23 +1101,20 @@ int sysctl_ip_dynaddr __read_mostly;
 static int inet_sk_reselect_saddr(struct sock *sk)
 {
 	struct inet_sock *inet = inet_sk(sk);
-	int err;
-	struct rtable *rt;
 	__be32 old_saddr = inet->inet_saddr;
-	__be32 new_saddr;
 	__be32 daddr = inet->inet_daddr;
+	struct rtable *rt;
+	__be32 new_saddr;
 
 	if (inet->opt && inet->opt->srr)
 		daddr = inet->opt->faddr;
 
 	/* Query new route. */
-	err = ip_route_connect(&rt, daddr, 0,
-			       RT_CONN_FLAGS(sk),
-			       sk->sk_bound_dev_if,
-			       sk->sk_protocol,
-			       inet->inet_sport, inet->inet_dport, sk, false);
-	if (err)
-		return err;
+	rt = ip_route_connect(daddr, 0, RT_CONN_FLAGS(sk),
+			      sk->sk_bound_dev_if, sk->sk_protocol,
+			      inet->inet_sport, inet->inet_dport, sk, false);
+	if (IS_ERR(rt))
+		return PTR_ERR(rt);
 
 	sk_setup_caps(sk, &rt->dst);
 
@@ -1160,7 +1157,7 @@ int inet_sk_rebuild_header(struct sock *sk)
 	daddr = inet->inet_daddr;
 	if (inet->opt && inet->opt->srr)
 		daddr = inet->opt->faddr;
-{
+	{
 	struct flowi fl = {
 		.oif = sk->sk_bound_dev_if,
 		.mark = sk->sk_mark,
@@ -1174,11 +1171,14 @@ int inet_sk_rebuild_header(struct sock *sk)
 	};
 
 	security_sk_classify_flow(sk, &fl);
-	err = ip_route_output_flow(sock_net(sk), &rt, &fl, sk);
-}
-	if (!err)
+	rt = ip_route_output_flow(sock_net(sk), &fl, sk);
+	}
+	if (!IS_ERR(rt)) {
+		err = 0;
 		sk_setup_caps(sk, &rt->dst);
-	else {
+	} else {
+		err = PTR_ERR(rt);
+
 		/* Routing failed... */
 		sk->sk_route_caps = 0;
 		/*
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7927589..fa9988d 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -440,7 +440,8 @@ static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
 	/*unsigned long now; */
 	struct net *net = dev_net(dev);
 
-	if (ip_route_output_key(net, &rt, &fl) < 0)
+	rt = ip_route_output_key(net, &fl);
+	if (IS_ERR(rt))
 		return 1;
 	if (rt->dst.dev != dev) {
 		NET_INC_STATS_BH(net, LINUX_MIB_ARPFILTER);
@@ -1063,10 +1064,10 @@ static int arp_req_set(struct net *net, struct arpreq *r,
 	if (dev == NULL) {
 		struct flowi fl = { .fl4_dst = ip,
 				    .fl4_tos = RTO_ONLINK };
-		struct rtable *rt;
-		err = ip_route_output_key(net, &rt, &fl);
-		if (err != 0)
-			return err;
+		struct rtable *rt = ip_route_output_key(net, &fl);
+
+		if (IS_ERR(rt))
+			return PTR_ERR(rt);
 		dev = rt->dst.dev;
 		ip_rt_put(rt);
 		if (!dev)
@@ -1177,7 +1178,6 @@ static int arp_req_delete_public(struct net *net, struct arpreq *r,
 static int arp_req_delete(struct net *net, struct arpreq *r,
 			  struct net_device *dev)
 {
-	int err;
 	__be32 ip;
 
 	if (r->arp_flags & ATF_PUBL)
@@ -1187,10 +1187,9 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
 	if (dev == NULL) {
 		struct flowi fl = { .fl4_dst = ip,
 				    .fl4_tos = RTO_ONLINK };
-		struct rtable *rt;
-		err = ip_route_output_key(net, &rt, &fl);
-		if (err != 0)
-			return err;
+		struct rtable *rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
+			return PTR_ERR(rt);
 		dev = rt->dst.dev;
 		ip_rt_put(rt);
 		if (!dev)
diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index eaee1ed..85bd24c 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -46,11 +46,12 @@ int ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 		if (!saddr)
 			saddr = inet->mc_addr;
 	}
-	err = ip_route_connect(&rt, usin->sin_addr.s_addr, saddr,
-			       RT_CONN_FLAGS(sk), oif,
-			       sk->sk_protocol,
-			       inet->inet_sport, usin->sin_port, sk, true);
-	if (err) {
+	rt = ip_route_connect(usin->sin_addr.s_addr, saddr,
+			      RT_CONN_FLAGS(sk), oif,
+			      sk->sk_protocol,
+			      inet->inet_sport, usin->sin_port, sk, true);
+	if (IS_ERR(rt)) {
+		err = PTR_ERR(rt);
 		if (err == -ENETUNREACH)
 			IP_INC_STATS_BH(sock_net(sk), IPSTATS_MIB_OUTNOROUTES);
 		return err;
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index c23bd8c..994a785 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -358,7 +358,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
 				    .fl4_tos = RT_TOS(ip_hdr(skb)->tos),
 				    .proto = IPPROTO_ICMP };
 		security_skb_classify_flow(skb, &fl);
-		if (ip_route_output_key(net, &rt, &fl))
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			goto out_unlock;
 	}
 	if (icmpv4_xrlim_allow(net, rt, icmp_param->data.icmph.type,
@@ -388,9 +389,9 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
 	int err;
 
 	security_skb_classify_flow(skb_in, &fl);
-	err = __ip_route_output_key(net, &rt, &fl);
-	if (err)
-		return ERR_PTR(err);
+	rt = __ip_route_output_key(net, &fl);
+	if (IS_ERR(rt))
+		return rt;
 
 	/* No need to clone since we're just using its address. */
 	rt2 = rt;
@@ -412,15 +413,19 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
 		goto relookup_failed;
 
 	if (inet_addr_type(net, fl.fl4_src) == RTN_LOCAL) {
-		err = __ip_route_output_key(net, &rt2, &fl);
+		rt2 = __ip_route_output_key(net, &fl);
+		if (IS_ERR(rt2))
+			err = PTR_ERR(rt2);
 	} else {
 		struct flowi fl2 = {};
 		unsigned long orefdst;
 
 		fl2.fl4_dst = fl.fl4_src;
-		err = ip_route_output_key(net, &rt2, &fl2);
-		if (err)
+		rt2 = ip_route_output_key(net, &fl2);
+		if (IS_ERR(rt2)) {
+			err = PTR_ERR(rt2);
 			goto relookup_failed;
+		}
 		/* Ugh! */
 		orefdst = skb_in->_skb_refdst; /* save old refdst */
 		err = ip_route_input(skb_in, fl.fl4_dst, fl.fl4_src,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index e0e77e2..44ba906 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -325,7 +325,8 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
 		struct flowi fl = { .oif = dev->ifindex,
 				    .fl4_dst = IGMPV3_ALL_MCR,
 				    .proto = IPPROTO_IGMP };
-		if (ip_route_output_key(net, &rt, &fl)) {
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt)) {
 			kfree_skb(skb);
 			return NULL;
 		}
@@ -670,7 +671,8 @@ static int igmp_send_report(struct in_device *in_dev, struct ip_mc_list *pmc,
 		struct flowi fl = { .oif = dev->ifindex,
 				    .fl4_dst = dst,
 				    .proto = IPPROTO_IGMP };
-		if (ip_route_output_key(net, &rt, &fl))
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			return -1;
 	}
 	if (rt->rt_src == 0) {
@@ -1440,7 +1442,6 @@ void ip_mc_destroy_dev(struct in_device *in_dev)
 static struct in_device *ip_mc_find_dev(struct net *net, struct ip_mreqn *imr)
 {
 	struct flowi fl = { .fl4_dst = imr->imr_multiaddr.s_addr };
-	struct rtable *rt;
 	struct net_device *dev = NULL;
 	struct in_device *idev = NULL;
 
@@ -1454,9 +1455,12 @@ static struct in_device *ip_mc_find_dev(struct net *net, struct ip_mreqn *imr)
 			return NULL;
 	}
 
-	if (!dev && !ip_route_output_key(net, &rt, &fl)) {
-		dev = rt->dst.dev;
-		ip_rt_put(rt);
+	if (!dev) {
+		struct rtable *rt = ip_route_output_key(net, &fl);
+		if (!IS_ERR(rt)) {
+			dev = rt->dst.dev;
+			ip_rt_put(rt);
+		}
 	}
 	if (dev) {
 		imr->imr_ifindex = dev->ifindex;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 7f85d4a..e4e301a 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -369,7 +369,8 @@ struct dst_entry *inet_csk_route_req(struct sock *sk,
 	struct net *net = sock_net(sk);
 
 	security_req_classify_flow(req, &fl);
-	if (ip_route_output_flow(net, &rt, &fl, sk))
+	rt = ip_route_output_flow(net, &fl, sk);
+	if (IS_ERR(rt))
 		goto no_route;
 	if (opt && opt->is_strictroute && rt->rt_dst != rt->rt_gateway)
 		goto route_err;
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 6613edf..f9af98d 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -778,7 +778,8 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 			.proto = IPPROTO_GRE,
 			.fl_gre_key = tunnel->parms.o_key
 		};
-		if (ip_route_output_key(dev_net(dev), &rt, &fl)) {
+		rt = ip_route_output_key(dev_net(dev), &fl);
+		if (IS_ERR(rt)) {
 			dev->stats.tx_carrier_errors++;
 			goto tx_error;
 		}
@@ -953,9 +954,9 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
 			.proto = IPPROTO_GRE,
 			.fl_gre_key = tunnel->parms.o_key
 		};
-		struct rtable *rt;
+		struct rtable *rt = ip_route_output_key(dev_net(dev), &fl);
 
-		if (!ip_route_output_key(dev_net(dev), &rt, &fl)) {
+		if (!IS_ERR(rt)) {
 			tdev = rt->dst.dev;
 			ip_rt_put(rt);
 		}
@@ -1215,9 +1216,9 @@ static int ipgre_open(struct net_device *dev)
 			.proto = IPPROTO_GRE,
 			.fl_gre_key = t->parms.o_key
 		};
-		struct rtable *rt;
+		struct rtable *rt = ip_route_output_key(dev_net(dev), &fl);
 
-		if (ip_route_output_key(dev_net(dev), &rt, &fl))
+		if (IS_ERR(rt))
 			return -EADDRNOTAVAIL;
 		dev = rt->dst.dev;
 		ip_rt_put(rt);
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 33316b3..171f483 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -355,7 +355,8 @@ int ip_queue_xmit(struct sk_buff *skb)
 			 * itself out.
 			 */
 			security_sk_classify_flow(sk, &fl);
-			if (ip_route_output_flow(sock_net(sk), &rt, &fl, sk))
+			rt = ip_route_output_flow(sock_net(sk), &fl, sk);
+			if (IS_ERR(rt))
 				goto no_route;
 		}
 		sk_setup_caps(sk, &rt->dst);
@@ -1489,7 +1490,8 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
 				    .proto = sk->sk_protocol,
 				    .flags = ip_reply_arg_flowi_flags(arg) };
 		security_skb_classify_flow(skb, &fl);
-		if (ip_route_output_key(sock_net(sk), &rt, &fl))
+		rt = ip_route_output_key(sock_net(sk), &fl);
+		if (IS_ERR(rt))
 			return;
 	}
 
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 988f52f..e1e1757 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -469,7 +469,8 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 			.proto = IPPROTO_IPIP
 		};
 
-		if (ip_route_output_key(dev_net(dev), &rt, &fl)) {
+		rt = ip_route_output_key(dev_net(dev), &fl);
+		if (IS_ERR(rt)) {
 			dev->stats.tx_carrier_errors++;
 			goto tx_error_icmp;
 		}
@@ -590,9 +591,9 @@ static void ipip_tunnel_bind_dev(struct net_device *dev)
 			.fl4_tos = RT_TOS(iph->tos),
 			.proto = IPPROTO_IPIP
 		};
-		struct rtable *rt;
+		struct rtable *rt = ip_route_output_key(dev_net(dev), &fl);
 
-		if (!ip_route_output_key(dev_net(dev), &rt, &fl)) {
+		if (!IS_ERR(rt)) {
 			tdev = rt->dst.dev;
 			ip_rt_put(rt);
 		}
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 8b65a12..26ca2f2 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1618,8 +1618,8 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
 			.fl4_tos = RT_TOS(iph->tos),
 			.proto = IPPROTO_IPIP
 		};
-
-		if (ip_route_output_key(net, &rt, &fl))
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			goto out_free;
 		encap = sizeof(struct iphdr);
 	} else {
@@ -1629,8 +1629,8 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,
 			.fl4_tos = RT_TOS(iph->tos),
 			.proto = IPPROTO_IPIP
 		};
-
-		if (ip_route_output_key(net, &rt, &fl))
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			goto out_free;
 	}
 
diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index 9770bb4..67bf709 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -38,7 +38,8 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned addr_type)
 		fl.oif = skb->sk ? skb->sk->sk_bound_dev_if : 0;
 		fl.mark = skb->mark;
 		fl.flags = skb->sk ? inet_sk_flowi_flags(skb->sk) : 0;
-		if (ip_route_output_key(net, &rt, &fl) != 0)
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			return -1;
 
 		/* Drop old route. */
@@ -48,7 +49,8 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned addr_type)
 		/* non-local src, find valid iif to satisfy
 		 * rp-filter when calling ip_route_input. */
 		fl.fl4_dst = iph->saddr;
-		if (ip_route_output_key(net, &rt, &fl) != 0)
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			return -1;
 
 		orefdst = skb->_skb_refdst;
@@ -221,7 +223,11 @@ static __sum16 nf_ip_checksum_partial(struct sk_buff *skb, unsigned int hook,
 
 static int nf_ip_route(struct dst_entry **dst, struct flowi *fl)
 {
-	return ip_route_output_key(&init_net, (struct rtable **)dst, fl);
+	struct rtable *rt = ip_route_output_key(&init_net, fl);
+	if (IS_ERR(rt))
+		return PTR_ERR(rt);
+	*dst = &rt->dst;
+	return 0;
 }
 
 static const struct nf_afinfo nf_ip_afinfo = {
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index d7a2d1e..467d570 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -564,10 +564,12 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		}
 
 		security_sk_classify_flow(sk, &fl);
-		err = ip_route_output_flow(sock_net(sk), &rt, &fl, sk);
+		rt = ip_route_output_flow(sock_net(sk), &fl, sk);
+		if (IS_ERR(rt)) {
+			err = PTR_ERR(rt);
+			goto done;
+		}
 	}
-	if (err)
-		goto done;
 
 	err = -EACCES;
 	if (rt->rt_flags & RTCF_BROADCAST && !sock_flag(sk, SOCK_BROADCAST))
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 63d3700..5090e95 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1014,8 +1014,8 @@ static int slow_chain_length(const struct rtable *head)
 	return length >> FRACT_BITS;
 }
 
-static int rt_intern_hash(unsigned hash, struct rtable *rt,
-			  struct rtable **rp, struct sk_buff *skb, int ifindex)
+static struct rtable *rt_intern_hash(unsigned hash, struct rtable *rt,
+				     struct sk_buff *skb, int ifindex)
 {
 	struct rtable	*rth, *cand;
 	struct rtable __rcu **rthp, **candp;
@@ -1056,7 +1056,7 @@ restart:
 					printk(KERN_WARNING
 					    "Neighbour table failure & not caching routes.\n");
 				ip_rt_put(rt);
-				return err;
+				return ERR_PTR(err);
 			}
 		}
 
@@ -1093,11 +1093,9 @@ restart:
 			spin_unlock_bh(rt_hash_lock_addr(hash));
 
 			rt_drop(rt);
-			if (rp)
-				*rp = rth;
-			else
+			if (skb)
 				skb_dst_set(skb, &rth->dst);
-			return 0;
+			return rth;
 		}
 
 		if (!atomic_read(&rth->dst.__refcnt)) {
@@ -1154,7 +1152,7 @@ restart:
 
 			if (err != -ENOBUFS) {
 				rt_drop(rt);
-				return err;
+				return ERR_PTR(err);
 			}
 
 			/* Neighbour tables are full and nothing
@@ -1175,7 +1173,7 @@ restart:
 			if (net_ratelimit())
 				printk(KERN_WARNING "ipv4: Neighbour table overflow.\n");
 			rt_drop(rt);
-			return -ENOBUFS;
+			return ERR_PTR(-ENOBUFS);
 		}
 	}
 
@@ -1201,11 +1199,9 @@ restart:
 	spin_unlock_bh(rt_hash_lock_addr(hash));
 
 skip_hashing:
-	if (rp)
-		*rp = rt;
-	else
+	if (skb)
 		skb_dst_set(skb, &rt->dst);
-	return 0;
+	return rt;
 }
 
 static atomic_t __rt_peer_genid = ATOMIC_INIT(0);
@@ -1896,7 +1892,10 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	RT_CACHE_STAT_INC(in_slow_mc);
 
 	hash = rt_hash(daddr, saddr, dev->ifindex, rt_genid(dev_net(dev)));
-	return rt_intern_hash(hash, rth, NULL, skb, dev->ifindex);
+	rth = rt_intern_hash(hash, rth, skb, dev->ifindex);
+	err = 0;
+	if (IS_ERR(rth))
+		err = PTR_ERR(rth);
 
 e_nobufs:
 	return -ENOBUFS;
@@ -2051,7 +2050,10 @@ static int ip_mkroute_input(struct sk_buff *skb,
 	/* put it into the cache */
 	hash = rt_hash(daddr, saddr, fl->iif,
 		       rt_genid(dev_net(rth->dst.dev)));
-	return rt_intern_hash(hash, rth, NULL, skb, fl->iif);
+	rth = rt_intern_hash(hash, rth, skb, fl->iif);
+	if (IS_ERR(rth))
+		return PTR_ERR(rth);
+	return 0;
 }
 
 /*
@@ -2194,7 +2196,10 @@ local_input:
 	}
 	rth->rt_type	= res.type;
 	hash = rt_hash(daddr, saddr, fl.iif, rt_genid(net));
-	err = rt_intern_hash(hash, rth, NULL, skb, fl.iif);
+	rth = rt_intern_hash(hash, rth, skb, fl.iif);
+	err = 0;
+	if (IS_ERR(rth))
+		err = PTR_ERR(rth);
 	goto out;
 
 no_route:
@@ -2422,8 +2427,8 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
  * called with rcu_read_lock();
  */
 
-static int ip_route_output_slow(struct net *net, struct rtable **rp,
-				const struct flowi *oldflp)
+static struct rtable *ip_route_output_slow(struct net *net,
+					   const struct flowi *oldflp)
 {
 	u32 tos	= RT_FL_TOS(oldflp);
 	struct flowi fl = { .fl4_dst = oldflp->fl4_dst,
@@ -2438,8 +2443,6 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 	unsigned int flags = 0;
 	struct net_device *dev_out = NULL;
 	struct rtable *rth;
-	int err;
-
 
 	res.fi		= NULL;
 #ifdef CONFIG_IP_MULTIPLE_TABLES
@@ -2448,7 +2451,7 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 
 	rcu_read_lock();
 	if (oldflp->fl4_src) {
-		err = -EINVAL;
+		rth = ERR_PTR(-EINVAL);
 		if (ipv4_is_multicast(oldflp->fl4_src) ||
 		    ipv4_is_lbcast(oldflp->fl4_src) ||
 		    ipv4_is_zeronet(oldflp->fl4_src))
@@ -2499,13 +2502,13 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 
 	if (oldflp->oif) {
 		dev_out = dev_get_by_index_rcu(net, oldflp->oif);
-		err = -ENODEV;
+		rth = ERR_PTR(-ENODEV);
 		if (dev_out == NULL)
 			goto out;
 
 		/* RACE: Check return value of inet_select_addr instead. */
 		if (!(dev_out->flags & IFF_UP) || !__in_dev_get_rcu(dev_out)) {
-			err = -ENETUNREACH;
+			rth = ERR_PTR(-ENETUNREACH);
 			goto out;
 		}
 		if (ipv4_is_local_multicast(oldflp->fl4_dst) ||
@@ -2563,7 +2566,7 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 			res.type = RTN_UNICAST;
 			goto make_route;
 		}
-		err = -ENETUNREACH;
+		rth = ERR_PTR(-ENETUNREACH);
 		goto out;
 	}
 
@@ -2598,23 +2601,20 @@ static int ip_route_output_slow(struct net *net, struct rtable **rp,
 
 make_route:
 	rth = __mkroute_output(&res, &fl, oldflp, dev_out, flags);
-	if (IS_ERR(rth))
-		err = PTR_ERR(rth);
-	else {
+	if (!IS_ERR(rth)) {
 		unsigned int hash;
 
 		hash = rt_hash(oldflp->fl4_dst, oldflp->fl4_src, oldflp->oif,
 			       rt_genid(dev_net(dev_out)));
-		err = rt_intern_hash(hash, rth, rp, NULL, oldflp->oif);
+		rth = rt_intern_hash(hash, rth, NULL, oldflp->oif);
 	}
 
 out:
 	rcu_read_unlock();
-	return err;
+	return rth;
 }
 
-int __ip_route_output_key(struct net *net, struct rtable **rp,
-			  const struct flowi *flp)
+struct rtable *__ip_route_output_key(struct net *net, const struct flowi *flp)
 {
 	struct rtable *rth;
 	unsigned int hash;
@@ -2639,15 +2639,14 @@ int __ip_route_output_key(struct net *net, struct rtable **rp,
 			dst_use(&rth->dst, jiffies);
 			RT_CACHE_STAT_INC(out_hit);
 			rcu_read_unlock_bh();
-			*rp = rth;
-			return 0;
+			return rth;
 		}
 		RT_CACHE_STAT_INC(out_hlist_search);
 	}
 	rcu_read_unlock_bh();
 
 slow_output:
-	return ip_route_output_slow(net, rp, flp);
+	return ip_route_output_slow(net, flp);
 }
 EXPORT_SYMBOL_GPL(__ip_route_output_key);
 
@@ -2717,34 +2716,29 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
 	return rt ? &rt->dst : ERR_PTR(-ENOMEM);
 }
 
-int ip_route_output_flow(struct net *net, struct rtable **rp, struct flowi *flp,
-			 struct sock *sk)
+struct rtable *ip_route_output_flow(struct net *net, struct flowi *flp,
+				    struct sock *sk)
 {
-	int err;
+	struct rtable *rt = __ip_route_output_key(net, flp);
 
-	if ((err = __ip_route_output_key(net, rp, flp)) != 0)
-		return err;
+	if (IS_ERR(rt))
+		return rt;
 
 	if (flp->proto) {
 		if (!flp->fl4_src)
-			flp->fl4_src = (*rp)->rt_src;
+			flp->fl4_src = rt->rt_src;
 		if (!flp->fl4_dst)
-			flp->fl4_dst = (*rp)->rt_dst;
-		*rp = (struct rtable *) xfrm_lookup(net, &(*rp)->dst, flp, sk, 0);
-		if (IS_ERR(*rp)) {
-			err = PTR_ERR(*rp);
-			*rp = NULL;
-			return err;
-		}
+			flp->fl4_dst = rt->rt_dst;
+		rt = (struct rtable *) xfrm_lookup(net, &rt->dst, flp, sk, 0);
 	}
 
-	return 0;
+	return rt;
 }
 EXPORT_SYMBOL_GPL(ip_route_output_flow);
 
-int ip_route_output_key(struct net *net, struct rtable **rp, struct flowi *flp)
+struct rtable *ip_route_output_key(struct net *net, struct flowi *flp)
 {
-	return ip_route_output_flow(net, rp, flp, NULL);
+	return ip_route_output_flow(net, flp, NULL);
 }
 EXPORT_SYMBOL(ip_route_output_key);
 
@@ -2915,7 +2909,11 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 			.oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0,
 			.mark = mark,
 		};
-		err = ip_route_output_key(net, &rt, &fl);
+		rt = ip_route_output_key(net, &fl);
+
+		err = 0;
+		if (IS_ERR(rt))
+			err = PTR_ERR(rt);
 	}
 
 	if (err)
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 4751920..0ad6ddf 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -355,7 +355,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
 				    .fl_ip_sport = th->dest,
 				    .fl_ip_dport = th->source };
 		security_req_classify_flow(req, &fl);
-		if (ip_route_output_key(sock_net(sk), &rt, &fl)) {
+		rt = ip_route_output_key(sock_net(sk), &fl);
+		if (IS_ERR(rt)) {
 			reqsk_free(req);
 			goto out;
 		}
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 05bc6d9..f7e6c2c 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -152,7 +152,6 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	__be16 orig_sport, orig_dport;
 	struct rtable *rt;
 	__be32 daddr, nexthop;
-	int tmp;
 	int err;
 
 	if (addr_len < sizeof(struct sockaddr_in))
@@ -170,14 +169,15 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 
 	orig_sport = inet->inet_sport;
 	orig_dport = usin->sin_port;
-	tmp = ip_route_connect(&rt, nexthop, inet->inet_saddr,
-			       RT_CONN_FLAGS(sk), sk->sk_bound_dev_if,
-			       IPPROTO_TCP,
-			       orig_sport, orig_dport, sk, true);
-	if (tmp < 0) {
-		if (tmp == -ENETUNREACH)
+	rt = ip_route_connect(nexthop, inet->inet_saddr,
+			      RT_CONN_FLAGS(sk), sk->sk_bound_dev_if,
+			      IPPROTO_TCP,
+			      orig_sport, orig_dport, sk, true);
+	if (IS_ERR(rt)) {
+		err = PTR_ERR(rt);
+		if (err == -ENETUNREACH)
 			IP_INC_STATS_BH(sock_net(sk), IPSTATS_MIB_OUTNOROUTES);
-		return tmp;
+		return err;
 	}
 
 	if (rt->rt_flags & (RTCF_MULTICAST | RTCF_BROADCAST)) {
@@ -236,12 +236,14 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	if (err)
 		goto failure;
 
-	err = ip_route_newports(&rt, IPPROTO_TCP,
-				orig_sport, orig_dport,
-				inet->inet_sport, inet->inet_dport, sk);
-	if (err)
+	rt = ip_route_newports(rt, IPPROTO_TCP,
+			       orig_sport, orig_dport,
+			       inet->inet_sport, inet->inet_dport, sk);
+	if (IS_ERR(rt)) {
+		err = PTR_ERR(rt);
+		rt = NULL;
 		goto failure;
-
+	}
 	/* OK, now commit destination to socket.  */
 	sk->sk_gso_type = SKB_GSO_TCPV4;
 	sk_setup_caps(sk, &rt->dst);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index ed9a5b7..95e0c2c 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -922,8 +922,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		struct net *net = sock_net(sk);
 
 		security_sk_classify_flow(sk, &fl);
-		err = ip_route_output_flow(net, &rt, &fl, sk);
-		if (err) {
+		rt = ip_route_output_flow(net, &fl, sk);
+		if (IS_ERR(rt)) {
+			err = PTR_ERR(rt);
 			if (err == -ENETUNREACH)
 				IP_INC_STATS_BH(net, IPSTATS_MIB_OUTNOROUTES);
 			goto out;
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 5f0f058..45b8214 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -26,18 +26,16 @@ static struct dst_entry *xfrm4_dst_lookup(struct net *net, int tos,
 		.fl4_dst = daddr->a4,
 		.fl4_tos = tos,
 	};
-	struct dst_entry *dst;
 	struct rtable *rt;
-	int err;
 
 	if (saddr)
 		fl.fl4_src = saddr->a4;
 
-	err = __ip_route_output_key(net, &rt, &fl);
-	dst = &rt->dst;
-	if (err)
-		dst = ERR_PTR(err);
-	return dst;
+	rt = __ip_route_output_key(net, &fl);
+	if (!IS_ERR(rt))
+		return &rt->dst;
+
+	return ERR_CAST(rt);
 }
 
 static int xfrm4_get_saddr(struct net *net,
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index da43038..02730ef 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -581,7 +581,8 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	fl.fl4_dst = eiph->saddr;
 	fl.fl4_tos = RT_TOS(eiph->tos);
 	fl.proto = IPPROTO_IPIP;
-	if (ip_route_output_key(dev_net(skb->dev), &rt, &fl))
+	rt = ip_route_output_key(dev_net(skb->dev), &fl);
+	if (IS_ERR(rt))
 		goto out;
 
 	skb2->dev = rt->dst.dev;
@@ -593,12 +594,14 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		fl.fl4_dst = eiph->daddr;
 		fl.fl4_src = eiph->saddr;
 		fl.fl4_tos = eiph->tos;
-		if (ip_route_output_key(dev_net(skb->dev), &rt, &fl) ||
+		rt = ip_route_output_key(dev_net(skb->dev), &fl);
+		if (IS_ERR(rt) ||
 		    rt->dst.dev->type != ARPHRD_TUNNEL) {
-			ip_rt_put(rt);
+			if (!IS_ERR(rt))
+				ip_rt_put(rt);
 			goto out;
 		}
-		skb_dst_set(skb2, (struct dst_entry *)rt);
+		skb_dst_set(skb2, &rt->dst);
 	} else {
 		ip_rt_put(rt);
 		if (ip_route_input(skb2, eiph->daddr, eiph->saddr, eiph->tos,
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index b1599a3..b8c8adb 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -738,7 +738,8 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
 				    .fl4_tos = RT_TOS(tos),
 				    .oif = tunnel->parms.link,
 				    .proto = IPPROTO_IPV6 };
-		if (ip_route_output_key(dev_net(dev), &rt, &fl)) {
+		rt = ip_route_output_key(dev_net(dev), &fl);
+		if (IS_ERR(rt)) {
 			dev->stats.tx_carrier_errors++;
 			goto tx_error_icmp;
 		}
@@ -862,8 +863,9 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
 				    .fl4_tos = RT_TOS(iph->tos),
 				    .oif = tunnel->parms.link,
 				    .proto = IPPROTO_IPV6 };
-		struct rtable *rt;
-		if (!ip_route_output_key(dev_net(dev), &rt, &fl)) {
+		struct rtable *rt = ip_route_output_key(dev_net(dev), &fl);
+
+		if (!IS_ERR(rt)) {
 			tdev = rt->dst.dev;
 			ip_rt_put(rt);
 		}
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 5381ceb..2a698ff 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -320,11 +320,12 @@ static int l2tp_ip_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
 	if (ipv4_is_multicast(lsa->l2tp_addr.s_addr))
 		goto out;
 
-	rc = ip_route_connect(&rt, lsa->l2tp_addr.s_addr, saddr,
+	rt = ip_route_connect(lsa->l2tp_addr.s_addr, saddr,
 			      RT_CONN_FLAGS(sk), oif,
 			      IPPROTO_L2TP,
 			      0, 0, sk, true);
-	if (rc) {
+	if (IS_ERR(rt)) {
+		rc = PTR_ERR(rt);
 		if (rc == -ENETUNREACH)
 			IP_INC_STATS_BH(&init_net, IPSTATS_MIB_OUTNOROUTES);
 		goto out;
@@ -489,7 +490,8 @@ static int l2tp_ip_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *m
 			 * itself out.
 			 */
 			security_sk_classify_flow(sk, &fl);
-			if (ip_route_output_flow(sock_net(sk), &rt, &fl, sk))
+			rt = ip_route_output_flow(sock_net(sk), &fl, sk);
+			if (IS_ERR(rt))
 				goto no_route;
 		}
 		sk_setup_caps(sk, &rt->dst);
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 6264219..878f6dd 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -103,7 +103,8 @@ __ip_vs_get_out_rt(struct sk_buff *skb, struct ip_vs_dest *dest,
 				.fl4_tos = rtos,
 			};
 
-			if (ip_route_output_key(net, &rt, &fl)) {
+			rt = ip_route_output_key(net, &fl);
+			if (IS_ERR(rt)) {
 				spin_unlock(&dest->dst_lock);
 				IP_VS_DBG_RL("ip_route_output error, dest: %pI4\n",
 					     &dest->addr.ip);
@@ -121,7 +122,8 @@ __ip_vs_get_out_rt(struct sk_buff *skb, struct ip_vs_dest *dest,
 			.fl4_tos = rtos,
 		};
 
-		if (ip_route_output_key(net, &rt, &fl)) {
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt)) {
 			IP_VS_DBG_RL("ip_route_output error, dest: %pI4\n",
 				     &daddr);
 			return NULL;
@@ -180,7 +182,8 @@ __ip_vs_reroute_locally(struct sk_buff *skb)
 			.mark = skb->mark,
 		};
 
-		if (ip_route_output_key(net, &rt, &fl))
+		rt = ip_route_output_key(net, &fl);
+		if (IS_ERR(rt))
 			return 0;
 		if (!(rt->rt_flags & RTCF_LOCAL)) {
 			ip_rt_put(rt);
diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c
index 5128a6c..624725b 100644
--- a/net/netfilter/xt_TEE.c
+++ b/net/netfilter/xt_TEE.c
@@ -73,7 +73,8 @@ tee_tg_route4(struct sk_buff *skb, const struct xt_tee_tginfo *info)
 	fl.fl4_dst = info->gw.ip;
 	fl.fl4_tos = RT_TOS(iph->tos);
 	fl.fl4_scope = RT_SCOPE_UNIVERSE;
-	if (ip_route_output_key(net, &rt, &fl) != 0)
+	rt = ip_route_output_key(net, &fl);
+	if (IS_ERR(rt))
 		return false;
 
 	skb_dst_drop(skb);
diff --git a/net/rxrpc/ar-peer.c b/net/rxrpc/ar-peer.c
index a53fb25..3620c56 100644
--- a/net/rxrpc/ar-peer.c
+++ b/net/rxrpc/ar-peer.c
@@ -37,7 +37,6 @@ static void rxrpc_assess_MTU_size(struct rxrpc_peer *peer)
 {
 	struct rtable *rt;
 	struct flowi fl;
-	int ret;
 
 	peer->if_mtu = 1500;
 
@@ -58,9 +57,9 @@ static void rxrpc_assess_MTU_size(struct rxrpc_peer *peer)
 		BUG();
 	}
 
-	ret = ip_route_output_key(&init_net, &rt, &fl);
-	if (ret < 0) {
-		_leave(" [route err %d]", ret);
+	rt = ip_route_output_key(&init_net, &fl);
+	if (IS_ERR(rt)) {
+		_leave(" [route err %ld]", PTR_ERR(rt));
 		return;
 	}
 
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index e58f947..4e55e6c 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -491,9 +491,9 @@ static struct dst_entry *sctp_v4_get_dst(struct sctp_association *asoc,
 	SCTP_DEBUG_PRINTK("%s: DST:%pI4, SRC:%pI4 - ",
 			  __func__, &fl.fl4_dst, &fl.fl4_src);
 
-	if (!ip_route_output_key(&init_net, &rt, &fl)) {
+	rt = ip_route_output_key(&init_net, &fl);
+	if (!IS_ERR(rt))
 		dst = &rt->dst;
-	}
 
 	/* If there is no association or if a source address is passed, no
 	 * more validation is required.
@@ -535,7 +535,8 @@ static struct dst_entry *sctp_v4_get_dst(struct sctp_association *asoc,
 		    (AF_INET == laddr->a.sa.sa_family)) {
 			fl.fl4_src = laddr->a.v4.sin_addr.s_addr;
 			fl.fl_ip_sport = laddr->a.v4.sin_port;
-			if (!ip_route_output_key(&init_net, &rt, &fl)) {
+			rt = ip_route_output_key(&init_net, &fl);
+			if (!IS_ERR(rt)) {
 				dst = &rt->dst;
 				goto out_unlock;
 			}
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCH] e1000: fix race condition while driver unload/reset using kref count
From: Brandeburg, Jesse @ 2011-03-02 22:50 UTC (permalink / raw)
  To: prasanna.panchamukhi@riverbed.com
  Cc: Allan, Bruce W, Kirsher, Jeffrey T, Pieper, Jeffrey E,
	e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <1299091597-28409-1-git-send-email-prasanna.panchamukhi@riverbed.com>



On Wed, 2 Mar 2011, prasanna.panchamukhi@riverbed.com wrote:

> This race conditions occurs with reentrant e1000_down(), which gets
> called by multiple threads while driver unload or reset.
> This patch fixes the race condition when one thread tries to destroy
> the memory allocated for tx buffer_info, while another thread still
> happen to access tx buffer_info.
> This patch fixes the above race condition using kref count.

I'm very interested in any test cases that you might have come up with to 
reproduce this issue.

The patch itself looks interesting, and probably okay, but we really need 
a reproduction case.

Also, do we need to adjust the rtnl_lock stuff or do this for rx 
buffer_info structs?

I'm concerned that maybe we don't understand the full flow of events 
leading up to this failure.

Jesse
 
> Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com>
> ---
>  drivers/net/e1000/e1000.h      |    2 +
>  drivers/net/e1000/e1000_main.c |   41 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
> index a881dd0..36f55b3 100644
> --- a/drivers/net/e1000/e1000.h
> +++ b/drivers/net/e1000/e1000.h
> @@ -168,6 +168,8 @@ struct e1000_tx_ring {
>  	unsigned int next_to_clean;
>  	/* array of buffer information structs */
>  	struct e1000_buffer *buffer_info;
> +	spinlock_t bufinfo_lock; /* protect access to buffer_info */
> +	struct kref bufinfo_refcount; /* refcount access to buffer info */
>  
>  	u16 tdh;
>  	u16 tdt;
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index beec573..336d3e1 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -1531,6 +1531,8 @@ setup_tx_desc_die:
>  
>  	txdr->next_to_use = 0;
>  	txdr->next_to_clean = 0;
> +	spin_lock_init(&txdr->bufinfo_lock);
> +	kref_init(&txdr->bufinfo_refcount);
>  
>  	return 0;
>  }
> @@ -1880,6 +1882,22 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
>  	ew32(RCTL, rctl);
>  }
>  
> +/*
> + * Free tx buffer info resources, only when no other thread is
> + * accessing it. Access to buffer_info is refcounted by bufinfo_refcount,
> + * hence memory allocated must be destroyed when bufinfo_refcount
> + * becomes zero. This routine gets executed when bufinfo_refcount
> + * becomes zero.
> + */
> +static void e1000_free_tx_buffer_info(struct kref *ref)
> +{
> +	struct e1000_tx_ring *tx_ring =
> +		container_of(ref, struct e1000_tx_ring, bufinfo_refcount);
> +
> +	vfree(tx_ring->buffer_info);
> +	tx_ring->buffer_info = NULL;
> +}
> +
>  /**
>   * e1000_free_tx_resources - Free Tx Resources per Queue
>   * @adapter: board private structure
> @@ -1895,8 +1913,9 @@ static void e1000_free_tx_resources(struct e1000_adapter *adapter,
>  
>  	e1000_clean_tx_ring(adapter, tx_ring);
>  
> -	vfree(tx_ring->buffer_info);
> -	tx_ring->buffer_info = NULL;
> +	spin_lock(&tx_ring->bufinfo_lock);
> +	kref_put(&tx_ring->bufinfo_refcount, e1000_free_tx_buffer_info);
> +	spin_unlock(&tx_ring->bufinfo_lock);
>  
>  	dma_free_coherent(&pdev->dev, tx_ring->size, tx_ring->desc,
>  			  tx_ring->dma);
> @@ -1954,8 +1973,20 @@ static void e1000_clean_tx_ring(struct e1000_adapter *adapter,
>  	unsigned long size;
>  	unsigned int i;
>  
> -	/* Free all the Tx ring sk_buffs */
> +	spin_lock(&tx_ring->bufinfo_lock);
> +	/*
> +	 * Check buffer_info is not NULL, and
> +	 * increment the refcount to prevent
> +	 * the buffer getting freed underneath.
> +	 */
> +	if (tx_ring->buffer_info == NULL) {
> +		spin_unlock(&tx_ring->bufinfo_lock);
> +		return;
> +	}
> +	kref_get(&tx_ring->bufinfo_refcount);
> +	spin_unlock(&tx_ring->bufinfo_lock);
>  
> +	/* Free all the Tx ring sk_buffs */
>  	for (i = 0; i < tx_ring->count; i++) {
>  		buffer_info = &tx_ring->buffer_info[i];
>  		e1000_unmap_and_free_tx_resource(adapter, buffer_info);
> @@ -1968,6 +1999,10 @@ static void e1000_clean_tx_ring(struct e1000_adapter *adapter,
>  
>  	memset(tx_ring->desc, 0, tx_ring->size);
>  
> +	spin_lock(&tx_ring->bufinfo_lock);
> +	kref_put(&tx_ring->bufinfo_refcount, e1000_free_tx_buffer_info);
> +	spin_unlock(&tx_ring->bufinfo_lock);
> +
>  	tx_ring->next_to_use = 0;
>  	tx_ring->next_to_clean = 0;
>  	tx_ring->last_tx_tso = 0;
> 

^ permalink raw reply

* [PATCH] ipv4: ip_route_output_key() is better as an inline.
From: David Miller @ 2011-03-02 23:03 UTC (permalink / raw)
  To: netdev


This avoid a stack frame at zero cost.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/route.h |    6 +++++-
 net/ipv4/route.c    |    6 ------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index 088a186..60daf74 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -119,11 +119,15 @@ extern void		ip_rt_redirect(__be32 old_gw, __be32 dst, __be32 new_gw,
 extern void		rt_cache_flush(struct net *net, int how);
 extern void		rt_cache_flush_batch(struct net *net);
 extern struct rtable *__ip_route_output_key(struct net *, const struct flowi *flp);
-extern struct rtable *ip_route_output_key(struct net *, struct flowi *flp);
 extern struct rtable *ip_route_output_flow(struct net *, struct flowi *flp,
 					   struct sock *sk);
 extern struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_orig);
 
+static inline struct rtable *ip_route_output_key(struct net *net, struct flowi *flp)
+{
+	return ip_route_output_flow(net, flp, NULL);
+}
+
 extern int ip_route_input_common(struct sk_buff *skb, __be32 dst, __be32 src,
 				 u8 tos, struct net_device *devin, bool noref);
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 5090e95..432eee6 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2736,12 +2736,6 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi *flp,
 }
 EXPORT_SYMBOL_GPL(ip_route_output_flow);
 
-struct rtable *ip_route_output_key(struct net *net, struct flowi *flp)
-{
-	return ip_route_output_flow(net, flp, NULL);
-}
-EXPORT_SYMBOL(ip_route_output_key);
-
 static int rt_fill_info(struct net *net,
 			struct sk_buff *skb, u32 pid, u32 seq, int event,
 			int nowait, unsigned int flags)
-- 
1.7.4.1


^ permalink raw reply related

* Re: [net-2.6 PATCH] net: dcbnl: check correct ops in dcbnl_ieee_set()
From: David Miller @ 2011-03-02 23:05 UTC (permalink / raw)
  To: john.r.fastabend; +Cc: netdev, shmulikr
In-Reply-To: <20110302203533.2465.11134.stgit@jf-dev1-dcblab>

From: John Fastabend <john.r.fastabend@intel.com>
Date: Wed, 02 Mar 2011 12:35:33 -0800

> The incorrect ops routine was being tested for in
> DCB_ATTR_IEEE_PFC attributes. This patch corrects
> it.
> 
> Currently, every driver implementing ieee_setets also
> implements ieee_setpfc so this bug is not actualized
> yet.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Applied, thanks John.

^ permalink raw reply

* Re: [net-2.6 0/3][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2011-03-02 23:06 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1299066341-13820-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  2 Mar 2011 03:45:38 -0800

> The following series contains fixes for sparse warnings for e1000
> and igb, as well as a fix for e1000e.
> 
> The following are changes since commit e3fa3aff0cb198e7c53d894f52146121d9592872:
>   net: fix nla_policy_len to actually _iterate_ over the policy
> 
> and are available in the git repository at:
>   master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-2.6 master

Pulled, thanks Jeff.

^ permalink raw reply

* [PATCH] net: mac80211: fix compilation warning
From: bookjovi @ 2011-03-02 23:32 UTC (permalink / raw)
  To: bookjovi
  Cc: John W. Linville, Johannes Berg, David S. Miller,
	open list:NETWORKING [WIREL..., open list:NETWORKING [GENERAL],
	open list

From: Jovi Zhang <bookjovi@gmail.com>

this commit fix compilation warning as following:
net/mac80211/tx.c:1753: warning: unused variable mppath

Signed-off-by: Jovi Zhang <bookjovi@gmail.com>
---
 net/mac80211/tx.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index b0beaa5..e87b07f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1750,7 +1750,6 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 	__le16 fc;
 	struct ieee80211_hdr hdr;
 	struct ieee80211s_hdr mesh_hdr __maybe_unused;
-	struct mesh_path *mppath = NULL;
 	const u8 *encaps_data;
 	int encaps_len, skip_header_bytes;
 	int nh_pos, h_pos;
@@ -1805,6 +1804,8 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		break;
 #ifdef CONFIG_MAC80211_MESH
 	case NL80211_IFTYPE_MESH_POINT:
+		struct mesh_path *mppath = NULL;
+
 		if (!sdata->u.mesh.mshcfg.dot11MeshTTL) {
 			/* Do not send frames with mesh_ttl == 0 */
 			sdata->u.mesh.mshstats.dropped_frames_ttl++;
-- 
1.7.2.3

^ permalink raw reply related

* [PATCH net-2.6 2/2] cnic: Fix lost interrupt on bnx2x
From: Michael Chan @ 2011-03-02 23:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, Michael Chan
In-Reply-To: <1299106850-23645-1-git-send-email-mchan@broadcom.com>

We service 2 queues (kcq1 and kcq2) in cnic_service_bnx2x_bh().  If
the status block index has changed when servicing the kcq2, we must
go back and check kcq1.  The latest status block index will be used
to acknowledge the interrupt, and without looping back to check kcq1,
we may miss events on kcq1.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/cnic.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index b0d9e4a..302be4a 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -2914,26 +2914,35 @@ static void cnic_service_bnx2x_bh(unsigned long data)
 {
 	struct cnic_dev *dev = (struct cnic_dev *) data;
 	struct cnic_local *cp = dev->cnic_priv;
-	u32 status_idx;
+	u32 status_idx, new_status_idx;
 
 	if (unlikely(!test_bit(CNIC_F_CNIC_UP, &dev->flags)))
 		return;
 
-	status_idx = cnic_service_bnx2x_kcq(dev, &cp->kcq1);
+	while (1) {
+		status_idx = cnic_service_bnx2x_kcq(dev, &cp->kcq1);
 
-	CNIC_WR16(dev, cp->kcq1.io_addr, cp->kcq1.sw_prod_idx + MAX_KCQ_IDX);
+		CNIC_WR16(dev, cp->kcq1.io_addr,
+			  cp->kcq1.sw_prod_idx + MAX_KCQ_IDX);
 
-	if (BNX2X_CHIP_IS_E2(cp->chip_id)) {
-		status_idx = cnic_service_bnx2x_kcq(dev, &cp->kcq2);
+		if (!BNX2X_CHIP_IS_E2(cp->chip_id)) {
+			cnic_ack_bnx2x_int(dev, cp->bnx2x_igu_sb_id, USTORM_ID,
+					   status_idx, IGU_INT_ENABLE, 1);
+			break;
+		}
+
+		new_status_idx = cnic_service_bnx2x_kcq(dev, &cp->kcq2);
+
+		if (new_status_idx != status_idx)
+			continue;
 
 		CNIC_WR16(dev, cp->kcq2.io_addr, cp->kcq2.sw_prod_idx +
 			  MAX_KCQ_IDX);
 
 		cnic_ack_igu_sb(dev, cp->bnx2x_igu_sb_id, IGU_SEG_ACCESS_DEF,
 				status_idx, IGU_INT_ENABLE, 1);
-	} else {
-		cnic_ack_bnx2x_int(dev, cp->bnx2x_igu_sb_id, USTORM_ID,
-				   status_idx, IGU_INT_ENABLE, 1);
+
+		break;
 	}
 }
 
-- 
1.6.4.GIT



^ permalink raw reply related

* [PATCH net-2.6 1/2] cnic: Prevent status block race conditions with hardware
From: Michael Chan @ 2011-03-02 23:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, Michael Chan

The status block index is used to acknowledge interrupt events and must
be read before checking for the interrupt events, so we need to add rmb()
to guarantee that.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/cnic.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 7ff170c..b0d9e4a 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -2760,6 +2760,8 @@ static u32 cnic_service_bnx2_queues(struct cnic_dev *dev)
 	u32 status_idx = (u16) *cp->kcq1.status_idx_ptr;
 	int kcqe_cnt;
 
+	/* status block index must be read before reading other fields */
+	rmb();
 	cp->kwq_con_idx = *cp->kwq_con_idx_ptr;
 
 	while ((kcqe_cnt = cnic_get_kcqes(dev, &cp->kcq1))) {
@@ -2770,6 +2772,8 @@ static u32 cnic_service_bnx2_queues(struct cnic_dev *dev)
 		barrier();
 		if (status_idx != *cp->kcq1.status_idx_ptr) {
 			status_idx = (u16) *cp->kcq1.status_idx_ptr;
+			/* status block index must be read first */
+			rmb();
 			cp->kwq_con_idx = *cp->kwq_con_idx_ptr;
 		} else
 			break;
@@ -2888,6 +2892,8 @@ static u32 cnic_service_bnx2x_kcq(struct cnic_dev *dev, struct kcq_info *info)
 	u32 last_status = *info->status_idx_ptr;
 	int kcqe_cnt;
 
+	/* status block index must be read before reading the KCQ */
+	rmb();
 	while ((kcqe_cnt = cnic_get_kcqes(dev, info))) {
 
 		service_kcqes(dev, kcqe_cnt);
@@ -2898,6 +2904,8 @@ static u32 cnic_service_bnx2x_kcq(struct cnic_dev *dev, struct kcq_info *info)
 			break;
 
 		last_status = *info->status_idx_ptr;
+		/* status block index must be read before reading the KCQ */
+		rmb();
 	}
 	return last_status;
 }
-- 
1.6.4.GIT



^ permalink raw reply related

* Re: [PATCH] sched: QFQ - quick fair queue scheduler (v2)
From: Stephen Hemminger @ 2011-03-02 23:55 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Fabio Checconi, David Miller, Luigi Rizzo, netdev
In-Reply-To: <1299087087.2920.27.camel@edumazet-laptop>

On Wed, 02 Mar 2011 18:31:27 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le mercredi 02 mars 2011 à 17:18 +0100, Eric Dumazet a écrit :
> > Le mercredi 02 mars 2011 à 08:11 -0800, Stephen Hemminger a écrit :
> > 
> > > I put the iproute2 code into the repository in the experimental branch.
> > > 
> > 
> > Thanks
> > 
> > It seems as soon as packets are dropped, qdisc is frozen (no more
> > packets dequeued)
> > 
> > Hmm...
> > 
> 
> It seems class deletes are buggy.
> 
> After one "tc class del dev $ETH classid 11:1 ..."
> 
> a "tc -s -d qdisc show dev $ETH" triggers an Oops
> 
> [  414.517709] general protection fault: 0000 [#1] SMP 
> [  414.517894] last sysfs file: /sys/devices/virtual/net/gre34/ifindex
> [  414.517956] CPU 3 
> [  414.517995] Modules linked in: sch_qfq sch_cbq ip_gre gre dummy ipmi_devintf ipmi_si ipmi_msghandler dm_mod video tg3 libphy sg [last unloaded: ip_tables]
> [  414.518663] 
> [  414.518717] Pid: 4692, comm: tc Not tainted 2.6.38-rc5-02726-gd486b8c-dirty #554 HP ProLiant BL460c G6
> [  414.518905] RIP: 0010:[<ffffffff8145118e>]  [<ffffffff8145118e>] tc_fill_qdisc+0xbe/0x300
> [  414.519025] RSP: 0018:ffff88011a123878  EFLAGS: 00010283
> [  414.519085] RAX: a00e6660ffffffff RBX: 0000000000000002 RCX: 0000000000000024
> [  414.519149] RDX: ffff880078683334 RSI: 0000000000000000 RDI: ffff88007867b400
> [  414.519212] RBP: ffff88011a123928 R08: ffff880078683000 R09: 0000000000000002
> [  414.519276] R10: 00000000000001f0 R11: 00000000000001e0 R12: ffff88007867b400
> [  414.519340] R13: ffff88011cb1779c R14: ffff880078683324 R15: 0000000000001254
> [  414.519405] FS:  0000000000000000(0000) GS:ffff88007fc40000(0063) knlGS:00000000f77778e0
> [  414.519486] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
> [  414.519553] CR2: 00000000005ffc80 CR3: 0000000078087000 CR4: 00000000000006e0
> [  414.519616] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  414.519680] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [  414.519744] Process tc (pid: 4692, threadinfo ffff88011a122000, task ffff88011a1715d0)
> [  414.519823] Stack:
> [  414.519876]  ffff88011bbfb570 000000004d6e7c8e ffff880078683324 0000000000000000
> [  414.520095]  ffff88011cb1789c ffff88007867b400 ffff8800786832bc 0000000400000003
> [  414.520312]  0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [  414.520531] Call Trace:
> [  414.520602]  [<ffffffff81451469>] tc_dump_qdisc_root+0x99/0x100
> [  414.520666]  [<ffffffff81452680>] tc_dump_qdisc+0x80/0x100
> [  414.520729]  [<ffffffff8146a50b>] netlink_dump+0x6b/0x1e0
> [  414.520791]  [<ffffffff810f5d16>] ? kmem_cache_alloc_trace+0xb6/0x100
> [  414.520855]  [<ffffffff8146d2ef>] netlink_dump_start+0x16f/0x190
> [  414.520918]  [<ffffffff81452600>] ? tc_dump_qdisc+0x0/0x100
> [  414.520981]  [<ffffffff81443b36>] rtnetlink_rcv_msg+0xb6/0x270
> [  414.521043]  [<ffffffff81443a80>] ? rtnetlink_rcv_msg+0x0/0x270
> [  414.521106]  [<ffffffff8146b609>] netlink_rcv_skb+0x99/0xc0
> [  414.521167]  [<ffffffff81443a65>] rtnetlink_rcv+0x25/0x40
> [  414.521229]  [<ffffffff8146b379>] netlink_unicast+0x2a9/0x2b0
> [  414.521292]  [<ffffffff8142c3e3>] ? memcpy_fromiovec+0x63/0x80
> [  414.521354]  [<ffffffff8146c30d>] netlink_sendmsg+0x24d/0x390
> [  414.521418]  [<ffffffff81421510>] sock_sendmsg+0xc0/0xf0
> [  414.521482]  [<ffffffff810b699a>] ? unlock_page+0x2a/0x40
> [  414.521545]  [<ffffffff81421262>] ? move_addr_to_kernel+0x62/0x70
> [  414.521608]  [<ffffffff8144d59f>] ? verify_compat_iovec+0x8f/0x100
> [  414.521685]  [<ffffffff814222b0>] sys_sendmsg+0x180/0x300
> [  414.521747]  [<ffffffff810d682c>] ? __pte_alloc+0xdc/0x100
> [  414.521809]  [<ffffffff810d6992>] ? handle_mm_fault+0x142/0x1c0
> [  414.521872]  [<ffffffff81569594>] ? do_page_fault+0x274/0x490
> [  414.521935]  [<ffffffff81421f41>] ? sys_getsockname+0xa1/0xb0
> [  414.521997]  [<ffffffff81421909>] ? sys_recvmsg+0x49/0x80
> [  414.522059]  [<ffffffff8144d164>] compat_sys_sendmsg+0x14/0x20
> [  414.522121]  [<ffffffff8144e08d>] compat_sys_socketcall+0x19d/0x1f0
> [  414.522185]  [<ffffffff8102d640>] sysenter_dispatch+0x7/0x2e
> [  414.522246] Code: 06 49 8d 56 10 45 89 7e 0c 66 41 89 46 04 8b 85 58 ff ff ff 41 c6 46 10 00 41 89 46 08 c6 42 01 00 66 c7 42 02 00 00 49 8b 45 68 <48> 8b 00 8b 80 c0 00 00 00 89 42 04 8b 85 5c ff ff ff 89 42 0c 
> [  414.524636] RIP  [<ffffffff8145118e>] tc_fill_qdisc+0xbe/0x300
> [  414.524734]  RSP <ffff88011a123878>
> [  414.524799] ---[ end trace 513a4307e5c34d00 ]---
> 
> 

Most of the class code was copied from DRR. Does it have the
same problem?

^ permalink raw reply

* Re: [PATCH] e1000: fix race condition while driver unload/reset using kref count
From: Prasanna Panchamukhi @ 2011-03-02 23:56 UTC (permalink / raw)
  To: Brandeburg, Jesse
  Cc: e1000-devel@lists.sourceforge.net, Allan, Bruce W,
	netdev@vger.kernel.org
In-Reply-To: <alpine.WNT.2.00.1103021337560.8204@JBRANDEB-DESK2.amr.corp.intel.com>


[-- Attachment #1.1: Type: text/plain, Size: 8389 bytes --]

On 03/02/2011 02:50 PM, Brandeburg, Jesse wrote:
>
> On Wed, 2 Mar 2011, prasanna.panchamukhi@riverbed.com wrote:
>
>> This race conditions occurs with reentrant e1000_down(), which gets
>> called by multiple threads while driver unload or reset.
>> This patch fixes the race condition when one thread tries to destroy
>> the memory allocated for tx buffer_info, while another thread still
>> happen to access tx buffer_info.
>> This patch fixes the above race condition using kref count.
> I'm very interested in any test cases that you might have come up with to
> reproduce this issue.
This patch is an alternative approach to the below commit:

commit 338c15e470d818f215d651505dc169d4e92f36a4
Author: Jesse Brandeburg <jesse.brandeburg@intel.com>
Date:   Wed Sep 22 18:22:42 2010 +0000

     e1000: fix occasional panic on unload


Its a very rare event.
One of the ways is to tweak the driver to reset more frequently(5 times 
per second) & do ifconfig up/down in tight loop.
Another way could be to do a module load/unload or keep rebooting the 
system in tight loop.
> The patch itself looks interesting, and probably okay, but we really need
> a reproduction case.
Yeah, I understand your point.
> Also, do we need to adjust the rtnl_lock stuff or do this for rx
> buffer_info structs?
kref counting  buffer_info struct is good enough.
> I'm concerned that maybe we don't understand the full flow of events
> leading up to this failure.

possible race condition because of e1000_down() being re-entrant. Multiple threads can end up calling
e1000_down() either during driver unload or reboot/reset.

below is the kernel traces
Oct 10 11:59:48 localhost kernel: EIP:    0060:[put_page+2/117]    Tainted: PF
  B VLI
Oct 10 11:59:48 localhost kernel: EIP:    0060:[<e0148432>]    Tainted: PF   B
VLI
Oct 10 11:59:48 localhost kernel: EFLAGS: 00010202   (2.6.9-34.EL-rbt-1784SMP)
Oct 10 11:59:48 localhost kernel: EIP is at put_page+0x2/0x75
Oct 10 11:59:48 localhost kernel: eax: 12a051a5   ebx: 00000002   ecx: e04b3300
   edx: 12a051a5
Oct 10 11:59:48 localhost kernel: esi: f72e8b80   edi: 00000cb7   ebp: f7d09000
   esp: e2158ebc
Oct 10 11:59:48 localhost kernel: ds: 007b   es: 007b   ss: 0068
Oct 10 11:59:48 localhost kernel: Process events/1 (pid: 7, threadinfo=e2158000
task=e2182b70)
Oct 10 11:59:48 localhost kernel: Stack: e03cea9c f72e8b80 f8b6db70 e03ceada
00000000 e03ceb98 e02d8e80 e2158ee0
Oct 10 11:59:48 localhost kernel:        f88c0000 00000000 f72e8b80 f7ce2180
e02c4a88 f7ce2180 f7d092a0 e02c4ab4
Oct 10 11:59:48 localhost kernel:        f7d092a0 00000000 0103f0f8 f7d09000
e02c4b5d f88c0000 f7d092a0 e02c2247
Oct 10 11:59:48 localhost kernel: Call Trace:

Oct 10 11:59:48 localhost kernel:  [skb_release_data+57/111]
skb_release_data+0x39/0x6f
Oct 10 11:59:48 localhost kernel:  [<e03cea9c>] skb_release_data+0x39/0x6f
Oct 10 11:59:48 localhost kernel:  [kfree_skbmem+8/21] kfree_skbmem+0x8/0x15
Oct 10 11:59:48 localhost kernel:  [<e03ceada>] kfree_skbmem+0x8/0x15
Oct 10 11:59:48 localhost kernel:  [__kfree_skb+177/343] __kfree_skb+0xb1/0x157
Oct 10 11:59:48 localhost kernel:  [<e03ceb98>] __kfree_skb+0xb1/0x157
Oct 10 11:59:48 localhost kernel:  [e1000_get_phy_info_m88+56/281]
e1000_get_phy_info_m88+0x38/0x119
Oct 10 11:59:48 localhost kernel:  [<e02d8e80>]
e1000_get_phy_info_m88+0x38/0x119
Oct 10 11:59:48 localhost kernel:  [e1000_unmap_and_free_tx_resource+156/165]
e1000_unmap_and_free_tx_resource+0x9c/0xa5
Oct 10 11:59:48 localhost kernel:  [<e02c4a88>]
e1000_unmap_and_free_tx_resource+0x9c/0xa5
Oct 10 11:59:48 localhost kernel:  [e1000_clean_tx_ring+35/162]
e1000_clean_tx_ring+0x23/0xa2
Oct 10 11:59:48 localhost kernel:  [<e02c4ab4>] e1000_clean_tx_ring+0x23/0xa2
Oct 10 11:59:48 localhost kernel:  [e1000_clean_all_tx_rings+42/56]
e1000_clean_all_tx_rings+0x2a/0x38
Oct 10 11:59:48 localhost kernel:  [<e02c4b5d>]
e1000_clean_all_tx_rings+0x2a/0x38
Oct 10 11:59:48 localhost kernel:  [e1000_down+356/380] e1000_down+0x164/0x17c
Oct 10 11:59:48 localhost kernel:  [<e02c2247>] e1000_down+0x164/0x17c
Oct 10 11:59:48 localhost kernel:  [e1000_reinit_locked+120/146]
e1000_reinit_locked+0x78/0x92
Oct 10 11:59:48 localhost kernel:  [<e02c22d7>] e1000_reinit_locked+0x78/0x92
Oct 10 11:59:48 localhost kernel:  [e1000_reset_task+21/33]
e1000_reset_task+0x15/0x21
Oct 10 11:59:48 localhost kernel:  [<e02c74a3>] e1000_reset_task+0x15/0x21
Oct 10 11:59:48 localhost kernel:  [worker_thread+432/570]
worker_thread+0x1b0/0x23a
Oct 10 11:59:48 localhost kernel:  [<e0132f8d>] worker_thread+0x1b0/0x23a
Oct 10 11:59:48 localhost kernel:  [schedule+808/2803] schedule+0x328/0xaf3
Oct 10 11:59:48 localhost kernel:  [<e0431e18>] schedule+0x328/0xaf3
Oct 10 11:59:48 localhost kernel:  [e1000_reset_task+0/33]

Thanks
Prasanna


> Jesse
>
>> Signed-off-by: Prasanna S. Panchamukhi<prasanna.panchamukhi@riverbed.com>
>> ---
>>   drivers/net/e1000/e1000.h      |    2 +
>>   drivers/net/e1000/e1000_main.c |   41 +++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
>> index a881dd0..36f55b3 100644
>> --- a/drivers/net/e1000/e1000.h
>> +++ b/drivers/net/e1000/e1000.h
>> @@ -168,6 +168,8 @@ struct e1000_tx_ring {
>>   	unsigned int next_to_clean;
>>   	/* array of buffer information structs */
>>   	struct e1000_buffer *buffer_info;
>> +	spinlock_t bufinfo_lock; /* protect access to buffer_info */
>> +	struct kref bufinfo_refcount; /* refcount access to buffer info */
>>
>>   	u16 tdh;
>>   	u16 tdt;
>> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
>> index beec573..336d3e1 100644
>> --- a/drivers/net/e1000/e1000_main.c
>> +++ b/drivers/net/e1000/e1000_main.c
>> @@ -1531,6 +1531,8 @@ setup_tx_desc_die:
>>
>>   	txdr->next_to_use = 0;
>>   	txdr->next_to_clean = 0;
>> +	spin_lock_init(&txdr->bufinfo_lock);
>> +	kref_init(&txdr->bufinfo_refcount);
>>
>>   	return 0;
>>   }
>> @@ -1880,6 +1882,22 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
>>   	ew32(RCTL, rctl);
>>   }
>>
>> +/*
>> + * Free tx buffer info resources, only when no other thread is
>> + * accessing it. Access to buffer_info is refcounted by bufinfo_refcount,
>> + * hence memory allocated must be destroyed when bufinfo_refcount
>> + * becomes zero. This routine gets executed when bufinfo_refcount
>> + * becomes zero.
>> + */
>> +static void e1000_free_tx_buffer_info(struct kref *ref)
>> +{
>> +	struct e1000_tx_ring *tx_ring =
>> +		container_of(ref, struct e1000_tx_ring, bufinfo_refcount);
>> +
>> +	vfree(tx_ring->buffer_info);
>> +	tx_ring->buffer_info = NULL;
>> +}
>> +
>>   /**
>>    * e1000_free_tx_resources - Free Tx Resources per Queue
>>    * @adapter: board private structure
>> @@ -1895,8 +1913,9 @@ static void e1000_free_tx_resources(struct e1000_adapter *adapter,
>>
>>   	e1000_clean_tx_ring(adapter, tx_ring);
>>
>> -	vfree(tx_ring->buffer_info);
>> -	tx_ring->buffer_info = NULL;
>> +	spin_lock(&tx_ring->bufinfo_lock);
>> +	kref_put(&tx_ring->bufinfo_refcount, e1000_free_tx_buffer_info);
>> +	spin_unlock(&tx_ring->bufinfo_lock);
>>
>>   	dma_free_coherent(&pdev->dev, tx_ring->size, tx_ring->desc,
>>   			  tx_ring->dma);
>> @@ -1954,8 +1973,20 @@ static void e1000_clean_tx_ring(struct e1000_adapter *adapter,
>>   	unsigned long size;
>>   	unsigned int i;
>>
>> -	/* Free all the Tx ring sk_buffs */
>> +	spin_lock(&tx_ring->bufinfo_lock);
>> +	/*
>> +	 * Check buffer_info is not NULL, and
>> +	 * increment the refcount to prevent
>> +	 * the buffer getting freed underneath.
>> +	 */
>> +	if (tx_ring->buffer_info == NULL) {
>> +		spin_unlock(&tx_ring->bufinfo_lock);
>> +		return;
>> +	}
>> +	kref_get(&tx_ring->bufinfo_refcount);
>> +	spin_unlock(&tx_ring->bufinfo_lock);
>>
>> +	/* Free all the Tx ring sk_buffs */
>>   	for (i = 0; i<  tx_ring->count; i++) {
>>   		buffer_info =&tx_ring->buffer_info[i];
>>   		e1000_unmap_and_free_tx_resource(adapter, buffer_info);
>> @@ -1968,6 +1999,10 @@ static void e1000_clean_tx_ring(struct e1000_adapter *adapter,
>>
>>   	memset(tx_ring->desc, 0, tx_ring->size);
>>
>> +	spin_lock(&tx_ring->bufinfo_lock);
>> +	kref_put(&tx_ring->bufinfo_refcount, e1000_free_tx_buffer_info);
>> +	spin_unlock(&tx_ring->bufinfo_lock);
>> +
>>   	tx_ring->next_to_use = 0;
>>   	tx_ring->next_to_clean = 0;
>>   	tx_ring->last_tx_tso = 0;
>>


[-- Attachment #2: Type: text/plain, Size: 429 bytes --]

------------------------------------------------------------------------------
Free Software Download: Index, Search & Analyze Logs and other IT data in 
Real-Time with Splunk. Collect, index and harness all the fast moving IT data 
generated by your applications, servers and devices whether physical, virtual
or in the cloud. Deliver compliance at lower cost and gain new business 
insights. http://p.sf.net/sfu/splunk-dev2dev 

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH net-2.6 1/2] cnic: Prevent status block race conditions with hardware
From: David Miller @ 2011-03-03  0:00 UTC (permalink / raw)
  To: mchan; +Cc: netdev
In-Reply-To: <1299106850-23645-1-git-send-email-mchan@broadcom.com>

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 2 Mar 2011 15:00:49 -0800

> The status block index is used to acknowledge interrupt events and must
> be read before checking for the interrupt events, so we need to add rmb()
> to guarantee that.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-2.6 2/2] cnic: Fix lost interrupt on bnx2x
From: David Miller @ 2011-03-03  0:00 UTC (permalink / raw)
  To: mchan; +Cc: netdev
In-Reply-To: <1299106850-23645-2-git-send-email-mchan@broadcom.com>

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 2 Mar 2011 15:00:50 -0800

> We service 2 queues (kcq1 and kcq2) in cnic_service_bnx2x_bh().  If
> the status block index has changed when servicing the kcq2, we must
> go back and check kcq1.  The latest status block index will be used
> to acknowledge the interrupt, and without looping back to check kcq1,
> we may miss events on kcq1.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

^ permalink raw reply

* [BUG] VPN broken in net-next
From: Stephen Hemminger @ 2011-03-03  0:28 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

My PPTP VPN is broken with net-next tree, it works fine with 2.6.38-rc6
but not in the next tree. Bisected the problem down to the following commit.

Debugging to see which address request is failing with the new code.

9435eb1cf0b76b323019cebf8d16762a50a12a19 is the first bad commit
commit 9435eb1cf0b76b323019cebf8d16762a50a12a19
Author: David S. Miller <davem@davemloft.net>
Date:   Fri Feb 18 12:43:09 2011 -0800

    ipv4: Implement __ip_dev_find using new interface address hash.
    
    Much quicker than going through the FIB tables.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

:040000 040000 265fa1882ee3430f74d59c9b38b6aca0858396d6 28a01a4941f99f7c266fd84f36cc27b933d426c1 M	net

^ permalink raw reply

* Re: [BUG] VPN broken in net-next
From: Stephen Hemminger @ 2011-03-03  0:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20110302162826.59f52c77@nehalam>

On Wed, 2 Mar 2011 16:28:26 -0800
Stephen Hemminger <shemminger@vyatta.com> wrote:

> My PPTP VPN is broken with net-next tree, it works fine with 2.6.38-rc6
> but not in the next tree. Bisected the problem down to the following commit.
> 
> Debugging to see which address request is failing with the new code.
> 
> 9435eb1cf0b76b323019cebf8d16762a50a12a19 is the first bad commit
> commit 9435eb1cf0b76b323019cebf8d16762a50a12a19
> Author: David S. Miller <davem@davemloft.net>
> Date:   Fri Feb 18 12:43:09 2011 -0800
> 
>     ipv4: Implement __ip_dev_find using new interface address hash.
>     
>     Much quicker than going through the FIB tables.
>     
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> :040000 040000 265fa1882ee3430f74d59c9b38b6aca0858396d6 28a01a4941f99f7c266fd84f36cc27b933d426c1 M	net
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

__ip_dev_find is being called with 10.250.0.105 and failing.
At the time the route tables are:

When VPN is up, the following is in the local table:

local 10.250.0.105 dev ppp0  proto kernel  scope host  src 10.250.0.105 

and it isn't getting matched...

In main table the difference is:
+10.255.254.0 dev ppp0  proto kernel  scope link  src 10.250.0.105 
-default via 192.168.1.1 dev eth0  proto static 

^ permalink raw reply


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