[Ns-developers] ARP pending queue

[Gustavo Carneiro]

I would really like to try this patch, to see if it fixes a problem I've
been having.  However I can't copy paste the patch from this email inline
because of line wrapping:

patching file src/internet-node/arp-cache.cc
Hunk #2 FAILED at 33.
patch: **** malformed patch at line 41:
("MaxPackets",Uinteger(m_unresQlen));

Also the mailing-list just strips attachments.  But the "right way" would be
to have the patch as attachment.

IMHO this policy of mailing list stripping attachments is beginning to be
really annoying and counter productive.  _Please_ can't this be fixed once
and for all? :-/

Thanks.

On 03/04/2008, Mathieu Lacage <mathieu.lacage at sophia.inria.fr> wrote:
>
> hi guillaume,
>
>
> On Thu, 2008-04-03 at 18:55 +0200, Guillaume Vu-Brugier wrote:
> > Hi,
> >
> > I recently spent a lot of time debugging a ns-3 program involving TCP
> and
> > UDP flows over a CSMA network.
> > The problem was that 2 flows were competing at the beginning of the
> > simulation and a TCP SYN segment was lost during the ARP address
> resolution
> > step, thus delaying this flow for 3s.
> >
> > I believe that the ARP implementation should buffer more than one
> outgoing
> > packets while waiting for the ARP reply :
> > The Linux kernel seems to do this (with a default 3-packet buffer
> configured
> > in /proc/sys/net/ipv4/neigh/DEV/unres_qlen [1]) and the Omnet++ INET
> > framework too [2].
>
>
> You are not the first person to complain about this but you are the
> first to actually produce a working patch :)
>
>
> > The attached patch modifies arp-cache and arp-l3-protocol to provide
> such a
> > functionality. It is not complete nor thoroughly tested but solved my
> > initial problem, comments are welcomed.
>
>
> Although I am a bit worried about the idea of creating an extra
> DropTailQueue per arp cache entry, I see no way to avoid having a
> per-entry queue so, I guess that the basic approach is sound.
>
> I would tend, however, to go with a "smaller" approach which avoids the
> overhead of the DropTailQueue itself and uses a per-entry
> std::list<Ptr<Packet>>. It would also be nice to avoid storing in each
> entry the queue length so, I would suggest:
>
>   - add an attribute to the ArpCache which indicates the size of the
> queue (you will have to make ArpCache derive from Object and add a
> GetTypeId method to it, and make users of ArpCache refer to it with
> Ptr<ArpCache> rather than ArpCache *).
>
>   - make said attribute go through a pair of getter/setter methods as
> shown below
>
>   - remove UpdateWaitReply and implement it in ArpCache
>
>   - move the packet management code to ArpCache methods rather than
> ArpCache::Entry methods (which means removing their Ptr<Packet>
> arguments which you have already started doing).
>
> void
> ArpCache::EnsureMaxPackets (ArpCache::Entry *entry, uint32_t length)
> {
>   while (entry->queue.size () > length)
>     {
>       entry->queue.pop_back ();
>     }
> }
>
> void ArpCache::SetPerEntryQueueSize (uint32_t length)
> {
>   for (CacheI i = m_arpCache.begin (); i != m_arpCache.end (); ++i)
>     {
>       ArpCache::Entry *entry = i->second;
>       EnsureMaxPackets (entry, length);
>     }
>   m_queueLength = length;
> }
>
> void ArpCache::GetPerEntryQueueSize (void) const
> {
>   return m_queueLength;
> }
>
> TypeId ArpCache::GetTypeId (void)
> {
>   static TypeId tid = TypeId ("ns3::ArpCache")
>     .SetParent<Object> ()
>     .AddConstructor<ArpCache> ()
>     .AddAttribute ("PerEntryQueueSize",
>                    "Help text here: XXX",
>                    Uinteger (3),
>                    MakeUintegerAccessor
> (&ArpCache::SetPerEntryQueueSize, &ArpCache::GetPerEntryQueueSize),
>                    MakeUintegerChecker<uint32_t> (1))
>   ;
>   return tid;
> }
>
> The above will allow you to control the size of the cache through
>
> Config::SetDefault ("ns3::ArpCache::PerEntryQueueSize", Uinteger (10));
>
> You will get bonus points if you manage to make the other cache
> parameters (the various delays) be attributes. And another set of bonus
> points (these will be much harder to get though) if you manage to make
> these attributes configurable also with Config::Set (instead of only
> with Config::SetDefault). The latter will require that you patch
> ArpL3Protocol to add an attribute which gives access to the list of
> cache objects. Hint: src/core/object-vector.h will be needed.
>
> I hope the above helps,
>
> Mathieu
>
>
>


-- 
Gustavo J. A. M. Carneiro
INESC Porto, Telecommunications and Multimedia Unit
"The universe is always one step beyond logic." -- Frank Herbert