[Ns-bugs] [Bug 253] ARP does not retry upon loss

[bugzilla-daemon@nsnam-www.ece.gatech.edu]

http://www.nsnam.org/bugzilla/show_bug.cgi?id=253





------- Comment #16 from tomh at tomh.org  2008-07-29 19:27 -------
(In reply to comment #15)
> (In reply to comment #14)
> > Created an attachment (id=216)
 --> (http://www.nsnam.org/bugzilla/attachment.cgi?id=216&action=view) [details] [details]
> > small fix based on Mathieu's comment
> > 
> > Mathieu, I agree with your comment; I think the below should avoid
> > retransmitting entries that have just moved to waitreply state.  In general,
> > entries will be first retransmitted after they have waited between 1 and 2
> > waitreply timeout intervals.
> > 
> >   for (CacheI i = m_arpCache.begin (); i != m_arpCache.end (); i++) 
> >      {
> >        entry = (*i).second;
> > -      if (entry != 0 && entry->IsWaitReply ())
> > +      if (entry != 0 && entry->IsWaitReply () && entry->IsExpired ())
> 
> Yes, I had something similar in mind.
> 
> A small nit: you have a method ClearRetries and you don't use it from within
> the ArpCache::Entry code. It might make debugging easier to use it rather than
> access m_retries directly.

OK
> 
> This has incorrect indentation:
> +    /**
> +     * This function is an event handler for the event that the
> +     * ArpCache wants to check whether it must retry any Arp requests.
> +     * If there are no Arp requests pending, this event is not scheduled.
> +     */
> +    void HandleWaitReplyTimeout (void);
> 
> Let's make this ASSERT a NS_FATAL_ERROR which asks for a bug report if it is
> ever hit and remove the code in the else block.

OK

> 
> @@ -218,6 +220,7 @@ ArpL3Protocol::Lookup (Ptr<Packet> packe
>              } 
>            else if (entry->IsWaitReply ()) 
>              {
> +              NS_ASSERT (false); // Test for unreachable code, remove later
> 
> Why do you have ArpL3Protocol::GetDropTrace ?

Trying to reuse the ArpL3Protocol in ArpCache without introducing a dependency
in ArpCache on ArpL3Protocol.  
> 
> ArpCache::SetDropTrace is not needed because ArpCache derives from Object and
> is accessible under the /NodeList/xx/$ArpL3Protocol/CacheList/xx/ so, you can
> make m_dropTrace a simple member of ArpCache which is never accessed by
> ArpL3Protocol.
> 

OK, I will add this separate drop trace instead.


-- 
Configure bugmail: http://www.nsnam.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.