From owner-dev-commits-src-main@freebsd.org Sun Aug 1 10:50:31 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B510165ADE0; Sun, 1 Aug 2021 10:50:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GcybC4VQcz3NHh; Sun, 1 Aug 2021 10:50:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 829272AFC; Sun, 1 Aug 2021 10:50:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 171AoVq7010044; Sun, 1 Aug 2021 10:50:31 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 171AoViq010043; Sun, 1 Aug 2021 10:50:31 GMT (envelope-from git) Date: Sun, 1 Aug 2021 10:50:31 GMT Message-Id: <202108011050.171AoViq010043@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Alexander V. Chernikov" Subject: git: 054948bd81bb - main - [multipath][nhops] Fix random crashes with high route churn rate. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: melifaro X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 054948bd81bb9e4e32449cf351b62e501b8831ff Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Aug 2021 10:50:31 -0000 The branch main has been updated by melifaro: URL: https://cgit.FreeBSD.org/src/commit/?id=054948bd81bb9e4e32449cf351b62e501b8831ff commit 054948bd81bb9e4e32449cf351b62e501b8831ff Author: Alexander V. Chernikov AuthorDate: 2021-08-01 09:46:05 +0000 Commit: Alexander V. Chernikov CommitDate: 2021-08-01 10:07:37 +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 MFC after: 1 week --- 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 f763cc25fd5c..982ff2a72f15 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++) { \