Date: Mon, 10 Sep 2012 11:38:02 +0000 (UTC) From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r240305 - in stable/9/sys: net netinet netinet6 Message-ID: <201209101138.q8ABc2Up046308@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: glebius Date: Mon Sep 10 11:38:02 2012 New Revision: 240305 URL: http://svn.freebsd.org/changeset/base/240305 Log: Merge r238002, r238016, r238092 from head: r238002 by tuexen@: Remove dead code (on FreeBSD) as suggested by glebius@. r238016: Remove route caching from IP multicast routing code. There is no reason to do that, and also, cached route never got unreferenced, which meant a reference leak. Reviewed by: bms r238092: When ip_output()/ip6_output() is supplied a struct route *ro argument, it skips FLOWTABLE lookup. However, the non-NULL ro has dual meaning here: it may be supplied to provide route, and it may be supplied to store and return to caller the route that ip_output()/ip6_output() finds. In the latter case skipping FLOWTABLE lookup is pessimisation. The difference between struct route filled by FLOWTABLE and filled by rtalloc() family is that the former doesn't hold a reference on its rtentry. Reference is hold by flow entry, and it is about to be released in future. Thus, route filled by FLOWTABLE shouldn't be passed to RTFREE() macro. - Introduce new flag for struct route/route_in6, that marks route not holding a reference on rtentry. - Introduce new macro RO_RTFREE() that cleans up a struct route depending on its kind. - All callers to ip_output()/ip6_output() that do supply non-NULL but empty route should use RO_RTFREE() to free results of lookup. - ip_output()/ip6_output() now do FLOWTABLE lookup always when ro->ro_rt == NULL. Modified: stable/9/sys/net/flowtable.c stable/9/sys/net/route.h stable/9/sys/netinet/ip_input.c stable/9/sys/netinet/ip_mroute.c stable/9/sys/netinet/ip_mroute.h stable/9/sys/netinet/ip_output.c stable/9/sys/netinet/sctp_output.c stable/9/sys/netinet6/ip6_mroute.c stable/9/sys/netinet6/ip6_mroute.h stable/9/sys/netinet6/ip6_output.c stable/9/sys/netinet6/nd6_nbr.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/net/flowtable.c ============================================================================== --- stable/9/sys/net/flowtable.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/net/flowtable.c Mon Sep 10 11:38:02 2012 (r240305) @@ -618,6 +618,7 @@ flow_to_route(struct flentry *fle, struc sin->sin_addr.s_addr = hashkey[2]; ro->ro_rt = __DEVOLATILE(struct rtentry *, fle->f_rt); ro->ro_lle = __DEVOLATILE(struct llentry *, fle->f_lle); + ro->ro_flags |= RT_NORTREF; } #endif /* INET */ @@ -825,7 +826,7 @@ flow_to_route_in6(struct flentry *fle, s memcpy(&sin6->sin6_addr, &hashkey[5], sizeof (struct in6_addr)); ro->ro_rt = __DEVOLATILE(struct rtentry *, fle->f_rt); ro->ro_lle = __DEVOLATILE(struct llentry *, fle->f_lle); - + ro->ro_flags |= RT_NORTREF; } #endif /* INET6 */ Modified: stable/9/sys/net/route.h ============================================================================== --- stable/9/sys/net/route.h Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/net/route.h Mon Sep 10 11:38:02 2012 (r240305) @@ -54,7 +54,8 @@ struct route { struct sockaddr ro_dst; }; -#define RT_CACHING_CONTEXT 0x1 +#define RT_CACHING_CONTEXT 0x1 /* XXX: not used anywhere */ +#define RT_NORTREF 0x2 /* doesn't hold reference on ro_rt */ /* * These numbers are used by reliable protocols for determining @@ -341,6 +342,18 @@ struct rt_addrinfo { RTFREE_LOCKED(_rt); \ } while (0) +#define RO_RTFREE(_ro) do { \ + if ((_ro)->ro_rt) { \ + if ((_ro)->ro_flags & RT_NORTREF) { \ + (_ro)->ro_flags &= ~RT_NORTREF; \ + (_ro)->ro_rt = NULL; \ + } else { \ + RT_LOCK((_ro)->ro_rt); \ + RTFREE_LOCKED((_ro)->ro_rt); \ + } \ + } \ +} while (0) + struct radix_node_head *rt_tables_get_rnh(int, int); struct ifmultiaddr; Modified: stable/9/sys/netinet/ip_input.c ============================================================================== --- stable/9/sys/netinet/ip_input.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet/ip_input.c Mon Sep 10 11:38:02 2012 (r240305) @@ -1495,8 +1495,7 @@ ip_forward(struct mbuf *m, int srcrt) if (error == EMSGSIZE && ro.ro_rt) mtu = ro.ro_rt->rt_rmx.rmx_mtu; - if (ro.ro_rt) - RTFREE(ro.ro_rt); + RO_RTFREE(&ro); if (error) IPSTAT_INC(ips_cantforward); Modified: stable/9/sys/netinet/ip_mroute.c ============================================================================== --- stable/9/sys/netinet/ip_mroute.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet/ip_mroute.c Mon Sep 10 11:38:02 2012 (r240305) @@ -924,7 +924,6 @@ add_vif(struct vifctl *vifcp) vifp->v_pkt_out = 0; vifp->v_bytes_in = 0; vifp->v_bytes_out = 0; - bzero(&vifp->v_route, sizeof(vifp->v_route)); /* Adjust numvifs up if the vifi is higher than numvifs */ if (V_numvifs <= vifcp->vifc_vifi) @@ -1702,7 +1701,7 @@ send_packet(struct vif *vifp, struct mbu * should get rejected because they appear to come from * the loopback interface, thus preventing looping. */ - error = ip_output(m, NULL, &vifp->v_route, IP_FORWARDING, &imo, NULL); + error = ip_output(m, NULL, NULL, IP_FORWARDING, &imo, NULL); CTR3(KTR_IPMF, "%s: vif %td err %d", __func__, (ptrdiff_t)(vifp - V_viftable), error); } Modified: stable/9/sys/netinet/ip_mroute.h ============================================================================== --- stable/9/sys/netinet/ip_mroute.h Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet/ip_mroute.h Mon Sep 10 11:38:02 2012 (r240305) @@ -262,7 +262,6 @@ struct vif { u_long v_pkt_out; /* # pkts out on interface */ u_long v_bytes_in; /* # bytes in on interface */ u_long v_bytes_out; /* # bytes out on interface */ - struct route v_route; /* cached route */ }; #ifdef _KERNEL Modified: stable/9/sys/netinet/ip_output.c ============================================================================== --- stable/9/sys/netinet/ip_output.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet/ip_output.c Mon Sep 10 11:38:02 2012 (r240305) @@ -105,6 +105,10 @@ extern struct protosw inetsw[]; * ip_len and ip_off are in host format. * The mbuf chain containing the packet will be freed. * The mbuf opt, if present, will not be freed. + * If route ro is present and has ro_rt initialized, route lookup would be + * skipped and ro->ro_rt would be used. If ro is present but ro->ro_rt is NULL, + * then result of route lookup is stored in ro->ro_rt. + * * In the IP forwarding case, the packet will arrive with options already * inserted, so must have a NULL opt pointer. */ @@ -119,7 +123,6 @@ ip_output(struct mbuf *m, struct mbuf *o int mtu; int n; /* scratchpad */ int error = 0; - int nortfree = 0; struct sockaddr_in *dst; struct in_ifaddr *ia; int isbroadcast, sw_csum; @@ -146,24 +149,23 @@ ip_output(struct mbuf *m, struct mbuf *o if (ro == NULL) { ro = &iproute; bzero(ro, sizeof (*ro)); + } #ifdef FLOWTABLE - { - struct flentry *fle; + if (ro->ro_rt == NULL) { + struct flentry *fle; - /* - * The flow table returns route entries valid for up to 30 - * seconds; we rely on the remainder of ip_output() taking no - * longer than that long for the stability of ro_rt. The - * flow ID assignment must have happened before this point. - */ - if ((fle = flowtable_lookup_mbuf(V_ip_ft, m, AF_INET)) != NULL) { - flow_to_route(fle, ro); - nortfree = 1; - } - } -#endif + /* + * The flow table returns route entries valid for up to 30 + * seconds; we rely on the remainder of ip_output() taking no + * longer than that long for the stability of ro_rt. The + * flow ID assignment must have happened before this point. + */ + fle = flowtable_lookup_mbuf(V_ip_ft, m, AF_INET); + if (fle != NULL) + flow_to_route(fle, ro); } +#endif if (opt) { int len = 0; @@ -210,10 +212,9 @@ again: !RT_LINK_IS_UP(rte->rt_ifp) || dst->sin_family != AF_INET || dst->sin_addr.s_addr != ip->ip_dst.s_addr)) { - if (!nortfree) - RTFREE(rte); - rte = ro->ro_rt = (struct rtentry *)NULL; - ro->ro_lle = (struct llentry *)NULL; + RO_RTFREE(ro); + ro->ro_lle = NULL; + rte = NULL; } #ifdef IPFIREWALL_FORWARD if (rte == NULL && fwd_tag == NULL) { @@ -678,9 +679,8 @@ passout: IPSTAT_INC(ips_fragmented); done: - if (ro == &iproute && ro->ro_rt && !nortfree) { - RTFREE(ro->ro_rt); - } + if (ro == &iproute) + RO_RTFREE(ro); if (ia != NULL) ifa_free(&ia->ia_ifa); return (error); Modified: stable/9/sys/netinet/sctp_output.c ============================================================================== --- stable/9/sys/netinet/sctp_output.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet/sctp_output.c Mon Sep 10 11:38:02 2012 (r240305) @@ -4163,10 +4163,7 @@ sctp_lowlevel_chunk_output(struct sctp_i SCTPDBG(SCTP_DEBUG_OUTPUT3, "IP output returns %d\n", ret); if (net == NULL) { /* free tempy routes */ - if (ro->ro_rt) { - RTFREE(ro->ro_rt); - ro->ro_rt = NULL; - } + RO_RTFREE(ro); } else { /* * PMTU check versus smallest asoc MTU goes @@ -4520,9 +4517,7 @@ sctp_lowlevel_chunk_output(struct sctp_i } if (net == NULL) { /* Now if we had a temp route free it */ - if (ro->ro_rt) { - RTFREE(ro->ro_rt); - } + RO_RTFREE(ro); } else { /* * PMTU check versus smallest asoc MTU goes @@ -10902,7 +10897,6 @@ sctp_send_resp_msg(struct mbuf *m, struc int len, cause_len, padding_len, ret; #ifdef INET - sctp_route_t ro; struct ip *iph_out; #endif @@ -11066,8 +11060,6 @@ sctp_send_resp_msg(struct mbuf *m, struc SCTP_ATTACH_CHAIN(o_pak, mout, len); #ifdef INET if (iph_out != NULL) { - /* zap the stack pointer to the route */ - bzero(&ro, sizeof(sctp_route_t)); if (port) { if (V_udp_cksum) { udp->uh_sum = in_pseudo(iph_out->ip_src.s_addr, iph_out->ip_dst.s_addr, udp->uh_ulen + htons(IPPROTO_UDP)); @@ -11100,11 +11092,7 @@ sctp_send_resp_msg(struct mbuf *m, struc SCTP_STAT_INCR(sctps_sendhwcrc); #endif } - SCTP_IP_OUTPUT(ret, o_pak, &ro, NULL, vrf_id); - /* Free the route if we got one back */ - if (ro.ro_rt) { - RTFREE(ro.ro_rt); - } + SCTP_IP_OUTPUT(ret, o_pak, NULL, NULL, vrf_id); } #endif #ifdef INET6 Modified: stable/9/sys/netinet6/ip6_mroute.c ============================================================================== --- stable/9/sys/netinet6/ip6_mroute.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet6/ip6_mroute.c Mon Sep 10 11:38:02 2012 (r240305) @@ -717,7 +717,6 @@ add_m6if(struct mif6ctl *mifcp) mifp->m6_pkt_out = 0; mifp->m6_bytes_in = 0; mifp->m6_bytes_out = 0; - bzero(&mifp->m6_route, sizeof(mifp->m6_route)); /* Adjust nummifs up if the mifi is higher than nummifs */ if (nummifs <= mifcp->mif6c_mifi) @@ -1576,11 +1575,8 @@ phyint_send(struct ip6_hdr *ip6, struct struct mbuf *mb_copy; struct ifnet *ifp = mifp->m6_ifp; int error = 0; - struct sockaddr_in6 *dst6; u_long linkmtu; - dst6 = &mifp->m6_route.ro_dst; - /* * Make a new reference to the packet; make sure that * the IPv6 header is actually copied, not just referenced, @@ -1610,8 +1606,8 @@ phyint_send(struct ip6_hdr *ip6, struct /* XXX: ip6_output will override ip6->ip6_hlim */ im6o.im6o_multicast_hlim = ip6->ip6_hlim; im6o.im6o_multicast_loop = 1; - error = ip6_output(mb_copy, NULL, &mifp->m6_route, - IPV6_FORWARDING, &im6o, NULL, NULL); + error = ip6_output(mb_copy, NULL, NULL, IPV6_FORWARDING, &im6o, + NULL, NULL); #ifdef MRT6DEBUG if (V_mrt6debug & DEBUG_XMIT) @@ -1626,10 +1622,13 @@ phyint_send(struct ip6_hdr *ip6, struct * loop back a copy now. */ if (in6_mcast_loop) { - dst6->sin6_len = sizeof(struct sockaddr_in6); - dst6->sin6_family = AF_INET6; - dst6->sin6_addr = ip6->ip6_dst; - ip6_mloopback(ifp, m, &mifp->m6_route.ro_dst); + struct sockaddr_in6 dst6; + + bzero(&dst6, sizeof(dst6)); + dst6.sin6_len = sizeof(struct sockaddr_in6); + dst6.sin6_family = AF_INET6; + dst6.sin6_addr = ip6->ip6_dst; + ip6_mloopback(ifp, m, &dst6); } /* @@ -1638,15 +1637,18 @@ phyint_send(struct ip6_hdr *ip6, struct */ linkmtu = IN6_LINKMTU(ifp); if (mb_copy->m_pkthdr.len <= linkmtu || linkmtu < IPV6_MMTU) { - dst6->sin6_len = sizeof(struct sockaddr_in6); - dst6->sin6_family = AF_INET6; - dst6->sin6_addr = ip6->ip6_dst; + struct sockaddr_in6 dst6; + + bzero(&dst6, sizeof(dst6)); + dst6.sin6_len = sizeof(struct sockaddr_in6); + dst6.sin6_family = AF_INET6; + dst6.sin6_addr = ip6->ip6_dst; /* * We just call if_output instead of nd6_output here, since * we need no ND for a multicast forwarded packet...right? */ error = (*ifp->if_output)(ifp, mb_copy, - (struct sockaddr *)&mifp->m6_route.ro_dst, NULL); + (struct sockaddr *)&dst6, NULL); #ifdef MRT6DEBUG if (V_mrt6debug & DEBUG_XMIT) log(LOG_DEBUG, "phyint_send on mif %d err %d\n", Modified: stable/9/sys/netinet6/ip6_mroute.h ============================================================================== --- stable/9/sys/netinet6/ip6_mroute.h Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet6/ip6_mroute.h Mon Sep 10 11:38:02 2012 (r240305) @@ -212,7 +212,6 @@ struct mif6 { u_quad_t m6_pkt_out; /* # pkts out on interface */ u_quad_t m6_bytes_in; /* # bytes in on interface */ u_quad_t m6_bytes_out; /* # bytes out on interface */ - struct route_in6 m6_route; /* cached route */ #ifdef notyet u_int m6_rsvp_on; /* RSVP listening on this vif */ struct socket *m6_rsvpd; /* RSVP daemon socket */ Modified: stable/9/sys/netinet6/ip6_output.c ============================================================================== --- stable/9/sys/netinet6/ip6_output.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet6/ip6_output.c Mon Sep 10 11:38:02 2012 (r240305) @@ -214,6 +214,9 @@ in6_delayed_cksum(struct mbuf *m, uint32 * This function may modify ver and hlim only. * The mbuf chain containing the packet will be freed. * The mbuf opt, if present, will not be freed. + * If route_in6 ro is present and has ro_rt initialized, route lookup would be + * skipped and ro->ro_rt would be used. If ro is present but ro->ro_rt is NULL, + * then result of route lookup is stored in ro->ro_rt. * * type of "mtu": rt_rmx.rmx_mtu is u_long, ifnet.ifr_mtu is int, and * nd_ifinfo.linkmtu is u_int32_t. so we use u_long to hold largest one, @@ -244,7 +247,6 @@ ip6_output(struct mbuf *m0, struct ip6_p struct in6_addr finaldst, src0, dst0; u_int32_t zone; struct route_in6 *ro_pmtu = NULL; - int flevalid = 0; int hdrsplit = 0; int needipsec = 0; int sw_csum, tso; @@ -521,7 +523,7 @@ skip_ipsec2:; ro = &opt->ip6po_route; dst = (struct sockaddr_in6 *)&ro->ro_dst; #ifdef FLOWTABLE - if (ro == &ip6route) { + if (ro->ro_rt == NULL) { struct flentry *fle; /* @@ -530,11 +532,9 @@ skip_ipsec2:; * longer than that long for the stability of ro_rt. The * flow ID assignment must have happened before this point. */ - if ((fle = flowtable_lookup_mbuf(V_ip6_ft, m, AF_INET6)) != NULL) { + fle = flowtable_lookup_mbuf(V_ip6_ft, m, AF_INET6); + if (fle != NULL) flow_to_route_in6(fle, ro); - if (ro->ro_rt != NULL && ro->ro_lle != NULL) - flevalid = 1; - } } #endif again: @@ -642,7 +642,7 @@ again: dst_sa.sin6_family = AF_INET6; dst_sa.sin6_len = sizeof(dst_sa); dst_sa.sin6_addr = ip6->ip6_dst; - if (flevalid) { + if (ro->ro_rt) { rt = ro->ro_rt; ifp = ro->ro_rt->rt_ifp; } else if ((error = in6_selectroute_fib(&dst_sa, opt, im6o, ro, @@ -1197,13 +1197,10 @@ sendorfree: V_ip6stat.ip6s_fragmented++; done: - if (ro == &ip6route && ro->ro_rt && flevalid == 0) { - /* brace necessary for RTFREE */ - RTFREE(ro->ro_rt); - } else if (ro_pmtu == &ip6route && ro_pmtu->ro_rt && - ((flevalid == 0) || (ro_pmtu != ro))) { - RTFREE(ro_pmtu->ro_rt); - } + if (ro == &ip6route) + RO_RTFREE(ro); + if (ro_pmtu == &ip6route) + RO_RTFREE(ro_pmtu); #ifdef IPSEC if (sp != NULL) KEY_FREESP(&sp); Modified: stable/9/sys/netinet6/nd6_nbr.c ============================================================================== --- stable/9/sys/netinet6/nd6_nbr.c Mon Sep 10 11:08:08 2012 (r240304) +++ stable/9/sys/netinet6/nd6_nbr.c Mon Sep 10 11:38:02 2012 (r240305) @@ -595,9 +595,9 @@ nd6_ns_output(struct ifnet *ifp, const s icmp6_ifstat_inc(ifp, ifs6_out_neighborsolicit); ICMP6STAT_INC(icp6s_outhist[ND_NEIGHBOR_SOLICIT]); - if (ro.ro_rt) { /* we don't cache this route. */ - RTFREE(ro.ro_rt); - } + /* We don't cache this route. */ + RO_RTFREE(&ro); + return; bad: @@ -1117,9 +1117,9 @@ nd6_na_output_fib(struct ifnet *ifp, con icmp6_ifstat_inc(ifp, ifs6_out_neighboradvert); ICMP6STAT_INC(icp6s_outhist[ND_NEIGHBOR_ADVERT]); - if (ro.ro_rt) { /* we don't cache this route. */ - RTFREE(ro.ro_rt); - } + /* We don't cache this route. */ + RO_RTFREE(&ro); + return; bad:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201209101138.q8ABc2Up046308>