Netdev List
 help / color / mirror / Atom feed
* [RFC][PATCH] Fixing SA/SP dumps on netlink/af_key
From: Timo Teräs @ 2008-01-13 12:26 UTC (permalink / raw)
  To: netdev

Hi all,

The problem with IPsec SP/SA dumping is that they don't work well with large SPD/SADB:s. In netlink the dumping code is O(n^2) and can miss some entries/dump duplicates if the DB is changed between the recv() calls to read the dump entries. This is due to the entry index counting done in xfrm_user code. With af_key the dump entries are just dropped when the socket receive queue is filled.

I tried to address all these problems, and added the xfrm_policy and xfrm_state structure to a new linked list that is used solely for dumping. This way the dumps can be done in O(n) and have an iterator point to them. I also modified to af_key to stop dumping when receive queue is filling up and continue it from a callback at recvfrom() when there's more room.

This patch is against 2.6.23 vanilla. I wanted to get some feedback before I make it against the git tree.

But I'd like to hear about:
- if the approach is ok (adding the entries in one more list)?
- if the new/modified variables, structures and function names are ok?
- if I should port these against net-2.6 or net-2.6.25 git tree?
- if I should split this to more than one commit? (maybe xfrm core, xfrm user and af_key parts?)

Cheers,
  Timo

Index: linux-2.6.23/include/net/xfrm.h
===================================================================
--- linux-2.6.23.orig/include/net/xfrm.h	2008-01-13 14:13:21.000000000 +0200
+++ linux-2.6.23/include/net/xfrm.h	2008-01-13 14:13:34.000000000 +0200
@@ -104,6 +104,7 @@
 struct xfrm_state
 {
 	/* Note: bydst is re-used during gc */
+	struct list_head	all;
 	struct hlist_node	bydst;
 	struct hlist_node	bysrc;
 	struct hlist_node	byspi;
@@ -346,6 +347,7 @@
 struct xfrm_policy
 {
 	struct xfrm_policy	*next;
+	struct list_head	bytype;
 	struct hlist_node	bydst;
 	struct hlist_node	byidx;
 
@@ -912,6 +914,17 @@
 	int priority;
 };
 
+struct xfrm_state_walker {
+	struct xfrm_state *state;
+	int count;
+};
+
+struct xfrm_policy_walker {
+	struct xfrm_policy *policy;
+	int count;
+	int type;
+};
+
 extern void xfrm_init(void);
 extern void xfrm4_init(void);
 extern void xfrm6_init(void);
@@ -921,7 +934,15 @@
 extern void xfrm6_state_init(void);
 extern void xfrm6_state_fini(void);
 
-extern int xfrm_state_walk(u8 proto, int (*func)(struct xfrm_state *, int, void*), void *);
+static inline void xfrm_state_walk_start(struct xfrm_state_walker *walk)
+{
+	walk->state = NULL;
+	walk->count = 0;
+}
+
+extern int xfrm_state_walk(struct xfrm_state_walker *walk, u8 proto,
+			   int (*func)(struct xfrm_state *, int, void*), void *);
+extern void xfrm_state_walk_end(struct xfrm_state_walker *walk);
 extern struct xfrm_state *xfrm_state_alloc(void);
 extern struct xfrm_state *xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t *saddr, 
 					  struct flowi *fl, struct xfrm_tmpl *tmpl,
@@ -1025,7 +1046,17 @@
 #endif
 
 struct xfrm_policy *xfrm_policy_alloc(gfp_t gfp);
-extern int xfrm_policy_walk(u8 type, int (*func)(struct xfrm_policy *, int, int, void*), void *);
+
+static inline void xfrm_policy_walk_start(struct xfrm_policy_walker *walk)
+{
+	walk->policy = NULL;
+	walk->count = 0;
+	walk->type = XFRM_POLICY_TYPE_MAIN;
+}
+
+extern int xfrm_policy_walk(struct xfrm_policy_walker *walk, u8 type,
+	int (*func)(struct xfrm_policy *, int, int, void*), void *);
+extern void xfrm_policy_walk_end(struct xfrm_policy_walker *walk);
 int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl);
 struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
 					  struct xfrm_selector *sel,
Index: linux-2.6.23/net/key/af_key.c
===================================================================
--- linux-2.6.23.orig/net/key/af_key.c	2008-01-13 14:13:21.000000000 +0200
+++ linux-2.6.23/net/key/af_key.c	2008-01-13 14:13:34.000000000 +0200
@@ -48,6 +48,16 @@
 	struct sock	sk;
 	int		registered;
 	int		promisc;
+
+	u8              dump_type;
+	uint8_t		dump_msg_version;
+	uint32_t	dump_msg_pid;
+	int		(*dump)(struct pfkey_sock *sk);
+	void		(*dump_done)(struct pfkey_sock *sk);
+	union {
+		struct xfrm_policy_walker	policy;
+		struct xfrm_state_walker	state;
+	} u;
 };
 
 static inline struct pfkey_sock *pfkey_sk(struct sock *sk)
@@ -55,6 +65,30 @@
 	return (struct pfkey_sock *)sk;
 }
 
+static int pfkey_can_dump(struct sock *sk)
+{
+	if (3 * atomic_read(&sk->sk_rmem_alloc) <= 2 * sk->sk_rcvbuf)
+		return 1;
+	return 0;
+}
+
+static int pfkey_do_dump(struct pfkey_sock *pfk)
+{
+	int rc;
+
+	rc = pfk->dump(pfk);
+	if (rc != 0) {
+		if (rc == -ENOBUFS)
+			return 0;
+		return rc;
+	}
+
+	pfk->dump_done(pfk);
+	pfk->dump = NULL;
+	pfk->dump_done = NULL;
+	return 0;
+}
+
 static void pfkey_sock_destruct(struct sock *sk)
 {
 	skb_queue_purge(&sk->sk_receive_queue);
@@ -1705,45 +1739,62 @@
 	return 0;
 }
 
-struct pfkey_dump_data
-{
-	struct sk_buff *skb;
-	struct sadb_msg *hdr;
-	struct sock *sk;
-};
-
 static int dump_sa(struct xfrm_state *x, int count, void *ptr)
 {
-	struct pfkey_dump_data *data = ptr;
+	struct pfkey_sock *pfk = ptr;
 	struct sk_buff *out_skb;
 	struct sadb_msg *out_hdr;
 
+	if (!pfkey_can_dump(&pfk->sk))
+		return -ENOBUFS;
+
 	out_skb = pfkey_xfrm_state2msg(x, 1, 3);
 	if (IS_ERR(out_skb))
 		return PTR_ERR(out_skb);
 
 	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = data->hdr->sadb_msg_version;
+	out_hdr->sadb_msg_version = pfk->dump_msg_version;
 	out_hdr->sadb_msg_type = SADB_DUMP;
 	out_hdr->sadb_msg_satype = pfkey_proto2satype(x->id.proto);
 	out_hdr->sadb_msg_errno = 0;
 	out_hdr->sadb_msg_reserved = 0;
 	out_hdr->sadb_msg_seq = count;
-	out_hdr->sadb_msg_pid = data->hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, data->sk);
+	out_hdr->sadb_msg_pid = pfk->dump_msg_pid;
+	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, &pfk->sk);
 	return 0;
 }
 
+static int pfkey_dump_sa(struct pfkey_sock *pfk)
+{
+	return xfrm_state_walk(&pfk->u.state, pfk->dump_type,
+			       dump_sa, (void *) pfk);
+}
+
+static void pfkey_dump_sa_done(struct pfkey_sock *pfk)
+{
+	xfrm_state_walk_end(&pfk->u.state);
+}
+
 static int pfkey_dump(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
+	struct pfkey_sock *pfk = pfkey_sk(sk);
 	u8 proto;
-	struct pfkey_dump_data data = { .skb = skb, .hdr = hdr, .sk = sk };
+
+	if (pfk->dump != NULL)
+		return -EBUSY;
 
 	proto = pfkey_satype2proto(hdr->sadb_msg_satype);
 	if (proto == 0)
 		return -EINVAL;
 
-	return xfrm_state_walk(proto, dump_sa, &data);
+	pfk->dump_type = proto;
+	pfk->dump_msg_version = hdr->sadb_msg_version;
+	pfk->dump_msg_pid = hdr->sadb_msg_pid;
+	pfk->dump = pfkey_dump_sa;
+	pfk->dump_done = pfkey_dump_sa_done;
+	xfrm_state_walk_start(&pfk->u.state);
+
+	return pfkey_do_dump(pfk);
 }
 
 static int pfkey_promisc(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
@@ -1776,6 +1827,7 @@
 
 static u32 gen_reqid(void)
 {
+	struct xfrm_policy_walker walker;
 	u32 start;
 	static u32 reqid = IPSEC_MANUAL_REQID_MAX;
 
@@ -1784,9 +1836,11 @@
 		++reqid;
 		if (reqid == 0)
 			reqid = IPSEC_MANUAL_REQID_MAX+1;
-		if (xfrm_policy_walk(XFRM_POLICY_TYPE_MAIN, check_reqid,
+		xfrm_policy_walk_start(&walker);
+		if (xfrm_policy_walk(&walker, XFRM_POLICY_TYPE_MAIN, check_reqid,
 				     (void*)&reqid) != -EEXIST)
 			return reqid;
+		xfrm_policy_walk_end(&walker);
 	} while (reqid != start);
 	return 0;
 }
@@ -2634,11 +2688,14 @@
 
 static int dump_sp(struct xfrm_policy *xp, int dir, int count, void *ptr)
 {
-	struct pfkey_dump_data *data = ptr;
+	struct pfkey_sock *pfk = ptr;
 	struct sk_buff *out_skb;
 	struct sadb_msg *out_hdr;
 	int err;
 
+	if (!pfkey_can_dump(&pfk->sk))
+		return -ENOBUFS;
+
 	out_skb = pfkey_xfrm_policy2msg_prep(xp);
 	if (IS_ERR(out_skb))
 		return PTR_ERR(out_skb);
@@ -2648,21 +2705,42 @@
 		return err;
 
 	out_hdr = (struct sadb_msg *) out_skb->data;
-	out_hdr->sadb_msg_version = data->hdr->sadb_msg_version;
+	out_hdr->sadb_msg_version = pfk->dump_msg_version;
 	out_hdr->sadb_msg_type = SADB_X_SPDDUMP;
 	out_hdr->sadb_msg_satype = SADB_SATYPE_UNSPEC;
 	out_hdr->sadb_msg_errno = 0;
 	out_hdr->sadb_msg_seq = count;
-	out_hdr->sadb_msg_pid = data->hdr->sadb_msg_pid;
-	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, data->sk);
+	out_hdr->sadb_msg_pid = pfk->dump_msg_pid;
+	pfkey_broadcast(out_skb, GFP_ATOMIC, BROADCAST_ONE, &pfk->sk);
 	return 0;
 }
 
+static int pfkey_dump_sp(struct pfkey_sock *pfk)
+{
+	return xfrm_policy_walk(&pfk->u.policy, pfk->dump_type,
+				dump_sp, (void *) pfk);
+}
+
+static void pfkey_dump_sp_done(struct pfkey_sock *pfk)
+{
+	xfrm_policy_walk_end(&pfk->u.policy);
+}
+
 static int pfkey_spddump(struct sock *sk, struct sk_buff *skb, struct sadb_msg *hdr, void **ext_hdrs)
 {
-	struct pfkey_dump_data data = { .skb = skb, .hdr = hdr, .sk = sk };
+	struct pfkey_sock *pfk = pfkey_sk(sk);
+
+	if (pfk->dump != NULL)
+		return -EBUSY;
+
+	pfk->dump_type = XFRM_POLICY_TYPE_MAIN;
+	pfk->dump_msg_version = hdr->sadb_msg_version;
+	pfk->dump_msg_pid = hdr->sadb_msg_pid;
+	pfk->dump = pfkey_dump_sp;
+	pfk->dump_done = pfkey_dump_sp_done;
+	xfrm_policy_walk_start(&pfk->u.policy);
 
-	return xfrm_policy_walk(XFRM_POLICY_TYPE_MAIN, dump_sp, &data);
+	return pfkey_do_dump(pfk);
 }
 
 static int key_notify_policy_flush(struct km_event *c)
@@ -3656,6 +3734,7 @@
 			 int flags)
 {
 	struct sock *sk = sock->sk;
+	struct pfkey_sock *pfk = pfkey_sk(sk);
 	struct sk_buff *skb;
 	int copied, err;
 
@@ -3681,6 +3760,10 @@
 
 	sock_recv_timestamp(msg, sk, skb);
 
+	if (pfk->dump != NULL &&
+	    3 * atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf)
+		pfkey_do_dump(pfk);
+
 	err = (flags & MSG_TRUNC) ? skb->len : copied;
 
 out_free:
Index: linux-2.6.23/net/xfrm/xfrm_policy.c
===================================================================
--- linux-2.6.23.orig/net/xfrm/xfrm_policy.c	2008-01-13 14:13:21.000000000 +0200
+++ linux-2.6.23/net/xfrm/xfrm_policy.c	2008-01-13 14:13:34.000000000 +0200
@@ -36,6 +36,7 @@
 
 static DEFINE_RWLOCK(xfrm_policy_lock);
 
+static struct list_head xfrm_policy_bytype[XFRM_POLICY_TYPE_MAX];
 unsigned int xfrm_policy_count[XFRM_POLICY_MAX*2];
 EXPORT_SYMBOL(xfrm_policy_count);
 
@@ -349,6 +350,7 @@
 	policy = kzalloc(sizeof(struct xfrm_policy), gfp);
 
 	if (policy) {
+		INIT_LIST_HEAD(&policy->bytype);
 		INIT_HLIST_NODE(&policy->bydst);
 		INIT_HLIST_NODE(&policy->byidx);
 		rwlock_init(&policy->lock);
@@ -372,6 +374,10 @@
 	if (del_timer(&policy->timer))
 		BUG();
 
+	write_lock_bh(&xfrm_policy_lock);
+	list_del(&policy->bytype);
+	write_unlock_bh(&xfrm_policy_lock);
+
 	security_xfrm_policy_free(policy);
 	kfree(policy);
 }
@@ -710,6 +716,7 @@
 	policy->curlft.use_time = 0;
 	if (!mod_timer(&policy->timer, jiffies + HZ))
 		xfrm_pol_hold(policy);
+	list_add_tail(&policy->bytype, &xfrm_policy_bytype[policy->type]);
 	write_unlock_bh(&xfrm_policy_lock);
 
 	if (delpol)
@@ -952,61 +959,71 @@
 }
 EXPORT_SYMBOL(xfrm_policy_flush);
 
-int xfrm_policy_walk(u8 type, int (*func)(struct xfrm_policy *, int, int, void*),
+int xfrm_policy_walk(struct xfrm_policy_walker *walk, u8 type,
+		     int (*func)(struct xfrm_policy *, int, int, void*),
 		     void *data)
 {
-	struct xfrm_policy *pol, *last = NULL;
-	struct hlist_node *entry;
-	int dir, last_dir = 0, count, error;
+	struct xfrm_policy *old, *pol, *last = NULL;
+	int error = 0;
+
+	if (type >= XFRM_POLICY_TYPE_MAX && type != XFRM_POLICY_TYPE_ANY)
+		return -EINVAL;
+
+	if (walk->policy == NULL && walk->count != 0)
+		return 0;
 
+	old = pol = walk->policy;
+	walk->policy = NULL;
 	read_lock_bh(&xfrm_policy_lock);
-	count = 0;
 
-	for (dir = 0; dir < 2*XFRM_POLICY_MAX; dir++) {
-		struct hlist_head *table = xfrm_policy_bydst[dir].table;
-		int i;
+	for (; walk->type < XFRM_POLICY_TYPE_MAX; walk->type++) {
+		if (type != walk->type && type != XFRM_POLICY_TYPE_ANY)
+			continue;
 
-		hlist_for_each_entry(pol, entry,
-				     &xfrm_policy_inexact[dir], bydst) {
-			if (pol->type != type)
+		if (pol == NULL) {
+			pol = list_first_entry(&xfrm_policy_bytype[walk->type],
+					       struct xfrm_policy, bytype);
+		}
+		list_for_each_entry_from(pol, &xfrm_policy_bytype[walk->type], bytype) {
+			if (pol->dead)
 				continue;
 			if (last) {
-				error = func(last, last_dir % XFRM_POLICY_MAX,
-					     count, data);
-				if (error)
+				error = func(last, xfrm_policy_id2dir(last->index),
+					     walk->count, data);
+				if (error) {
+					xfrm_pol_hold(last);
+					walk->policy = last;
 					goto out;
-			}
-			last = pol;
-			last_dir = dir;
-			count++;
-		}
-		for (i = xfrm_policy_bydst[dir].hmask; i >= 0; i--) {
-			hlist_for_each_entry(pol, entry, table + i, bydst) {
-				if (pol->type != type)
-					continue;
-				if (last) {
-					error = func(last, last_dir % XFRM_POLICY_MAX,
-						     count, data);
-					if (error)
-						goto out;
 				}
-				last = pol;
-				last_dir = dir;
-				count++;
 			}
+			last = pol;
+			walk->count++;
 		}
+		pol = NULL;
 	}
-	if (count == 0) {
+	if (walk->count == 0) {
 		error = -ENOENT;
 		goto out;
 	}
-	error = func(last, last_dir % XFRM_POLICY_MAX, 0, data);
+	if (last)
+		error = func(last, xfrm_policy_id2dir(last->index), 0, data);
 out:
 	read_unlock_bh(&xfrm_policy_lock);
+	if (old != NULL)
+		xfrm_pol_put(old);
 	return error;
 }
 EXPORT_SYMBOL(xfrm_policy_walk);
 
+void xfrm_policy_walk_end(struct xfrm_policy_walker *walker)
+{
+	if (walker->policy != NULL) {
+		xfrm_pol_put(walker->policy);
+		walker->policy = NULL;
+	}
+}
+EXPORT_SYMBOL(xfrm_policy_walk_end);
+
 /*
  * Find policy to apply to this flow.
  *
@@ -2401,6 +2418,9 @@
 			panic("XFRM: failed to allocate bydst hash\n");
 	}
 
+	for (dir = 0; dir < XFRM_POLICY_TYPE_MAX; dir++)
+		INIT_LIST_HEAD(&xfrm_policy_bytype[dir]);
+
 	INIT_WORK(&xfrm_policy_gc_work, xfrm_policy_gc_task);
 	register_netdevice_notifier(&xfrm_dev_notifier);
 }
Index: linux-2.6.23/net/xfrm/xfrm_state.c
===================================================================
--- linux-2.6.23.orig/net/xfrm/xfrm_state.c	2008-01-13 14:13:21.000000000 +0200
+++ linux-2.6.23/net/xfrm/xfrm_state.c	2008-01-13 14:13:34.000000000 +0200
@@ -50,6 +50,7 @@
  * Main use is finding SA after policy selected tunnel or transport mode.
  * Also, it can be used by ah/esp icmp error handler to find offending SA.
  */
+static LIST_HEAD(xfrm_state_all);
 static struct hlist_head *xfrm_state_bydst __read_mostly;
 static struct hlist_head *xfrm_state_bysrc __read_mostly;
 static struct hlist_head *xfrm_state_byspi __read_mostly;
@@ -319,6 +320,7 @@
 	if (x) {
 		atomic_set(&x->refcnt, 1);
 		atomic_set(&x->tunnel_users, 0);
+		INIT_LIST_HEAD(&x->all);
 		INIT_HLIST_NODE(&x->bydst);
 		INIT_HLIST_NODE(&x->bysrc);
 		INIT_HLIST_NODE(&x->byspi);
@@ -345,6 +347,10 @@
 {
 	BUG_TRAP(x->km.state == XFRM_STATE_DEAD);
 
+	spin_lock_bh(&xfrm_state_lock);
+	list_del(&x->all);
+	spin_unlock_bh(&xfrm_state_lock);
+
 	spin_lock_bh(&xfrm_state_gc_lock);
 	hlist_add_head(&x->bydst, &xfrm_state_gc_list);
 	spin_unlock_bh(&xfrm_state_gc_lock);
@@ -722,6 +728,8 @@
 
 	x->genid = ++xfrm_state_genid;
 
+	list_add_tail(&x->all, &xfrm_state_all);
+
 	h = xfrm_dst_hash(&x->id.daddr, &x->props.saddr,
 			  x->props.reqid, x->props.family);
 	hlist_add_head(&x->bydst, xfrm_state_bydst+h);
@@ -1343,40 +1351,59 @@
 }
 EXPORT_SYMBOL(xfrm_alloc_spi);
 
-int xfrm_state_walk(u8 proto, int (*func)(struct xfrm_state *, int, void*),
+int xfrm_state_walk(struct xfrm_state_walker *walk, u8 proto,
+		    int (*func)(struct xfrm_state *, int, void*),
 		    void *data)
 {
-	int i;
-	struct xfrm_state *x, *last = NULL;
-	struct hlist_node *entry;
-	int count = 0;
+	struct xfrm_state *old, *x, *last = NULL;
 	int err = 0;
 
+	if (walk->state == NULL && walk->count != 0)
+		return 0;
+
+	old = x = walk->state;
+	walk->state = NULL;
 	spin_lock_bh(&xfrm_state_lock);
-	for (i = 0; i <= xfrm_state_hmask; i++) {
-		hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
-			if (!xfrm_id_proto_match(x->id.proto, proto))
-				continue;
-			if (last) {
-				err = func(last, count, data);
-				if (err)
-					goto out;
+	if (x == NULL)
+		x = list_first_entry(&xfrm_state_all, struct xfrm_state, all);
+	list_for_each_entry_from(x, &xfrm_state_all, all) {
+		if (x->km.state == XFRM_STATE_DEAD)
+			continue;
+		if (!xfrm_id_proto_match(x->id.proto, proto))
+			continue;
+		if (last) {
+			err = func(last, walk->count, data);
+			if (err) {
+				xfrm_state_hold(last);
+				walk->state = last;
+				goto out;
 			}
-			last = x;
-			count++;
 		}
+		last = x;
+		walk->count++;
 	}
-	if (count == 0) {
+	if (walk->count == 0) {
 		err = -ENOENT;
 		goto out;
 	}
-	err = func(last, 0, data);
+	if (last)
+		err = func(last, 0, data);
 out:
 	spin_unlock_bh(&xfrm_state_lock);
+	if (old != NULL)
+		xfrm_state_put(old);
 	return err;
 }
 EXPORT_SYMBOL(xfrm_state_walk);
 
+void xfrm_state_walk_end(struct xfrm_state_walker *walk)
+{
+	if (walk->state != NULL) {
+		xfrm_state_put(walk->state);
+		walk->state = NULL;
+	}
+}
+EXPORT_SYMBOL(xfrm_state_walk_end);
 
 void xfrm_replay_notify(struct xfrm_state *x, int event)
 {
Index: linux-2.6.23/net/xfrm/xfrm_user.c
===================================================================
--- linux-2.6.23.orig/net/xfrm/xfrm_user.c	2008-01-13 14:13:21.000000000 +0200
+++ linux-2.6.23/net/xfrm/xfrm_user.c	2008-01-13 14:13:34.000000000 +0200
@@ -572,8 +572,6 @@
 	struct sk_buff *out_skb;
 	u32 nlmsg_seq;
 	u16 nlmsg_flags;
-	int start_idx;
-	int this_idx;
 };
 
 static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
@@ -585,9 +583,6 @@
 	struct nlmsghdr *nlh;
 	unsigned char *b = skb_tail_pointer(skb);
 
-	if (sp->this_idx < sp->start_idx)
-		goto out;
-
 	nlh = NLMSG_PUT(skb, NETLINK_CB(in_skb).pid,
 			sp->nlmsg_seq,
 			XFRM_MSG_NEWSA, sizeof(*p));
@@ -629,8 +624,6 @@
 		RTA_PUT(skb, XFRMA_LASTUSED, sizeof(x->lastused), &x->lastused);
 
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-out:
-	sp->this_idx++;
 	return 0;
 
 nlmsg_failure:
@@ -639,18 +632,23 @@
 	return -1;
 }
 
+static int xfrm_dump_sa_done(struct netlink_callback *cb)
+{
+	struct xfrm_state_walker *walker = (struct xfrm_state_walker *) cb->args;
+	xfrm_state_walk_end(walker);
+	return 0;
+}
+
 static int xfrm_dump_sa(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct xfrm_state_walker *walker = (struct xfrm_state_walker *) cb->args;
 	struct xfrm_dump_info info;
 
 	info.in_skb = cb->skb;
 	info.out_skb = skb;
 	info.nlmsg_seq = cb->nlh->nlmsg_seq;
 	info.nlmsg_flags = NLM_F_MULTI;
-	info.this_idx = 0;
-	info.start_idx = cb->args[0];
-	(void) xfrm_state_walk(0, dump_one_state, &info);
-	cb->args[0] = info.this_idx;
+	(void) xfrm_state_walk(walker, 0, dump_one_state, &info);
 
 	return skb->len;
 }
@@ -669,7 +667,6 @@
 	info.out_skb = skb;
 	info.nlmsg_seq = seq;
 	info.nlmsg_flags = 0;
-	info.this_idx = info.start_idx = 0;
 
 	if (dump_one_state(x, 0, &info)) {
 		kfree_skb(skb);
@@ -1273,9 +1270,6 @@
 	struct nlmsghdr *nlh;
 	unsigned char *b = skb_tail_pointer(skb);
 
-	if (sp->this_idx < sp->start_idx)
-		goto out;
-
 	nlh = NLMSG_PUT(skb, NETLINK_CB(in_skb).pid,
 			sp->nlmsg_seq,
 			XFRM_MSG_NEWPOLICY, sizeof(*p));
@@ -1291,8 +1285,6 @@
 		goto nlmsg_failure;
 
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-out:
-	sp->this_idx++;
 	return 0;
 
 nlmsg_failure:
@@ -1300,21 +1292,26 @@
 	return -1;
 }
 
+static int xfrm_dump_policy_done(struct netlink_callback *cb)
+{
+	struct xfrm_policy_walker *walker = (struct xfrm_policy_walker *) cb->args;
+
+	xfrm_policy_walk_end(walker);
+	return 0;
+}
+
 static int xfrm_dump_policy(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct xfrm_policy_walker *walker = (struct xfrm_policy_walker *) cb->args;
 	struct xfrm_dump_info info;
 
 	info.in_skb = cb->skb;
 	info.out_skb = skb;
 	info.nlmsg_seq = cb->nlh->nlmsg_seq;
 	info.nlmsg_flags = NLM_F_MULTI;
-	info.this_idx = 0;
-	info.start_idx = cb->args[0];
-	(void) xfrm_policy_walk(XFRM_POLICY_TYPE_MAIN, dump_one_policy, &info);
-#ifdef CONFIG_XFRM_SUB_POLICY
-	(void) xfrm_policy_walk(XFRM_POLICY_TYPE_SUB, dump_one_policy, &info);
-#endif
-	cb->args[0] = info.this_idx;
+
+	(void) xfrm_policy_walk(walker, XFRM_POLICY_TYPE_ANY,
+				dump_one_policy, &info);
 
 	return skb->len;
 }
@@ -1334,7 +1331,6 @@
 	info.out_skb = skb;
 	info.nlmsg_seq = seq;
 	info.nlmsg_flags = 0;
-	info.this_idx = info.start_idx = 0;
 
 	if (dump_one_policy(xp, dir, 0, &info) < 0) {
 		kfree_skb(skb);
@@ -1951,15 +1947,18 @@
 static struct xfrm_link {
 	int (*doit)(struct sk_buff *, struct nlmsghdr *, struct rtattr **);
 	int (*dump)(struct sk_buff *, struct netlink_callback *);
+	int (*done)(struct netlink_callback *);
 } xfrm_dispatch[XFRM_NR_MSGTYPES] = {
 	[XFRM_MSG_NEWSA       - XFRM_MSG_BASE] = { .doit = xfrm_add_sa        },
 	[XFRM_MSG_DELSA       - XFRM_MSG_BASE] = { .doit = xfrm_del_sa        },
 	[XFRM_MSG_GETSA       - XFRM_MSG_BASE] = { .doit = xfrm_get_sa,
-						   .dump = xfrm_dump_sa       },
+						   .dump = xfrm_dump_sa,
+						   .done = xfrm_dump_sa_done  },
 	[XFRM_MSG_NEWPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_add_policy    },
 	[XFRM_MSG_DELPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_get_policy    },
 	[XFRM_MSG_GETPOLICY   - XFRM_MSG_BASE] = { .doit = xfrm_get_policy,
-						   .dump = xfrm_dump_policy   },
+						   .dump = xfrm_dump_policy,
+						   .done = xfrm_dump_policy_done },
 	[XFRM_MSG_ALLOCSPI    - XFRM_MSG_BASE] = { .doit = xfrm_alloc_userspi },
 	[XFRM_MSG_ACQUIRE     - XFRM_MSG_BASE] = { .doit = xfrm_add_acquire   },
 	[XFRM_MSG_EXPIRE      - XFRM_MSG_BASE] = { .doit = xfrm_add_sa_expire },
@@ -1998,7 +1997,7 @@
 		if (link->dump == NULL)
 			return -EINVAL;
 
-		return netlink_dump_start(xfrm_nl, skb, nlh, link->dump, NULL);
+		return netlink_dump_start(xfrm_nl, skb, nlh, link->dump, link->done);
 	}
 
 	memset(xfrma, 0, sizeof(xfrma));
Index: linux-2.6.23/include/linux/xfrm.h
===================================================================
--- linux-2.6.23.orig/include/linux/xfrm.h	2007-10-09 23:31:38.000000000 +0300
+++ linux-2.6.23/include/linux/xfrm.h	2008-01-13 14:13:35.000000000 +0200
@@ -106,7 +106,8 @@
 {
 	XFRM_POLICY_TYPE_MAIN	= 0,
 	XFRM_POLICY_TYPE_SUB	= 1,
-	XFRM_POLICY_TYPE_MAX	= 2
+	XFRM_POLICY_TYPE_MAX	= 2,
+	XFRM_POLICY_TYPE_ANY	= 255
 };
 
 enum


^ permalink raw reply

* [AX25]: sparse cleanups
From: Eric Dumazet @ 2008-01-13 11:01 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

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

net/ax25/ax25_route.c:251:13: warning: context imbalance in 
'ax25_rt_seq_start' - wrong count at exit
net/ax25/ax25_route.c:276:13: warning: context imbalance in 'ax25_rt_seq_stop' 
- unexpected unlock
net/ax25/ax25_std_timer.c:65:25: warning: expensive signed divide
net/ax25/ax25_uid.c:46:1: warning: symbol 'ax25_uid_list' was not declared. 
Should it be static?
net/ax25/ax25_uid.c:146:13: warning: context imbalance in 'ax25_uid_seq_start' 
- wrong count at exit
net/ax25/ax25_uid.c:169:13: warning: context imbalance in 'ax25_uid_seq_stop' 
- unexpected unlock
net/ax25/af_ax25.c:573:28: warning: expensive signed divide
net/ax25/af_ax25.c:1865:13: warning: context imbalance in 'ax25_info_start' - 
wrong count at exit
net/ax25/af_ax25.c:1888:13: warning: context imbalance in 'ax25_info_stop' - 
unexpected unlock
net/ax25/ax25_ds_timer.c:133:25: warning: expensive signed divide

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

  net/ax25/af_ax25.c        |    4 +++-
  net/ax25/ax25_ds_timer.c  |    2 +-
  net/ax25/ax25_route.c     |    2 ++
  net/ax25/ax25_std_timer.c |    4 ++--
  net/ax25/ax25_uid.c       |    6 ++++--
  5 files changed, 12 insertions(+), 6 deletions(-)


[-- Attachment #2: ax25.patch --]
[-- Type: text/plain, Size: 3574 bytes --]

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index a028d37..1bc0e85 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -570,7 +570,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 			res = -EINVAL;
 			break;
 		}
-		ax25->rtt = (opt * HZ) / 2;
+		ax25->rtt = (opt * HZ) >> 1;
 		ax25->t1  = opt * HZ;
 		break;
 
@@ -1863,6 +1863,7 @@ static int ax25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 #ifdef CONFIG_PROC_FS
 
 static void *ax25_info_start(struct seq_file *seq, loff_t *pos)
+	__acquires(ax25_list_lock)
 {
 	struct ax25_cb *ax25;
 	struct hlist_node *node;
@@ -1886,6 +1887,7 @@ static void *ax25_info_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void ax25_info_stop(struct seq_file *seq, void *v)
+	__releases(ax25_list_lock)
 {
 	spin_unlock_bh(&ax25_list_lock);
 }
diff --git a/net/ax25/ax25_ds_timer.c b/net/ax25/ax25_ds_timer.c
index 4f44185..c4e3b02 100644
--- a/net/ax25/ax25_ds_timer.c
+++ b/net/ax25/ax25_ds_timer.c
@@ -130,7 +130,7 @@ void ax25_ds_heartbeat_expiry(ax25_cb *ax25)
 		 */
 		if (sk != NULL) {
 			if (atomic_read(&sk->sk_rmem_alloc) <
-			    (sk->sk_rcvbuf / 2) &&
+			    (sk->sk_rcvbuf >> 1) &&
 			    (ax25->condition & AX25_COND_OWN_RX_BUSY)) {
 				ax25->condition &= ~AX25_COND_OWN_RX_BUSY;
 				ax25->condition &= ~AX25_COND_ACK_PENDING;
diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
index 9ecf6f1..38c7f30 100644
--- a/net/ax25/ax25_route.c
+++ b/net/ax25/ax25_route.c
@@ -249,6 +249,7 @@ int ax25_rt_ioctl(unsigned int cmd, void __user *arg)
 #ifdef CONFIG_PROC_FS
 
 static void *ax25_rt_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(ax25_route_lock)
 {
 	struct ax25_route *ax25_rt;
 	int i = 1;
@@ -274,6 +275,7 @@ static void *ax25_rt_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void ax25_rt_seq_stop(struct seq_file *seq, void *v)
+	__releases(ax25_route_lock)
 {
 	read_unlock(&ax25_route_lock);
 }
diff --git a/net/ax25/ax25_std_timer.c b/net/ax25/ax25_std_timer.c
index f2f6918..96e4b92 100644
--- a/net/ax25/ax25_std_timer.c
+++ b/net/ax25/ax25_std_timer.c
@@ -32,7 +32,7 @@
 
 void ax25_std_heartbeat_expiry(ax25_cb *ax25)
 {
-	struct sock *sk=ax25->sk;
+	struct sock *sk = ax25->sk;
 
 	if (sk)
 		bh_lock_sock(sk);
@@ -62,7 +62,7 @@ void ax25_std_heartbeat_expiry(ax25_cb *ax25)
 		 */
 		if (sk != NULL) {
 			if (atomic_read(&sk->sk_rmem_alloc) <
-			    (sk->sk_rcvbuf / 2) &&
+			    (sk->sk_rcvbuf >> 1) &&
 			    (ax25->condition & AX25_COND_OWN_RX_BUSY)) {
 				ax25->condition &= ~AX25_COND_OWN_RX_BUSY;
 				ax25->condition &= ~AX25_COND_ACK_PENDING;
diff --git a/net/ax25/ax25_uid.c b/net/ax25/ax25_uid.c
index ce0b13d..5f4eb73 100644
--- a/net/ax25/ax25_uid.c
+++ b/net/ax25/ax25_uid.c
@@ -43,10 +43,10 @@
  *	Callsign/UID mapper. This is in kernel space for security on multi-amateur machines.
  */
 
-HLIST_HEAD(ax25_uid_list);
+static HLIST_HEAD(ax25_uid_list);
 static DEFINE_RWLOCK(ax25_uid_lock);
 
-int ax25_uid_policy = 0;
+int ax25_uid_policy;
 
 EXPORT_SYMBOL(ax25_uid_policy);
 
@@ -144,6 +144,7 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 #ifdef CONFIG_PROC_FS
 
 static void *ax25_uid_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(ax25_uid_lock)
 {
 	struct ax25_uid_assoc *pt;
 	struct hlist_node *node;
@@ -167,6 +168,7 @@ static void *ax25_uid_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void ax25_uid_seq_stop(struct seq_file *seq, void *v)
+	__releases(ax25_uid_lock)
 {
 	read_unlock(&ax25_uid_lock);
 }

^ permalink raw reply related

* [X25]: Avoid divides and sparse warnings
From: Eric Dumazet @ 2008-01-13 10:33 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

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

   CHECK   net/x25/af_x25.c
net/x25/af_x25.c:117:46: warning: expensive signed divide
   CHECK   net/x25/x25_facilities.c
net/x25/x25_facilities.c:209:30: warning: expensive signed divide
   CHECK   net/x25/x25_in.c
net/x25/x25_in.c:250:26: warning: expensive signed divide
   CHECK   net/x25/x25_proc.c
net/x25/x25_proc.c:48:11: warning: context imbalance in 'x25_seq_route_start' 
- wrong count at exit
net/x25/x25_proc.c:72:13: warning: context imbalance in 'x25_seq_route_stop' - 
unexpected unlock
net/x25/x25_proc.c:112:11: warning: context imbalance in 
'x25_seq_socket_start' - wrong count at exit
net/x25/x25_proc.c:129:13: warning: context imbalance in 'x25_seq_socket_stop' 
- unexpected unlock
net/x25/x25_proc.c:190:11: warning: context imbalance in 
'x25_seq_forward_start' - wrong count at exit
net/x25/x25_proc.c:215:13: warning: context imbalance in 
'x25_seq_forward_stop' - unexpected unlock
   CHECK   net/x25/x25_subr.c
net/x25/x25_subr.c:362:57: warning: expensive signed divide

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

  net/x25/af_x25.c         |    4 ++--
  net/x25/x25_facilities.c |    4 +---
  net/x25/x25_in.c         |    2 +-
  net/x25/x25_proc.c       |    6 ++++++
  net/x25/x25_subr.c       |    2 +-
  5 files changed, 11 insertions(+), 7 deletions(-)


[-- Attachment #2: x25.patch --]
[-- Type: text/plain, Size: 3081 bytes --]

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 92cfe8e..07fad7c 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -83,9 +83,9 @@ struct compat_x25_subscrip_struct {
 int x25_addr_ntoa(unsigned char *p, struct x25_address *called_addr,
 		  struct x25_address *calling_addr)
 {
-	int called_len, calling_len;
+	unsigned int called_len, calling_len;
 	char *called, *calling;
-	int i;
+	unsigned int i;
 
 	called_len  = (*p >> 0) & 0x0F;
 	calling_len = (*p >> 4) & 0x0F;
diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index dec404a..a21f664 100644
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -205,9 +205,7 @@ int x25_create_facilities(unsigned char *buffer,
 	}
 
 	if (dte_facs->calling_len && (facil_mask & X25_MASK_CALLING_AE)) {
-		unsigned bytecount = (dte_facs->calling_len % 2) ?
-					dte_facs->calling_len / 2 + 1 :
-					dte_facs->calling_len / 2;
+		unsigned bytecount = (dte_facs->calling_len + 1) >> 1;
 		*p++ = X25_FAC_CALLING_AE;
 		*p++ = 1 + bytecount;
 		*p++ = dte_facs->calling_len;
diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
index 1c88762..7d7c3ab 100644
--- a/net/x25/x25_in.c
+++ b/net/x25/x25_in.c
@@ -247,7 +247,7 @@ static int x25_state3_machine(struct sock *sk, struct sk_buff *skb, int frametyp
 					break;
 				}
 				if (atomic_read(&sk->sk_rmem_alloc) >
-				    (sk->sk_rcvbuf / 2))
+				    (sk->sk_rcvbuf >> 1))
 					x25->condition |= X25_COND_OWN_RX_BUSY;
 			}
 			/*
diff --git a/net/x25/x25_proc.c b/net/x25/x25_proc.c
index 7d55e50..78b0534 100644
--- a/net/x25/x25_proc.c
+++ b/net/x25/x25_proc.c
@@ -41,6 +41,7 @@ found:
 }
 
 static void *x25_seq_route_start(struct seq_file *seq, loff_t *pos)
+	__acquires(x25_route_list_lock)
 {
 	loff_t l = *pos;
 
@@ -70,6 +71,7 @@ out:
 }
 
 static void x25_seq_route_stop(struct seq_file *seq, void *v)
+	__releases(x25_route_list_lock)
 {
 	read_unlock_bh(&x25_route_list_lock);
 }
@@ -105,6 +107,7 @@ found:
 }
 
 static void *x25_seq_socket_start(struct seq_file *seq, loff_t *pos)
+	__acquires(x25_list_lock)
 {
 	loff_t l = *pos;
 
@@ -127,6 +130,7 @@ out:
 }
 
 static void x25_seq_socket_stop(struct seq_file *seq, void *v)
+	__releases(x25_list_lock)
 {
 	read_unlock_bh(&x25_list_lock);
 }
@@ -183,6 +187,7 @@ found:
 }
 
 static void *x25_seq_forward_start(struct seq_file *seq, loff_t *pos)
+	__acquires(x25_forward_list_lock)
 {
 	loff_t l = *pos;
 
@@ -213,6 +218,7 @@ out:
 }
 
 static void x25_seq_forward_stop(struct seq_file *seq, void *v)
+	__releases(x25_forward_list_lock)
 {
 	read_unlock_bh(&x25_forward_list_lock);
 }
diff --git a/net/x25/x25_subr.c b/net/x25/x25_subr.c
index 8d6220a..511a598 100644
--- a/net/x25/x25_subr.c
+++ b/net/x25/x25_subr.c
@@ -359,7 +359,7 @@ void x25_check_rbuf(struct sock *sk)
 {
 	struct x25_sock *x25 = x25_sk(sk);
 
-	if (atomic_read(&sk->sk_rmem_alloc) < (sk->sk_rcvbuf / 2) &&
+	if (atomic_read(&sk->sk_rmem_alloc) < (sk->sk_rcvbuf >> 1) &&
 	    (x25->condition & X25_COND_OWN_RX_BUSY)) {
 		x25->condition &= ~X25_COND_OWN_RX_BUSY;
 		x25->condition &= ~X25_COND_ACK_PENDING;

^ permalink raw reply related

* Re: [PATCH] Add me as maintainer of the RDC r6040 driver
From: Florian Fainelli @ 2008-01-13 10:28 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, Francois Romieu
In-Reply-To: <47893DB6.3080201@garzik.org>

Hi Jeff,

Le samedi 12 janvier 2008, Jeff Garzik a écrit :
> applied

Thank you. I think you will get this change twice when you pull Francoi's 
netdev-2.6 repository which has the r6040 patches I sent already.

-- 
Cordialement, Florian Fainelli
------------------------------

^ permalink raw reply

* Re: sky2 driver stopped working after upgrading from 2.6.18-4
From: Andrew Paprocki @ 2008-01-13 10:26 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger
In-Reply-To: <76366b180801130024j78225c66j17974245e86a183f@mail.gmail.com>

It appears this problem might be caused by the system HPET timer not
functioning properly. I was noticing strange behavior in my rc0.d
shutdown scripts where the script would freeze in a 'sleep 1'. Sure
enough, even running a 'sleep 1' form a normal prompt just made the
system hang indefinitely (until I hit ctrl-C). If I unload sky2 and
then install a different clock source (jiffies, tsc, etc) and reload
the module, it all appears to work properly. This is very, very
strange. I'll bring this up on the kernel mailing list in a different
post focusing on the HPET config.

Thanks,
-Andrew

On Jan 13, 2008 3:24 AM, Andrew Paprocki <andrew@ishiboo.com> wrote:
> I have never been able to get my Yukon-EC Ultra (0xb4) rev 2 working
> on anything past 2.6.18-4 that I've tried. (All ~.20+)

^ permalink raw reply

* Re: [FIB]: removes a memset() call in tnode_new()
From: David Miller @ 2008-01-13  8:43 UTC (permalink / raw)
  To: dada1; +Cc: netdev
In-Reply-To: <4789BF2B.3040103@cosmosbay.com>

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Sun, 13 Jan 2008 08:35:07 +0100

> tnode_alloc() already clears allocated memory, using kcalloc()
> or alloc_pages(GFP_KERNEL|__GFP_ZERO, ...)
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Applied, thanks Eric.

^ permalink raw reply

* sky2 driver stopped working after upgrading from 2.6.18-4
From: Andrew Paprocki @ 2008-01-13  8:24 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

I have never been able to get my Yukon-EC Ultra (0xb4) rev 2 working
on anything past 2.6.18-4 that I've tried. (All ~.20+)

Below is the debug output from the driver on both versions. Notice the
'phy interrupt status' that each version prints out in case it is
involved in the problem:

2.6.18-4:  sky2 eth0: phy interrupt status 0x1c40 0xbf48
2.6.23.13: sky2 eth0: phy interrupt status 0x1c40 0xbf08

Things I've tried:
- disabling MSI
- building git head (2.4.24-rc7) to get the latest driver (same problem)
- manually changing some sky2.c code back to 18-4 code to see if I
could produce different behavior

Things I see:
- the right-hand green LED never lights up using the new code, it
blinks under the old code
- when I manually config an IP and ping, it reports a single packet
response w/ 0.000ms time and then nothing else

I added a printk into gm_phy_write() and this is what prints out under
2.6.23.13:

ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 18 (level, low) -> IRQ 20
PCI: Setting latency timer of device 0000:02:00.0 to 64
sky2 0000:02:00.0: v1.18 addr 0xfddfc000 irq 20 Yukon-EC Ultra (0xb4) rev 2
<NULL>: reg 0x0012 val 0x0000
sky2 eth0: addr 00:0a:48:00:0a:53
sky2 eth0: enabling interface
eth0: reg 0x0010 val 0x2860
eth0: reg 0x0009 val 0x0300
eth0: reg 0x0004 val 0x0de1
eth0: reg 0x0000 val 0x9200
eth0: reg 0x0016 val 0x0003
eth0: reg 0x0010 val 0x1877
eth0: reg 0x0012 val 0x4100
eth0: reg 0x0016 val 0x0000
eth0: reg 0x0016 val 0x00ff
eth0: reg 0x0018 val 0xaa99
eth0: reg 0x0017 val 0x2011
eth0: reg 0x0018 val 0xa204
eth0: reg 0x0017 val 0x2002
eth0: reg 0x0016 val 0x0000
eth0: reg 0x0012 val 0x0800
sky2 eth0: phy interrupt status 0x1c40 0xbf48
eth0: reg 0x0012 val 0x6400
eth0: reg 0x0016 val 0x0003
eth0: reg 0x0010 val 0x1007
eth0: reg 0x0016 val 0x0000
sky2 eth0: Link is up at 1000 Mbps, full duplex, flow control both

Please let me know if there is any more useful information I can dig up.

Thanks,
-Andrew

------------------------------------------------------------------------

Linux 2.6.18-4

$ modprobe -r sky2
sky2 eth0: disabling interface
ACPI: PCI interrupt for device 0000:02:00.0 disabled

$ modprobe sky2 debug=16
PCI: Enabling device 0000:02:00.0 (0000 -> 0003)
ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 18 (level, low) -> IRQ 201
PCI: Setting latency timer of device 0000:02:00.0 to 64
sky2 v1.5 addr 0xfddfc000 irq 201 Yukon-EC Ultra (0xb4) rev 2
sky2 eth0: addr 00:0a:48:00:0a:53
sky2 eth0: enabling interface
ADDRCONF(NETDEV_UP): eth0: link is not ready
sky2 eth0: phy interrupt status 0x1c40 0xbf48
sky2 eth0: Link is up at 1000 Mbps, full duplex, flow control both
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
eth0: tx queued, slot 0, len 90
eth0: tx done, up to 1
eth0: tx queued, slot 1, len 342
eth0: tx done, up to 2
sky2 eth0: rx slot 0 status 0x1560100 len 342
eth0: tx queued, slot 2, len 42
sky2 eth0: rx slot 1 status 0x3c0100 len 60
eth0: tx queued, slot 3, len 81
eth0: tx done, up to 3
sky2 eth0: rx slot 2 status 0x6d0100 len 109
eth0: tx done, up to 5
eth0: tx queued, slot 5, len 81
sky2 eth0: rx slot 3 status 0xbd0100 len 189
eth0: tx done, up to 7
... <continues for pages>

------------------------------------------------------------------------

Linux 2.6.23.13

$ modprobe -r sky2
sky2 eth0: disabling interface
ACPI: PCI interrupt for device 0000:02:00.0 disabled

$ modprobe sky2 debug=16
ACPI: PCI Interrupt 0000:02:00.0[A] -> GSI 18 (level, low) -> IRQ 20
PCI: Setting latency timer of device 0000:02:00.0 to 64
sky2 0000:02:00.0: v1.18 addr 0xfddfc000 irq 20 Yukon-EC Ultra (0xb4) rev 2
sky2 eth0: addr 00:0a:48:00:0a:53
sky2 eth0: enabling interface
sky2 eth0: phy interrupt status 0x1c40 0xbf08
sky2 eth0: Link is up at 1000 Mbps, full duplex, flow control both
sky2 eth0: rx slot 0 status 0x3c0300 len 60
sky2 eth0: rx slot 1 status 0xf30300 len 243
sky2 eth0: rx slot 2 status 0xf30300 len 243
sky2 eth0: rx slot 3 status 0x3c0300 len 60
sky2 eth0: rx slot 4 status 0x3c0300 len 60
... <hangs here, nothing else prints out from sky2>

------------------------------------------------------------------------

(printed from under 2.6.18-4)
# lspci -vv -s 02:00.0
02:00.0 Ethernet controller: Marvell Technology Group Ltd. Unknown
device 4364 (rev 12)
        Subsystem: Marvell Technology Group Ltd. Unknown device 5621
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR-
        Latency: 0, Cache Line Size: 4 bytes
        Interrupt: pin A routed to IRQ 225
        Region 0: Memory at fddfc000 (64-bit, non-prefetchable) [size=16K]
        Region 2: I/O ports at ee00 [size=256]
        [virtual] Expansion ROM at fdc00000 [disabled] [size=128K]
        Capabilities: [48] Power Management version 3
                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA
PME(D0+,D1+,D2+,D3hot+,D3cold+)
                Status: D0 PME-Enable- DSel=0 DScale=1 PME-
        Capabilities: [50] Vital Product Data
        Capabilities: [5c] Message Signalled Interrupts: Mask- 64bit+
Queue=0/0 Enable+
                Address: 00000000fee00000  Data: 40e1
        Capabilities: [e0] Express Legacy Endpoint IRQ 0
                Device: Supported: MaxPayload 128 bytes, PhantFunc 0, ExtTag-
                Device: Latency L0s unlimited, L1 unlimited
                Device: AtnBtn- AtnInd- PwrInd-
                Device: Errors: Correctable- Non-Fatal- Fatal- Unsupported-
                Device: RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
                Device: MaxPayload 128 bytes, MaxReadReq 512 bytes
                Link: Supported Speed 2.5Gb/s, Width x1, ASPM L0s L1, Port 3
                Link: Latency L0s <256ns, L1 unlimited
                Link: ASPM Disabled RCB 128 bytes CommClk+ ExtSynch-
                Link: Speed 2.5Gb/s, Width x1
        Capabilities: [100] Advanced Error Reporting

^ permalink raw reply

* 2.6.24-rc6-mm1 - oddness with IPv4/v6 mapped sockets hanging...
From: Valdis.Kletnieks @ 2008-01-13  7:35 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, netdev

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

I'm seeing problems with Sendmail on 24-rc6-mm1, where the main Sendmail is
listening on ::1/25, and Fetchmail connects to 127.0.0.1:25 to inject mail it
has just fetched from an outside server via IMAP - it will often just hang and
not make any further progress. Looking at netstat shows something interesting:

% netstat -n -a -A inet | grep 25
tcp        0   5108 127.0.0.1:59355             127.0.0.1:25                ESTABLISHED 
% netstat -n -a -A inet6 | grep 25
tcp        0      0 :::25                       :::*                        LISTEN      
tcp        0      0 ::ffff:127.0.0.1:25         ::ffff:127.0.0.1:59355      ESTABLISHED 
% netstat -n -a -A inet | grep 25
tcp        0   5108 127.0.0.1:59355             127.0.0.1:25                ESTABLISHED 
% netstat -n -a -A inet6 | grep 25
tcp        0      0 :::25                       :::*                        LISTEN      
tcp        0      0 ::ffff:127.0.0.1:25         ::ffff:127.0.0.1:59355      ESTABLISHED 
% netstat -n -a -A inet | grep 25
tcp        0   5108 127.0.0.1:59355             127.0.0.1:25                ESTABLISHED 
% netstat -n -a -A inet6 | grep 25
tcp        0      0 :::25                       :::*                        LISTEN      
tcp        0      0 ::ffff:127.0.0.1:25         ::ffff:127.0.0.1:59355      ESTABLISHED 

On the IPv4 side, it thinks it's got 5108 bytes in the send queue - but on
the IPv6 side of that same connection, it's showing 0 in the receive queue,
and we're stuck there.

It's not consistent - sometimes Fetchmail will wedge on the very first mail,
and do so several times in a row.  Other times, it will do well for a while -
at the moment, it's gone through 471 of the 1,470 currently queued mails just
fine, only to get wedged again on number 472.

For what it's worth, here's what 'echo w > /proc/sysrq-trigger' got, although I
don't see anything that looks odd to me given the netstat output above -
procmail has sent data, and is waiting for a response back, and sendmail is
waiting for data to arrive:

fetchmail     S ffffffff8053c520  5360 17612   9902
 ffff81007d37bb08 0000000000000086 0000000000000000 000200d000000000
 ffff81006bf826c0 ffffffff80687360 ffff81006bf82918 0000000000000001
 0000000000000003 ffff81007d37bb88 0000000000000000 0000000000000000
Call Trace:
 [<ffffffff80522682>] schedule_timeout+0x22/0xb4
 [<ffffffff80523bd3>] _spin_lock_bh+0x11/0x38
 [<ffffffff80523ac4>] _spin_unlock_bh+0x1e/0x20
 [<ffffffff8047cec6>] release_sock+0xa3/0xac
 [<ffffffff8047d98f>] sk_wait_data+0x8a/0xcf
 [<ffffffff80249b99>] autoremove_wake_function+0x0/0x38
 [<ffffffff804abdca>] tcp_recvmsg+0x35a/0x86b
 [<ffffffff8047c7be>] sock_common_recvmsg+0x32/0x47
 [<ffffffff803288be>] selinux_socket_recvmsg+0x1d/0x1f
 [<ffffffff8047af38>] sock_recvmsg+0x10e/0x12f
 [<ffffffff80249b99>] autoremove_wake_function+0x0/0x38
 [<ffffffff8032425d>] avc_has_perm+0x4c/0x5e
 [<ffffffff803ac952>] pty_write+0x3a/0x44
 [<ffffffff80249dd8>] remove_wait_queue+0x2f/0x3b
 [<ffffffff8047c06b>] sys_recvfrom+0xa4/0xf5
 [<ffffffff8024c850>] hrtimer_start+0x11f/0x131
 [<ffffffff8023aa6e>] do_setitimer+0x184/0x326
 [<ffffffff8020c03b>] system_call_after_swapgs+0x7b/0x80

sendmail      S ffff81007d30a400  5360 17613  16992
 ffff81006bc419e8 0000000000000086 ffff81006bc41998 ffffffff8023f6a5
 ffff81007d30a400 ffff81007d24f200 ffff81007d30a658 0000000100000286
 ffff81006bc419e8 ffffffff8023f851 000000004789b768 ffff81000100eb20
Call Trace:
 [<ffffffff8023f6a5>] lock_timer_base+0x26/0x4a
 [<ffffffff8023f851>] __mod_timer+0xc4/0xd6
 [<ffffffff805226ed>] schedule_timeout+0x8d/0xb4
 [<ffffffff8023f37c>] process_timeout+0x0/0xb
 [<ffffffff805226e8>] schedule_timeout+0x88/0xb4
 [<ffffffff8029cd26>] do_select+0x4a9/0x50b
 [<ffffffff8029d22d>] __pollwait+0x0/0xdf
 [<ffffffff8022d7b9>] default_wake_function+0x0/0xf
 [<ffffffff80523bd3>] _spin_lock_bh+0x11/0x38
 [<ffffffff8047cf74>] lock_sock_nested+0xa5/0xb2
 [<ffffffff80523bd3>] _spin_lock_bh+0x11/0x38
 [<ffffffff80523ac4>] _spin_unlock_bh+0x1e/0x20
 [<ffffffff8047cec6>] release_sock+0xa3/0xac
 [<ffffffff804ac1c9>] tcp_recvmsg+0x759/0x86b
 [<ffffffff8047c7be>] sock_common_recvmsg+0x32/0x47
 [<ffffffff803288be>] selinux_socket_recvmsg+0x1d/0x1f
 [<ffffffff8047a924>] sock_aio_read+0x121/0x139
 [<ffffffff8032425d>] avc_has_perm+0x4c/0x5e
 [<ffffffff8029cf7a>] core_sys_select+0x1f2/0x2a0
 [<ffffffff80282f50>] page_add_new_anon_rmap+0x20/0x22
 [<ffffffff803251f5>] file_has_perm+0xa5/0xb4
 [<ffffffff80249b99>] autoremove_wake_function+0x0/0x38
 [<ffffffff8029d45c>] sys_select+0x150/0x17b
 [<ffffffff8020c03b>] system_call_after_swapgs+0x7b/0x80

Any ideas?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply

* [FIB]: removes a memset() call in tnode_new()
From: Eric Dumazet @ 2008-01-13  7:35 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

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

tnode_alloc() already clears allocated memory, using kcalloc()
or alloc_pages(GFP_KERNEL|__GFP_ZERO, ...)

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>


[-- Attachment #2: fib_trie.patch --]
[-- Type: text/plain, Size: 355 bytes --]

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index a418498..f26ba31 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -391,7 +391,6 @@ static struct tnode* tnode_new(t_key key, int pos, int bits)
 	struct tnode *tn = tnode_alloc(sz);
 
 	if (tn) {
-		memset(tn, 0, sz);
 		tn->parent = T_TNODE;
 		tn->pos = pos;
 		tn->bits = bits;

^ permalink raw reply related

* Re: [PATCH 4/9] statistics improvements
From: David Miller @ 2008-01-13  5:44 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <4789A29C.6080000@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Sat, 12 Jan 2008 21:33:16 -0800

> The size field is added to /proc/net/fib_triestat that was the point.

Not from what I can see.

davem@sunset:~/src/GIT/net-2.6.25$ git grep -e "->size" net/ipv4/fib_trie.c
net/ipv4/fib_trie.c:	t->size++;
net/ipv4/fib_trie.c:	t->size--;
davem@sunset:~/src/GIT/net-2.6.25$ git grep -e "\.size" net/ipv4/fib_trie.c
davem@sunset:~/src/GIT/net-2.6.25$ 

Nothing uses the field, it is merely incremented and decremented.

trie_show_stats() and trie_show_usage() do not access this field.

^ permalink raw reply

* Re: [PATCH] fib_semantics: prevent long hash chains in access server config
From: David Miller @ 2008-01-13  5:38 UTC (permalink / raw)
  To: bcrl; +Cc: netdev
In-Reply-To: <20080112185819.GA12775@kvack.org>

From: Benjamin LaHaise <bcrl@kvack.org>
Date: Sat, 12 Jan 2008 13:58:19 -0500

> This is a patch from a while ago that I'm resending.  Basically, in
> access server configurations, a lot of routes have the same local ip
> address but on different devices.  This fixes the long chains that
> result from not including the device index in the hash.

I'm not applying this for the same reason I didn't apply it last time.

Please listen to the reason this time, and do not resubmit this until
the problem with this patch is resolved.

The fib_dev is an attribute of the first nexthop, ie. the
fib_info->fib_nh[0] member.

There can be multiple nexthops.

It is pointless to salt the hash with one of the nexthop
device indexes if you do not also compare the index in the
hash lookup comparisons.

And guess why we don't do this?  Because it's not part of
the key.  Other aspects of the base fib_info and nexthops
provide the uniqueness, not the devindex of the first hop.

So you'll need to find another way to do this.

^ permalink raw reply

* Re: [PATCH 4/9] statistics improvements
From: Stephen Hemminger @ 2008-01-13  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112.205520.55747078.davem@davemloft.net>

David Miller wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Fri, 11 Jan 2008 22:45:17 -0800
>
>   
>> Turn the unused size field into a useful counter for the number
>> of routes.
>>
>> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>
>>     
>
> It's not useful if nothing reports it's value.  I'm dropping
> this.
>
> Unless you add code to provide this information via procfs
> or similar, better to just remove it.
>   
The size field is added to /proc/net/fib_triestat that was the point.

^ permalink raw reply

* [PATCH v2] Documentation: add a guideline for hard_start_xmit method
From: Matti Linnanvuori @ 2008-01-13  5:33 UTC (permalink / raw)
  To: jgarzik, netdev

From: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
 
Add a guideline for hard_start_xmit method not to
modify SKB.
 
Signed-off-by: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
---
--- a/Documentation/networking/driver.txt 2008-01-13 07:14:01.608291500 +0200
+++ b/Documentation/networking/driver.txt 2008-01-13 07:13:40.535421500 +0200
@@ -61,7 +61,9 @@
 2) Do not forget to update netdev->trans_start to jiffies after
    each new tx packet is given to the hardware.
 
-3) Do not forget that once you return 0 from your hard_start_xmit
+3) A hard_start_xmit method must not modify the shared parts of the SKB.
+
+4) Do not forget that once you return 0 from your hard_start_xmit
    method, it is your driver's responsibility to free up the SKB
    and in some finite amount of time.


      ____________________________________________________________________________________
Looking for last minute shopping deals?  
Find them fast with Yahoo! Search.  http://tools.search.yahoo.com/newsearch/category.php?category=shopping

^ permalink raw reply

* Re: [XFRM]: alg_key_len should be unsigned to avoid integer divides
From: David Miller @ 2008-01-13  5:32 UTC (permalink / raw)
  To: dada1; +Cc: netdev
In-Reply-To: <4788F8FC.3010001@cosmosbay.com>

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Sat, 12 Jan 2008 18:29:32 +0100

> alg_key_len is currently defined as 'signed int'. This unfortunatly leads
> to integer divides in several paths.
> 
> Converting it to unsigned is safe and saves 208 bytes of text on i386.
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Applied.

I realigned the struct members in a follow-on changeset.

commit 7305e737926be49e09718df53f4285bf69cc3755
Author: David S. Miller <davem@davemloft.net>
Date:   Sat Jan 12 21:31:29 2008 -0800

    [XFRM]: Fix struct xfrm_algo code formatting.
    
    Realign struct members.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index f8507ee..9b5b00c 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -91,9 +91,9 @@ struct xfrm_replay_state
 };
 
 struct xfrm_algo {
-	char	alg_name[64];
+	char		alg_name[64];
 	unsigned int	alg_key_len;    /* in bits */
-	char	alg_key[0];
+	char		alg_key[0];
 };
 
 struct xfrm_stats {

^ permalink raw reply related

* Re: [PATCH 2/2] [HTB]: htb_classid is dead static inline
From: David Miller @ 2008-01-13  5:29 UTC (permalink / raw)
  To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <12001388163255-git-send-email-ilpo.jarvinen@helsinki.fi>

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Sat, 12 Jan 2008 13:53:36 +0200

> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] [NET] core/utils.c: digit2bin is dead static inline
From: David Miller @ 2008-01-13  5:28 UTC (permalink / raw)
  To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <1200138816244-git-send-email-ilpo.jarvinen@helsinki.fi>

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Sat, 12 Jan 2008 13:53:35 +0200

> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Applied.

^ permalink raw reply

* Re: [PATCH 9/9] fix sparse warnings
From: David Miller @ 2008-01-13  5:28 UTC (permalink / raw)
  To: stephen.hemminger; +Cc: dada1, robert.olsson, netdev
In-Reply-To: <20080112130946.4d83eca7@deepthought>

From: Stephen Hemminger <stephen.hemminger@vyatta.com>
Date: Sat, 12 Jan 2008 13:09:46 -0800

> On Sat, 12 Jan 2008 12:16:13 +0100
> Eric Dumazet <dada1@cosmosbay.com> wrote:
> 
> > [FIB]: Reduce text size of net/ipv4/fib_trie.o
> > 
> > In struct tnode, we use two fields of 5 bits for 'pos' and 'bits'.
> > Switching to plain 'unsigned char' (8 bits) take the same space
> > because of compiler alignments, and reduce text size by 435 bytes
> > on i386.
> > 
> > On i386 :
> > $ size net/ipv4/fib_trie.o.before_patch net/ipv4/fib_trie.o
> >     text    data     bss     dec     hex filename
> >    13714       4      64   13782    35d6 net/ipv4/fib_trie.o.before
> >    13279       4      64   13347    3423 net/ipv4/fib_trie.o
> > 
> > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
> > 
> 
> I agree they should not have been bitfields in the first place.

Applied.

^ permalink raw reply

* Re: [PATCH 6/8] [NETFILTER] xt_policy.c: kill some bloat
From: David Miller @ 2008-01-13  5:26 UTC (permalink / raw)
  To: kaber; +Cc: ilpo.jarvinen, netdev, netfilter-devel
In-Reply-To: <47890F03.5080503@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 12 Jan 2008 20:03:31 +0100

> David Miller wrote:
> > From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
> > Date: Sat, 12 Jan 2008 11:34:27 +0200
> > 
> > Ilpo, please post netfilter patches to netfilter-devel,
> > CC:'ing Patrick McHardy.
> > 
> > Patrick, please review, thanks.
> 
> This looks fine to me, thanks Ilpo.

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 9/9] fix sparse warnings
From: David Miller @ 2008-01-13  5:25 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112064646.659443238@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:22 -0800

> Make FIB TRIE go through sparse checker without warnings.
> 
> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>

Also applied, thanks.

^ permalink raw reply

* Re: [PATCH 8/9] add statistics
From: David Miller @ 2008-01-13  5:23 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112064646.583836190@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:21 -0800

> The FIB TRIE code has a bunch of statistics, but the code is hidden
> behind an ifdef that was never implemented. Since it was dead code,
> it was broken as well.
> 
> This patch fixes that by making it a config option.
> 
> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 7/9] printk related cleanups
From: David Miller @ 2008-01-13  4:58 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev
In-Reply-To: <20080112064646.507015655@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:20 -0800

> printk related cleanups:
>  * Get rid of unused printk wrappers.
>  * Make bug checks into KERN_WARNING because KERN_DEBUG gets ignored
>  * Turn one cryptic old message into something real
>  * Make sure all messages have KERN_XXX

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 6/9] : fib_insert_node cleanup
From: David Miller @ 2008-01-13  4:57 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112064646.432200237@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:19 -0800

> The only error from fib_insert_node is if memory allocation fails,
> so instead of passing by reference, just use the convention of returning
> NULL.
> 
> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>

I'd be a hypocrite if I didn't apply this one :-)

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 5/9] use %u for unsigned printfs
From: David Miller @ 2008-01-13  4:56 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112064646.356466158@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:18 -0800

> Use %u instead of %d when printing unsigned values.
> 
> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 4/9] statistics improvements
From: David Miller @ 2008-01-13  4:55 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112064646.282104074@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:17 -0800

> Turn the unused size field into a useful counter for the number
> of routes.
> 
> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>

It's not useful if nothing reports it's value.  I'm dropping
this.

Unless you add code to provide this information via procfs
or similar, better to just remove it.

^ permalink raw reply

* Re: [PATCH 3/9] move size information to pr_debug()
From: David Miller @ 2008-01-13  4:53 UTC (permalink / raw)
  To: shemminger; +Cc: robert.olsson, netdev, stephen.hemminger
In-Reply-To: <20080112064646.207183428@linux-foundation.org>

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Fri, 11 Jan 2008 22:45:16 -0800

> The size of structures is a debug thing, not something that needs to
> be part of a /proc api.
> 
> Signed-off-by: Stephen Hemminger <stephen.hemminger@vyatta.com>

I don't necessarily agree with this one, the size of the
structures is an important inherent attribute contributing
directly to memory usage and performance of the algorithms.
So knowing those values are necessary to diagnose problems.

^ 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