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