Bug 1697 - ICMPv6 Redirect trigger contains multiple bugs

CHANGES.html

@@ -66,6 +66,11 @@ us a note on ns-developers mailing list.</p>
   Users of the event garbage collector, previously in tools, will now 
   include it from the core module.
   </li>
+  <li> The Ipv6 UnicastForwardCallback and  MulticastForwardCallback 
+  have a new parmater, the NetDevice the packet has been received from.
+  Existing Ipv6RoutingProtocols should update their RouteInput function
+  accordingly. E.g., from ucb (rtentry, p, header); to ucb (idev, rtentry, p, header);
+  </li>
 </ul>
 
 <h2>Changes to build system:</h2>

RELEASE_NOTES

@@ -24,6 +24,7 @@ New user-visible features
 
 Bugs fixed
 ----------
+- Bug 1390 - ICMPv6 Redirect are handled correctly only for /64 networks
 - Bug 1643 - NdiscCache creation and existence checks
 - Bug 1646 - ICMPv6 Redirect are sent from global address instead of link-local
 - Bug 1662 - m_type not set for Ipv6OptionRouterAlertHeader
@@ -32,6 +33,7 @@ Bugs fixed
 - Bug 1669 - ns-3 should support binding two and three (possibly more) arguments
 - Bug 1688 - Routers should advertise themselves from the link-local address
 - Bug 1689 - IPv6 shouldn't add a default gateway without checking the Router lifetime
+- Bug 1697 - ICMPv6 Redirect trigger contains multiple bugs
 - Bug 1700 - Ipv6RawSocket does not honor the bound address when sending packets
 - Bug 1701 - Ipv6StaticRouting: the source address should match the destination scope
 - Bug 1703 - Nodes don't react to a DAD

src/internet/model/ipv6-l3-protocol.cc

@@ -883,7 +883,7 @@ void Ipv6L3Protocol::SendRealOut (Ptr<Ipv6Route> route, Ptr<Packet> packet, Ipv6
     }
 }
 
-void Ipv6L3Protocol::IpForward (Ptr<Ipv6Route> rtentry, Ptr<const Packet> p, const Ipv6Header& header)
+void Ipv6L3Protocol::IpForward (Ptr<const NetDevice> idev, Ptr<Ipv6Route> rtentry, Ptr<const Packet> p, const Ipv6Header& header)
 {
   NS_LOG_FUNCTION (this << rtentry << p << header);
   NS_LOG_LOGIC ("Forwarding logic for node: " << m_node->GetId ());
@@ -920,9 +920,12 @@ void Ipv6L3Protocol::IpForward (Ptr<Ipv6Route> rtentry, Ptr<const Packet> p, con
    * exists.
    */
 
-  if (m_sendIcmpv6Redirect &&
-      ((!rtentry->GetGateway ().IsAny () && rtentry->GetGateway ().CombinePrefix (Ipv6Prefix (64)) == header.GetSourceAddress ().CombinePrefix (Ipv6Prefix (64)))
-      || (rtentry->GetDestination ().CombinePrefix (Ipv6Prefix (64)) == header.GetSourceAddress ().CombinePrefix (Ipv6Prefix (64)))))
+  /* Theoretically we should also check if the redirect target is on the same network
+   * as the source node. On the other hand, we are sure that the router we're redirecting to
+   * used a link-local address. As a consequence, they MUST be on the same network, the link-local net.
+   */
+
+  if (m_sendIcmpv6Redirect && (rtentry->GetOutputDevice ()==idev))
     {
       NS_LOG_LOGIC ("ICMPv6 redirect!");
       Ptr<Icmpv6L4Protocol> icmpv6 = GetIcmpv6 ();
@@ -953,7 +956,7 @@ void Ipv6L3Protocol::IpForward (Ptr<Ipv6Route> rtentry, Ptr<const Packet> p, con
   SendRealOut (rtentry, packet, ipHeader);
 }
 
-void Ipv6L3Protocol::IpMulticastForward (Ptr<Ipv6MulticastRoute> mrtentry, Ptr<const Packet> p, const Ipv6Header& header)
+void Ipv6L3Protocol::IpMulticastForward (Ptr<const NetDevice> idev, Ptr<Ipv6MulticastRoute> mrtentry, Ptr<const Packet> p, const Ipv6Header& header)
 {
   NS_LOG_FUNCTION (this << mrtentry << p << header);
   NS_LOG_LOGIC ("Multicast forwarding logic for node: " << m_node->GetId ());

src/internet/model/ipv6-l3-protocol.h

@@ -420,19 +420,21 @@ private:
 
   /**
    * \brief Forward a packet.
+   * \param idev Pointer to ingress network device
    * \param rtentry route 
    * \param p packet to forward
    * \param header IPv6 header to add to the packet
    */
-  void IpForward (Ptr<Ipv6Route> rtentry, Ptr<const Packet> p, const Ipv6Header& header);
+  void IpForward (Ptr<const NetDevice> idev, Ptr<Ipv6Route> rtentry, Ptr<const Packet> p, const Ipv6Header& header);
 
   /**
    * \brief Forward a packet in multicast.
+   * \param idev Pointer to ingress network device
    * \param mrtentry route 
    * \param p packet to forward
    * \param header IPv6 header to add to the packet
    */
-  void IpMulticastForward (Ptr<Ipv6MulticastRoute> mrtentry, Ptr<const Packet> p, const Ipv6Header& header);
+  void IpMulticastForward (Ptr<const NetDevice> idev, Ptr<Ipv6MulticastRoute> mrtentry, Ptr<const Packet> p, const Ipv6Header& header);
 
   /**
    * \brief Deliver a packet.

src/internet/model/ipv6-routing-protocol.h

@@ -56,8 +56,8 @@ class Ipv6RoutingProtocol : public Object
 public:
   static TypeId GetTypeId (void);
 
-  typedef Callback<void, Ptr<Ipv6Route>, Ptr<const Packet>, const Ipv6Header &> UnicastForwardCallback;
-  typedef Callback<void, Ptr<Ipv6MulticastRoute>, Ptr<const Packet>, const Ipv6Header &> MulticastForwardCallback;
+  typedef Callback<void, Ptr<const NetDevice>, Ptr<Ipv6Route>, Ptr<const Packet>, const Ipv6Header &> UnicastForwardCallback;
+  typedef Callback<void, Ptr<const NetDevice>, Ptr<Ipv6MulticastRoute>, Ptr<const Packet>, const Ipv6Header &> MulticastForwardCallback;
   typedef Callback<void, Ptr<const Packet>, const Ipv6Header &, uint32_t > LocalDeliverCallback;
   typedef Callback<void, Ptr<const Packet>, const Ipv6Header &, Socket::SocketErrno > ErrorCallback;
 

src/internet/model/ipv6-static-routing.cc

@@ -559,7 +559,7 @@ bool Ipv6StaticRouting::RouteInput (Ptr<const Packet> p, const Ipv6Header &heade
       if (mrtentry)
         {
           NS_LOG_LOGIC ("Multicast route found");
-          mcb (mrtentry, p, header); // multicast forwarding callback
+          mcb (idev, mrtentry, p, header); // multicast forwarding callback
           return true;
         }
       else
@@ -611,7 +611,7 @@ bool Ipv6StaticRouting::RouteInput (Ptr<const Packet> p, const Ipv6Header &heade
   if (rtentry != 0)
     {
       NS_LOG_LOGIC ("Found unicast destination- calling unicast callback");
-      ucb (rtentry, p, header);  // unicast forwarding callback
+      ucb (idev, rtentry, p, header);  // unicast forwarding callback
       return true;
     }
   else