Skip to contentSkip to conversation

neighbor: gc_list changes should be protected by table lock

PATCH Applied David Ahern 2 participants 2 messages Dec 10, 2018 – Dec 11, 2018

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

PATCH 1/1 neighbor: gc_list changes should be protected by table lock

From: David Ahern <[email protected]>

Adding and removing neighbor entries to / from the gc_list need to be done while holding the table lock; a couple of places were missed in the original patch.

Move the list_add_tail in neigh_alloc to ___neigh_create where the lock is already obtained. Since neighbor entries should rarely be moved to/from PERMANENT state, add lock/unlock around the gc_list changes in neigh_change_state rather than extending the lock hold around all neighbor updates.

Fixes 58956317c8de ("neighbor: Improve garbage collection")
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
---
 net/core/neighbour.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
net/core/neighbour.c +10 -5
index c3b58712e98b..03fdc5ae66b0 100644
@@ -138,11 +138,17 @@ static void neigh_change_state(struct neighbour *n, u8 new)
 	 * add to the gc list if new state is not permanent
 	 */
 	if (new_is_perm && on_gc_list) {
+		write_lock_bh(&n->tbl->lock);
 		list_del_init(&n->gc_list);
+		write_unlock_bh(&n->tbl->lock);
+
 		atomic_dec(&n->tbl->gc_entries);
 	} else if (!new_is_perm && !on_gc_list) {
 		/* add entries to the tail; cleaning removes from the front */
+		write_lock_bh(&n->tbl->lock);
 		list_add_tail(&n->gc_list, &n->tbl->gc_list);
+		write_unlock_bh(&n->tbl->lock);
+
 		atomic_inc(&n->tbl->gc_entries);
 	}
 }
@@ -390,11 +396,7 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
 	n->tbl		  = tbl;
 	refcount_set(&n->refcnt, 1);
 	n->dead		  = 1;
-
-	if (!permanent)
-		list_add_tail(&n->gc_list, &n->tbl->gc_list);
-	else
-		INIT_LIST_HEAD(&n->gc_list);
+	INIT_LIST_HEAD(&n->gc_list);
 
 	atomic_inc(&tbl->entries);
 out:
@@ -616,6 +618,9 @@ static struct neighbour *___neigh_create(struct neigh_table *tbl,
 	}
 
 	n->dead = 0;
+	if (!permanent)
+		list_add_tail(&n->gc_list, &n->tbl->gc_list);
+
 	if (want_ref)
 		neigh_hold(n);
 	rcu_assign_pointer(n->next,
Signature · 1 line
2.11.0
maintainer Applied
#1 lore raw

From: David Ahern <[email protected]>
Date: Mon, 10 Dec 2018 13:54:07 -0800

Quoted 12 lines

From: David Ahern <[email protected]>

Adding and removing neighbor entries to / from the gc_list need to be done while holding the table lock; a couple of places were missed in the original patch.

Move the list_add_tail in neigh_alloc to ___neigh_create where the lock is already obtained. Since neighbor entries should rarely be moved to/from PERMANENT state, add lock/unlock around the gc_list changes in neigh_change_state rather than extending the lock hold around all neighbor updates.

Quoted 9 lines
Fixes 58956317c8de ("neighbor: Improve garbage collection")
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]
Reported-by [email protected]

Applied, thanks for fixing this so quickly.