d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[multipath][nhops] Fix random crashes with high route churn rate.

Browse Source 
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
MFC after:	1 week
This commit is contained in:
Alexander V. Chernikov 2021-08-01 09:46:05 +00:00
parent 60fb9e10c7
commit 054948bd81
3 changed files with 5 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sys/net/route/nhgrp.c
View File

@ -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!");

2
sys/net/route/nhop.c
View File

@ -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;

23
sys/net/route/nhop_utils.h
View File

@ -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++) { \