On Tue, Jun 28, 2016 at 08:30:16AM +0200, Martin Pieuchot wrote: > With this diff if your next hop becomes invalid after being cached you'll > also need two ICMP6_PACKET_TOO_BIG to restore the MTU, is this wanted?
No, a single ICMP6_PACKET_TOO_BIG packet is enough. I have written a test that checks all cases. https://github.com/bluhm/pmtu-regress After the issue has been fixed, I would like to commit it to /usr/src/regress/sys/netinet/pmtu . > I would prefer if we could cache the gw route in a single place. So > I'm wondering if the check for reseting PMTUD makes sense (where it is). After testing TCP, TCP6, UDP6 I still think my diff is correct. It makes sense to reset the PMTU discovery after the gateway route is gone. I the current implementation rtalloc() returns a route with a valid gateway, while rtrequest(RTM_ADD) returns a route with an invalid gateway. I think both methods should provide a similar route, so rt_setgwroute() should fix the gateway route in both functions. bluhm > > Index: net/if_spppsubr.c > > =================================================================== > > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_spppsubr.c,v > > retrieving revision 1.154 > > diff -u -p -r1.154 if_spppsubr.c > > --- net/if_spppsubr.c 14 Jun 2016 20:44:43 -0000 1.154 > > +++ net/if_spppsubr.c 24 Jun 2016 22:32:28 -0000 > > @@ -4161,7 +4161,7 @@ sppp_update_gw_walker(struct rtentry *rt > > rt->rt_gateway->sa_family || > > !ISSET(rt->rt_flags, RTF_GATEWAY)) > > return (0); /* do not modify non-gateway routes */ > > - rt_setgate(rt, rt->rt_ifa->ifa_dstaddr); > > + rt_setgate(rt, rt->rt_ifa->ifa_dstaddr, ifp->if_rdomain); > > } > > return (0); > > } > > Index: net/route.c > > =================================================================== > > RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v > > retrieving revision 1.309 > > diff -u -p -r1.309 route.c > > --- net/route.c 14 Jun 2016 09:48:52 -0000 1.309 > > +++ net/route.c 24 Jun 2016 22:37:45 -0000 > > @@ -154,6 +154,7 @@ struct pool rttimer_pool; /* pool for r > > > > void rt_timer_init(void); > > int rt_setaddr(struct rtentry *, struct sockaddr *); > > +void rt_setgwroute(struct rtentry *, u_int); > > int rtflushclone1(struct rtentry *, void *, u_int); > > void rtflushclone(unsigned int, struct rtentry *); > > int rt_if_remove_rtdelete(struct rtentry *, void *, u_int); > > @@ -369,7 +370,7 @@ rtalloc(struct sockaddr *dst, int flags, > > struct rtentry * > > _rtalloc(struct sockaddr *dst, uint32_t *src, int flags, unsigned int > > rtableid) > > { > > - struct rtentry *rt, *nhrt; > > + struct rtentry *rt; > > > > rt = rt_match(dst, src, flags, rtableid); > > > > @@ -381,6 +382,16 @@ _rtalloc(struct sockaddr *dst, uint32_t > > if (rtisvalid(rt->rt_gwroute)) > > return (rt); > > > > + rt_setgwroute(rt, rtableid); > > + > > + return (rt); > > +} > > + > > +void > > +rt_setgwroute(struct rtentry *rt, u_int rtableid) > > +{ > > + struct rtentry *nhrt; > > + > > rtfree(rt->rt_gwroute); > > rt->rt_gwroute = NULL; > > > > @@ -392,10 +403,9 @@ _rtalloc(struct sockaddr *dst, uint32_t > > * this behavior. But it is safe since rt_checkgate() wont > > * allow us to us this route later on. > > */ > > - nhrt = rt_match(rt->rt_gateway, NULL, flags | RT_RESOLVE, > > - rtable_l2(rtableid)); > > + nhrt = rt_match(rt->rt_gateway, NULL, RT_RESOLVE, rtable_l2(rtableid)); > > if (nhrt == NULL) > > - return (rt); > > + return; > > > > /* > > * Next hop must be reachable, this also prevents rtentry > > @@ -403,13 +413,13 @@ _rtalloc(struct sockaddr *dst, uint32_t > > */ > > if (ISSET(nhrt->rt_flags, RTF_CLONING|RTF_GATEWAY)) { > > rtfree(nhrt); > > - return (rt); > > + return; > > } > > > > /* Next hop entry must be UP and on the same interface. */ > > if (!ISSET(nhrt->rt_flags, RTF_UP) || nhrt->rt_ifidx != rt->rt_ifidx) { > > rtfree(nhrt); > > - return (rt); > > + return; > > } > > > > /* > > @@ -424,8 +434,6 @@ _rtalloc(struct sockaddr *dst, uint32_t > > * do the magic for us. > > */ > > rt->rt_gwroute = nhrt; > > - > > - return (rt); > > } > > > > void > > @@ -595,7 +603,7 @@ create: > > flags |= RTF_MODIFIED; > > prio = rt->rt_priority; > > stat = &rtstat.rts_newgateway; > > - rt_setgate(rt, gateway); > > + rt_setgate(rt, gateway, rdomain); > > } > > } else > > error = EHOSTUNREACH; > > @@ -1089,7 +1097,8 @@ rtrequest(int req, struct rt_addrinfo *i > > * it to (re)order routes. > > */ > > if ((error = rt_setaddr(rt, ifa->ifa_addr)) || > > - (error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY]))) { > > + (error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY], > > + tableid))) { > > ifafree(ifa); > > rtfree(rt->rt_parent); > > rtfree(rt->rt_gwroute); > > @@ -1169,7 +1178,7 @@ rt_setaddr(struct rtentry *rt, struct so > > } > > > > int > > -rt_setgate(struct rtentry *rt, struct sockaddr *gate) > > +rt_setgate(struct rtentry *rt, struct sockaddr *gate, u_int rtableid) > > { > > int glen = ROUNDUP(gate->sa_len); > > struct sockaddr *sa; > > @@ -1183,8 +1192,8 @@ rt_setgate(struct rtentry *rt, struct so > > } > > memmove(rt->rt_gateway, gate, glen); > > > > - rtfree(rt->rt_gwroute); > > - rt->rt_gwroute = NULL; > > + if (ISSET(rt->rt_flags, RTF_GATEWAY)) > > + rt_setgwroute(rt, rtableid); > > > > return (0); > > } > > Index: net/route.h > > =================================================================== > > RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.h,v > > retrieving revision 1.138 > > diff -u -p -r1.138 route.h > > --- net/route.h 14 Jun 2016 09:48:52 -0000 1.138 > > +++ net/route.h 24 Jun 2016 22:29:45 -0000 > > @@ -363,7 +363,7 @@ struct sockaddr *rt_plen2mask(struct rte > > void rt_sendmsg(struct rtentry *, int, u_int); > > void rt_sendaddrmsg(struct rtentry *, int, struct ifaddr *); > > void rt_missmsg(int, struct rt_addrinfo *, int, uint8_t, u_int, > > int, u_int); > > -int rt_setgate(struct rtentry *, struct sockaddr *); > > +int rt_setgate(struct rtentry *, struct sockaddr *, u_int); > > int rt_checkgate(struct rtentry *, struct rtentry **); > > void rt_setmetrics(u_long, const struct rt_metrics *, struct > > rt_kmetrics *); > > void rt_getmetrics(const struct rt_kmetrics *, struct rt_metrics *); > > Index: net/rtsock.c > > =================================================================== > > RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v > > retrieving revision 1.192 > > diff -u -p -r1.192 rtsock.c > > --- net/rtsock.c 14 Jun 2016 09:48:52 -0000 1.192 > > +++ net/rtsock.c 24 Jun 2016 22:33:04 -0000 > > @@ -748,7 +748,8 @@ report: > > ifa = info.rti_ifa; > > } > > if (info.rti_info[RTAX_GATEWAY] != NULL && (error = > > - rt_setgate(rt, info.rti_info[RTAX_GATEWAY]))) > > + rt_setgate(rt, info.rti_info[RTAX_GATEWAY], > > + tableid))) > > goto flush; > > if (ifa) { > > if (rt->rt_ifa != ifa) { > >
