On Fri, Mar 16, 2001 at 08:19:16PM +0200, Ruslan Ermilov wrote:
> Hi!
> 
> First of all, I would like to commit the attached patch; it removes
> duplicate code.  Please review.
> 
This got already committed as part of another bugfix.
See ip_input.c, revision 1.164.

> Also, I found a nasty bug in IP routing.  The new route added may not
> take immediate effect for routing decisions, because ip_forward() may
> use the cached route (rt_forwarding).
> 
The attached patch fixes this.

> DEMO (only relevant routes are shown).
> 
> Step 1.  On a router, add a route to the network (192.168.1).
> 
> : # route add -net 192.168.1 gateway
> : add net 192.168.1: gateway gateway
> : 
> : # netstat -rn
> : Routing tables
> : 
> : Internet:
> : Destination        Gateway            Flags     Refs     Use     Netif Expire
> : 192.168.1          gateway            UGSc        0        0      rl0
> 
> Step 2.  From some other host for which this machine is the default router,
>          run ``traceroute -m 2 -n 192.168.1.1''.  Observe, on the router,
>          that the reference count grown on the 192.168.1 route.
> 
> : # netstat -rn
> : Routing tables
> : 
> : Internet:
> : Destination        Gateway            Flags     Refs     Use     Netif Expire
> : 192.168.1          gateway            UGSc        1        3      rl0
> 
> Step 3.  Add RTF_REJECT host route to the destination:
> 
> : # route add 192.168.1.1 -iface lo0 -reject
> : add host 192.168.1.1: gateway lo0
> : # netstat -rn
> : Routing tables
> : 
> : Internet:
> : Destination        Gateway            Flags     Refs     Use     Netif Expire
> : 192.168.1          gateway            UGSc        1        3      rl0
> : 192.168.1.1        lo0                UHRS        0        0      lo0
> 
> Step 4.  The fun begins.  What you would expect if you run traceroute to
>          192.168.1.1 again?  Obviously host route should take precedence
>          over network route, and I expected ICMP Destination Unreachable.
>          But... what's a hell? the new route did not take immediate effect,
>          traceroute succeeded (192.168.1.1 still has zero refcound and use):
> 
> : # netstat -rn
> : Routing tables
> : 
> : Internet:
> : Destination        Gateway            Flags     Refs     Use     Netif Expire
> : 192.168.1          gateway            UGSc        1        6      rl0
> : 192.168.1.1        lo0                UHRS        0        0      lo0
> 
> Step 5.  The fun continues.  From another host, run traceroute to another
>          destination (to help router change rt_forward.ro_dst).
>          traceroute -m 2 -n 192.168.1.2:
> 
> : # netstat -rn
> : Routing tables
> : 
> : Internet:
> : Destination        Gateway            Flags     Refs     Use     Netif Expire
> : 192.168.1          gateway            UGSc        1        9      rl0
> : 192.168.1.1        lo0                UHRS        0        0      lo0
> 
> Step 6.  Run traceroute again to 192.168.1.1.  The fun ends.
> 
> 
> The solution I found so far is to unstaticize the ``rt_forward'', and
> invalidate the rt_forward.ro_rt in in_addroute() (in_rmx.c).  Any better
> ideas?


-- 
Ruslan Ermilov          Oracle Developer/DBA,
[EMAIL PROTECTED]           Sunbay Software AG,
[EMAIL PROTECTED]          FreeBSD committer,
+380.652.512.251        Simferopol, Ukraine

http://www.FreeBSD.org  The Power To Serve
http://www.oracle.com   Enabling The Information Age
Index: in_rmx.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/in_rmx.c,v
retrieving revision 1.38
diff -u -p -r1.38 in_rmx.c
--- in_rmx.c    2001/03/15 14:52:12     1.38
+++ in_rmx.c    2001/03/18 15:08:25
@@ -54,6 +54,7 @@
 #include <net/route.h>
 #include <netinet/in.h>
 #include <netinet/in_var.h>
+#include <netinet/ip_var.h>
 
 extern int     in_inithead __P((void **head, int off));
 
@@ -137,6 +138,16 @@ in_addroute(void *v_arg, void *n_arg, st
                        RTFREE(rt2);
                }
        }
+
+       /*
+        * If the new route successfully added, and we are forwarding,
+        * and there is a cached route, free it.
+        */
+       if (ret != NULL && ipforwarding && ipforward_rt.ro_rt) {
+               RTFREE(ipforward_rt.ro_rt);
+               ipforward_rt.ro_rt = 0;
+       }
+
        return ret;
 }
 
Index: ip_input.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.164
diff -u -p -r1.164 ip_input.c
--- ip_input.c  2001/03/18 13:04:07     1.164
+++ ip_input.c  2001/03/18 15:08:26
@@ -257,7 +257,7 @@ ip_init()
 }
 
 static struct  sockaddr_in ipaddr = { sizeof(ipaddr), AF_INET };
-static struct  route ipforward_rt;
+struct route ipforward_rt;
 
 /*
  * Ip input routine.  Checksum and byte swap header.  If fragmented
Index: ip_var.h
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.53
diff -u -p -r1.53 ip_var.h
--- ip_var.h    2001/03/16 20:00:53     1.53
+++ ip_var.h    2001/03/18 15:08:26
@@ -141,6 +141,7 @@ extern struct       ipstat  ipstat;
 extern u_short ip_id;                          /* ip packet ctr, for ids */
 extern int     ip_defttl;                      /* default IP ttl */
 extern int     ipforwarding;                   /* ip forwarding */
+extern struct route ipforward_rt;              /* ip forwarding cached route */
 extern u_char  ip_protox[];
 extern struct socket *ip_rsvpd;        /* reservation protocol daemon */
 extern struct socket *ip_mrouter; /* multicast routing daemon */

Reply via email to