On Tue, Sep 01, 2015 at 04:47:31PM +0200, Martin Pieuchot wrote:
> Smaller diff now that rtisvalid(9) is in!
> +rt_isvalid(struct rtentry *rt)
> +/*
> + * Returns 1 if the (cached) ``rt'' entry is still valid, 0 otherwise.
> + */
> +int
> +rtisvalid(struct rtentry *rt)
> +{
> + if (!rt_isvalid(rt))
> + return (0);
> +
> + /* Next hop must also be valid. */
> + if ((rt->rt_flags & RTF_GATEWAY) && !rt_isvalid(rt->rt_gwroute))
> + return (0);
> +
> + return (1);
> +}
Please do not call two functions rt_isvalid() and rtisvalid(), that
is confusing. Maybe rt_isvalid() could be called rtisvalidnotgw().
Is it possible that a gateway route has another gateway route as
gateway? rt_checkgate() checks and fixes that. What do you think
about this? Maybe the panic() is too aggressive and should be replaced
with a return (0) to avoid recursion.
bluhm
Index: net/route.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
retrieving revision 1.228
diff -u -p -r1.228 route.c
--- net/route.c 1 Sep 2015 12:50:03 -0000 1.228
+++ net/route.c 2 Sep 2015 14:54:54 -0000
@@ -316,6 +316,15 @@ rtisvalid(struct rtentry *rt)
if (rt->rt_ifa == NULL || rt->rt_ifa->ifa_ifp == NULL)
return (0);
+ /* Next hop must also be valid. */
+ if (rt->rt_flags & RTF_GATEWAY) {
+ if (rt->rt_gwroute && (rt->rt_gwroute->rt_flags & RTF_GATEWAY))
+ panic("%s: route %p has gateway route %p with gateway",
+ __func__, rt, rt->rt_gwroute);
+ if (!rtisvalid(rt->rt_gwroute))
+ return (0);
+ }
+
return (1);
}
