Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Aug 2009 10:05:54 +0000 (UTC)
From:      "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net>
To:        Qing Li <qingli@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r196609 - head/sys/net
Message-ID:  <20090828094234.G93661@maildrop.int.zabbadoz.net>
In-Reply-To: <200908280701.n7S71JB7015826@svn.freebsd.org>
References:  <200908280701.n7S71JB7015826@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 28 Aug 2009, Qing Li wrote:

> Author: qingli
> Date: Fri Aug 28 07:01:09 2009
> New Revision: 196609
> URL: http://svn.freebsd.org/changeset/base/196609
>
> Log:
>  In ip_output(), the flow-table module must not try to cache L2/L3
>  information for interface of IFF_POINTOPOINT or IFF_LOOPBACK type.
>  Since the L2 information (rt_lle) is invalid for these interface
>  types, accidental caching attempt will trigger panic when the invalid
>  rt_lle reference is accessed.
>
>  When installing a new route, or when updating an existing route, the
>  user supplied gateway address may be an interface address (this is
>  particularly true for point-to-point interface related modules such
>  as ppp, if_tun, if_gif). Currently the routing command handler always
>  set the RTF_GATEWAY flag if the gateway address is given as part of the
>  command paramters. Therefore the gateway address must be verified against
>  interface addresses or else the route would be treated as an indirect
>  route, thus making that route unusable.
>
>  Reviewed by:	kmacy, julia, rwatson
>  Verified by:	marcus
>  MFC after:	3 days


Am I right that this is two entirely separate issues in one commit?



> Modified:
>  head/sys/net/flowtable.c
>  head/sys/net/rtsock.c
>
> Modified: head/sys/net/flowtable.c
> ==============================================================================
> --- head/sys/net/flowtable.c	Fri Aug 28 05:37:31 2009	(r196608)
> +++ head/sys/net/flowtable.c	Fri Aug 28 07:01:09 2009	(r196609)
> @@ -692,6 +692,12 @@ uncached:
> 		struct rtentry *rt = ro->ro_rt;
> 		struct ifnet *ifp = rt->rt_ifp;
>
> +		if (ifp->if_flags & (IFF_POINTOPOINT | IFF_LOOPBACK)) {
> +			RTFREE(rt);
> +			ro->ro_rt = NULL;
> +			return (ENOENT);
> +		}
> +
> 		if (rt->rt_flags & RTF_GATEWAY)
> 			l3addr = rt->rt_gateway;
> 		else
>
> Modified: head/sys/net/rtsock.c
> ==============================================================================
> --- head/sys/net/rtsock.c	Fri Aug 28 05:37:31 2009	(r196608)
> +++ head/sys/net/rtsock.c	Fri Aug 28 07:01:09 2009	(r196609)
> @@ -513,6 +513,39 @@ route_output(struct mbuf *m, struct sock
> 			senderr(error);
> 	}
>
> +	/*
> +	 * The given gateway address may be an interface address.
> +	 * For example, issuing a "route change" command on a route
> +	 * entry that was created from a tunnel, and the gateway
> +	 * address given is the local end point. In this case the
> +	 * RTF_GATEWAY flag must be cleared or the destination will
> +	 * not be reachable even though there is no error message.
> +	 */
> +	if (info.rti_info[RTAX_GATEWAY] != NULL &&
> +	    info.rti_info[RTAX_GATEWAY]->sa_family != AF_LINK) {
> +		struct route gw_ro;
> +
> +		bzero(&gw_ro, sizeof(gw_ro));
> +		gw_ro.ro_dst = *info.rti_info[RTAX_GATEWAY];
> +		rtalloc_ign(&gw_ro, 0);

This ignores the FIB?  Shouldn't it be:
 	rtalloc_ign_fib(&gw_ro, 0, so->so_fibnum);

Considering that "julia" reviewed it, I wonder what I am missing here?



> +		/*
> +		 * A host route through the loopback interface is
> +		 * installed for each interface adddress. In pre 8.0
> +		 * releases the interface address of a PPP link type
> +		 * is not reachable locally. This behavior is fixed as
> +		 * part of the new L2/L3 redesign and rewrite work. The
> +		 * signature of this interface address route is the
> +		 * AF_LINK sa_family type of the rt_gateway, and the
> +		 * rt_ifp has the IFF_LOOPBACK flag set.
> +		 */
> +		if (gw_ro.ro_rt != NULL &&
> +		    gw_ro.ro_rt->rt_gateway->sa_family == AF_LINK &&
> +		    gw_ro.ro_rt->rt_ifp->if_flags & IFF_LOOPBACK)
> +			info.rti_flags &= ~RTF_GATEWAY;
> +		if (gw_ro.ro_rt != NULL)
> +			RTFREE(gw_ro.ro_rt);
> +	}
> +
> 	switch (rtm->rtm_type) {
> 		struct rtentry *saved_nrt;
>
> @@ -714,7 +747,7 @@ route_output(struct mbuf *m, struct sock
> 					RT_UNLOCK(rt);
> 					senderr(error);
> 				}
> -				rt->rt_flags |= RTF_GATEWAY;
> +				rt->rt_flags |= (RTF_GATEWAY & info.rti_flags);
> 			}
> 			if (info.rti_ifa != NULL &&
> 			    info.rti_ifa != rt->rt_ifa) {


Thanks for handling this!


/bz

-- 
Bjoern A. Zeeb           What was I talking about and who are you again?



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090828094234.G93661>