[Click] IPv6 AddressTranslator - Possible bug

[Tim]

Hello,

I have just recently learn about click, and after spending just a couple
of hours with it, I was quite impressed.  I'm currently doing IPv6
research, and it took me very little time to build myself a basic IPv6
router, all without touching any code.  Very nice.

In any case, I've started working with the AddressTranslator, as it
appears this module could greatly help me in my research.  I've been
fighting with it for several days now, and I think I may have finally
tracked down my troubles to a bug.  (It's either that, or my own
misunderstanding.)

Here's a snippet of my configuration:


// Dynamically mapped
at :: AddressTranslator(0, 1, 0, 1, 2002:c0a8:1302::);
//...

// Outward
rt[1] -> dhl1 :: DecIP6HLIM()
      -> IP6Print("Outward")
      -> at
      -> CheckIP6Header()
      -> GetIP6Address(24)
      -> IP6Print("Outward, translated")
      -> if0Solicitor
      -> if0out;

// Inward
rt[2] -> dhl2 :: DecIP6HLIM()
      -> IP6Print("Inward")
      -> [1]at[1]
      -> CheckIP6Header()
      -> GetIP6Address(24)
      -> IP6Print("Inward, translated")
      -> if1Solicitor
      -> if1out;
//...


My goal is to have a simple IPv6 router with one exception.  I'd like
to map all incoming packets, regardless of their destination, to the
internal destination address '2002:c0a8:1302::'.  Then, when the
responses are sent back over the same connection, the source addresses
should be re-written based on the original external destination address.
It seems like the dynamic mapping feature of this module should handle
this situation. (Though I can't be completely sure... the documentation
is currently sparse.)

I got very close to realizing this setup with this configuration.  The
incoming destination address of ICMPv6 packets was rewritten to my
internal host, and the response came back into this element.  However,
the dynamic re-mapping of the source address failed on the way back out.
It just dropped the packets outright.

After reading through much code, I made a small change in the way
lookup() is called in the handle_outward() function, and everything
started to work.  Based on how handle_inward() is written, it seems like
this is a good thing to change, but I may be totally wrong.  I have
attached a patch for the changes I've made (it is very short).  If you
could let me know if this is the "right thing" to do, I'd appreciate it.
If it is a good change to make, I'd appreciate it even more if it were
committed to CVS. =)

If my changes are misguided, I'd also appreciate some guidance in this
matter.

Thanks for a great piece of software,
tim

PS- The patch is against the current release version, 1.4.3.  I noticed
    this file hasn't changed in CVS since, so hopefully that's ok.
-------------- next part --------------
--- addresstranslator.cc	2003-10-10 00:40:25.000000000 -0400
+++ addresstranslator.cc.new	2006-01-27 20:07:21.000000000 -0500
@@ -565,7 +565,7 @@
     {
 
       unsigned char * icmp6_start = (unsigned char *)(ip6_new +1);
-      if (lookup(ip6_src, dport, ip6_msrc, mport, ip6_dst, sport, _dynamic_mapping_allocation_direction))
+      if (lookup(ip6_src, dport, ip6_msrc, mport, ip6_dst, sport, 0))
 	{
 	 
 	  ip6_new->ip6_src = ip6_msrc;
@@ -611,7 +611,7 @@
        sport = ntohs(tcp_new->th_sport);
        dport = ntohs(tcp_new->th_dport);
 
-      if (lookup(ip6_src, sport, ip6_msrc, mport, ip6_dst, dport, _dynamic_mapping_allocation_direction))
+      if (lookup(ip6_src, sport, ip6_msrc, mport, ip6_dst, dport, 0))
 	{
 	  ip6_new->ip6_src = ip6_msrc;
 	  tcp_new->th_sport = htons(mport);
@@ -634,7 +634,7 @@
       sport = ntohs(udp_new->uh_sport);
       dport = ntohs(udp_new->uh_dport);
 
-      if (lookup(ip6_src, sport, ip6_msrc, mport, ip6_dst, dport, _dynamic_mapping_allocation_direction))
+      if (lookup(ip6_src, sport, ip6_msrc, mport, ip6_dst, dport, 0))
 	{
 	  ip6_new->ip6_src = ip6_msrc;
 	  udp_new->uh_sport = htons(mport);