On Tue, Aug 25, 2015 at 01:01:38PM +0200, Martin Pieuchot wrote:
> I want to remove this chunked introduce in r7.19 in 1991 by sklower@
> because it no longer makes any sense, it is a layer violation and
> does no play well with rt refcounting.
>
> When this chunk was introduced rtrequest1(RTM_DELETE...) was *not* doing
> a route lookup. So this check is now (and since a long time) redundant
> with the rtable_lookup()+rtable_mpath_match() done inside rtrequest1(9).
>
> Remember that when this code was introduced ``ifa'' where linked to
> ``rt'' in 1:1 fashion and rtrequest() started by rtfree(9)ing route
> entries. That's why you wanted to be sure that the following line was
> correct:
>
> error = rtrequest(cmd, dst, ifa->ifa_addr, ifa->ifa_netmask,
> flags | ifa->ifa_flags, &ifa->ifa_rt)
>
> Ok?
It looks that the error changes from EHOSTUNREACH/ENETUNREACH to
ESRCH. That is good, when you try to remove an non existing
address/route.
The check in the old code and in rtable_mpath_match() is not
equivalent. The ifa was used to compare it with every route. In
rtable_mpath_match() the RTAX_GATEWAY is used for that purpose.
But this field is only set, if RTF_LLINFO is set in flags.
So at least the old check does not look completely redundant. But
it still looks wrong to search for a route by comparing rt_ifa with
ifa and, after that has been found, delete another one.
So I would say, delete that chunk.
bluhm
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.225
> diff -u -p -r1.225 route.c
> --- net/route.c 24 Aug 2015 22:11:33 -0000 1.225
> +++ net/route.c 25 Aug 2015 10:43:32 -0000
> @@ -1230,21 +1230,6 @@ rt_ifa_del(struct ifaddr *ifa, int flags
> rt_maskedcopy(dst, deldst, ifa->ifa_netmask);
> dst = deldst;
> }
> - if ((rt = rtalloc(dst, 0, rtableid)) != NULL) {
> - rt->rt_refcnt--;
> -#ifndef ART
> - /* try to find the right route */
> - while (rt && rt->rt_ifa != ifa)
> - rt = (struct rtentry *)
> - ((struct radix_node *)rt)->rn_dupedkey;
> - if (!rt) {
> - if (m != NULL)
> - (void) m_free(m);
> - return (flags & RTF_HOST ? EHOSTUNREACH
> - : ENETUNREACH);
> - }
> -#endif
> - }
>
> memset(&info, 0, sizeof(info));
> info.rti_ifa = ifa;
