[Ns-developers] ARP pending queue

[Mathieu Lacage]

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