git: 406a4b37a24e - stable/13 - [multipath][nhops] Fix random crashes with high route churn rate.

Alexander V. Chernikov melifaro at FreeBSD.org
Tue Aug 17 21:15:17 UTC 2021


The branch stable/13 has been updated by melifaro:

URL: https://cgit.FreeBSD.org/src/commit/?id=406a4b37a24e655b8a5d8f3401e636ba43f37978

commit 406a4b37a24e655b8a5d8f3401e636ba43f37978
Author:     Alexander V. Chernikov <melifaro at FreeBSD.org>
AuthorDate: 2021-08-01 09:46:05 +0000
Commit:     Alexander V. Chernikov <melifaro at FreeBSD.org>
CommitDate: 2021-08-17 21:14:42 +0000

    [multipath][nhops] Fix random crashes with high route churn rate.
    
    When certain multipath route begins flapping really fast, it may
     result in creating multiple identical nexthop groups. The code
     responsible for unlinking unused nexthop groups had an implicit
     assumption that there could be only one nexthop group for the
     same combination of nexthops with weights. This assumption resulted
     in always unlinking the first "identical" group, instead of the
     desired one. Such action, in turn, produced a used-but-unlinked
     nhg along with freed-and-linked nhg, ending up in random crashes.
    
    Similarly, it is possible that multiple identical nexthops gets
     created in the case of high route churn, resulting in the same
     problem when deleting one of such nexthops.
    
    Fix by matching the nexthop/nexhop group pointer when deleting the item.
    
    Reported by:    avg
    (cherry picked from commit 054948bd81bb9e4e32449cf351b62e501b8831ff)
---
 sys/net/route/nhgrp.c      |  2 +-
 sys/net/route/nhop.c       |  2 +-
 sys/net/route/nhop_utils.h | 23 +++--------------------
 3 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/sys/net/route/nhgrp.c b/sys/net/route/nhgrp.c
index f6638b12ebea..f40ee3c2981c 100644
--- a/sys/net/route/nhgrp.c
+++ b/sys/net/route/nhgrp.c
@@ -187,7 +187,7 @@ unlink_nhgrp(struct nh_control *ctl, struct nhgrp_priv *key)
 
 	NHOPS_WLOCK(ctl);
 
-	CHT_SLIST_REMOVE_BYOBJ(&ctl->gr_head, mpath, key, nhg_priv_ret);
+	CHT_SLIST_REMOVE(&ctl->gr_head, mpath, key, nhg_priv_ret);
 
 	if (nhg_priv_ret == NULL) {
 		DPRINTF("Unable to find nhop group!");
diff --git a/sys/net/route/nhop.c b/sys/net/route/nhop.c
index 0db47db9916e..ab5e393ae56a 100644
--- a/sys/net/route/nhop.c
+++ b/sys/net/route/nhop.c
@@ -337,7 +337,7 @@ unlink_nhop(struct nh_control *ctl, struct nhop_priv *nh_priv_del)
 	idx = 0;
 
 	NHOPS_WLOCK(ctl);
-	CHT_SLIST_REMOVE_BYOBJ(&ctl->nh_head, nhops, nh_priv_del, priv_ret);
+	CHT_SLIST_REMOVE(&ctl->nh_head, nhops, nh_priv_del, priv_ret);
 
 	if (priv_ret != NULL) {
 		idx = priv_ret->nh_idx;
diff --git a/sys/net/route/nhop_utils.h b/sys/net/route/nhop_utils.h
index a0d7cd564e72..1f56f4cb8b0b 100644
--- a/sys/net/route/nhop_utils.h
+++ b/sys/net/route/nhop_utils.h
@@ -115,31 +115,13 @@ struct _HNAME##_head {				\
 	(_head)->items_count++;						\
 } while(0)
 
-#define	CHT_SLIST_REMOVE(_head, _PX, _key, _ret) do {			\
-	typeof(*(_head)->ptr) _tmp;					\
-	uint32_t _buck = CHT_GET_BUCK(_head, _PX, _key);		\
-	_ret = CHT_FIRST(_head, _buck);					\
-	_tmp = NULL;							\
-	for ( ; _ret != NULL; _tmp = _ret, _ret = _PX##_next(_ret)) {	\
-		if (_PX##_cmp(_key, _ret))				\
-			break;						\
-	}								\
-	if (_ret != NULL) {						\
-		if (_tmp == NULL)					\
-			CHT_FIRST(_head, _buck) = _PX##_next(_ret);	\
-		else							\
-			_PX##_next(_tmp) = _PX##_next(_ret);		\
-		(_head)->items_count--;					\
-	}								\
-} while(0)
-
-#define	CHT_SLIST_REMOVE_BYOBJ(_head, _PX, _obj, _ret) do {		\
+#define	CHT_SLIST_REMOVE(_head, _PX, _obj, _ret) do {			\
 	typeof(*(_head)->ptr) _tmp;					\
 	uint32_t _buck = CHT_GET_BUCK_OBJ(_head, _PX, _obj);		\
 	_ret = CHT_FIRST(_head, _buck);					\
 	_tmp = NULL;							\
 	for ( ; _ret != NULL; _tmp = _ret, _ret = _PX##_next(_ret)) {	\
-		if (_PX##_cmp(_obj, _ret))				\
+		if (_obj == _ret)					\
 			break;						\
 	}								\
 	if (_ret != NULL) {						\
@@ -150,6 +132,7 @@ struct _HNAME##_head {				\
 		(_head)->items_count--;					\
 	}								\
 } while(0)
+#define	CHT_SLIST_REMOVE_BYOBJ	CHT_SLIST_REMOVE
 
 #define	CHT_SLIST_FOREACH(_head, _PX, _x)				\
 	for (uint32_t _i = 0; _i < (_head)->hash_size; _i++) {		\


More information about the dev-commits-src-all mailing list