Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/netinet Restore to working order; this has apparently be...



details:   https://anonhg.NetBSD.org/src/rev/f201b8d1278e
branches:  trunk
changeset: 583378:f201b8d1278e
user:      gdt <gdt%NetBSD.org@localhost>
date:      Wed Aug 03 18:20:11 2005 +0000

description:
Restore to working order; this has apparently been nonworking since
the decapsulator dispatch changes in 2001.  Problems found and fixed
by Christine Jones of BBN.  Specifically:

Check for a packet's protocol to be ENCAP_PROTO, not AF_INET.

Remove one-back cache for last vif, because vif_encapcheck is called
for each vif, rather than being expected to find the appropriate vif.
The cache usage caused packets to be input on the wrong vif and hence
usually dropped.

In vif_encapcheck, verify the local source as well.  While mrouted
endeavors not to create multiple tunnels with a peer, a packet
arriving with the wrong local address is still wrong and should not be
accepted.  (This is a correctness nit, not a security issue.)  Order
checks to fail quickly for packets being checked to see if they match
a vif other than the one they belong on (essentially, check peer
source address in outer header first).

Claim 69 bits of match (32 each from outer src/dst and 5 from checking
that inner dst is within 224/5).  This should result in the vif having
a higher priority for multicast packets compared to a parallel gif(4)
tunnel, and that both seems appropriate if both are configured and
seems to match the semantics expected by the decapsulator dispatch
machinery.

(These changes were made in 2.99.15 and about a dozen nodes are
running them with many vifs.  ip_mroute.c has not changed
significantly since then (February 2005) and the changes applied
cleanly to current and compile cleanly.)

diffstat:

 sys/netinet/ip_mroute.c |  89 ++++++++++++++++++++++++++++--------------------
 1 files changed, 51 insertions(+), 38 deletions(-)

diffs (151 lines):

diff -r 7e66dd53f068 -r f201b8d1278e sys/netinet/ip_mroute.c
--- a/sys/netinet/ip_mroute.c   Wed Aug 03 18:05:16 2005 +0000
+++ b/sys/netinet/ip_mroute.c   Wed Aug 03 18:20:11 2005 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ip_mroute.c,v 1.94 2005/06/06 06:06:50 martin Exp $    */
+/*     $NetBSD: ip_mroute.c,v 1.95 2005/08/03 18:20:11 gdt Exp $       */
 
 /*
  * Copyright (c) 1992, 1993
@@ -93,7 +93,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_mroute.c,v 1.94 2005/06/06 06:06:50 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_mroute.c,v 1.95 2005/08/03 18:20:11 gdt Exp $");
 
 #include "opt_inet.h"
 #include "opt_ipsec.h"
@@ -382,13 +382,6 @@
 static struct callout expire_upcalls_ch;
 
 /*
- * one-back cache used by vif_encapcheck to locate a tunnel's vif
- * given a datagram's src ip address.
- */
-static struct in_addr last_encap_src;
-static struct vif *last_encap_vif;
-
-/*
  * whether or not special PIM assert processing is enabled.
  */
 static int pim_assert;
@@ -926,6 +919,13 @@
                }
 
                /* attach this vif to decapsulator dispatch table */
+               /*
+                * XXX Use addresses in registration so that matching
+                * can be done with radix tree in decapsulator.  But,
+                * we need to check inner header for multicast, so
+                * this requires both radix tree lookup and then a
+                * function to check, and this is not supported yet.
+                */
                vifp->v_encap_cookie = encap_attach_func(AF_INET, IPPROTO_IPV4,
                    vif_encapcheck, &vif_protosw, vifp);
                if (!vifp->v_encap_cookie)
@@ -1037,13 +1037,9 @@
                m_freem(m);
        }
 
-       if (vifp->v_flags & VIFF_TUNNEL) {
+       if (vifp->v_flags & VIFF_TUNNEL)
                free(vifp->v_ifp, M_MRTABLE);
-               if (vifp == last_encap_vif) {
-                       last_encap_vif = NULL;
-                       last_encap_src = zeroin_addr;
-               }
-       } else if (vifp->v_flags & VIFF_REGISTER) {
+       else if (vifp->v_flags & VIFF_REGISTER) {
 #ifdef PIM
                reg_vif_num = VIFI_INVALID;
 #endif
@@ -1984,7 +1980,7 @@
        va_end(ap);
 
        vifp = (struct vif *)encap_getarg(m);
-       if (!vifp || proto != AF_INET) {
+       if (!vifp || proto != ENCAP_PROTO) {
                m_freem(m);
                mrtstat.mrts_bad_tunnel++;
                return;
@@ -2011,7 +2007,9 @@
 }
 
 /*
- * Check if the packet should be grabbed by us.
+ * Check if the packet should be received on the vif denoted by arg.
+ * (The encap selection code will call this once per vif since each is
+ * registered separately.)
  */
 static int
 vif_encapcheck(struct mbuf *m, int off, int proto, void *arg)
@@ -2025,32 +2023,47 @@
 #endif
 
        /*
-        * do not grab the packet if it's not to a multicast destination or if
-        * we don't have an encapsulating tunnel with the source.
-        * Note:  This code assumes that the remote site IP address
-        * uniquely identifies the tunnel (i.e., that this site has
-        * at most one tunnel with the remote site).
+        * Accept the packet only if the inner heaader is multicast
+        * and the outer header matches a tunnel-mode vif.  Order
+        * checks in the hope that common non-matching packets will be
+        * rejected quickly.  Assume that unicast IPv4 traffic in a
+        * parallel tunnel (e.g. gif(4)) is unlikely.
         */
 
-       m_copydata(m, off, sizeof(ip), (caddr_t)&ip);
+       /* Obtain the outer IP header and the vif pointer. */
+       m_copydata((struct mbuf *)m, 0, sizeof(ip), (caddr_t)&ip);
+       vifp = (struct vif *)arg;
+
+       /*
+        * The outer source must match the vif's remote peer address.
+        * For a multicast router with several tunnels, this is the
+        * only check that will fail on packets in other tunnels,
+        * assuming the local address is the same.         
+        */
+       if (!in_hosteq(vifp->v_rmt_addr, ip.ip_src))
+               return 0;
+
+       /* The outer destination must match the vif's local address. */
+       if (!in_hosteq(vifp->v_lcl_addr, ip.ip_dst))
+               return 0;
+
+       /* The vif must be of tunnel type. */
+       if ((vifp->v_flags & VIFF_TUNNEL) == 0)
+               return 0;
+
+       /* Check that the inner destination is multicast. */
+       m_copydata((struct mbuf *)m, off, sizeof(ip), (caddr_t)&ip);
        if (!IN_MULTICAST(ip.ip_dst.s_addr))
                return 0;
 
-       m_copydata(m, 0, sizeof(ip), (caddr_t)&ip);
-       if (!in_hosteq(ip.ip_src, last_encap_src)) {
-               vifp = (struct vif *)arg;
-               if (vifp->v_flags & VIFF_TUNNEL &&
-                   in_hosteq(vifp->v_rmt_addr, ip.ip_src))
-                       ;
-               else
-                       return 0;
-               last_encap_vif = vifp;
-               last_encap_src = ip.ip_src;
-       } else
-               vifp = last_encap_vif;
-
-       /* 32bit match, since we have checked ip_src only */
-       return 32;
+       /*
+        * We have checked that both the outer src and dst addresses
+        * match the vif, and that the inner destination is multicast
+        * (224/5).  By claiming more than 64, we intend to
+        * preferentially take packets that also match a parallel
+        * gif(4).
+        */
+       return 32 + 32 + 5;
 }
 
 /*



Home | Main Index | Thread Index | Old Index