Add route generation number to route cache.

The outgoing route is cached at the inpcb.  This cache was only
invalidated when the socket closes or if the route gets invalid.
More specific routes were not detected.  Especially with dynamic
routing protocols, sockets must be closed and reopened to use the
correct route.  Running ping during a route change shows the problem.

To solve this, add a route generation number that is updated whenever
the routing table changes.  The lookup in struct route is put into
the route_cache() function.  If the generation number is too old,
the cached route gets discarded.

Implement route_cache() for ip_output() and ip_forward() first.
IPv6 and more places will follow.

OK claudio@

diff --git a/sys/net/route.c b/sys/net/route.c
index 0b97254..991b480 100644 (file)
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: route.c,v 1.426 2023/11/13 17:18:27 bluhm Exp $       */
+/*     $OpenBSD: route.c,v 1.427 2024/01/31 14:56:42 bluhm Exp $       */
 /*     $NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $      */
 
 /*
@@ -140,6 +140,7 @@
 
 /*
  * Locks used to protect struct members:
+ *      a       atomic operations
  *      I       immutable after creation
  *      L       rtlabel_mtx
  *      T       rttimer_mtx
@@ -152,8 +153,9 @@ static uint32_t             rt_hashjitter;
 
 extern unsigned int    rtmap_limit;
 
-struct cpumem *                rtcounters;
-int                    rttrash;        /* routes not in table but not freed */
+struct cpumem  *rtcounters;
+int             rttrash;       /* [a] routes not in table but not freed */
+u_long          rtgeneration;  /* [a] generation number, routes changed */
 
 struct pool    rtentry_pool;           /* pool for rtentry structures */
 struct pool    rttimer_pool;           /* pool for rttimer structures */
@@ -199,6 +201,33 @@ route_init(void)
 #endif
 }
 
+void
+route_cache(struct route *ro, struct in_addr addr, u_int rtableid)
+{
+       u_long gen;
+
+       gen = atomic_load_long(&rtgeneration);
+       membar_consumer();
+
+       if (rtisvalid(ro->ro_rt) &&
+           ro->ro_generation == gen &&
+           ro->ro_tableid == rtableid &&
+           ro->ro_dst.sa_family == AF_INET &&
+           satosin(&ro->ro_dst)->sin_addr.s_addr == addr.s_addr) {
+               return;
+       }
+
+       rtfree(ro->ro_rt);
+       ro->ro_rt = NULL;
+       ro->ro_generation = gen;
+       ro->ro_tableid = rtableid;
+
+       memset(&ro->ro_dst, 0, sizeof(ro->ro_dst));
+       satosin(&ro->ro_dst)->sin_family = AF_INET;
+       satosin(&ro->ro_dst)->sin_len = sizeof(struct sockaddr_in);
+       satosin(&ro->ro_dst)->sin_addr = addr;
+}
+
 /*
  * Returns 1 if the (cached) ``rt'' entry is still valid, 0 otherwise.
  */
@@ -824,6 +853,9 @@ rtrequest_delete(struct rt_addrinfo *info, u_int8_t prio, struct ifnet *ifp,
        else
                rtfree(rt);
 
+       membar_producer();
+       atomic_inc_long(&rtgeneration);
+
        return (0);
 }
 
@@ -992,6 +1024,10 @@ rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio,
                        *ret_nrt = rt;
                else
                        rtfree(rt);
+
+               membar_producer();
+               atomic_inc_long(&rtgeneration);
+
                break;
        }
 
@@ -1829,6 +1865,9 @@ rt_if_linkstate_change(struct rtentry *rt, void *arg, u_int id)
        }
        if_group_routechange(rt_key(rt), rt_plen2mask(rt, &sa_mask));
 
+       membar_producer();
+       atomic_inc_long(&rtgeneration);
+
        return (error);
 }
 

diff --git a/sys/net/route.h b/sys/net/route.h
index 96a76d7..2d2a31d 100644 (file)
--- a/sys/net/route.h
+++ b/sys/net/route.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: route.h,v 1.203 2023/11/12 17:51:40 bluhm Exp $       */
+/*     $OpenBSD: route.h,v 1.204 2024/01/31 14:56:42 bluhm Exp $       */
 /*     $NetBSD: route.h,v 1.9 1996/02/13 22:00:49 christos Exp $       */
 
 /*
@@ -377,6 +377,7 @@ struct sockaddr_rtsearch {
  */
 struct route {
        struct  rtentry *ro_rt;
+       u_long           ro_generation;
        u_long           ro_tableid;    /* u_long because of alignment */
        struct  sockaddr ro_dst;
 };
@@ -438,15 +439,18 @@ void               rtlabel_unref(u_int16_t);
 #define        RT_RESOLVE      1
 
 extern struct rtstat rtstat;
+extern u_long rtgeneration;
 
 struct mbuf;
 struct socket;
 struct ifnet;
+struct in_addr;
 struct sockaddr_in6;
 struct if_ieee80211_data;
 struct bfd_config;
 
 void    route_init(void);
+void    route_cache(struct route *, struct in_addr, u_int);
 void    rtm_ifchg(struct ifnet *);
 void    rtm_ifannounce(struct ifnet *, int);
 void    rtm_bfd(struct bfd_config *);

diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 2792523..6bbbcf6 100644 (file)
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_input.c,v 1.387 2023/09/16 09:33:27 mpi Exp $      */
+/*     $OpenBSD: ip_input.c,v 1.388 2024/01/31 14:56:42 bluhm Exp $    */
 /*     $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $   */
 
 /*
@@ -1475,7 +1475,6 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
 {
        struct mbuf mfake, *mcopy = NULL;
        struct ip *ip = mtod(m, struct ip *);
-       struct sockaddr_in *sin;
        struct route ro;
        int error = 0, type = 0, code = 0, destmtu = 0, fake = 0, len;
        u_int32_t dest;
@@ -1491,15 +1490,11 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
                goto freecopy;
        }
 
-       memset(&ro, 0, sizeof(ro));
-       sin = satosin(&ro.ro_dst);
-       sin->sin_family = AF_INET;
-       sin->sin_len = sizeof(*sin);
-       sin->sin_addr = ip->ip_dst;
-
+       ro.ro_rt = NULL;
+       route_cache(&ro, ip->ip_dst, m->m_pkthdr.ph_rtableid);
        if (!rtisvalid(rt)) {
                rtfree(rt);
-               rt = rtalloc_mpath(sintosa(sin), &ip->ip_src.s_addr,
+               rt = rtalloc_mpath(&ro.ro_dst, &ip->ip_src.s_addr,
                    m->m_pkthdr.ph_rtableid);
                if (rt == NULL) {
                        ipstat_inc(ips_noroute);
@@ -1507,6 +1502,7 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
                        return;
                }
        }
+       ro.ro_rt = rt;
 
        /*
         * Save at most 68 bytes of the packet in case
@@ -1557,8 +1553,6 @@ ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
                }
        }
 
-       ro.ro_rt = rt;
-       ro.ro_tableid = m->m_pkthdr.ph_rtableid;
        error = ip_output(m, NULL, &ro,
            (IP_FORWARDING | (ip_directedbcast ? IP_ALLOWBROADCAST : 0)),
            NULL, NULL, 0);

diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index 761d063..98d0cd5 100644 (file)
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_output.c,v 1.393 2024/01/18 11:03:16 claudio Exp $ */
+/*     $OpenBSD: ip_output.c,v 1.394 2024/01/31 14:56:43 bluhm Exp $   */
 /*     $NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $  */
 
 /*
@@ -159,28 +159,15 @@ reroute:
         */
        if (ro == NULL) {
                ro = &iproute;
-               memset(ro, 0, sizeof(*ro));
+               ro->ro_rt = NULL;
        }
 
-       dst = satosin(&ro->ro_dst);
-
        /*
         * If there is a cached route, check that it is to the same
         * destination and is still up.  If not, free it and try again.
         */
-       if (!rtisvalid(ro->ro_rt) ||
-           dst->sin_addr.s_addr != ip->ip_dst.s_addr ||
-           ro->ro_tableid != m->m_pkthdr.ph_rtableid) {
-               rtfree(ro->ro_rt);
-               ro->ro_rt = NULL;
-       }
-
-       if (ro->ro_rt == NULL) {
-               dst->sin_family = AF_INET;
-               dst->sin_len = sizeof(*dst);
-               dst->sin_addr = ip->ip_dst;
-               ro->ro_tableid = m->m_pkthdr.ph_rtableid;
-       }
+       route_cache(ro, ip->ip_dst, m->m_pkthdr.ph_rtableid);
+       dst = satosin(&ro->ro_dst);
 
        if ((IN_MULTICAST(ip->ip_dst.s_addr) ||
            (ip->ip_dst.s_addr == INADDR_BROADCAST)) &&

diff --git a/sys/netinet6/in6.h b/sys/netinet6/in6.h
index 98384bf..909166c 100644 (file)
--- a/sys/netinet6/in6.h
+++ b/sys/netinet6/in6.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in6.h,v 1.112 2024/01/27 21:13:46 bluhm Exp $ */
+/*     $OpenBSD: in6.h,v 1.113 2024/01/31 14:56:43 bluhm Exp $ */
 /*     $KAME: in6.h,v 1.83 2001/03/29 02:55:07 jinmei Exp $    */
 
 /*
@@ -145,10 +145,11 @@ extern const struct in6_addr in6addr_linklocal_allrouters;
 
 #if __BSD_VISIBLE
 /*
- * IPv6 route structure
+ * IPv6 route structure, keep fields in sync with struct route
  */
 struct route_in6 {
        struct  rtentry *ro_rt;
+       u_long           ro_generation;
        u_long           ro_tableid;    /* padded to long for alignment */
        struct  sockaddr_in6 ro_dst;
 };