On Mon, Aug 15, 2016 at 01:52:46PM +0200, Martin Pieuchot wrote:
> More tests, comments, ok are welcome.
There is an issue with path mtu discovery that my regression test
found, but i think that can be fixed with this diff in tree.
> +/*
> + * Invalidate the cached route entry of the gateway entry ``rt''.
> + */
> +void
> +rt_putgwroute(struct rtentry *rt)
> +{
> + struct rtentry *nhrt = rt->rt_gwroute;
> +
> + KERNEL_ASSERT_LOCKED();
> +
> + if (!ISSET(rt->rt_flags, RTF_GATEWAY) || nhrt == NULL)
> + return;
> +
> + KASSERT(nhrt->rt_cachecnt > 0);
Could you put a KASSERT(ISSET(rt->rt_flags, RTF_CACHED)) before
accessing rt_cachecnt?
> @@ -615,7 +615,27 @@ route_output(struct mbuf *m, ...)
> }
> break;
> case RTM_DELETE:
> - error = rtrequest(RTM_DELETE, &info, prio, &rt, tableid);
...
> + error = rtrequest(RTM_DELETE, &info, prio, NULL, tableid);
> if (error == 0)
> goto report;
> break;
I think you should keep passing &rt instead of NULL, otherwise rt
might be NULL when you goto report.
> +void
> +arpinvalidate(struct rtentry *rt)
> +{
> + struct llinfo_arp *la = (struct llinfo_arp *)rt->rt_llinfo;
> + struct sockaddr_dl *sdl = satosdl(rt->rt_gateway);
> +
> + la_hold_total -= ml_purge(&la->la_ml);
> + sdl->sdl_alen = 0;
> + la->la_asked = 0;
> +}
Is it safe to modify the la and sdl while another CPU might use it?
Especially ml_purge() looks dangerous.
> +void
> +nd6_invalidate(struct rtentry *rt)
> +{
> + struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo;
> +
> + m_freem(ln->ln_hold);
> + ln->ln_hold = NULL;
> + ln->ln_state = ND6_LLINFO_INCOMPLETE;
> + ln->ln_asked = 0;
> +}
Same here. Is it safe to free ln_hold?
> @@ -1495,18 +1512,13 @@ nd6_resolve(struct ifnet *ifp, struct rt
> struct sockaddr_dl *sdl;
> struct rtentry *rt;
> struct llinfo_nd6 *ln = NULL;
> - int error;
>
> if (m->m_flags & M_MCAST) {
> ETHER_MAP_IPV6_MULTICAST(&satosin6(dst)->sin6_addr, desten);
> return (0);
> }
>
> - error = rt_checkgate(rt0, &rt);
> - if (error) {
> - m_freem(m);
> - return (error);
> - }
> + rt = rt_getll(rt0);
Should we check for RTF_REJECT like it was done in rt_checkgate()
before?
bluhm
