The branch main has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=1e9482f4331bdce775061bea66ff54a6a79d5245
commit 1e9482f4331bdce775061bea66ff54a6a79d5245 Author: Alexander Motin <[email protected]> AuthorDate: 2022-10-08 17:10:07 +0000 Commit: Alexander Motin <[email protected]> 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
