From e686086e7d44315bccc6becf173656a6185bfcba Mon Sep 17 00:00:00 2001 From: mvs Date: Sat, 21 Jan 2023 17:35:01 +0000 Subject: [PATCH] Introduce `rt_lock' rwlock(9) and use it instead of kernel lock to serialize arpcache() and arpresolve(). In fact, net stack already has sleep points, so the rwlock(9) is better here because we avoid intersection with the rest of kernel locked paths. Also this new lock assumed to use to route layer protection instead of netlock. Hrvoje Popovski had tested this diff and found no visible performance impact. ok bluhm@ --- sys/net/route.c | 9 ++++++--- sys/net/route.h | 9 ++++++++- sys/netinet/if_ether.c | 20 ++++++++++---------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/sys/net/route.c b/sys/net/route.c index 6f5cb089aec..d6dcc3110d2 100644 --- a/sys/net/route.c +++ b/sys/net/route.c @@ -1,4 +1,4 @@ -/* $OpenBSD: route.c,v 1.414 2022/08/29 07:51:45 bluhm Exp $ */ +/* $OpenBSD: route.c,v 1.415 2023/01/21 17:35:01 mvs Exp $ */ /* $NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $ */ /* @@ -113,6 +113,7 @@ #include #include #include +#include #include #include @@ -139,6 +140,8 @@ #define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long)) +struct rwlock rt_lock = RWLOCK_INITIALIZER("rtlck"); + /* Give some jitter to hash, to avoid synchronization between routers. */ static uint32_t rt_hashjitter; @@ -253,10 +256,10 @@ rt_clone(struct rtentry **rtp, struct sockaddr *dst, unsigned int rtableid) * It should also be higher to let the ARP layer find * cloned routes instead of the cloning one. */ - KERNEL_LOCK(); + RT_LOCK(); error = rtrequest(RTM_RESOLVE, &info, rt->rt_priority - 1, &rt, rtableid); - KERNEL_UNLOCK(); + RT_UNLOCK(); if (error) { rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, error, rtableid); } else { diff --git a/sys/net/route.h b/sys/net/route.h index 2178e401731..4c0c38577dc 100644 --- a/sys/net/route.h +++ b/sys/net/route.h @@ -1,4 +1,4 @@ -/* $OpenBSD: route.h,v 1.196 2022/06/28 10:01:13 bluhm Exp $ */ +/* $OpenBSD: route.h,v 1.197 2023/01/21 17:35:01 mvs Exp $ */ /* $NetBSD: route.h,v 1.9 1996/02/13 22:00:49 christos Exp $ */ /* @@ -383,6 +383,13 @@ struct rt_addrinfo { #ifdef _KERNEL #include +#include + +extern struct rwlock rt_lock; + +#define RT_LOCK() rw_enter_write(&rt_lock) +#define RT_UNLOCK() rw_exit_write(&rt_lock) +#define RT_ASSERT_LOCKED() rw_assert_wrlock(&rt_lock) enum rtstat_counters { rts_badredirect, /* bogus redirect calls */ diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c index 6edb24bc6c0..7edc263361c 100644 --- a/sys/netinet/if_ether.c +++ b/sys/netinet/if_ether.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_ether.c,v 1.252 2022/12/07 14:38:29 claudio Exp $ */ +/* $OpenBSD: if_ether.c,v 1.253 2023/01/21 17:35:01 mvs Exp $ */ /* $NetBSD: if_ether.c,v 1.31 1996/05/11 12:59:58 mycroft Exp $ */ /* @@ -412,16 +412,16 @@ arpresolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m, if (ifp->if_flags & (IFF_NOARP|IFF_STATICARP)) goto bad; - KERNEL_LOCK(); + RT_LOCK(); /* - * Re-check since we grab the kernel lock after the first check. + * Re-check since we grab the route lock after the first check. * rtrequest_delete() can be called with shared netlock. From * there arp_rtrequest() is reached which touches RTF_LLINFO - * and rt_llinfo. As this is called with kernel lock we grab the - * kernel lock here and are safe. XXXSMP + * and rt_llinfo. As this is called with route lock we grab the + * route lock here and are safe. XXXSMP */ if (!ISSET(rt->rt_flags, RTF_LLINFO)) { - KERNEL_UNLOCK(); + RT_UNLOCK(); goto bad; } la = (struct llinfo_arp *)rt->rt_llinfo; @@ -471,7 +471,7 @@ arpresolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m, } } - KERNEL_UNLOCK(); + RT_UNLOCK(); return (EAGAIN); bad: @@ -611,9 +611,9 @@ in_arpinput(struct ifnet *ifp, struct mbuf *m) } else if (rt != NULL) { int error; - KERNEL_LOCK(); + RT_LOCK(); error = arpcache(ifp, ea, rt); - KERNEL_UNLOCK(); + RT_UNLOCK(); if (error) goto out; } @@ -659,7 +659,7 @@ arpcache(struct ifnet *ifp, struct ether_arp *ea, struct rtentry *rt) unsigned int len; int changed = 0; - KERNEL_ASSERT_LOCKED(); + RT_ASSERT_LOCKED(); KASSERT(sdl != NULL); /* -- 2.20.1