d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

inet: Simplify if_multiaddrs iteration.

Browse Source 
Similar to 2cd6ad766e 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
This commit is contained in:
Alexander Motin 2022-10-08 13:10:07 -04:00
parent 133935d26f
commit 1e9482f433
4 changed files with 51 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
sys/netinet/igmp.c
View File

@ -673,8 +673,9 @@ igi_alloc_locked(/*const*/ struct ifnet *ifp)
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__));

17
sys/netinet/in.c
View File

@ -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);

19
sys/netinet/in_mcast.c
View File

@ -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;

16
sys/netinet/in_var.h
View File

@ -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