From: Alexander Bluhm Subject: unlock igmp slow timeout To: tech@openbsd.org Date: Tue, 11 Nov 2025 10:49:59 +0100 Hi, I would like to remove net lock from igmp_slowtimo(). Replace it with a mutex that protects the router_info list. ok? bluhm Index: netinet/igmp.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v diff -u -p -r1.88 igmp.c --- netinet/igmp.c 8 Jul 2025 00:47:41 -0000 1.88 +++ netinet/igmp.c 10 Nov 2025 22:26:28 -0000 @@ -77,6 +77,7 @@ #include #include +#include #include #include #include @@ -94,15 +95,23 @@ #define IP_MULTICASTOPTS 0 +/* + * Locks used to protect global data and struct members: + * I immutable after creation + * a atomic + * G global igmp mutex igmp_mtx + */ + int igmp_timers_are_running; /* [a] shortcut for fast timer */ -static LIST_HEAD(, router_info) rti_head; +struct mutex igmp_mtx = MUTEX_INITIALIZER(IPL_SOFTNET); +static LIST_HEAD(, router_info) rti_head; /* [G] */ static struct mbuf *router_alert; struct cpumem *igmpcounters; int igmp_checktimer(struct ifnet *); void igmp_sendpkt(struct ifnet *, struct in_multi *, int, in_addr_t); int rti_fill(struct in_multi *); -struct router_info * rti_find(struct ifnet *); +int rti_reset(struct ifnet *); int igmp_input_if(struct ifnet *, struct mbuf **, int *, int, int, struct netstack *); int igmp_sysctl_igmpstat(void *, size_t *, void *); @@ -137,61 +146,96 @@ igmp_init(void) router_alert->m_len = sizeof(ra->ipopt_dst) + ra->ipopt_list[1]; } -int -rti_fill(struct in_multi *inm) +static struct router_info * +rti_find_locked(unsigned int ifidx) { struct router_info *rti; + MUTEX_ASSERT_LOCKED(&igmp_mtx); + LIST_FOREACH(rti, &rti_head, rti_list) { - if (rti->rti_ifidx == inm->inm_ifidx) { - inm->inm_rti = rti; - if (rti->rti_type == IGMP_v1_ROUTER) - return (IGMP_v1_HOST_MEMBERSHIP_REPORT); - else - return (IGMP_v2_HOST_MEMBERSHIP_REPORT); - } + if (rti->rti_ifidx == ifidx) + return (rti); } + return (NULL); +} - rti = malloc(sizeof(*rti), M_MRTABLE, M_WAITOK); +int +rti_fill(struct in_multi *inm) +{ + struct router_info *rti, *new_rti = NULL; + int type; + + mtx_enter(&igmp_mtx); + rti = rti_find_locked(inm->inm_ifidx); + if (rti) + goto found; + mtx_leave(&igmp_mtx); + + new_rti = malloc(sizeof(struct router_info), M_MRTABLE, M_WAITOK); + + mtx_enter(&igmp_mtx); + /* check again after unlock and lock */ + rti = rti_find_locked(inm->inm_ifidx); + if (rti) + goto found; + rti = new_rti; rti->rti_ifidx = inm->inm_ifidx; rti->rti_type = IGMP_v2_ROUTER; LIST_INSERT_HEAD(&rti_head, rti, rti_list); inm->inm_rti = rti; + mtx_leave(&igmp_mtx); return (IGMP_v2_HOST_MEMBERSHIP_REPORT); + + found: + inm->inm_rti = rti; + type = rti->rti_type; + mtx_leave(&igmp_mtx); + free(new_rti, M_MRTABLE, sizeof(struct router_info)); + if (type == IGMP_v1_ROUTER) + return (IGMP_v1_HOST_MEMBERSHIP_REPORT); + else + return (IGMP_v2_HOST_MEMBERSHIP_REPORT); } -struct router_info * -rti_find(struct ifnet *ifp) +int +rti_reset(struct ifnet *ifp) { struct router_info *rti; - KERNEL_ASSERT_LOCKED(); - LIST_FOREACH(rti, &rti_head, rti_list) { - if (rti->rti_ifidx == ifp->if_index) - return (rti); + mtx_enter(&igmp_mtx); + rti = rti_find_locked(ifp->if_index); + if (rti) { + rti->rti_type = IGMP_v1_ROUTER; + rti->rti_age = 0; + mtx_leave(&igmp_mtx); + return (0); } rti = malloc(sizeof(*rti), M_MRTABLE, M_NOWAIT); - if (rti == NULL) - return (NULL); + if (rti == NULL) { + mtx_leave(&igmp_mtx); + return (ENOBUFS); + } rti->rti_ifidx = ifp->if_index; - rti->rti_type = IGMP_v2_ROUTER; + rti->rti_type = IGMP_v1_ROUTER; + rti->rti_age = 0; LIST_INSERT_HEAD(&rti_head, rti, rti_list); - return (rti); + mtx_leave(&igmp_mtx); + return (0); } void rti_delete(struct ifnet *ifp) { - struct router_info *rti, *trti; + struct router_info *rti; - LIST_FOREACH_SAFE(rti, &rti_head, rti_list, trti) { - if (rti->rti_ifidx == ifp->if_index) { - LIST_REMOVE(rti, rti_list); - free(rti, M_MRTABLE, sizeof(*rti)); - break; - } - } + mtx_enter(&igmp_mtx); + rti = rti_find_locked(ifp->if_index); + if (rti) + LIST_REMOVE(rti, rti_list); + mtx_leave(&igmp_mtx); + free(rti, M_MRTABLE, sizeof(*rti)); } int @@ -226,9 +270,8 @@ igmp_input_if(struct ifnet *ifp, struct int minlen; struct ifmaddr *ifma; struct in_multi *inm; - struct router_info *rti; struct in_ifaddr *ia; - int timer, running = 0; + int error, timer, running = 0; igmplen = ntohs(ip->ip_len) - iphlen; @@ -271,13 +314,11 @@ igmp_input_if(struct ifnet *ifp, struct break; if (igmp->igmp_code == 0) { - rti = rti_find(ifp); - if (rti == NULL) { + error = rti_reset(ifp); + if (error) { m_freem(m); return IPPROTO_DONE; } - rti->rti_type = IGMP_v1_ROUTER; - rti->rti_age = 0; if (ip->ip_dst.s_addr != INADDR_ALLHOSTS_GROUP) { igmpstat_inc(igps_rcv_badqueries); @@ -599,16 +640,14 @@ igmp_slowtimo(void) { struct router_info *rti; - NET_LOCK(); - + mtx_enter(&igmp_mtx); LIST_FOREACH(rti, &rti_head, rti_list) { if (rti->rti_type == IGMP_v1_ROUTER && ++rti->rti_age >= IGMP_AGE_THRESHOLD) { rti->rti_type = IGMP_v2_ROUTER; } } - - NET_UNLOCK(); + mtx_leave(&igmp_mtx); } void Index: netinet/in_var.h =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_var.h,v diff -u -p -r1.41 in_var.h --- netinet/in_var.h 18 Oct 2018 15:23:04 -0000 1.41 +++ netinet/in_var.h 10 Nov 2025 22:02:57 -0000 @@ -99,10 +99,10 @@ do { \ * Per-interface router version information. */ struct router_info { - unsigned int rti_ifidx; - int rti_type; /* type of router on this interface */ - int rti_age; /* time since last v1 query */ - LIST_ENTRY(router_info) rti_list; + unsigned int rti_ifidx; /* [I] */ + int rti_type; /* [G] type of router on interface */ + int rti_age; /* [G] time since last v1 query */ + LIST_ENTRY(router_info) rti_list; /* [G] */ }; #ifdef _KERNEL