svn commit: r240305 - in stable/9/sys: net netinet netinet6

Gleb Smirnoff glebius at FreeBSD.org
Mon Sep 10 11:38:03 UTC 2012


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 at .
  
  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:


More information about the svn-src-stable-9 mailing list