From nobody Mon May 01 14:16:07 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Q94y01lJgz48bcF; Mon, 1 May 2023 14:16:08 +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 4Q94y01CzTz3Q4K; Mon, 1 May 2023 14:16:08 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682950568; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=EMzxwqnznXN6yYeDNWoCw3sHWd86KjeIep24/QNu1UU=; b=Sb7wYMcT9Kgd6uBL9l1oskUQJ5mEvoHbd+J/MpBXTfqMFyVCDfT7i8xLm9A9jFe8qlZmyD qj9rMRvfJgXN8hVya4XEAHZ04zFHGC/FKxUMyiZvl4T9d0w12VRrGYHJTIIr38ZB4Smvkn PBV2oJJemHe25U/ouGwYyiKY3De8G7Q33gYhj7bVCv0iqX8lECjj9uUSdtKAz9YAwIJG6z DQGBO5YoBBCMIsmzKMVVHi2GPMXyX1lLGOYs6gHYkWveEk19JsVHt7lQOneCkSFrHFkZdK jlz6wRvLJGuyIx0Y0wUDRkZHl0uRb1N/lsRynfAMI6QSYLLENFeUAOzHBd5HyQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1682950568; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=EMzxwqnznXN6yYeDNWoCw3sHWd86KjeIep24/QNu1UU=; b=jVAQCv29YsiVcvZLfXf99z2zU6Qd0FqFvsD1aoEmM29ptXaT1pFpRbYLcgbEWoaramXfHi Ag25R4c8DdCqqwhjQEiN4ZAkIHvnq5I330vjrEqGyCQUehZ54yv36EpL3zezO0FHhSVzAb belg9YmTAoDraQYMNpiY4El7IEMg8IEvs0HEQ17k3oC3mtYbw7izWcFbUXlCJnHzvVcZro IH6PVTSYhyxM6DFiQoRMyhNtD8owiYOEk7Zf1KOjagXHMjdWf+AqPrxOSnvX4KzEdSi/dT 8NOEfL8QYY4nsIQ3W/QtIem8MSqfuAiT6FLdeEZSvyhv/GFtAL1RoNPvu3nYnQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1682950568; a=rsa-sha256; cv=none; b=r1NlrNG0ABBw9NuRdVILRbN/6Fdn3xWcUI9hKX44sP/qIoxV3La9z/R8BA8PbJO6oL75FA cXzIn0bflemctrS5F3xvnpaHyyrdKg3m/wT5VFVjsQCAbttksRTS1IYgrGgD0rlUwZVuKI JdiIeW/VEAVzRoHd5QSmNYr011E9L9XceHTYUMN9GCHIWFaXWgWAq4bNJzSHBgLMIoPQ6w uDNht6Pe7VffxTnao5YM+PFMsRUnty5cSdK71SNvhP+oNJdsTNvlO36Hpn89UTQZJ7xPeK 0QfejyLbHWUyeoNsjRbi7AEI9tAv2Jqous9GDyTb1VZ2S47jvmFP5/EWvZg/qA== 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 4Q94y001Cpz17sK; Mon, 1 May 2023 14:16:08 +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 341EG7xw073567; Mon, 1 May 2023 14:16:07 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 341EG7me073566; Mon, 1 May 2023 14:16:07 GMT (envelope-from git) Date: Mon, 1 May 2023 14:16:07 GMT Message-Id: <202305011416.341EG7me073566@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: db0a2bfd0c6d - main - pf: reduce number of hashing operations when handling source nodes List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: db0a2bfd0c6df48ae1c5346198b97c83f746d219 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=db0a2bfd0c6df48ae1c5346198b97c83f746d219 commit db0a2bfd0c6df48ae1c5346198b97c83f746d219 Author: Kajetan Staszkiewicz AuthorDate: 2023-05-01 14:07:42 +0000 Commit: Kristof Provost CommitDate: 2023-05-01 14:15:30 +0000 pf: reduce number of hashing operations when handling source nodes Reduce number of hashing operations when handling source nodes by always having a pointer to the hash row mutex in the source node. Provide macros for handling and asserting the mutex. Calculate the hash only once in pf_find_src_node() and then use this hash in subsequent operations. Cherry-picked from development of D39880 Reviewed by: kp, mjg Sponsored by: InnoGames GmbH Differential Revision: https://reviews.freebsd.org/D39888 --- sys/net/pfvar.h | 39 ++++++++++++++++++++++++++++++++++- sys/netpfil/pf/pf.c | 55 ++++++++++++++++++++++++-------------------------- sys/netpfil/pf/pf_lb.c | 3 ++- 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index a82735f71c8c..c1550f2c30e6 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -375,6 +375,41 @@ struct pfi_dynaddr { #define PF_STATE_LOCK_ASSERT(s) do {} while (0) #endif /* INVARIANTS */ +#ifdef INVARIANTS +#define PF_SRC_NODE_LOCK(sn) \ + do { \ + struct pf_ksrc_node *_sn = (sn); \ + struct pf_srchash *_sh = &V_pf_srchash[ \ + pf_hashsrc(&_sn->addr, _sn->af)]; \ + MPASS(_sn->lock == &_sh->lock); \ + mtx_lock(_sn->lock); \ + } while (0) +#define PF_SRC_NODE_UNLOCK(sn) \ + do { \ + struct pf_ksrc_node *_sn = (sn); \ + struct pf_srchash *_sh = &V_pf_srchash[ \ + pf_hashsrc(&_sn->addr, _sn->af)]; \ + MPASS(_sn->lock == &_sh->lock); \ + mtx_unlock(_sn->lock); \ + } while (0) +#else +#define PF_SRC_NODE_LOCK(sn) mtx_lock((sn)->lock) +#define PF_SRC_NODE_UNLOCK(sn) mtx_unlock((sn)->lock) +#endif + +#ifdef INVARIANTS +#define PF_SRC_NODE_LOCK_ASSERT(sn) \ + do { \ + struct pf_ksrc_node *_sn = (sn); \ + struct pf_srchash *_sh = &V_pf_srchash[ \ + pf_hashsrc(&_sn->addr, _sn->af)]; \ + MPASS(_sn->lock == &_sh->lock); \ + PF_HASHROW_ASSERT(_sh); \ + } while (0) +#else /* !INVARIANTS */ +#define PF_SRC_NODE_LOCK_ASSERT(sn) do {} while (0) +#endif /* INVARIANTS */ + extern struct mtx_padalign pf_unlnkdrules_mtx; #define PF_UNLNKDRULES_LOCK() mtx_lock(&pf_unlnkdrules_mtx) #define PF_UNLNKDRULES_UNLOCK() mtx_unlock(&pf_unlnkdrules_mtx) @@ -845,6 +880,7 @@ struct pf_ksrc_node { u_int32_t expire; sa_family_t af; u_int8_t ruletype; + struct mtx *lock; }; #endif @@ -2120,7 +2156,8 @@ extern struct pf_kstate *pf_find_state_all(struct pf_state_key_cmp *, extern bool pf_find_state_all_exists(struct pf_state_key_cmp *, u_int); extern struct pf_ksrc_node *pf_find_src_node(struct pf_addr *, - struct pf_krule *, sa_family_t, int); + struct pf_krule *, sa_family_t, + struct pf_srchash **, bool); extern void pf_unlink_src_node(struct pf_ksrc_node *); extern u_int pf_free_src_nodes(struct pf_ksrc_node_list *); extern void pf_print_state(struct pf_kstate *); diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index a8da800dd814..e5f2a5ea57a0 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -683,6 +683,10 @@ pf_src_connlimit(struct pf_kstate **state) int bad = 0; PF_STATE_LOCK_ASSERT(*state); + /* + * XXXKS: The src node is accessed unlocked! + * PF_SRC_NODE_LOCK_ASSERT((*state)->src_node); + */ (*state)->src_node->conn++; (*state)->src.tcp_est = 1; @@ -827,25 +831,25 @@ pf_overload_task(void *v, int pending) */ struct pf_ksrc_node * pf_find_src_node(struct pf_addr *src, struct pf_krule *rule, sa_family_t af, - int returnlocked) + struct pf_srchash **sh, bool returnlocked) { - struct pf_srchash *sh; struct pf_ksrc_node *n; counter_u64_add(V_pf_status.scounters[SCNT_SRC_NODE_SEARCH], 1); - sh = &V_pf_srchash[pf_hashsrc(src, af)]; - PF_HASHROW_LOCK(sh); - LIST_FOREACH(n, &sh->nodes, entry) + *sh = &V_pf_srchash[pf_hashsrc(src, af)]; + PF_HASHROW_LOCK(*sh); + LIST_FOREACH(n, &(*sh)->nodes, entry) if (n->rule.ptr == rule && n->af == af && ((af == AF_INET && n->addr.v4.s_addr == src->v4.s_addr) || (af == AF_INET6 && bcmp(&n->addr, src, sizeof(*src)) == 0))) break; + if (n != NULL) { n->states++; - PF_HASHROW_UNLOCK(sh); - } else if (returnlocked == 0) - PF_HASHROW_UNLOCK(sh); + PF_HASHROW_UNLOCK(*sh); + } else if (returnlocked == false) + PF_HASHROW_UNLOCK(*sh); return (n); } @@ -865,17 +869,16 @@ static int pf_insert_src_node(struct pf_ksrc_node **sn, struct pf_krule *rule, struct pf_addr *src, sa_family_t af) { + struct pf_srchash *sh = NULL; KASSERT((rule->rule_flag & PFRULE_SRCTRACK || rule->rpool.opts & PF_POOL_STICKYADDR), ("%s for non-tracking rule %p", __func__, rule)); if (*sn == NULL) - *sn = pf_find_src_node(src, rule, af, 1); + *sn = pf_find_src_node(src, rule, af, &sh, true); if (*sn == NULL) { - struct pf_srchash *sh = &V_pf_srchash[pf_hashsrc(src, af)]; - PF_HASHROW_ASSERT(sh); if (!rule->max_src_nodes || @@ -904,6 +907,9 @@ pf_insert_src_node(struct pf_ksrc_node **sn, struct pf_krule *rule, rule->max_src_conn_rate.limit, rule->max_src_conn_rate.seconds); + MPASS((*sn)->lock == NULL); + (*sn)->lock = &sh->lock; + (*sn)->af = af; (*sn)->rule.ptr = rule; PF_ACPY(&(*sn)->addr, src, af); @@ -929,8 +935,8 @@ pf_insert_src_node(struct pf_ksrc_node **sn, struct pf_krule *rule, void pf_unlink_src_node(struct pf_ksrc_node *src) { + PF_SRC_NODE_LOCK_ASSERT(src); - PF_HASHROW_ASSERT(&V_pf_srchash[pf_hashsrc(&src->addr, src->af)]); LIST_REMOVE(src, entry); if (src->rule.ptr) counter_u64_add(src->rule.ptr->src_nodes, -1); @@ -1982,7 +1988,6 @@ static void pf_src_tree_remove_state(struct pf_kstate *s) { struct pf_ksrc_node *sn; - struct pf_srchash *sh; uint32_t timeout; timeout = s->rule.ptr->timeout[PFTM_SRC_NODE] ? @@ -1991,21 +1996,19 @@ pf_src_tree_remove_state(struct pf_kstate *s) if (s->src_node != NULL) { sn = s->src_node; - sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)]; - PF_HASHROW_LOCK(sh); + PF_SRC_NODE_LOCK(sn); if (s->src.tcp_est) --sn->conn; if (--sn->states == 0) sn->expire = time_uptime + timeout; - PF_HASHROW_UNLOCK(sh); + PF_SRC_NODE_UNLOCK(sn); } if (s->nat_src_node != s->src_node && s->nat_src_node != NULL) { sn = s->nat_src_node; - sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)]; - PF_HASHROW_LOCK(sh); + PF_SRC_NODE_LOCK(sn); if (--sn->states == 0) sn->expire = time_uptime + timeout; - PF_HASHROW_UNLOCK(sh); + PF_SRC_NODE_UNLOCK(sn); } s->src_node = s->nat_src_node = NULL; } @@ -4805,31 +4808,25 @@ csfailed: uma_zfree(V_pf_state_key_z, nk); if (sn != NULL) { - struct pf_srchash *sh; - - sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)]; - PF_HASHROW_LOCK(sh); + PF_SRC_NODE_LOCK(sn); if (--sn->states == 0 && sn->expire == 0) { pf_unlink_src_node(sn); uma_zfree(V_pf_sources_z, sn); counter_u64_add( V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1); } - PF_HASHROW_UNLOCK(sh); + PF_SRC_NODE_UNLOCK(sn); } if (nsn != sn && nsn != NULL) { - struct pf_srchash *sh; - - sh = &V_pf_srchash[pf_hashsrc(&nsn->addr, nsn->af)]; - PF_HASHROW_LOCK(sh); + PF_SRC_NODE_LOCK(nsn); if (--nsn->states == 0 && nsn->expire == 0) { pf_unlink_src_node(nsn); uma_zfree(V_pf_sources_z, nsn); counter_u64_add( V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1); } - PF_HASHROW_UNLOCK(sh); + PF_SRC_NODE_UNLOCK(nsn); } return (PF_DROP); diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c index 186634edbd56..ca8593a1c37d 100644 --- a/sys/netpfil/pf/pf_lb.c +++ b/sys/netpfil/pf/pf_lb.c @@ -348,12 +348,13 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr, { struct pf_kpool *rpool = &r->rpool; struct pf_addr *raddr = NULL, *rmask = NULL; + struct pf_srchash *sh = NULL; /* Try to find a src_node if none was given and this is a sticky-address rule. */ if (*sn == NULL && r->rpool.opts & PF_POOL_STICKYADDR && (r->rpool.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE) - *sn = pf_find_src_node(saddr, r, af, 0); + *sn = pf_find_src_node(saddr, r, af, &sh, false); /* If a src_node was found or explicitly given and it has a non-zero route address, use this address. A zeroed address is found if the