Download raw body.
unlock igmp slow timeout
On Tue, Nov 11, 2025 at 10:49:59AM +0100, Alexander Bluhm wrote:
> I would like to remove net lock from igmp_slowtimo(). Replace it
> with a mutex that protects the router_info list.
And here rebase, cleanup, fix race, add mutex.
If you prefer to review the whole thing.
ok?
bluhm
Index: netinet/igmp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
diff -u -p -r1.90 igmp.c
--- netinet/igmp.c 12 Nov 2025 11:37:08 -0000 1.90
+++ netinet/igmp.c 12 Nov 2025 16:29:23 -0000
@@ -77,6 +77,7 @@
#include <sys/param.h>
#include <sys/mbuf.h>
+#include <sys/mutex.h>
#include <sys/systm.h>
#include <sys/socket.h>
#include <sys/protosw.h>
@@ -95,24 +96,32 @@
#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
+ */
+
+/*
* Per-interface router version information.
*/
struct router_info {
- LIST_ENTRY(router_info) rti_list;
- 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; /* [G] */
+ unsigned int rti_ifidx; /* [I] */
+ int rti_type; /* [G] type of router on interface */
+ int rti_age; /* [G] time since last v1 query */
};
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 *);
@@ -147,61 +156,95 @@ 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(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(inm->inm_ifidx);
+ if (rti != NULL)
+ goto found;
+ mtx_leave(&igmp_mtx);
+
+ new_rti = malloc(sizeof(*rti), M_MRTABLE, M_WAITOK);
+
+ mtx_enter(&igmp_mtx);
+ /* check again after unlock and lock */
+ rti = rti_find(inm->inm_ifidx);
+ if (rti != NULL)
+ 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(*rti));
+ return (type == IGMP_v1_ROUTER ?
+ IGMP_v1_HOST_MEMBERSHIP_REPORT : 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(ifp->if_index);
+ if (rti != NULL)
+ goto found;
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;
LIST_INSERT_HEAD(&rti_head, rti, rti_list);
- return (rti);
+ found:
+ rti->rti_type = IGMP_v1_ROUTER;
+ rti->rti_age = 0;
+ 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(ifp->if_index);
+ if (rti != NULL)
+ LIST_REMOVE(rti, rti_list);
+ mtx_leave(&igmp_mtx);
+
+ free(rti, M_MRTABLE, sizeof(*rti));
}
int
@@ -236,9 +279,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;
@@ -281,13 +323,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);
@@ -609,16 +649,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
unlock igmp slow timeout