Re: Mobile IPv6 patch for NetBSD-current (revised)

[David Young <dyoung%pobox.com@localhost>]

On Thu, Jan 31, 2008 at 10:52:40PM +0900, Keiichi SHIMA wrote:
> Hello all,
> 
> Long time ago, I proposed Mobile IPv6 patch for NetBSD current.  I  
> received several comments, but could not release revised patch until  
> now.  Sorry for my long silence.

> I'm now releasing the patch again with some updates.  The modified  
> points from the previous patch are:
> 
> - less '#ifdef MOBILE_IPV6' especially in 'if' conditions
>   to improve code readability

I think that you can reduce the #ifdefs a bit more by using
ntohs() or htons() in cases such as this,

+#if BYTE_ORDER == BIG_ENDIAN
+#define MIP6_HAINFO_FLAG_MR     0x8000
+#endif /* BIG_ENDIAN */
+#if BYTE_ORDER == LITTLE_ENDIAN
+#define MIP6_HAINFO_FLAG_MR     0x0080
+#endif /* LITTLE_ENDIAN */

> - mip(4) manual (it is still very basic, though)
> 
> Some suggested me that the code should support FAST_IPSEC.  I  
> completely understand the idea, however, the patch is not ready to  
> support it yet.  I will support it once the patch is imported to the  
> repository.
> 
> Others suggested that it would be great if there was an implementation  
> document (like IP, IPv6, IPsec).  I haven't written that kind of  
> documents yet.
> 
> The patch (and new files) is available from  
> http://www.mobileip.jp/~keiichi/tmp/mobile-ipv6-20080125.tgz
> 
> The overview of the change is:
> 
> - the type 2 routing header processing
> - the home address option (one of the destination options)
>  processing.
> - some new ICMPv6 messages defined in Mobile IPv6
> - the mobility header processing (the new extension header for
>  Mobile IPv6 signaling messages)
> - the mip pseudo interface to represent a home network
> - the mobility information message exchange mechanism between
>  the kernel and user space programs (PF_MOBILITY socket)
> - new IPv6 address properties (e.g. the HOME flag,
>  the DEREGISTERING flag)
> - the type 2 routing header socket API
> - neighbor discovery extension (e.g. retuning home detection for
>  mobile nodes, packet intercept by home agents)
> - source address selection extension (home addresses are preferred)
> - IPsec SA/SP db update according to the movement of mobile nodes.
> 
> With this change, the following files are modified:
>     distrib/sets/lists/comp/mi
>     lib/libc/net/rthdr.c
>     sbin/ifconfig/af_inet6.c
>     sbin/ifconfig/ifconfig.c
>     sys/conf/files
>     sys/net/Makefile
>     sys/net/if_types.h
>     sys/net/pfkeyv2.h
>     sys/net/route.h
>     sys/net/rtsock.c
>     sys/netinet/Makefile
>     sys/netinet/icmp6.h
>     sys/netinet/in.h
>     sys/netinet/ip6.h
>     sys/netinet6/Makefile
>     sys/netinet6/dest6.c
>     sys/netinet6/files.netinet6
>     sys/netinet6/in6.c
>     sys/netinet6/in6.h
>     sys/netinet6/in6_proto.c
>     sys/netinet6/in6_src.c
>     sys/netinet6/in6_var.h
>     sys/netinet6/ip6_forward.c

Why is there a special case for home addresses?  It seems that you can
avoid the special case by putting a route to each home address in the IPv6
forwarding table; route through a Mobile IP pseudo-interface that adds
the encapsulation.  You may benefit from the IPv6 fast-forwarding code
in this way.  It may make it easier for you to get FAST_IPSEC to work.

Strive for zero modifications to the IPv6 forwarding code.

>     sys/netinet6/ip6_input.c

Be careful, rtcache_validate() can return NULL:

+               if ((rtcache_validate(&ip6_forward_rt)->rt_flags & 
RTF_ANNOUNCE) != 0) {        

What is this processing in ip6_input.c all about?  Perhaps you can do
this processing after the protocol switch?

>     sys/netinet6/ip6_output.c

What is going on here?

>     sys/netinet6/ip6_var.h
>     sys/netinet6/ipsec.c
>     sys/netinet6/nd6.c

Tunnelling?  In Neighbor Discovery code?  What is going on here?

>     sys/netinet6/nd6.h
>     sys/netinet6/nd6_nbr.c
>     sys/netinet6/nd6_rtr.c
>     sys/netinet6/raw_ip6.c
>     sys/netinet6/route6.c
>     sys/netkey/key.c
>     sys/netkey/key.h
>     sys/sys/socket.h
> 
> The change introduces these new files
>     share/man/man4/mip.4

I could not find this file.

>     sys/net/mipsock.c
>     sys/net/mipsock.h

Why a socket here?  Why not a character device (advantages: simplicity,
delegation w/ chmod/chown/chgrp)?  ioctls on the MIP pseudo-interface?

The general feeling I get by reading the patch is that it touches much
more code than is necessary or desirable.  I think that more of the
Mobile IPv6 functions can be put into the MIP pseudo-interface, with
the nice side-effect that you can probably use MIPv6 with the existing
fast-forwarding code, you can tap and filter MIPv6 packets before and
after encapsulation, and you keep clutter out of the IPv6 stack.

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933 ext 24