Date: Sat, 4 Oct 2003 11:04:25 -0700 (PDT) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 39159 for review Message-ID: <200310041804.h94I4P0Q018732@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=39159 Change 39159 by sam@sam_ebb on 2003/10/04 11:03:37 interlock access to the ip forwarding route cache Affected files ... .. //depot/projects/netperf/sys/netinet/in_rmx.c#6 edit .. //depot/projects/netperf/sys/netinet/ip_input.c#8 edit .. //depot/projects/netperf/sys/netinet/ip_var.h#6 edit Differences ... ==== //depot/projects/netperf/sys/netinet/in_rmx.c#6 (text+ko) ==== @@ -26,7 +26,7 @@ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * - * $FreeBSD: src/sys/netinet/in_rmx.c,v 1.44 2003/02/10 22:01:34 hsu Exp $ + * $FreeBSD: src/sys/netinet/in_rmx.c,v 1.45 2003/10/04 03:44:49 sam Exp $ */ /* @@ -141,13 +141,10 @@ /* * If the new route created successfully, and we are forwarding, - * and there is a cached route, free it. Otherwise, we may end - * up using the wrong route. + * flush any cached routes to avoid using a stale value. */ - if (ret != NULL && ipforwarding && ipforward_rt.ro_rt) { - RTFREE(ipforward_rt.ro_rt); - ipforward_rt.ro_rt = 0; - } + if (ret != NULL && ipforwarding) + ip_forward_cacheinval(); return ret; } ==== //depot/projects/netperf/sys/netinet/ip_input.c#8 (text+ko) ==== @@ -189,8 +189,8 @@ #define IPQ_LOCK() mtx_lock(&ipqlock) #define IPQ_UNLOCK() mtx_unlock(&ipqlock) -#define IPQ_LOCK_INIT() mtx_init(&ipqlock, "ipqlock", NULL, MTX_DEF); -#define IPQ_LOCK_ASSERT() mtx_assert(&ipqlock, MA_OWNED); +#define IPQ_LOCK_INIT() mtx_init(&ipqlock, "ipqlock", NULL, MTX_DEF) +#define IPQ_LOCK_ASSERT() mtx_assert(&ipqlock, MA_OWNED) #ifdef IPCTL_DEFMTU SYSCTL_INT(_net_inet_ip, IPCTL_DEFMTU, mtu, CTLFLAG_RW, @@ -212,8 +212,42 @@ /* Dummynet hooks */ ip_dn_io_t *ip_dn_io_ptr; +/* + * One deep route cache for ip forwarding. + */ +static struct rtcache { + struct route rc_ro; /* most recently used route */ + struct mtx rc_mtx; /* update lock for cache */ +} ip_fwdcache; + +#define RTCACHE_LOCK() mtx_lock(&ip_fwdcache.rc_mtx) +#define RTCACHE_UNLOCK() mtx_unlock(&ip_fwdcache.rc_mtx) +#define RTCACHE_LOCK_INIT() \ + mtx_init(&ip_fwdcache.rc_mtx, "route cache", NULL, MTX_DEF); +#define RTCACHE_LOCK_ASSERT() mtx_assert(&ip_fwdcache.rc_mtx, MA_OWNED) + +/* + * Get the current route cache contents. + */ +#define RTCACHE_GET(_ro) do { \ + RTCACHE_LOCK(); \ + *(_ro) = ip_fwdcache.rc_ro; \ + RTCACHE_UNLOCK(); \ +} while (0); /* + * Update the cache contents. We optimize this using + * the routing table reference. XXX is this safe? + */ +#define RTCACHE_UPDATE(_ro) do { \ + if ((_ro)->ro_rt != ip_fwdcache.rc_ro.ro_rt) { \ + RTCACHE_LOCK(); \ + ip_fwdcache.rc_ro = *(_ro); \ + RTCACHE_UNLOCK(); \ + } \ +} while (0); + +/* * XXX this is ugly -- the following two global variables are * used to store packet state while it travels through the stack. * Note that the code even makes assumptions on the size and @@ -237,7 +271,7 @@ static void save_rte(u_char *, struct in_addr); static int ip_dooptions(struct mbuf *m, int, struct sockaddr_in *next_hop); -static void ip_forward(struct mbuf *m, int srcrt, +static void ip_forward(struct mbuf *m, struct route *, int srcrt, struct sockaddr_in *next_hop); static void ip_freef(struct ipqhead *, struct ipq *); static struct mbuf *ip_reass(struct mbuf *, struct ipqhead *, @@ -278,6 +312,9 @@ for (i = 0; i < IPREASS_NHASH; i++) TAILQ_INIT(&ipq[i]); + bzero(&ip_fwdcache, sizeof(ip_fwdcache)); + RTCACHE_LOCK_INIT(); + maxnipq = nmbclusters / 32; maxfragsperpacket = 16; @@ -290,11 +327,21 @@ } /* - * XXX watch out this one. It is perhaps used as a cache for - * the most recently used route ? it is cleared in in_addroute() - * when a new route is successfully created. + * Invalidate any cached route used for forwarding. */ -struct route ipforward_rt; +void +ip_forward_cacheinval(void) +{ + struct rtentry *rt = NULL; + + RTCACHE_LOCK(); + rt = ip_fwdcache.rc_ro.ro_rt; + ip_fwdcache.rc_ro.ro_rt = 0; + RTCACHE_UNLOCK(); + + if (rt != NULL) + RTFREE(rt); +} /* * Ip input routine. Checksum and byte swap header. If fragmented @@ -312,6 +359,7 @@ struct in_addr pkt_dst; u_int32_t divert_info = 0; /* packet divert/tee info */ struct ip_fw_args args; + struct route cro; /* copy of cached route */ #ifdef FAST_IPSEC struct m_tag *mtag; struct tdb_ident *tdbi; @@ -712,7 +760,8 @@ goto bad; } #endif /* FAST_IPSEC */ - ip_forward(m, 0, args.next_hop); + RTCACHE_GET(&cro); + ip_forward(m, &cro, 0, args.next_hop); } return; @@ -1312,6 +1361,14 @@ struct in_addr *sin, dst; n_time ntime; struct sockaddr_in ipaddr = { sizeof(ipaddr), AF_INET }; + struct route cro; /* copy of cached route */ + + /* + * Grab a copy of the route cache in case we need + * to update to reflect source routing or the like. + * Could optimize this to do it later... + */ + RTCACHE_GET(&cro); dst = ip->ip_dst; cp = (u_char *)(ip + 1); @@ -1431,7 +1488,7 @@ if ((ia = (INA)ifa_ifwithdstaddr((SA)&ipaddr)) == 0) ia = (INA)ifa_ifwithnet((SA)&ipaddr); } else - ia = ip_rtaddr(ipaddr.sin_addr, &ipforward_rt); + ia = ip_rtaddr(ipaddr.sin_addr, &cro); if (ia == 0) { type = ICMP_UNREACH; code = ICMP_UNREACH_SRCFAIL; @@ -1473,8 +1530,7 @@ * use the incoming interface (should be same). */ if ((ia = (INA)ifa_ifwithaddr((SA)&ipaddr)) == 0 && - (ia = ip_rtaddr(ipaddr.sin_addr, - &ipforward_rt)) == 0) { + (ia = ip_rtaddr(ipaddr.sin_addr, &cro)) == 0) { type = ICMP_UNREACH; code = ICMP_UNREACH_HOST; goto bad; @@ -1554,7 +1610,7 @@ } } if (forward && ipforwarding) { - ip_forward(m, 1, next_hop); + ip_forward(m, &cro, 1, next_hop); return (1); } return (0); @@ -1739,7 +1795,8 @@ * via a source route. */ static void -ip_forward(struct mbuf *m, int srcrt, struct sockaddr_in *next_hop) +ip_forward(struct mbuf *m, struct route *ro, + int srcrt, struct sockaddr_in *next_hop) { struct ip *ip = mtod(m, struct ip *); struct rtentry *rt; @@ -1784,11 +1841,11 @@ } #endif - if (ip_rtaddr(pkt_dst, &ipforward_rt) == 0) { + if (ip_rtaddr(pkt_dst, ro) == 0) { icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0); return; } else - rt = ipforward_rt.ro_rt; + rt = ro->ro_rt; /* * Save the IP header and at most 8 bytes of the payload, @@ -1878,9 +1935,13 @@ tag.mh_next = m; m = (struct mbuf *)&tag; } - error = ip_output(m, (struct mbuf *)0, &ipforward_rt, - IP_FORWARDING, 0, NULL); + error = ip_output(m, (struct mbuf *)0, ro, IP_FORWARDING, 0, NULL); } + /* + * Update the ip forwarding cache with the route we used. + * We may want to do this more selectively; not sure. + */ + RTCACHE_UPDATE(ro); if (error) ipstat.ips_cantforward++; else { @@ -1889,7 +1950,7 @@ ipstat.ips_redirectsent++; else { if (mcopy) { - ipflow_create(&ipforward_rt, mcopy); + ipflow_create(ro, mcopy); m_freem(mcopy); } return; @@ -1924,11 +1985,10 @@ * tunnel MTU = if MTU - sizeof(IP) - ESP/AH hdrsiz * XXX quickhack!!! */ - if (ipforward_rt.ro_rt) { + if (ro->ro_rt) { struct secpolicy *sp = NULL; int ipsecerror; int ipsechdr; - struct route *ro; sp = ipsec4_getpolicybyaddr(mcopy, IPSEC_DIR_OUTBOUND, @@ -1936,7 +1996,7 @@ &ipsecerror); if (sp == NULL) - destifp = ipforward_rt.ro_rt->rt_ifp; + destifp = ro->ro_rt->rt_ifp; else { /* count IPsec header size */ ipsechdr = ipsec4_hdrsiz(mcopy, @@ -1956,10 +2016,11 @@ if (sp->req != NULL && sp->req->sav != NULL && sp->req->sav->sah != NULL) { - ro = &sp->req->sav->sah->sa_route; - if (ro->ro_rt && ro->ro_rt->rt_ifp) { + struct route *saro; + saro = &sp->req->sav->sah->sa_route; + if (saro->ro_rt && saro->ro_rt->rt_ifp) { dummyifp.if_mtu = - ro->ro_rt->rt_ifp->if_mtu; + saro->ro_rt->rt_ifp->if_mtu; dummyifp.if_mtu -= ipsechdr; destifp = &dummyifp; } @@ -1975,11 +2036,10 @@ * tunnel MTU = if MTU - sizeof(IP) - ESP/AH hdrsiz * XXX quickhack!!! */ - if (ipforward_rt.ro_rt) { + if (ro->ro_rt) { struct secpolicy *sp = NULL; int ipsecerror; int ipsechdr; - struct route *ro; sp = ipsec_getpolicybyaddr(mcopy, IPSEC_DIR_OUTBOUND, @@ -1987,7 +2047,7 @@ &ipsecerror); if (sp == NULL) - destifp = ipforward_rt.ro_rt->rt_ifp; + destifp = ro->ro_rt->rt_ifp; else { /* count IPsec header size */ ipsechdr = ipsec4_hdrsiz(mcopy, @@ -2007,10 +2067,11 @@ if (sp->req != NULL && sp->req->sav != NULL && sp->req->sav->sah != NULL) { - ro = &sp->req->sav->sah->sa_route; - if (ro->ro_rt && ro->ro_rt->rt_ifp) { + struct route *saro; + saro = &sp->req->sav->sah->sa_route; + if (saro->ro_rt && saro->ro_rt->rt_ifp) { dummyifp.if_mtu = - ro->ro_rt->rt_ifp->if_mtu; + saro->ro_rt->rt_ifp->if_mtu; dummyifp.if_mtu -= ipsechdr; destifp = &dummyifp; } @@ -2020,8 +2081,8 @@ } } #else /* !IPSEC && !FAST_IPSEC */ - if (ipforward_rt.ro_rt) - destifp = ipforward_rt.ro_rt->rt_ifp; + if (ro->ro_rt) + destifp = ro->ro_rt->rt_ifp; #endif /*IPSEC*/ ipstat.ips_cantfrag++; break; ==== //depot/projects/netperf/sys/netinet/ip_var.h#6 (text+ko) ==== @@ -154,7 +154,6 @@ #endif extern int ip_defttl; /* default IP ttl */ extern int ipforwarding; /* ip forwarding */ -extern struct route ipforward_rt; /* ip forwarding cached route */ extern u_char ip_protox[]; extern struct socket *ip_rsvpd; /* reservation protocol daemon */ extern struct socket *ip_mrouter; /* multicast routing daemon */ @@ -167,6 +166,7 @@ void ip_drain(void); int ip_fragment(struct ip *ip, struct mbuf **m_frag, int mtu, u_long if_hwassist_flags, int sw_csum); +void ip_forward_cacheinval(void); void ip_freemoptions(struct ip_moptions *); void ip_init(void); extern int (*ip_mforward)(struct ip *, struct ifnet *, struct mbuf *,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200310041804.h94I4P0Q018732>