From nobody Sun Oct 23 17:43:33 2022 X-Original-To: dev-commits-src-branches@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 4MwQY14J5Yz4fWZS; Sun, 23 Oct 2022 17:43:33 +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 4MwQY12yDsz3vc9; Sun, 23 Oct 2022 17:43:33 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1666547013; 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=hQYdHDkNp1HqGq/zgZmuqS/ncuJ/7s2cjPJypR58ypc=; b=Mr4qt05twaFiT/IyEqmOllTpdCRI6bFSS7et8GMnA/YE5ko/jo0R7ZSh23x79k5n7c6fZd NGP6DjE6DTJqF2WXL/GYHo3ujgTe5cwYoNMTKyrAdI1UGFqMj9/rM7vZOTpQTSVCjpy+5/ vIqgqcHwmXGYq+8G9rBfhIe7dxc/lPAz3+MBLuVQjtezILzir1ponJGOhFn6KtG4TZqzcr iKs/h2F7tDX889CMoZZB/YZi5HU5j0mwEydmAWtRi8SAzP2xKcNqLj1TVM++LCT5AQKVlq kHXdxCbZZO7aOgYWZ+7fJgNkF/wsllTJXjoU23ClWXSESKMIUQDMjCRznhlx2Q== 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 4MwQY122htzy8K; Sun, 23 Oct 2022 17:43:33 +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 29NHhXOp062208; Sun, 23 Oct 2022 17:43:33 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 29NHhXlS062207; Sun, 23 Oct 2022 17:43:33 GMT (envelope-from git) Date: Sun, 23 Oct 2022 17:43:33 GMT Message-Id: <202210231743.29NHhXlS062207@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Alexander Motin Subject: git: b948644cfa07 - stable/13 - inet: Simplify if_multiaddrs iteration. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mav X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: b948644cfa0741ce928256b8a66edb0dbe257879 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1666547013; 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=hQYdHDkNp1HqGq/zgZmuqS/ncuJ/7s2cjPJypR58ypc=; b=PiV0d3O4IuvJ0tSLA76YSMW2ZuOpWOu2Gzrt1oMIPm6/UoYwsXSsiBAu+92kHAcT+xpZKd cIyogdqX/0eyLlGdZusE9uRvvLVhu/+5nyuUJgomCVxbnDFZ7taIHFhtEYDvwpVDRgezSN szMbLVJ5YyMuZnPU2p9mySEP2hA8/EtYwQpctDhVyHerIOBeWQAXotLR/uVg+cFO6YLrRQ r/43G1xaIQYOAaM8e0v9q97oBjosnueTecI5zDJ0LyF/pIsap6IJlCC0xZz1X2wdrF8rQG kq5FqXR0CVWkr3AuuhCJXHWVHg5xr2lG4afk6xKK4t3dBCKImX9ueB3rV0PQwg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1666547013; a=rsa-sha256; cv=none; b=xifXUELcJMwZyLMyZhl3wpdqHWPUc3AWv5n6XEkbARNR0Pzr7OW07InAXANZeZiZHJYwD/ 0msX/9VaBQJAgVzfy3toDAIVyHjJun58BuKxMezl7ZQzJdkkkYfVUE3eBg3ro7xrgktGfk g/1KKalJZ3LcIR9bZOs6U/1/bbkdg6jxB68LWJXvspM4WvFm/c975dw/4C/eVH3IIPzKN1 yZ8RTnKmoTyaEuDnetmGyylUi/NXjd+iMw/1K4fuqXHNVr/XTgqfRLvSpUIG4xxnz9x0xB aTfKCv5iWlNkU/YNxQLiLBadoNFR/yUN09Y84jzQw1Vi4opiJJ7DNRoxLESiDg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=b948644cfa0741ce928256b8a66edb0dbe257879 commit b948644cfa0741ce928256b8a66edb0dbe257879 Author: Alexander Motin AuthorDate: 2022-10-08 17:10:07 +0000 Commit: Alexander Motin CommitDate: 2022-10-23 17:40:47 +0000 inet: Simplify if_multiaddrs iteration. Similar to 2cd6ad766eb23 for inet6 drop ifma_restart use, creating more problems than solving. It is no longer needed after epoch introduction. While there, add NULL check for ifma_ifp in igmp_change_state(), that sometimes caused panics on interface destruction. MFC after: 2 weeks (cherry picked from commit 1e9482f4331bdce775061bea66ff54a6a79d5245) --- sys/netinet/igmp.c | 58 ++++++++++++++++++++------------------------------ sys/netinet/in.c | 17 ++++++--------- sys/netinet/in_mcast.c | 19 ++++++----------- sys/netinet/in_var.h | 16 +++++++++++++- 4 files changed, 51 insertions(+), 59 deletions(-) diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c index ef0da5e5cb46..8b0a5d6e194a 100644 --- a/sys/netinet/igmp.c +++ b/sys/netinet/igmp.c @@ -673,8 +673,9 @@ out: void igmp_ifdetach(struct ifnet *ifp) { + struct epoch_tracker et; struct igmp_ifsoftc *igi; - struct ifmultiaddr *ifma, *next; + struct ifmultiaddr *ifma; struct in_multi *inm; struct in_multi_head inm_free_tmp; CTR3(KTR_IGMPV3, "%s: called for ifp %p(%s)", __func__, ifp, @@ -686,20 +687,16 @@ igmp_ifdetach(struct ifnet *ifp) igi = ((struct in_ifinfo *)ifp->if_afdata[AF_INET])->ii_igmp; if (igi->igi_version == IGMP_VERSION_3) { IF_ADDR_WLOCK(ifp); - restart: - CK_STAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link, next) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + NET_EPOCH_ENTER(et); + CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; if (inm->inm_state == IGMP_LEAVING_MEMBER) inm_rele_locked(&inm_free_tmp, inm); inm_clear_recorded(inm); - if (__predict_false(ifma_restart)) { - ifma_restart = false; - goto restart; - } } + NET_EPOCH_EXIT(et); IF_ADDR_WUNLOCK(ifp); inm_release_list_deferred(&inm_free_tmp); } @@ -800,10 +797,9 @@ igmp_input_v1_query(struct ifnet *ifp, const struct ip *ip, * except those which are already running. */ CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; if (inm->inm_timer != 0) continue; switch (inm->inm_state) { @@ -901,10 +897,9 @@ igmp_input_v2_query(struct ifnet *ifp, const struct ip *ip, CTR2(KTR_IGMPV3, "process v2 general query on ifp %p(%s)", ifp, ifp->if_xname); CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; igmp_v2_update_group(inm, timer); } } else { @@ -1687,7 +1682,7 @@ igmp_fasttimo_vnet(void) struct mbufq qrq; /* Query response packets */ struct ifnet *ifp; struct igmp_ifsoftc *igi; - struct ifmultiaddr *ifma, *next; + struct ifmultiaddr *ifma; struct in_multi *inm; struct in_multi_head inm_free_tmp; int loop, uri_fasthz; @@ -1752,12 +1747,10 @@ igmp_fasttimo_vnet(void) } IF_ADDR_WLOCK(ifp); - restart: - CK_STAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link, next) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; switch (igi->igi_version) { case IGMP_VERSION_1: case IGMP_VERSION_2: @@ -1769,10 +1762,6 @@ igmp_fasttimo_vnet(void) &scq, inm, uri_fasthz); break; } - if (__predict_false(ifma_restart)) { - ifma_restart = false; - goto restart; - } } IF_ADDR_WUNLOCK(ifp); @@ -2041,7 +2030,7 @@ igmp_set_version(struct igmp_ifsoftc *igi, const int version) static void igmp_v3_cancel_link_timers(struct igmp_ifsoftc *igi) { - struct ifmultiaddr *ifma, *ifmatmp; + struct ifmultiaddr *ifma; struct ifnet *ifp; struct in_multi *inm; struct in_multi_head inm_free_tmp; @@ -2068,11 +2057,10 @@ igmp_v3_cancel_link_timers(struct igmp_ifsoftc *igi) */ ifp = igi->igi_ifp; IF_ADDR_WLOCK(ifp); - CK_STAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link, ifmatmp) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; switch (inm->inm_state) { case IGMP_NOT_MEMBER: case IGMP_SILENT_MEMBER: @@ -2329,6 +2317,8 @@ igmp_change_state(struct in_multi *inm) */ KASSERT(inm->inm_ifma != NULL, ("%s: no ifma", __func__)); ifp = inm->inm_ifma->ifma_ifp; + if (ifp == NULL) + return (0); /* * Sanity check that netinet's notion of ifp is the * same as net's. @@ -3382,11 +3372,9 @@ igmp_v3_dispatch_general_query(struct igmp_ifsoftc *igi) ifp = igi->igi_ifp; CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - - inm = (struct in_multi *)ifma->ifma_protospec; KASSERT(ifp == inm->inm_ifp, ("%s: inconsistent ifp", __func__)); diff --git a/sys/netinet/in.c b/sys/netinet/in.c index 92b9c229ea5b..9fa9ab289fd3 100644 --- a/sys/netinet/in.c +++ b/sys/netinet/in.c @@ -1258,9 +1258,10 @@ in_ifdetach(struct ifnet *ifp) static void in_purgemaddrs(struct ifnet *ifp) { + struct epoch_tracker et; struct in_multi_head purgeinms; struct in_multi *inm; - struct ifmultiaddr *ifma, *next; + struct ifmultiaddr *ifma; SLIST_INIT(&purgeinms); IN_MULTI_LIST_LOCK(); @@ -1272,18 +1273,14 @@ in_purgemaddrs(struct ifnet *ifp) * by code further down. */ IF_ADDR_WLOCK(ifp); - restart: - CK_STAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link, next) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + NET_EPOCH_ENTER(et); + CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; inm_rele_locked(&purgeinms, inm); - if (__predict_false(ifma_restart)) { - ifma_restart = true; - goto restart; - } } + NET_EPOCH_EXIT(et); IF_ADDR_WUNLOCK(ifp); inm_release_list_deferred(&purgeinms); diff --git a/sys/netinet/in_mcast.c b/sys/netinet/in_mcast.c index 46658688bbc2..c88de648eabd 100644 --- a/sys/netinet/in_mcast.c +++ b/sys/netinet/in_mcast.c @@ -114,8 +114,6 @@ MTX_SYSINIT(in_multi_free_mtx, &in_multi_free_mtx, "in_multi_free_mtx", MTX_DEF) struct sx in_multi_sx; SX_SYSINIT(in_multi_sx, &in_multi_sx, "in_multi_sx"); -int ifma_restart; - /* * Functions with non-static linkage defined in this file should be * declared in in_var.h: @@ -288,7 +286,6 @@ inm_disconnect(struct in_multi *inm) } MCDPRINTF("removed ll_ifma: %p from %s\n", ll_ifma, ifp->if_xname); if_freemulti(ll_ifma); - ifma_restart = true; } } } @@ -375,17 +372,14 @@ inm_lookup_locked(struct ifnet *ifp, const struct in_addr ina) IN_MULTI_LIST_LOCK_ASSERT(); IF_ADDR_LOCK_ASSERT(ifp); - inm = NULL; CK_STAILQ_FOREACH(ifma, &((ifp)->if_multiaddrs), ifma_link) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; if (inm->inm_addr.s_addr == ina.s_addr) - break; - inm = NULL; + return (inm); } - return (inm); + return (NULL); } /* @@ -2954,10 +2948,9 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_ARGS) IN_MULTI_LIST_LOCK(); CK_STAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) { - if (ifma->ifma_addr->sa_family != AF_INET || - ifma->ifma_protospec == NULL) + inm = inm_ifmultiaddr_get_inm(ifma); + if (inm == NULL) continue; - inm = (struct in_multi *)ifma->ifma_protospec; if (!in_hosteq(inm->inm_addr, group)) continue; fmode = inm->inm_st[1].iss_fmode; diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h index c7ebff80e56d..18d78a46d815 100644 --- a/sys/netinet/in_var.h +++ b/sys/netinet/in_var.h @@ -395,7 +395,21 @@ extern struct sx in_multi_sx; #define IN_MULTI_UNLOCK_ASSERT() sx_assert(&in_multi_sx, SA_XUNLOCKED) void inm_disconnect(struct in_multi *inm); -extern int ifma_restart; + +/* + * Get the in_multi pointer from a ifmultiaddr. + * Returns NULL if ifmultiaddr is no longer valid. + */ +static __inline struct in_multi * +inm_ifmultiaddr_get_inm(struct ifmultiaddr *ifma) +{ + + NET_EPOCH_ASSERT(); + + return ((ifma->ifma_addr->sa_family != AF_INET || + (ifma->ifma_flags & IFMA_F_ENQUEUED) == 0) ? NULL : + ifma->ifma_protospec); +} /* Acquire an in_multi record. */ static __inline void