From nobody Sat Oct 08 17:17:46 2022 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 4MlBhC1ZQ7z4V9mS; Sat, 8 Oct 2022 17:17:47 +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 4MlBhC156Gz44bg; Sat, 8 Oct 2022 17:17:47 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1665249467; 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=+mJKbPNZk65W6pDuHdtrEHRlpDCDLRTSeAP7Lewjqac=; b=DJ53r25nYWLt4DZwSx/1rXtdV07XOKk+Jko9OGuxhgD5TZVX9EijRLapoRj7zjJFSK1VQq kXGcd3qbVJCvkE+0i7K3ikOwnMsfhR/YtG95oxcMAhmg3uy2xb5LcFVMskGszaelIKzaPw ztCv8PmcjHmAi4+fMqUDweDtX6PHpF+2K67vsmQkQWp1qpSwx4n3OpR9rzse1tABobX/QC xtqgWye5EencZb8tNLOze9meqSO5aHLP8+/exrB8YmQIaR74+V5i0930L2RgVcwbk2As93 UVs1qXJn3IWHji9zq0ei3p3jvuDw9AEF4EQaEhisKW7gLFJNnnwPXGtWcmKq1A== 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 4MlBhC02Zvz11Gt; Sat, 8 Oct 2022 17:17:47 +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 298HHkIr074885; Sat, 8 Oct 2022 17:17:46 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 298HHkXE074884; Sat, 8 Oct 2022 17:17:46 GMT (envelope-from git) Date: Sat, 8 Oct 2022 17:17:46 GMT Message-Id: <202210081717.298HHkXE074884@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Alexander Motin Subject: git: 1e9482f4331b - main - inet: Simplify if_multiaddrs iteration. 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: mav X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 1e9482f4331bdce775061bea66ff54a6a79d5245 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1665249467; 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=+mJKbPNZk65W6pDuHdtrEHRlpDCDLRTSeAP7Lewjqac=; b=fR4EIjWpZPW0RQho2Wu3sn1EzY3EHsWH3waC8CYiVefTyb57KPOr5m6gA9V1tIiC3gZFXu EAvepIcOpfQ3x/yP/tUMyH5kdaG2P++6x1yBCgTK27f+2MEfkWCpHRYcQukaHZpXXn2m2j cNRfEkrlAwiRrgW5vS3qARUr3v80RG5HSRac/iK6BH/BjTtXmfjw4wzGtGm6DKZj0pMnEN HadGfB8XFDci2TbZpg1uucwRU4LhlUThAOkfKQnmR7MPyky6DnGGPOdF0XMoWciuf4daU1 8Cwmf30RAVWSVjQtAw+4r72bzsDkvaUl0wL8r+LhI7laWhj3WOjNfvb7sHd+tQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1665249467; a=rsa-sha256; cv=none; b=WLnOl/uYkdlyH86ohqVZEz2cijYJQDuEakBVukaKICRNYEn9vzQbdryYddhP/Gkei5OF0S oaxUWcsi5QTcegiE4RyBBSV8/JVRpLWlAerSY98oXKGdyMWr/lGydBVcUH1dDt0rDTqVIr vSwrcszCStq8gS4LUa44BrnfS8grZ0SGvQgI7kdIDLVkbxGL3jZc6Ka5xJs6YJBHg07FE4 Zxr2PK+IfjHMz7QpT1pZmt4JuFIAvmT+HQQr3Sl1Lk+h6UIhCEDzJkcSmK2qWbr3NWH2VC 9PphtS9nbzpGcLS5c/fwOyftyKDAIrLfgNQti5juCORfvY9DxuZ1wEtkQZ3j5Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=1e9482f4331bdce775061bea66ff54a6a79d5245 commit 1e9482f4331bdce775061bea66ff54a6a79d5245 Author: Alexander Motin AuthorDate: 2022-10-08 17:10:07 +0000 Commit: Alexander Motin CommitDate: 2022-10-08 17:10:07 +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 --- 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 61ca24c4b519..cebde1798c6d 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 { @@ -1691,7 +1686,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; @@ -1756,12 +1751,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: @@ -1773,10 +1766,6 @@ igmp_fasttimo_vnet(void) &scq, inm, uri_fasthz); break; } - if (__predict_false(ifma_restart)) { - ifma_restart = false; - goto restart; - } } IF_ADDR_WUNLOCK(ifp); @@ -2045,7 +2034,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; @@ -2072,11 +2061,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: @@ -2339,6 +2327,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. @@ -3398,11 +3388,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 7f88a897ff44..b6b412042dad 100644 --- a/sys/netinet/in.c +++ b/sys/netinet/in.c @@ -1370,9 +1370,10 @@ EVENTHANDLER_DEFINE(ifnet_event, in_ifnet_event, NULL, EVENTHANDLER_PRI_ANY); 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(); @@ -1384,18 +1385,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 87de83da7a6a..b2bfce038088 100644 --- a/sys/netinet/in_mcast.c +++ b/sys/netinet/in_mcast.c @@ -113,8 +113,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: @@ -282,7 +280,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; } } } @@ -369,17 +366,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); } /* @@ -2901,10 +2895,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 c4cfeea66ba8..40955e26bd81 100644 --- a/sys/netinet/in_var.h +++ b/sys/netinet/in_var.h @@ -380,7 +380,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