Bug 2073 - NDisc cache entries update timer might be stuck in a loop
authorTommaso Pecorella <tommaso.pecorella@unifi.it>
Sat, 28 Feb 2015 15:16:47 +0100
changeset 11225 750bd0fc8706
parent 11224 7d0449e73dc8
child 11226 d1185f77286f
Bug 2073 - NDisc cache entries update timer might be stuck in a loop
RELEASE_NOTES
src/internet/bindings/modulegen__gcc_ILP32.py
src/internet/bindings/modulegen__gcc_LP64.py
src/internet/model/icmpv6-l4-protocol.cc
src/internet/model/ndisc-cache.cc
src/internet/model/ndisc-cache.h
--- a/RELEASE_NOTES	Fri Feb 27 07:48:34 2015 -0800
+++ b/RELEASE_NOTES	Sat Feb 28 15:16:47 2015 +0100
@@ -24,6 +24,7 @@
 
 Bugs fixed
 ----------
+- Bug 2073 - NDisc cache entries update timer might be stuck in a loop
 
 Known issues
 ------------
--- a/src/internet/bindings/modulegen__gcc_ILP32.py	Fri Feb 27 07:48:34 2015 -0800
+++ b/src/internet/bindings/modulegen__gcc_ILP32.py	Sat Feb 28 15:16:47 2015 +0100
@@ -5328,7 +5328,6 @@
     return
 
 def register_Ns3Int64x64_t_methods(root_module, cls):
-    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('!=')
     cls.add_inplace_numeric_operator('+=', param('ns3::int64x64_t const &', u'right'))
     cls.add_binary_numeric_operator('*', root_module['ns3::int64x64_t'], root_module['ns3::int64x64_t'], param('ns3::int64x64_t const &', u'right'))
@@ -5342,6 +5341,7 @@
     cls.add_inplace_numeric_operator('-=', param('ns3::int64x64_t const &', u'right'))
     cls.add_inplace_numeric_operator('/=', param('ns3::int64x64_t const &', u'right'))
     cls.add_output_stream_operator()
+    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('==')
     cls.add_binary_comparison_operator('>=')
     ## int64x64-double.h (module 'core'): ns3::int64x64_t::int64x64_t() [constructor]
@@ -10096,7 +10096,6 @@
     return
 
 def register_Ns3Time_methods(root_module, cls):
-    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('!=')
     cls.add_inplace_numeric_operator('+=', param('ns3::Time const &', u'right'))
     cls.add_binary_numeric_operator('*', root_module['ns3::Time'], root_module['ns3::Time'], param('int64_t const &', u'right'))
@@ -10107,6 +10106,7 @@
     cls.add_binary_comparison_operator('>')
     cls.add_inplace_numeric_operator('-=', param('ns3::Time const &', u'right'))
     cls.add_output_stream_operator()
+    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('==')
     cls.add_binary_comparison_operator('>=')
     ## nstime.h (module 'core'): ns3::Time::Time() [constructor]
@@ -14584,15 +14584,6 @@
                    'ns3::Address', 
                    [], 
                    is_const=True)
-    ## ndisc-cache.h (module 'internet'): uint8_t ns3::NdiscCache::Entry::GetNSRetransmit() const [member function]
-    cls.add_method('GetNSRetransmit', 
-                   'uint8_t', 
-                   [], 
-                   is_const=True)
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::IncNSRetransmit() [member function]
-    cls.add_method('IncNSRetransmit', 
-                   'void', 
-                   [])
     ## ndisc-cache.h (module 'internet'): bool ns3::NdiscCache::Entry::IsDelay() const [member function]
     cls.add_method('IsDelay', 
                    'bool', 
@@ -14651,10 +14642,6 @@
     cls.add_method('MarkStale', 
                    'void', 
                    [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::ResetNSRetransmit() [member function]
-    cls.add_method('ResetNSRetransmit', 
-                   'void', 
-                   [])
     ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::SetIpv6Address(ns3::Ipv6Address ipv6Address) [member function]
     cls.add_method('SetIpv6Address', 
                    'void', 
@@ -14683,20 +14670,8 @@
     cls.add_method('StartRetransmitTimer', 
                    'void', 
                    [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopDelayTimer() [member function]
-    cls.add_method('StopDelayTimer', 
-                   'void', 
-                   [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopProbeTimer() [member function]
-    cls.add_method('StopProbeTimer', 
-                   'void', 
-                   [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopReachableTimer() [member function]
-    cls.add_method('StopReachableTimer', 
-                   'void', 
-                   [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopRetransmitTimer() [member function]
-    cls.add_method('StopRetransmitTimer', 
+    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopNudTimer() [member function]
+    cls.add_method('StopNudTimer', 
                    'void', 
                    [])
     ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::UpdateLastReachabilityconfirmation() [member function]
--- a/src/internet/bindings/modulegen__gcc_LP64.py	Fri Feb 27 07:48:34 2015 -0800
+++ b/src/internet/bindings/modulegen__gcc_LP64.py	Sat Feb 28 15:16:47 2015 +0100
@@ -5328,7 +5328,6 @@
     return
 
 def register_Ns3Int64x64_t_methods(root_module, cls):
-    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('!=')
     cls.add_inplace_numeric_operator('+=', param('ns3::int64x64_t const &', u'right'))
     cls.add_binary_numeric_operator('*', root_module['ns3::int64x64_t'], root_module['ns3::int64x64_t'], param('ns3::int64x64_t const &', u'right'))
@@ -5342,6 +5341,7 @@
     cls.add_inplace_numeric_operator('-=', param('ns3::int64x64_t const &', u'right'))
     cls.add_inplace_numeric_operator('/=', param('ns3::int64x64_t const &', u'right'))
     cls.add_output_stream_operator()
+    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('==')
     cls.add_binary_comparison_operator('>=')
     ## int64x64-double.h (module 'core'): ns3::int64x64_t::int64x64_t() [constructor]
@@ -10096,7 +10096,6 @@
     return
 
 def register_Ns3Time_methods(root_module, cls):
-    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('!=')
     cls.add_inplace_numeric_operator('+=', param('ns3::Time const &', u'right'))
     cls.add_binary_numeric_operator('*', root_module['ns3::Time'], root_module['ns3::Time'], param('int64_t const &', u'right'))
@@ -10107,6 +10106,7 @@
     cls.add_binary_comparison_operator('>')
     cls.add_inplace_numeric_operator('-=', param('ns3::Time const &', u'right'))
     cls.add_output_stream_operator()
+    cls.add_binary_comparison_operator('<=')
     cls.add_binary_comparison_operator('==')
     cls.add_binary_comparison_operator('>=')
     ## nstime.h (module 'core'): ns3::Time::Time() [constructor]
@@ -14584,15 +14584,6 @@
                    'ns3::Address', 
                    [], 
                    is_const=True)
-    ## ndisc-cache.h (module 'internet'): uint8_t ns3::NdiscCache::Entry::GetNSRetransmit() const [member function]
-    cls.add_method('GetNSRetransmit', 
-                   'uint8_t', 
-                   [], 
-                   is_const=True)
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::IncNSRetransmit() [member function]
-    cls.add_method('IncNSRetransmit', 
-                   'void', 
-                   [])
     ## ndisc-cache.h (module 'internet'): bool ns3::NdiscCache::Entry::IsDelay() const [member function]
     cls.add_method('IsDelay', 
                    'bool', 
@@ -14651,10 +14642,6 @@
     cls.add_method('MarkStale', 
                    'void', 
                    [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::ResetNSRetransmit() [member function]
-    cls.add_method('ResetNSRetransmit', 
-                   'void', 
-                   [])
     ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::SetIpv6Address(ns3::Ipv6Address ipv6Address) [member function]
     cls.add_method('SetIpv6Address', 
                    'void', 
@@ -14683,20 +14670,8 @@
     cls.add_method('StartRetransmitTimer', 
                    'void', 
                    [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopDelayTimer() [member function]
-    cls.add_method('StopDelayTimer', 
-                   'void', 
-                   [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopProbeTimer() [member function]
-    cls.add_method('StopProbeTimer', 
-                   'void', 
-                   [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopReachableTimer() [member function]
-    cls.add_method('StopReachableTimer', 
-                   'void', 
-                   [])
-    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopRetransmitTimer() [member function]
-    cls.add_method('StopRetransmitTimer', 
+    ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::StopNudTimer() [member function]
+    cls.add_method('StopNudTimer', 
                    'void', 
                    [])
     ## ndisc-cache.h (module 'internet'): void ns3::NdiscCache::Entry::UpdateLastReachabilityconfirmation() [member function]
--- a/src/internet/model/icmpv6-l4-protocol.cc	Fri Feb 27 07:48:34 2015 -0800
+++ b/src/internet/model/icmpv6-l4-protocol.cc	Sat Feb 28 15:16:47 2015 +0100
@@ -379,10 +379,9 @@
       std::list<Ptr<Packet> > waiting;
       if (entry->IsIncomplete ())
         {
-          entry->StopRetransmitTimer ();
+          entry->StopNudTimer ();
           // mark it to reachable
           waiting = entry->MarkReachable (lla.GetAddress ());
-          entry->StopReachableTimer ();
           entry->StartReachableTimer ();
           // send out waiting packet
           for (std::list<Ptr<Packet> >::const_iterator it = waiting.begin (); it != waiting.end (); it++)
@@ -403,8 +402,7 @@
             {
               if (!entry->IsReachable ())
                 {
-                  entry->StopProbeTimer ();
-                  entry->StopDelayTimer ();
+                  entry->StopNudTimer ();
                   waiting = entry->MarkReachable (lla.GetAddress ());
                   if (entry->IsProbe ())
                     {
@@ -413,7 +411,6 @@
                           cache->GetInterface ()->Send (*it, src);
                         }
                     }
-                  entry->StopReachableTimer ();
                   entry->StartReachableTimer ();
                 }
             }
@@ -664,13 +661,12 @@
   if (entry->IsIncomplete ())
     {
       /* we receive a NA so stop the retransmission timer */
-      entry->StopRetransmitTimer ();
+      entry->StopNudTimer ();
 
       if (naHeader.GetFlagS ())
         {
           /* mark it to reachable */
           waiting = entry->MarkReachable (lla.GetAddress ());
-          entry->StopReachableTimer ();
           entry->StartReachableTimer ();
           /* send out waiting packet */
           for (std::list<Ptr<Packet> >::const_iterator it = waiting.begin (); it != waiting.end (); it++)
@@ -692,8 +688,7 @@
   else
     {
       /* we receive a NA so stop the probe timer or delay timer if any */
-      entry->StopProbeTimer ();
-      entry->StopDelayTimer ();
+      entry->StopNudTimer ();
 
       /* if the Flag O is clear and mac address differs from the cache */
       if (!naHeader.GetFlagO () && lla.GetAddress () != entry->GetMacAddress ())
@@ -728,7 +723,6 @@
                           entry->MarkReachable (lla.GetAddress ());
                         }
                     }
-                  entry->StopReachableTimer ();
                   entry->StartReachableTimer ();
                 }
               else if (lla.GetAddress () != entry->GetMacAddress ())
--- a/src/internet/model/ndisc-cache.cc	Fri Feb 27 07:48:34 2015 -0800
+++ b/src/internet/model/ndisc-cache.cc	Sat Feb 28 15:16:47 2015 +0100
@@ -197,10 +197,7 @@
   : m_ndCache (nd),
     m_waiting (),
     m_router (false),
-    m_reachableTimer (Timer::CANCEL_ON_DESTROY),
-    m_retransTimer (Timer::CANCEL_ON_DESTROY),
-    m_probeTimer (Timer::CANCEL_ON_DESTROY),
-    m_delayTimer (Timer::CANCEL_ON_DESTROY),
+    m_nudTimer (Timer::CANCEL_ON_DESTROY),
     m_lastReachabilityConfirmation (Seconds (0.0)),
     m_nsRetransmit (0)
 {
@@ -268,9 +265,9 @@
         }
     }
 
-  if (GetNSRetransmit () < icmpv6->MAX_MULTICAST_SOLICIT)
+  if (m_nsRetransmit < icmpv6->MAX_MULTICAST_SOLICIT)
     {
-      IncNSRetransmit ();
+      m_nsRetransmit++;
 
       icmpv6->SendNS (addr, Ipv6Address::MakeSolicitedAddress (m_ipv6Address), m_ipv6Address, m_ndCache->GetDevice ()->GetAddress ());
       /* arm the timer again */
@@ -323,8 +320,7 @@
   Ptr<Packet> p = icmpv6->ForgeNS (addr, m_ipv6Address, m_ipv6Address, m_ndCache->GetDevice ()->GetAddress ());
   m_ndCache->GetDevice ()->Send (p, this->GetMacAddress (), Ipv6L3Protocol::PROT_NUMBER);
 
-  ResetNSRetransmit ();
-  IncNSRetransmit ();
+  m_nsRetransmit = 1;
   StartProbeTimer ();
 }
 
@@ -334,8 +330,10 @@
   Ptr<Ipv6L3Protocol> ipv6 = m_ndCache->GetDevice ()->GetNode ()->GetObject<Ipv6L3Protocol> ();
   Ptr<Icmpv6L4Protocol> icmpv6 = ipv6->GetIcmpv6 ();
 
-  if (GetNSRetransmit () < icmpv6->MAX_UNICAST_SOLICIT)
+  if (m_nsRetransmit < icmpv6->MAX_UNICAST_SOLICIT)
     {
+      m_nsRetransmit++;
+
       Ipv6Address addr;
 
       if (m_ipv6Address.IsLinkLocal ())
@@ -358,7 +356,6 @@
           return;
         }
 
-      IncNSRetransmit ();
       /* icmpv6->SendNS (m_ndCache->GetInterface ()->GetLinkLocalAddress (), m_ipv6Address, m_ipv6Address, m_ndCache->GetDevice ()->GetAddress ()); */
       Ptr<Packet> p = icmpv6->ForgeNS (addr, m_ipv6Address, m_ipv6Address, m_ndCache->GetDevice ()->GetAddress ());
       m_ndCache->GetDevice ()->Send (p, this->GetMacAddress (), Ipv6L3Protocol::PROT_NUMBER);
@@ -379,24 +376,6 @@
   m_ipv6Address = ipv6Address;
 }
 
-uint8_t NdiscCache::Entry::GetNSRetransmit () const
-{
-  NS_LOG_FUNCTION_NOARGS ();
-  return m_nsRetransmit;
-}
-
-void NdiscCache::Entry::IncNSRetransmit ()
-{
-  NS_LOG_FUNCTION_NOARGS ();
-  m_nsRetransmit++;
-}
-
-void NdiscCache::Entry::ResetNSRetransmit ()
-{
-  NS_LOG_FUNCTION_NOARGS ();
-  m_nsRetransmit = 0;
-}
-
 Time NdiscCache::Entry::GetLastReachabilityConfirmation () const
 {
   NS_LOG_FUNCTION_NOARGS ();
@@ -411,77 +390,57 @@
 void NdiscCache::Entry::StartReachableTimer ()
 {
   NS_LOG_FUNCTION_NOARGS ();
-  if (m_reachableTimer.IsRunning ())
+  if (m_nudTimer.IsRunning ())
     {
-      m_reachableTimer.Cancel ();
+      m_nudTimer.Cancel ();
     }
-  m_reachableTimer.SetFunction (&NdiscCache::Entry::FunctionReachableTimeout, this);
-  m_reachableTimer.SetDelay (MilliSeconds (Icmpv6L4Protocol::REACHABLE_TIME));
-  m_reachableTimer.Schedule ();
-}
 
-void NdiscCache::Entry::StopReachableTimer ()
-{
-  NS_LOG_FUNCTION_NOARGS ();
-  m_reachableTimer.Cancel ();
+  m_nudTimer.SetFunction (&NdiscCache::Entry::FunctionReachableTimeout, this);
+  m_nudTimer.SetDelay (MilliSeconds (Icmpv6L4Protocol::REACHABLE_TIME));
+  m_nudTimer.Schedule ();
 }
 
 void NdiscCache::Entry::StartProbeTimer ()
 {
   NS_LOG_FUNCTION_NOARGS ();
-  if (m_probeTimer.IsRunning ())
+  if (m_nudTimer.IsRunning ())
     {
-      m_probeTimer.Cancel ();
+      m_nudTimer.Cancel ();
     }
-  m_probeTimer.SetFunction (&NdiscCache::Entry::FunctionProbeTimeout, this);
-  m_probeTimer.SetDelay (MilliSeconds (Icmpv6L4Protocol::RETRANS_TIMER));
-  m_probeTimer.Schedule ();
+  m_nudTimer.SetFunction (&NdiscCache::Entry::FunctionProbeTimeout, this);
+  m_nudTimer.SetDelay (MilliSeconds (Icmpv6L4Protocol::RETRANS_TIMER));
+  m_nudTimer.Schedule ();
 }
 
-void NdiscCache::Entry::StopProbeTimer ()
-{
-  NS_LOG_FUNCTION_NOARGS ();
-  m_probeTimer.Cancel ();
-  ResetNSRetransmit ();
-}
-
-
 void NdiscCache::Entry::StartDelayTimer ()
 {
   NS_LOG_FUNCTION_NOARGS ();
-  if (m_delayTimer.IsRunning ())
+  if (m_nudTimer.IsRunning ())
     {
-      m_delayTimer.Cancel ();
+      m_nudTimer.Cancel ();
     }
-  m_delayTimer.SetFunction (&NdiscCache::Entry::FunctionDelayTimeout, this);
-  m_delayTimer.SetDelay (Seconds (Icmpv6L4Protocol::DELAY_FIRST_PROBE_TIME));
-  m_delayTimer.Schedule ();
-}
-
-void NdiscCache::Entry::StopDelayTimer ()
-{
-  NS_LOG_FUNCTION_NOARGS ();
-  m_delayTimer.Cancel ();
-  ResetNSRetransmit ();
+  m_nudTimer.SetFunction (&NdiscCache::Entry::FunctionDelayTimeout, this);
+  m_nudTimer.SetDelay (Seconds (Icmpv6L4Protocol::DELAY_FIRST_PROBE_TIME));
+  m_nudTimer.Schedule ();
 }
 
 void NdiscCache::Entry::StartRetransmitTimer ()
 {
   NS_LOG_FUNCTION_NOARGS ();
-  if (m_retransTimer.IsRunning ())
+  if (m_nudTimer.IsRunning ())
     {
-      m_retransTimer.Cancel ();
+      m_nudTimer.Cancel ();
     }
-  m_retransTimer.SetFunction (&NdiscCache::Entry::FunctionRetransmitTimeout, this);
-  m_retransTimer.SetDelay (MilliSeconds (Icmpv6L4Protocol::RETRANS_TIMER));
-  m_retransTimer.Schedule ();
+  m_nudTimer.SetFunction (&NdiscCache::Entry::FunctionRetransmitTimeout, this);
+  m_nudTimer.SetDelay (MilliSeconds (Icmpv6L4Protocol::RETRANS_TIMER));
+  m_nudTimer.Schedule ();
 }
 
-void NdiscCache::Entry::StopRetransmitTimer ()
+void NdiscCache::Entry::StopNudTimer ()
 {
   NS_LOG_FUNCTION_NOARGS ();
-  m_retransTimer.Cancel ();
-  ResetNSRetransmit ();
+  m_nudTimer.Cancel ();
+  m_nsRetransmit = 0;
 }
 
 void NdiscCache::Entry::MarkIncomplete (Ptr<Packet> p)
@@ -573,7 +532,7 @@
 
 void NdiscCache::Entry::SetMacAddress (Address mac)
 {
-  NS_LOG_FUNCTION (this << mac);
+  NS_LOG_FUNCTION (this << mac << int(m_state));
   m_macAddress = mac;
 }
 
--- a/src/internet/model/ndisc-cache.h	Fri Feb 27 07:48:34 2015 -0800
+++ b/src/internet/model/ndisc-cache.h	Sat Feb 28 15:16:47 2015 +0100
@@ -250,22 +250,6 @@
     void SetRouter (bool router);
 
     /**
-     * \brief Get the number of NS retransmit.
-     * \return number of NS that have been retransmit
-     */
-    uint8_t GetNSRetransmit () const;
-
-    /**
-     * \brief Increment NS retransmit.
-     */
-    void IncNSRetransmit ();
-
-    /**
-     * \brief Reset NS retransmit (=0).
-     */
-    void ResetNSRetransmit ();
-
-    /**
      * \brief Get the time of last reachability confirmation.
      * \return time
      */
@@ -282,39 +266,24 @@
     void StartReachableTimer ();
 
     /**
-     * \brief Stop the reachable timer.
-     */
-    void StopReachableTimer ();
-
-    /**
      * \brief Start retransmit timer.
      */
     void StartRetransmitTimer ();
 
     /**
-     * \brief Stop retransmit timer.
-     */
-    void StopRetransmitTimer ();
-
-    /**
      * \brief Start probe timer.
      */
     void StartProbeTimer ();
 
     /**
-     * \brief Stop probe timer.
-     */
-    void StopProbeTimer ();
-
-    /**
      * \brief Start delay timer.
      */
     void StartDelayTimer ();
 
     /**
-     * \brief Stop delay timer.
+     * \brief Stop NUD timer and reset the NUD retransmission counter
      */
-    void StopDelayTimer ();
+    void StopNudTimer ();
 
     /**
      * \brief Function called when reachable timer timeout.
@@ -388,24 +357,9 @@
     bool m_router;
 
     /**
-     * \brief Reachable timer (used for NUD in REACHABLE state).
-     */
-    Timer m_reachableTimer;
-
-    /**
-     * \brief Retransmission timer (used for NUD in INCOMPLETE state).
+     * \brief Timer (used for NUD).
      */
-    Timer m_retransTimer;
-
-    /**
-     * \brief Probe timer (used for NUD in PROBE state).
-     */
-    Timer m_probeTimer;
-
-    /**
-     * \brief Delay timer (used for NUD when in DELAY state).
-     */
-    Timer m_delayTimer;
+    Timer m_nudTimer;
 
     /**
      * \brief Last time we see a reachability confirmation.