Skip to contentSkip to conversation

neighbour table lockless read

RFC In review Stephen Hemminger 3 participants 12 messages Aug 14, 2006 – Aug 15, 2006

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover neighbour table lockless read

This set of patches changes the neighbour table used by ARP and IPV6 neighbour discovery to use RCU and seqlock's. This improves performance by eliminated locked instructions in the common case.

--

#1 lore raw

RFC 1/7 net neighbor: convert top level list to RCU

Change the top level list of neighbor tables to use RCU. Minor change to BUG() if a neighbor table is registered twice.

---
 include/net/neighbour.h |    2 -
 net/core/neighbour.c    |   89 ++++++++++++++++++++++--------------------------
 2 files changed, 42 insertions(+), 49 deletions(-)
net/core/neighbour.c +41 -48
@@ -61,7 +61,7 @@ static void neigh_app_notify(struct neig
 static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
 void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
 
-static struct neigh_table *neigh_tables;
+static LIST_HEAD(neigh_tables);
 #ifdef CONFIG_PROC_FS
 static struct file_operations neigh_stat_seq_fops;
 #endif
@@ -93,11 +93,11 @@ static struct file_operations neigh_stat
    It is supposed, that dev->hard_header is simplistic and does
    not make callbacks to neighbour tables.
 
-   The last lock is neigh_tbl_lock. It is pure SMP lock, protecting
-   list of neighbour tables. This list is used only in process context,
+   The last lock is neigh_tbl_lock. It controls update to the
+   set of neighbor tables. Typically used during initialization
  */
 
-static DEFINE_RWLOCK(neigh_tbl_lock);
+static DEFINE_SPINLOCK(neigh_tbl_lock);
 
 static int neigh_blackhole(struct sk_buff *skb)
 {
@@ -1391,26 +1391,18 @@ void neigh_table_init(struct neigh_table
 	struct neigh_table *tmp;
 
 	neigh_table_init_no_netlink(tbl);
-	write_lock(&neigh_tbl_lock);
-	for (tmp = neigh_tables; tmp; tmp = tmp->next) {
-		if (tmp->family == tbl->family)
-			break;
-	}
-	tbl->next	= neigh_tables;
-	neigh_tables	= tbl;
-	write_unlock(&neigh_tbl_lock);
-
-	if (unlikely(tmp)) {
-		printk(KERN_ERR "NEIGH: Registering multiple tables for "
-		       "family %d\n", tbl->family);
-		dump_stack();
+
+	spin_lock(&neigh_tbl_lock);
+	list_for_each_entry(tmp, &neigh_tables, list) {
+		BUG_ON(tmp->family == tbl->family);
 	}
+
+	list_add_rcu(&tbl->list, &neigh_tables);
+	spin_unlock(&neigh_tbl_lock);
 }
 
 int neigh_table_clear(struct neigh_table *tbl)
 {
-	struct neigh_table **tp;
-
 	/* It is not clean... Fix it to unload IPv6 module safely */
 	del_timer_sync(&tbl->gc_timer);
 	del_timer_sync(&tbl->proxy_timer);
@@ -1418,14 +1410,12 @@ int neigh_table_clear(struct neigh_table
 	neigh_ifdown(tbl, NULL);
 	if (atomic_read(&tbl->entries))
 		printk(KERN_CRIT "neighbour leakage\n");
-	write_lock(&neigh_tbl_lock);
-	for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
-		if (*tp == tbl) {
-			*tp = tbl->next;
-			break;
-		}
-	}
-	write_unlock(&neigh_tbl_lock);
+
+	spin_lock(&neigh_tbl_lock);
+	list_del_rcu(&tbl->list);
+	spin_unlock(&neigh_tbl_lock);
+
+	synchronize_rcu();
 
 	neigh_hash_free(tbl->hash_buckets, tbl->hash_mask + 1);
 	tbl->hash_buckets = NULL;
@@ -1460,13 +1450,12 @@ int neigh_delete(struct sk_buff *skb, st
 		}
 	}
 
-	read_lock(&neigh_tbl_lock);
-	for (tbl = neigh_tables; tbl; tbl = tbl->next) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(tbl, &neigh_tables, list) {
 		struct neighbour *neigh;
 
 		if (tbl->family != ndm->ndm_family)
 			continue;
-		read_unlock(&neigh_tbl_lock);
 
 		if (nla_len(dst_attr) < tbl->key_len)
 			goto out_dev_put;
@@ -1491,12 +1480,12 @@ int neigh_delete(struct sk_buff *skb, st
 		neigh_release(neigh);
 		goto out_dev_put;
 	}
-	read_unlock(&neigh_tbl_lock);
 	err = -EAFNOSUPPORT;
 
 out_dev_put:
 	if (dev)
 		dev_put(dev);
+	rcu_read_unlock();
 out:
 	return err;
 }
@@ -1529,15 +1518,14 @@ int neigh_add(struct sk_buff *skb, struc
 			goto out_dev_put;
 	}
 
-	read_lock(&neigh_tbl_lock);
-	for (tbl = neigh_tables; tbl; tbl = tbl->next) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(tbl, &neigh_tables, list) {
 		int flags = NEIGH_UPDATE_F_ADMIN | NEIGH_UPDATE_F_OVERRIDE;
 		struct neighbour *neigh;
 		void *dst, *lladdr;
 
 		if (tbl->family != ndm->ndm_family)
 			continue;
-		read_unlock(&neigh_tbl_lock);
 
 		if (nla_len(tb[NDA_DST]) < tbl->key_len)
 			goto out_dev_put;
@@ -1582,12 +1570,12 @@ int neigh_add(struct sk_buff *skb, struc
 		goto out_dev_put;
 	}
 
-	read_unlock(&neigh_tbl_lock);
 	err = -EAFNOSUPPORT;
 
 out_dev_put:
 	if (dev)
 		dev_put(dev);
+	rcu_read_unlock();
 out:
 	return err;
 }
@@ -1792,8 +1780,8 @@ int neightbl_set(struct sk_buff *skb, st
 	}
 
 	ndtmsg = nlmsg_data(nlh);
-	read_lock(&neigh_tbl_lock);
-	for (tbl = neigh_tables; tbl; tbl = tbl->next) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(tbl, &neigh_tables, list) {
 		if (ndtmsg->ndtm_family && tbl->family != ndtmsg->ndtm_family)
 			continue;
 
@@ -1893,26 +1881,26 @@ int neightbl_set(struct sk_buff *skb, st
 errout_tbl_lock:
 	write_unlock_bh(&tbl->lock);
 errout_locked:
-	read_unlock(&neigh_tbl_lock);
+	rcu_read_unlock();
 errout:
 	return err;
 }
 
 int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	int family, tidx, nidx = 0;
+	int family, tidx = 0, nidx = 0;
 	int tbl_skip = cb->args[0];
 	int neigh_skip = cb->args[1];
 	struct neigh_table *tbl;
 
 	family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
 
-	read_lock(&neigh_tbl_lock);
-	for (tbl = neigh_tables, tidx = 0; tbl; tbl = tbl->next, tidx++) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(tbl, &neigh_tables, list) {
 		struct neigh_parms *p;
 
 		if (tidx < tbl_skip || (family && tbl->family != family))
-			continue;
+			goto next_tbl;
 
 		if (neightbl_fill_info(skb, tbl, NETLINK_CB(cb->skb).pid,
 				       cb->nlh->nlmsg_seq, RTM_NEWNEIGHTBL,
@@ -1932,9 +1920,11 @@ int neightbl_dump_info(struct sk_buff *s
 		}
 
 		neigh_skip = 0;
+next_tbl:
+		++tidx;
 	}
 out:
-	read_unlock(&neigh_tbl_lock);
+	rcu_read_unlock();
 	cb->args[0] = tidx;
 	cb->args[1] = nidx;
 
@@ -2024,22 +2014,25 @@ out:
 int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct neigh_table *tbl;
-	int t, family, s_t;
+	int t = 0, family, s_t;
+
 
-	read_lock(&neigh_tbl_lock);
 	family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
 	s_t = cb->args[0];
 
-	for (tbl = neigh_tables, t = 0; tbl; tbl = tbl->next, t++) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(tbl, &neigh_tables, list) {
 		if (t < s_t || (family && tbl->family != family))
-			continue;
+			goto next;
 		if (t > s_t)
 			memset(&cb->args[1], 0, sizeof(cb->args) -
 						sizeof(cb->args[0]));
 		if (neigh_dump_table(tbl, skb, cb) < 0)
 			break;
+next:
+		++t;
 	}
-	read_unlock(&neigh_tbl_lock);
+	rcu_read_unlock();
 
 	cb->args[0] = t;
 	return skb->len;
include/net/neighbour.h +1 -1
@@ -136,7 +136,7 @@ struct pneigh_entry
 
 struct neigh_table
 {
-	struct neigh_table	*next;
+	struct list_head	list;
 	int			family;
 	int			entry_size;
 	int			key_len;

--

#2 lore raw

RFC 2/7 neighbour: convert neighbour hash table to hlist

In reply to Stephen Hemminger

Change the neighbour table hash list to hlist from list.h to allow for easier later conversion to RCU.

---
 include/net/neighbour.h |    6 -
 net/core/neighbour.c    |  160 +++++++++++++++++++++++++-----------------------
 2 files changed, 88 insertions(+), 78 deletions(-)
include/net/neighbour.h +3 -3
@@ -88,10 +88,10 @@ struct neigh_statistics
 
 struct neighbour
 {
-	struct neighbour	*next;
+	struct hlist_node	hlist;
 	struct neigh_table	*tbl;
 	struct neigh_parms	*parms;
-	struct net_device		*dev;
+	struct net_device	*dev;
 	unsigned long		used;
 	unsigned long		confirmed;
 	unsigned long		updated;
@@ -161,7 +161,7 @@ struct neigh_table
 	unsigned long		last_rand;
 	kmem_cache_t		*kmem_cachep;
 	struct neigh_statistics	*stats;
-	struct neighbour	**hash_buckets;
+	struct hlist_head	*hash_buckets;
 	unsigned int		hash_mask;
 	__u32			hash_rnd;
 	unsigned int		hash_chain_gc;
net/core/neighbour.c +85 -75

380 lines not on this page · raw message

--

#3 lore raw

RFC 3/7 neighbour: convert pneigh hash table to hlist

In reply to Stephen Hemminger

Change the pneigh_entry table to hlist from list.h to allow for easier later conversion to RCU.

---
 include/net/neighbour.h |    6 ++--
 net/core/neighbour.c    |   58 ++++++++++++++++++++++++------------------------
 2 files changed, 33 insertions(+), 31 deletions(-)
include/net/neighbour.h +3 -3
@@ -124,8 +124,8 @@ struct neigh_ops
 
 struct pneigh_entry
 {
-	struct pneigh_entry	*next;
-	struct net_device		*dev;
+	struct hlist_node	hlist;
+	struct net_device	*dev;
 	u8			key[0];
 };
 
@@ -165,7 +165,7 @@ struct neigh_table
 	unsigned int		hash_mask;
 	__u32			hash_rnd;
 	unsigned int		hash_chain_gc;
-	struct pneigh_entry	**phash_buckets;
+	struct hlist_head	*phash_buckets;
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry	*pde;
 #endif
net/core/neighbour.c +30 -28

143 lines not on this page · raw message

--

#4 lore raw

RFC 4/7 net neighbour: convert to RCU

In reply to Stephen Hemminger

Use RCU to allow for lock less access to the neighbour table. This should speedup the send path because no atomic operations will be needed to lookup ARP entries, etc.

---
 include/net/neighbour.h |    4 -
 net/core/neighbour.c    |  158 +++++++++++++++++++++++++-----------------------
 2 files changed, 87 insertions(+), 75 deletions(-)
include/net/neighbour.h +3 -1
@@ -108,6 +108,7 @@ struct neighbour
 	struct sk_buff_head	arp_queue;
 	struct timer_list	timer;
 	struct neigh_ops	*ops;
+	struct rcu_head		rcu;
 	u8			primary_key[0];
 };
 
@@ -126,6 +127,7 @@ struct pneigh_entry
 {
 	struct hlist_node	hlist;
 	struct net_device	*dev;
+	struct rcu_head		rcu;
 	u8			key[0];
 };
 
@@ -157,7 +159,7 @@ struct neigh_table
 	struct timer_list 	proxy_timer;
 	struct sk_buff_head	proxy_queue;
 	atomic_t		entries;
-	rwlock_t		lock;
+	spinlock_t		lock;
 	unsigned long		last_rand;
 	kmem_cache_t		*kmem_cachep;
 	struct neigh_statistics	*stats;
net/core/neighbour.c +84 -74

472 lines not on this page · raw message

--

#5 lore raw

RFC 5/7 neighbour: convert lookup to sequence lock

In reply to Stephen Hemminger

The reading of the contents of a neighbour entry can be converted from a slow reader/writer lock to a fast lockless sequence number check.

---
 include/net/neighbour.h |    2 
 net/core/neighbour.c    |  102 +++++++++++++++++++++++++++---------------------
 net/ipv4/arp.c          |  101 ++++++++++++++++++++++++++++-------------------
 net/ipv6/ndisc.c        |   16 +++----
 net/ipv6/route.c        |   12 ++---
 net/sched/sch_teql.c    |   11 +++--
 6 files changed, 142 insertions(+), 102 deletions(-)
include/net/neighbour.h +1 -1
@@ -100,7 +100,7 @@ struct neighbour
 	__u8			type;
 	__u8			dead;
 	atomic_t		probes;
-	rwlock_t		lock;
+	seqlock_t		lock;
 	unsigned char		ha[ALIGN(MAX_ADDR_LEN, sizeof(unsigned long))];
 	struct hh_cache		*hh;
 	atomic_t		refcnt;
net/core/neighbour.c +59 -43

290 lines not on this page · raw message

net/ipv4/arp.c +61 -40

161 lines not on this page · raw message

net/ipv6/ndisc.c +8 -8
@@ -1405,15 +1405,15 @@ void ndisc_send_redirect(struct sk_buff 
 		return;
 	}
 
-	if (dev->addr_len) {
-		read_lock_bh(&neigh->lock);
-		if (neigh->nud_state & NUD_VALID) {
+	if (dev->addr_len && (neigh->nud_state & NUD_VALID)) {
+		unsigned seq;
+		do {
+			seq = read_seqbegin(&neigh->lock);
 			memcpy(ha_buf, neigh->ha, dev->addr_len);
-			read_unlock_bh(&neigh->lock);
-			ha = ha_buf;
-			len += ndisc_opt_addr_space(dev);
-		} else
-			read_unlock_bh(&neigh->lock);
+		} while (read_seqretry(&neigh->lock, seq));
+
+		ha = ha_buf;
+		len += ndisc_opt_addr_space(dev);
 	}
 
 	rd_len = min_t(unsigned int,
net/ipv6/route.c +5 -7
@@ -280,20 +280,19 @@ static void rt6_probe(struct rt6_info *r
 	 */
 	if (!neigh || (neigh->nud_state & NUD_VALID))
 		return;
-	read_lock_bh(&neigh->lock);
+
 	if (!(neigh->nud_state & NUD_VALID) &&
-	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
+	    time_after(jiffies,
+		       neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
 		struct in6_addr mcaddr;
 		struct in6_addr *target;
 
 		neigh->updated = jiffies;
-		read_unlock_bh(&neigh->lock);
 
 		target = (struct in6_addr *)&neigh->primary_key;
 		addrconf_addr_solict_mult(target, &mcaddr);
 		ndisc_send_ns(rt->rt6i_dev, NULL, target, &mcaddr, NULL);
-	} else
-		read_unlock_bh(&neigh->lock);
+	}
 }
 #else
 static inline void rt6_probe(struct rt6_info *rt)
@@ -324,10 +323,9 @@ static int inline rt6_check_neigh(struct
 	    !(rt->rt6i_flags & RTF_GATEWAY))
 		m = 1;
 	else if (neigh) {
-		read_lock_bh(&neigh->lock);
+		smp_rmb();
 		if (neigh->nud_state & NUD_VALID)
 			m = 2;
-		read_unlock_bh(&neigh->lock);
 	}
 	return m;
 }
net/sched/sch_teql.c +8 -3
@@ -248,9 +248,14 @@ __teql_resolve(struct sk_buff *skb, stru
 	}
 	if (neigh_event_send(n, skb_res) == 0) {
 		int err;
-		read_lock(&n->lock);
-		err = dev->hard_header(skb, dev, ntohs(skb->protocol), n->ha, NULL, skb->len);
-		read_unlock(&n->lock);
+		unsigned seq;
+
+		do {
+			seq = read_seqbegin(&n->lock);
+			err = dev->hard_header(skb, dev, ntohs(skb->protocol),
+					       n->ha, NULL, skb->len);
+		} while (read_seqretry(&n->lock, seq));
+
 		if (err < 0) {
 			neigh_release(n);
 			return -EINVAL;

--

#6 lore raw

Re: [RFC 5/7] neighbour: convert lookup to sequence lock

* Stephen Hemminger <[email protected]> 2006-08-14 14:20

Quoted 19 lines
net/core/neighbour.c +5 -3
@@ -1160,19 +1164,24 @@ int neigh_resolve_output(struct sk_buff 
 	if (!neigh_event_send(neigh, skb)) {
 		int err;
 		struct net_device *dev = neigh->dev;
+		unsigned short type = ntohs(skb->protocol);
+
 		if (dev->hard_header_cache && !dst->hh) {
-			write_lock_bh(&neigh->lock);
+			write_seqlock_bh(&neigh->lock);
 			if (!dst->hh)
 				neigh_hh_init(neigh, dst, dst->ops->protocol);
-			err = dev->hard_header(skb, dev, ntohs(skb->protocol),
+			err = dev->hard_header(skb, dev, type,
 					       neigh->ha, NULL, skb->len);
-			write_unlock_bh(&neigh->lock);
+			write_sequnlock_bh(&neigh->lock);
 		} else {

net/core/neighbour.c int neigh_resolve_output(struct sk_buff · line 1182

Quoted 10 lines
net/core/neighbour.c +6 -4
-			read_lock_bh(&neigh->lock);
-			err = dev->hard_header(skb, dev, ntohs(skb->protocol),
-					       neigh->ha, NULL, skb->len);
-			read_unlock_bh(&neigh->lock);
+			unsigned seq;
+			do {
+				seq = read_seqbegin(&neigh->lock);
+				err = dev->hard_header(skb, dev, type,
+						       neigh->ha, NULL, skb->len);
+			} while (read_seqretry(&neigh->lock, seq));

Don't you need to pull in order to undo the push that probably happened in dev->hard_header()?

#7 lore raw

Re: [RFC 5/7] neighbour: convert lookup to sequence lock

Quoted 25 lines · On Tue, 15 Aug 2006 00:22:20 +0200 Thomas Graf <[email protected]> wrote:

* Stephen Hemminger <[email protected]> 2006-08-14 14:20

Quoted 24 lines
net/core/neighbour.c +6 -7
@@ -1160,19 +1164,24 @@ int neigh_resolve_output(struct sk_buff 
 	if (!neigh_event_send(neigh, skb)) {
 		int err;
 		struct net_device *dev = neigh->dev;
+		unsigned short type = ntohs(skb->protocol);
+
 		if (dev->hard_header_cache && !dst->hh) {
-			write_lock_bh(&neigh->lock);
+			write_seqlock_bh(&neigh->lock);
 			if (!dst->hh)
 				neigh_hh_init(neigh, dst, dst->ops->protocol);
-			err = dev->hard_header(skb, dev, ntohs(skb->protocol),
+			err = dev->hard_header(skb, dev, type,
 					       neigh->ha, NULL, skb->len);
-			write_unlock_bh(&neigh->lock);
+			write_sequnlock_bh(&neigh->lock);
 		} else {
-			read_lock_bh(&neigh->lock);
-			err = dev->hard_header(skb, dev, ntohs(skb->protocol),
-					       neigh->ha, NULL, skb->len);
-			read_unlock_bh(&neigh->lock);
+			unsigned seq;
Quoted 8 lines

net/core/neighbour.c int neigh_resolve_output(struct sk_buff · line 1182

Quoted 5 lines
net/core/neighbour.c +5 -0
+			do {
+				seq = read_seqbegin(&neigh->lock);
+				err = dev->hard_header(skb, dev, type,
+						       neigh->ha, NULL, skb->len);
+			} while (read_seqretry(&neigh->lock, seq));

Don't you need to pull in order to undo the push that probably happened in dev->hard_header()?

Probably just record the headroom() and undo that?

#8 lore raw

Re: [RFC 5/7] neighbour: convert lookup to sequence lock

* Stephen Hemminger <[email protected]> 2006-08-14 15:37

Quoted 6 lines
Quoted 2 lines · On Tue, 15 Aug 2006 00:22:20 +0200 Thomas Graf <[email protected]> wrote:

Don't you need to pull in order to undo the push that probably happened in dev->hard_header()?

Probably just record the headroom() and undo that?

That wouldn't work if hard_header() ever expands the head. Fortunately hard_header() returns the length added even in case of an error so we can undo the absolute value returned.

#9 lore raw

Re: [RFC 5/7] neighbour: convert lookup to sequence lock

Hello!

Quoted 3 lines

That wouldn't work if hard_header() ever expands the head. Fortunately hard_header() returns the length added even in case of an error so we can undo the absolute value returned.

Yes.

Or probably it is safer to undo to skb->nh. Even if hard_header expands skb, skb->nh still remains valid.

Alexey

#10 lore raw

RFC 6/7 neighbour: convert hard header cache to sequence number

In reply to Stephen Hemminger

The reading of the hard header cache in the output path can be made lockless using seqlock.

---
 include/linux/netdevice.h |    3 ++-
 include/net/neighbour.h   |    2 ++
 net/core/neighbour.c      |   40 +++++++++++++++++++++++++++++++++++-----
 net/ipv4/ip_output.c      |   13 +++----------
 net/ipv6/ip6_output.c     |   13 +++----------
 5 files changed, 45 insertions(+), 26 deletions(-)
include/linux/netdevice.h +2 -1
@@ -193,7 +193,7 @@ struct hh_cache
                                          */
 	int		hh_len;		/* length of header */
 	int		(*hh_output)(struct sk_buff *skb);
-	rwlock_t	hh_lock;
+	seqlock_t	hh_lock;
 
 	/* cached hardware header; allow for machine alignment needs.        */
 #define HH_DATA_MOD	16
@@ -217,6 +217,7 @@ struct hh_cache
 #define LL_RESERVED_SPACE_EXTRA(dev,extra) \
 	((((dev)->hard_header_len+extra)&~(HH_DATA_MOD - 1)) + HH_DATA_MOD)
 
+
 /* These flag bits are private to the generic network queueing
  * layer, they may not be explicitly referenced by any other
  * code.
net/core/neighbour.c +35 -5
@@ -591,9 +591,11 @@ void neigh_destroy(struct neighbour *nei
 	while ((hh = neigh->hh) != NULL) {
 		neigh->hh = hh->hh_next;
 		hh->hh_next = NULL;
-		write_lock_bh(&hh->hh_lock);
+
+		write_seqlock_bh(&hh->hh_lock);
 		hh->hh_output = neigh_blackhole;
-		write_unlock_bh(&hh->hh_lock);
+		write_sequnlock_bh(&hh->hh_lock);
+
 		if (atomic_dec_and_test(&hh->hh_refcnt))
 			kfree(hh);
 	}
@@ -912,9 +914,9 @@ static __inline__ void neigh_update_hhs(
 
 	if (update) {
 		for (hh = neigh->hh; hh; hh = hh->hh_next) {
-			write_lock_bh(&hh->hh_lock);
+			write_seqlock_bh(&hh->hh_lock);
 			update(hh, neigh->dev, neigh->ha);
-			write_unlock_bh(&hh->hh_lock);
+			write_sequnlock_bh(&hh->hh_lock);
 		}
 	}
 }
@@ -1105,7 +1107,7 @@ static void neigh_hh_init(struct neighbo
 			break;
 
 	if (!hh && (hh = kzalloc(sizeof(*hh), GFP_ATOMIC)) != NULL) {
-		rwlock_init(&hh->hh_lock);
+		seqlock_init(&hh->hh_lock);
 		hh->hh_type = protocol;
 		atomic_set(&hh->hh_refcnt, 0);
 		hh->hh_next = NULL;
@@ -1128,6 +1130,33 @@ static void neigh_hh_init(struct neighbo
 	}
 }
 
+
+/*
+ * Add header to skb from hard header cache
+ * Handle case where cache gets changed.
+ */
+int neigh_hh_output(const struct hh_cache *hh, struct sk_buff *skb)
+{
+	int len, alen;
+	unsigned seq;
+	int (*output)(struct sk_buff *);
+
+	for(;;) {
+		seq = read_seqbegin(&hh->hh_lock);
+		len = hh->hh_len;
+		alen = HH_DATA_ALIGN(len);
+		output = hh->hh_output;
+		memcpy(skb->data - alen, hh->hh_data, alen);
+		skb_push(skb, len);
+
+		if (likely(!read_seqretry(&hh->hh_lock, seq)))
+			return output(skb);
+
+		/* undo and try again */
+		__skb_pull(skb, len);
+	}
+}
+
 /* This function can be used in contexts, where only old dev_queue_xmit
    worked, f.e. if you want to override normal output path (eql, shaper),
    but resolution is not made yet.
@@ -2762,6 +2791,7 @@ EXPORT_SYMBOL(neigh_delete);
 EXPORT_SYMBOL(neigh_destroy);
 EXPORT_SYMBOL(neigh_dump_info);
 EXPORT_SYMBOL(neigh_event_ns);
+EXPORT_SYMBOL(neigh_hh_output);
 EXPORT_SYMBOL(neigh_ifdown);
 EXPORT_SYMBOL(neigh_lookup);
 EXPORT_SYMBOL(neigh_lookup_nodev);
net/ipv4/ip_output.c +3 -10
@@ -182,16 +182,9 @@ static inline int ip_finish_output2(stru
 		skb = skb2;
 	}
 
-	if (hh) {
-		int hh_alen;
-
-		read_lock_bh(&hh->hh_lock);
-		hh_alen = HH_DATA_ALIGN(hh->hh_len);
-  		memcpy(skb->data - hh_alen, hh->hh_data, hh_alen);
-		read_unlock_bh(&hh->hh_lock);
-	        skb_push(skb, hh->hh_len);
-		return hh->hh_output(skb);
-	} else if (dst->neighbour)
+	if (hh)
+		return neigh_hh_output(hh, skb);
+	else if (dst->neighbour)
 		return dst->neighbour->output(skb);
 
 	if (net_ratelimit())
net/ipv6/ip6_output.c +3 -10
@@ -76,16 +76,9 @@ static inline int ip6_output_finish(stru
 	struct dst_entry *dst = skb->dst;
 	struct hh_cache *hh = dst->hh;
 
-	if (hh) {
-		int hh_alen;
-
-		read_lock_bh(&hh->hh_lock);
-		hh_alen = HH_DATA_ALIGN(hh->hh_len);
-		memcpy(skb->data - hh_alen, hh->hh_data, hh_alen);
-		read_unlock_bh(&hh->hh_lock);
-	        skb_push(skb, hh->hh_len);
-		return hh->hh_output(skb);
-	} else if (dst->neighbour)
+	if (hh)
+		return neigh_hh_output(hh, skb);
+	else if (dst->neighbour)
 		return dst->neighbour->output(skb);
 
 	IP6_INC_STATS_BH(IPSTATS_MIB_OUTNOROUTES);
include/net/neighbour.h +2 -0
@@ -193,6 +193,8 @@ extern struct neighbour *	neigh_create(s
 					     struct net_device *dev);
 extern void			neigh_destroy(struct neighbour *neigh);
 extern int			__neigh_event_send(struct neighbour *neigh, struct sk_buff *skb);
+extern int			neigh_hh_output(const struct hh_cache *hh, struct sk_buff *skb);
+
 extern int			neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, 
 					     u32 flags);
 extern void			neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);

--

#11 lore raw

RFC 7/7 neighbour: reduce exports

In reply to Stephen Hemminger

The function neigh_update_hhs is only used in neighbour.c and let compiler decide to inline or not.

There are several symbols only used by rtnetlink and since it can not be a module, there is no reason to export them.

---
 net/core/neighbour.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
net/core/neighbour.c +1 -5
@@ -906,7 +906,7 @@ out:
 	return rc;
 }
 
-static __inline__ void neigh_update_hhs(struct neighbour *neigh)
+static void neigh_update_hhs(struct neighbour *neigh)
 {
 	struct hh_cache *hh;
 	void (*update)(struct hh_cache*, struct net_device*, unsigned char *) =
@@ -2782,7 +2782,6 @@ void neigh_sysctl_unregister(struct neig
 #endif	/* CONFIG_SYSCTL */
 
 EXPORT_SYMBOL(__neigh_event_send);
-EXPORT_SYMBOL(neigh_add);
 EXPORT_SYMBOL(neigh_changeaddr);
 EXPORT_SYMBOL(neigh_compat_output);
 EXPORT_SYMBOL(neigh_connected_output);
@@ -2803,11 +2802,8 @@ EXPORT_SYMBOL(neigh_table_clear);
 EXPORT_SYMBOL(neigh_table_init);
 EXPORT_SYMBOL(neigh_table_init_no_netlink);
 EXPORT_SYMBOL(neigh_update);
-EXPORT_SYMBOL(neigh_update_hhs);
 EXPORT_SYMBOL(pneigh_enqueue);
 EXPORT_SYMBOL(pneigh_lookup);
-EXPORT_SYMBOL(neightbl_dump_info);
-EXPORT_SYMBOL(neightbl_set);
 
 #ifdef CONFIG_ARPD
 EXPORT_SYMBOL(neigh_app_ns);

--