From c8ee75f2315e8267ad814dc5b4645ef205f0e0e1 Mon Sep 17 00:00:00 2001 From: Gleb Smirnoff Date: Sun, 10 Oct 2021 10:02:26 -0700 Subject: Use network epoch to protect local IPv4 addresses hash. The modification to the hash are already naturally locked by in_control_sx. Convert the hash lists to CK lists. Remove the in_ifaddr_rmlock. Assert the network epoch where necessary. Most cases when the hash lookup is done the epoch is already entered. Cover a few cases, that need entering the epoch, which mostly is initial configuration of tunnel interfaces and multicast addresses. Reviewed by: melifaro Differential revision: https://reviews.freebsd.org/D32584 --- sys/netinet/if_ether.c | 13 +++---------- sys/netinet/in.c | 46 ++++++++++++++++++++-------------------------- sys/netinet/in.h | 2 +- sys/netinet/in_debug.c | 6 +++--- sys/netinet/in_gif.c | 3 +++ sys/netinet/in_mcast.c | 28 +++++++++++++++------------- sys/netinet/in_var.h | 24 +++++++----------------- sys/netinet/ip_gre.c | 3 +++ sys/netinet/ip_icmp.c | 7 +------ sys/netinet/ip_input.c | 12 ++++-------- 10 files changed, 60 insertions(+), 84 deletions(-) (limited to 'sys/netinet') diff --git a/sys/netinet/if_ether.c b/sys/netinet/if_ether.c index 45ce04117948..f54df9937936 100644 --- a/sys/netinet/if_ether.c +++ b/sys/netinet/if_ether.c @@ -52,7 +52,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include @@ -779,7 +778,6 @@ SYSCTL_INT(_net_link_ether_inet, OID_AUTO, allow_multicast, CTLFLAG_RW, static void in_arpinput(struct mbuf *m) { - struct rm_priotracker in_ifa_tracker; struct arphdr *ah; struct ifnet *ifp = m->m_pkthdr.rcvif; struct llentry *la = NULL, *la_tmp; @@ -846,24 +844,21 @@ in_arpinput(struct mbuf *m) * of the receive interface. (This will change slightly * when we have clusters of interfaces). */ - IN_IFADDR_RLOCK(&in_ifa_tracker); - LIST_FOREACH(ia, INADDR_HASH(itaddr.s_addr), ia_hash) { + CK_LIST_FOREACH(ia, INADDR_HASH(itaddr.s_addr), ia_hash) { if (((bridged && ia->ia_ifp->if_bridge == ifp->if_bridge) || ia->ia_ifp == ifp) && itaddr.s_addr == ia->ia_addr.sin_addr.s_addr && (ia->ia_ifa.ifa_carp == NULL || (*carp_iamatch_p)(&ia->ia_ifa, &enaddr))) { ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); goto match; } } - LIST_FOREACH(ia, INADDR_HASH(isaddr.s_addr), ia_hash) + CK_LIST_FOREACH(ia, INADDR_HASH(isaddr.s_addr), ia_hash) if (((bridged && ia->ia_ifp->if_bridge == ifp->if_bridge) || ia->ia_ifp == ifp) && isaddr.s_addr == ia->ia_addr.sin_addr.s_addr) { ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); goto match; } @@ -878,17 +873,15 @@ in_arpinput(struct mbuf *m) * meant to be destined to the bridge member. */ if (is_bridge) { - LIST_FOREACH(ia, INADDR_HASH(itaddr.s_addr), ia_hash) { + CK_LIST_FOREACH(ia, INADDR_HASH(itaddr.s_addr), ia_hash) { if (BDG_MEMBER_MATCHES_ARP(itaddr.s_addr, ifp, ia)) { ifa_ref(&ia->ia_ifa); ifp = ia->ia_ifp; - IN_IFADDR_RUNLOCK(&in_ifa_tracker); goto match; } } } #undef BDG_MEMBER_MATCHES_ARP - IN_IFADDR_RUNLOCK(&in_ifa_tracker); /* * No match, use the first inet address on the receive interface diff --git a/sys/netinet/in.c b/sys/netinet/in.c index aa87546be2d4..8eb20f0f2d27 100644 --- a/sys/netinet/in.c +++ b/sys/netinet/in.c @@ -46,7 +46,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -124,21 +123,18 @@ in_localaddr(struct in_addr in) * Return 1 if an internet address is for the local host and configured * on one of its interfaces. */ -int +bool in_localip(struct in_addr in) { - struct rm_priotracker in_ifa_tracker; struct in_ifaddr *ia; - IN_IFADDR_RLOCK(&in_ifa_tracker); - LIST_FOREACH(ia, INADDR_HASH(in.s_addr), ia_hash) { - if (IA_SIN(ia)->sin_addr.s_addr == in.s_addr) { - IN_IFADDR_RUNLOCK(&in_ifa_tracker); - return (1); - } - } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); - return (0); + NET_EPOCH_ASSERT(); + + CK_LIST_FOREACH(ia, INADDR_HASH(in.s_addr), ia_hash) + if (IA_SIN(ia)->sin_addr.s_addr == in.s_addr) + return (true); + + return (false); } /* @@ -170,24 +166,24 @@ in_ifhasaddr(struct ifnet *ifp, struct in_addr in) static struct in_ifaddr * in_localip_more(struct in_ifaddr *original_ia) { - struct rm_priotracker in_ifa_tracker; + struct epoch_tracker et; in_addr_t original_addr = IA_SIN(original_ia)->sin_addr.s_addr; uint32_t original_fib = original_ia->ia_ifa.ifa_ifp->if_fib; struct in_ifaddr *ia; - IN_IFADDR_RLOCK(&in_ifa_tracker); - LIST_FOREACH(ia, INADDR_HASH(original_addr), ia_hash) { + NET_EPOCH_ENTER(et); + CK_LIST_FOREACH(ia, INADDR_HASH(original_addr), ia_hash) { in_addr_t addr = IA_SIN(ia)->sin_addr.s_addr; uint32_t fib = ia->ia_ifa.ifa_ifp->if_fib; if (!V_rt_add_addr_allfibs && (original_fib != fib)) continue; if ((original_ia != ia) && (original_addr == addr)) { ifa_ref(&ia->ia_ifa); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + NET_EPOCH_EXIT(et); return (ia); } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + NET_EPOCH_EXIT(et); return (NULL); } @@ -500,10 +496,10 @@ in_aifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td) IF_ADDR_WUNLOCK(ifp); ifa_ref(ifa); /* in_ifaddrhead */ - IN_IFADDR_WLOCK(); + sx_assert(&in_control_sx, SA_XLOCKED); CK_STAILQ_INSERT_TAIL(&V_in_ifaddrhead, ia, ia_link); - LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr), ia, ia_hash); - IN_IFADDR_WUNLOCK(); + CK_LIST_INSERT_HEAD(INADDR_HASH(ia->ia_addr.sin_addr.s_addr), ia, + ia_hash); /* * Give the interface a chance to initialize @@ -575,10 +571,9 @@ fail1: IF_ADDR_WUNLOCK(ifp); ifa_free(&ia->ia_ifa); /* if_addrhead */ - IN_IFADDR_WLOCK(); + sx_assert(&in_control_sx, SA_XLOCKED); CK_STAILQ_REMOVE(&V_in_ifaddrhead, ia, in_ifaddr, ia_link); - LIST_REMOVE(ia, ia_hash); - IN_IFADDR_WUNLOCK(); + CK_LIST_REMOVE(ia, ia_hash); ifa_free(&ia->ia_ifa); /* in_ifaddrhead */ return (error); @@ -639,10 +634,9 @@ in_difaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td) IF_ADDR_WUNLOCK(ifp); ifa_free(&ia->ia_ifa); /* if_addrhead */ - IN_IFADDR_WLOCK(); + sx_assert(&in_control_sx, SA_XLOCKED); CK_STAILQ_REMOVE(&V_in_ifaddrhead, ia, in_ifaddr, ia_link); - LIST_REMOVE(ia, ia_hash); - IN_IFADDR_WUNLOCK(); + CK_LIST_REMOVE(ia, ia_hash); /* * in_scrubprefix() kills the interface route. diff --git a/sys/netinet/in.h b/sys/netinet/in.h index 0206fd16d2fe..0506f1739e9b 100644 --- a/sys/netinet/in.h +++ b/sys/netinet/in.h @@ -649,7 +649,7 @@ int in_broadcast(struct in_addr, struct ifnet *); int in_ifaddr_broadcast(struct in_addr, struct in_ifaddr *); int in_canforward(struct in_addr); int in_localaddr(struct in_addr); -int in_localip(struct in_addr); +bool in_localip(struct in_addr); int in_ifhasaddr(struct ifnet *, struct in_addr); struct in_ifaddr *in_findlocal(uint32_t, bool); int inet_aton(const char *, struct in_addr *); /* in libkern */ diff --git a/sys/netinet/in_debug.c b/sys/netinet/in_debug.c index d39c528dd9cd..00bc2636b436 100644 --- a/sys/netinet/in_debug.c +++ b/sys/netinet/in_debug.c @@ -90,9 +90,9 @@ in_show_in_ifaddr(struct in_ifaddr *ia) IA_DB_RPINTF_PTR("%p", ia_ifa); IA_DB_RPINTF("0x%08lx", ia_subnet); IA_DB_RPINTF("0x%08lx", ia_subnetmask); - IA_DB_RPINTF("%p", ia_hash.le_next); - IA_DB_RPINTF("%p", ia_hash.le_prev); - IA_DB_RPINTF_DPTR("%p", ia_hash.le_prev); + IA_DB_RPINTF("%p", ia_hash.cle_next); + IA_DB_RPINTF("%p", ia_hash.cle_prev); + IA_DB_RPINTF_DPTR("%p", ia_hash.cle_prev); IA_DB_RPINTF("%p", ia_link.cstqe_next); IA_DB_RPINTF_PTR("%p", ia_addr); IA_DB_RPINTF_PTR("%p", ia_dstaddr); diff --git a/sys/netinet/in_gif.c b/sys/netinet/in_gif.c index a9f3d384fb5a..6290de6cb31e 100644 --- a/sys/netinet/in_gif.c +++ b/sys/netinet/in_gif.c @@ -196,6 +196,7 @@ int in_gif_ioctl(struct gif_softc *sc, u_long cmd, caddr_t data) { struct ifreq *ifr = (struct ifreq *)data; + struct epoch_tracker et; struct sockaddr_in *dst, *src; struct ip *ip; int error; @@ -245,7 +246,9 @@ in_gif_ioctl(struct gif_softc *sc, u_long cmd, caddr_t data) sc->gif_family = AF_INET; sc->gif_iphdr = ip; in_gif_attach(sc); + NET_EPOCH_ENTER(et); in_gif_set_running(sc); + NET_EPOCH_EXIT(et); break; case SIOCGIFPSRCADDR: case SIOCGIFPDSTADDR: diff --git a/sys/netinet/in_mcast.c b/sys/netinet/in_mcast.c index f307be283e64..6ac81aa98e44 100644 --- a/sys/netinet/in_mcast.c +++ b/sys/netinet/in_mcast.c @@ -44,7 +44,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -1378,7 +1377,6 @@ static int inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) { struct group_source_req gsr; - struct rm_priotracker in_ifa_tracker; sockunion_t *gsa, *ssa; struct ifnet *ifp; struct in_mfilter *imf; @@ -1416,9 +1414,12 @@ inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) ssa->sin.sin_addr = mreqs.imr_sourceaddr; if (!in_nullhost(mreqs.imr_interface)) { - IN_IFADDR_RLOCK(&in_ifa_tracker); + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); INADDR_TO_IFP(mreqs.imr_interface, ifp); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + /* XXXGL: ifref? */ + NET_EPOCH_EXIT(et); } if (sopt->sopt_name == IP_BLOCK_SOURCE) doblock = 1; @@ -1871,7 +1872,6 @@ static struct ifnet * inp_lookup_mcast_ifp(const struct inpcb *inp, const struct sockaddr_in *gsin, const struct in_addr ina) { - struct rm_priotracker in_ifa_tracker; struct ifnet *ifp; struct nhop_object *nh; @@ -1883,11 +1883,9 @@ inp_lookup_mcast_ifp(const struct inpcb *inp, ifp = NULL; if (!in_nullhost(ina)) { - IN_IFADDR_RLOCK(&in_ifa_tracker); INADDR_TO_IFP(ina, ifp); if (ifp != NULL) if_ref(ifp); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); } else { nh = fib4_lookup(inp->inp_inc.inc_fibnum, gsin->sin_addr, 0, NHR_NONE, 0); if (nh != NULL) { @@ -2247,7 +2245,6 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sopt) { struct group_source_req gsr; struct ip_mreq_source mreqs; - struct rm_priotracker in_ifa_tracker; sockunion_t *gsa, *ssa; struct ifnet *ifp; struct in_mfilter *imf; @@ -2307,9 +2304,12 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sopt) * using an IPv4 address as a key is racy. */ if (!in_nullhost(mreqs.imr_interface)) { - IN_IFADDR_RLOCK(&in_ifa_tracker); + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); INADDR_TO_IFP(mreqs.imr_interface, ifp); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + /* XXXGL ifref? */ + NET_EPOCH_EXIT(et); } CTR3(KTR_IGMPV3, "%s: imr_interface = 0x%08x, ifp = %p", __func__, ntohl(mreqs.imr_interface.s_addr), ifp); @@ -2465,7 +2465,6 @@ out_inp_locked: static int inp_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) { - struct rm_priotracker in_ifa_tracker; struct in_addr addr; struct ip_mreqn mreqn; struct ifnet *ifp; @@ -2504,9 +2503,12 @@ inp_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) if (in_nullhost(addr)) { ifp = NULL; } else { - IN_IFADDR_RLOCK(&in_ifa_tracker); + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); INADDR_TO_IFP(addr, ifp); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); + /* XXXGL ifref? */ + NET_EPOCH_EXIT(et); if (ifp == NULL) return (EADDRNOTAVAIL); } diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h index c33098e2c79c..a6902159e739 100644 --- a/sys/netinet/in_var.h +++ b/sys/netinet/in_var.h @@ -79,7 +79,7 @@ struct in_ifaddr { /* ia_subnet{,mask} in host order */ u_long ia_subnet; /* subnet address */ u_long ia_subnetmask; /* mask of subnet */ - LIST_ENTRY(in_ifaddr) ia_hash; /* entry in bucket of inet addresses */ + CK_LIST_ENTRY(in_ifaddr) ia_hash; /* hash of internet addresses */ CK_STAILQ_ENTRY(in_ifaddr) ia_link; /* list of internet addresses */ struct sockaddr_in ia_addr; /* reserve space for interface name */ struct sockaddr_in ia_dstaddr; /* reserve space for broadcast addr */ @@ -108,7 +108,7 @@ extern u_char inetctlerrmap[]; * Hash table for IP addresses. */ CK_STAILQ_HEAD(in_ifaddrhead, in_ifaddr); -LIST_HEAD(in_ifaddrhashhead, in_ifaddr); +CK_LIST_HEAD(in_ifaddrhashhead, in_ifaddr); VNET_DECLARE(struct in_ifaddrhashhead *, in_ifaddrhashtbl); VNET_DECLARE(struct in_ifaddrhead, in_ifaddrhead); @@ -124,16 +124,6 @@ VNET_DECLARE(u_long, in_ifaddrhmask); /* mask for hash table */ #define INADDR_HASH(x) \ (&V_in_ifaddrhashtbl[INADDR_HASHVAL(x) & V_in_ifaddrhmask]) -extern struct rmlock in_ifaddr_lock; - -#define IN_IFADDR_LOCK_ASSERT() rm_assert(&in_ifaddr_lock, RA_LOCKED) -#define IN_IFADDR_RLOCK(t) rm_rlock(&in_ifaddr_lock, (t)) -#define IN_IFADDR_RLOCK_ASSERT() rm_assert(&in_ifaddr_lock, RA_RLOCKED) -#define IN_IFADDR_RUNLOCK(t) rm_runlock(&in_ifaddr_lock, (t)) -#define IN_IFADDR_WLOCK() rm_wlock(&in_ifaddr_lock) -#define IN_IFADDR_WLOCK_ASSERT() rm_assert(&in_ifaddr_lock, RA_WLOCKED) -#define IN_IFADDR_WUNLOCK() rm_wunlock(&in_ifaddr_lock) - /* * Macro for finding the internet address structure (in_ifaddr) * corresponding to one of our IP addresses (in_addr). @@ -141,11 +131,11 @@ extern struct rmlock in_ifaddr_lock; #define INADDR_TO_IFADDR(addr, ia) \ /* struct in_addr addr; */ \ /* struct in_ifaddr *ia; */ \ -do { \ -\ - LIST_FOREACH(ia, INADDR_HASH((addr).s_addr), ia_hash) \ - if (IA_SIN(ia)->sin_addr.s_addr == (addr).s_addr) \ - break; \ +do { \ + NET_EPOCH_ASSERT(); \ + CK_LIST_FOREACH(ia, INADDR_HASH((addr).s_addr), ia_hash) \ + if (IA_SIN(ia)->sin_addr.s_addr == (addr).s_addr) \ + break; \ } while (0) /* diff --git a/sys/netinet/ip_gre.c b/sys/netinet/ip_gre.c index 0afd490944a4..6a2135fa32cd 100644 --- a/sys/netinet/ip_gre.c +++ b/sys/netinet/ip_gre.c @@ -363,6 +363,7 @@ fail: static int in_gre_attach(struct gre_softc *sc) { + struct epoch_tracker et; struct grehdr *gh; int error; @@ -397,7 +398,9 @@ in_gre_attach(struct gre_softc *sc) sc, srchash); /* Set IFF_DRV_RUNNING if interface is ready */ + NET_EPOCH_ENTER(et); in_gre_set_running(sc); + NET_EPOCH_EXIT(et); return (0); } diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c index f8dfc21df8f3..463ac8c8e04d 100644 --- a/sys/netinet/ip_icmp.c +++ b/sys/netinet/ip_icmp.c @@ -44,7 +44,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include @@ -756,7 +755,6 @@ freeit: static void icmp_reflect(struct mbuf *m) { - struct rm_priotracker in_ifa_tracker; struct ip *ip = mtod(m, struct ip *); struct ifaddr *ifa; struct ifnet *ifp; @@ -785,15 +783,12 @@ icmp_reflect(struct mbuf *m) * If the incoming packet was addressed directly to one of our * own addresses, use dst as the src for the reply. */ - IN_IFADDR_RLOCK(&in_ifa_tracker); - LIST_FOREACH(ia, INADDR_HASH(t.s_addr), ia_hash) { + CK_LIST_FOREACH(ia, INADDR_HASH(t.s_addr), ia_hash) { if (t.s_addr == IA_SIN(ia)->sin_addr.s_addr) { t = IA_SIN(ia)->sin_addr; - IN_IFADDR_RUNLOCK(&in_ifa_tracker); goto match; } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); /* * If the incoming packet was addressed to one of our broadcast diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index 465c00e4dac7..dc122dd62e99 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -102,9 +102,6 @@ extern void ipreass_slowtimo(void); extern void ipreass_destroy(void); #endif -struct rmlock in_ifaddr_lock; -RM_SYSINIT(in_ifaddr_lock, &in_ifaddr_lock, "in_ifaddr_lock"); - VNET_DEFINE(int, rsvp_on); VNET_DEFINE(int, ipforwarding); @@ -180,6 +177,9 @@ VNET_DEFINE(struct in_ifaddrhead, in_ifaddrhead); /* first inet address */ VNET_DEFINE(struct in_ifaddrhashhead *, in_ifaddrhashtbl); /* inet addr hash table */ VNET_DEFINE(u_long, in_ifaddrhmask); /* mask for hash table */ +/* Make sure it is safe to use hashinit(9) on CK_LIST. */ +CTASSERT(sizeof(struct in_ifaddrhashhead) == sizeof(LIST_HEAD(, in_addr))); + #ifdef IPCTL_DEFMTU SYSCTL_INT(_net_inet_ip, IPCTL_DEFMTU, mtu, CTLFLAG_RW, &ip_mtu, 0, "Default MTU"); @@ -453,7 +453,6 @@ void ip_input(struct mbuf *m) { MROUTER_RLOCK_TRACKER; - struct rm_priotracker in_ifa_tracker; struct ip *ip = NULL; struct in_ifaddr *ia = NULL; struct ifaddr *ifa; @@ -689,8 +688,7 @@ passin: /* * Check for exact addresses in the hash bucket. */ - IN_IFADDR_RLOCK(&in_ifa_tracker); - LIST_FOREACH(ia, INADDR_HASH(ip->ip_dst.s_addr), ia_hash) { + CK_LIST_FOREACH(ia, INADDR_HASH(ip->ip_dst.s_addr), ia_hash) { /* * If the address matches, verify that the packet * arrived via the correct interface if checking is @@ -701,11 +699,9 @@ passin: counter_u64_add(ia->ia_ifa.ifa_ipackets, 1); counter_u64_add(ia->ia_ifa.ifa_ibytes, m->m_pkthdr.len); - IN_IFADDR_RUNLOCK(&in_ifa_tracker); goto ours; } } - IN_IFADDR_RUNLOCK(&in_ifa_tracker); /* * Check for broadcast addresses. -- cgit v1.2.3