git: 1e9482f4331b - main - inet: Simplify if_multiaddrs iteration.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 08 Oct 2022 17:17:46 UTC
The branch main has been updated by mav:
URL: https://cgit.FreeBSD.org/src/commit/?id=1e9482f4331bdce775061bea66ff54a6a79d5245
commit 1e9482f4331bdce775061bea66ff54a6a79d5245
Author: Alexander Motin <mav@FreeBSD.org>
AuthorDate: 2022-10-08 17:10:07 +0000
Commit: Alexander Motin <mav@FreeBSD.org>
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