Subject: Re: multicast routing over vifs broken?
To: None <tech-net@NetBSD.org>
From: Greg Troxel <gdt@NetBSD.org>
List: tech-net
Date: 08/03/2005 14:37:09
  gdt said:

  Earlier I wrote about an incorrect comparison in vif_encapcheck that
  caused multicast routing over vifs to have been broken since May 2001.
  (No one has told me that it has worked during this time; if so please
  speak up.)

  So, I propose to completely rip out the last_encap_* cache.
  If there is a performance problem, then registering with specific
  addresses so the radix tree is used should be done.   This may mean
  that we need to add a way to register both a filter and a function, so
  that the inner packet is checked for multicast destination by the
  function only if the outer src/dst matches.

  Also, I think the outer dst should be checked against the local tunnel
  endpoint.  While mrouted may take care to only create a single tunnel
  with a peer, a packet arriving with a different local address is
  incorrect and shouldn't be treated as on-vif.

No one spoke up other than to say 'code looks wrong to me, go ahead
and fix', so I interpreted that as

  no one thinks multicast routing on vifs works now, and
  no one objected to the proposed changes

and committed our local changes (the 2 key bugs were found and fixed
by Christine).

If anyone uses vifs, please check that the new code is still ok in
your environment, and if you've tried and given up they probably work
ok now.


From: Greg Troxel <gdt@netbsd.org>
Subject: CVS commit: src/sys/netinet
To: source-changes@NetBSD.org
Date: Wed,  3 Aug 2005 18:20:11 +0000 (UTC)
Reply-To: gdt@netbsd.org


Module Name:	src
Committed By:	gdt
Date:		Wed Aug  3 18:20:11 UTC 2005

Modified Files:
	src/sys/netinet: ip_mroute.c

Log Message:
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.)


To generate a diff of this commit:
cvs rdiff -r1.94 -r1.95 src/sys/netinet/ip_mroute.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


-- 
        Greg Troxel <gdt@ir.bbn.com>