Bug 1911 - AODV can not work on nodes with more than one NetDevice
AODV can not work on nodes with more than one NetDevice
Product: ns-3
Classification: Unclassified
Component: aodv
All All
: P5 normal
Assigned To: Tommaso Pecorella
Depends on:
  Show dependency treegraph
Reported: 2014-05-03 03:04 EDT by Tommaso Pecorella
Modified: 2016-05-06 14:28 EDT (History)
1 user (show)

See Also:

example script (4.60 KB, application/octet-stream)
2014-05-03 03:07 EDT, Tommaso Pecorella
Tentative patch (9.31 KB, patch)
2014-05-04 09:37 EDT, Tommaso Pecorella
Details | Diff
good patch (20.99 KB, patch)
2014-05-04 10:57 EDT, Tommaso Pecorella
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tommaso Pecorella 2014-05-03 03:04:32 EDT
It may be a bug or a feature. The problem is that AODV can be installed on a non-WiFi network, but then it doesn't work.

The problem has been raised by an user, see:

In Aodv, NotifyInterfaceUp there is the following statement:

  // Allow neighbor manager use this interface for layer 2 feedback if possible
  Ptr<WifiNetDevice> wifi = dev->GetObject<WifiNetDevice> ();
  if (wifi == 0)
  Ptr<WifiMac> mac = wifi->GetMac ();
  if (mac == 0)

  mac->TraceConnectWithoutContext ("TxErrHeader", m_nb.GetTxErrorCallback ());
  m_nb.AddArpCache (l3->GetInterface (i)->GetArpCache ());

which may be a hint.

Things to tests:
1) AODV on a non-WiFi interface (e.g., csma), and
2) AODV on a no-ARP interface (GetArpCache could return null).
Comment 1 Tommaso Pecorella 2014-05-03 03:07:02 EDT
Created attachment 1834 [details]
example script

An example program showing the bug.

The p2p link is used "as intended", but RREP are not forwarded (the intermediate node doesn't seems to be receiving the RREP at all).
Comment 2 Tommaso Pecorella 2014-05-04 04:04:47 EDT
The problem is a bit more complex than I thought.

AODV opens an UDP socket for each interface:
  // Create a socket to listen only on this interface
  Ptr<Socket> socket = Socket::CreateSocket (GetObject<Node> (),
                                             UdpSocketFactory::GetTypeId ());
  NS_ASSERT (socket != 0);
  socket->SetRecvCallback (MakeCallback (&RoutingProtocol::RecvAodv, this));
  int ret = socket->Bind (InetSocketAddress (Ipv4Address::GetAny (), AODV_PORT));
  socket->BindToNetDevice (l3->GetNetDevice (i));

Any interface other than the first one will fail to open the socket, because the address is already in use.
As a matter of fact, it's a bad idea to open a socket on an "any" interface and then bind it to a NetDevice...

A possible solution is to open two sockets for each interface, one on the unicast address and one on the subnet-broadcast.

Note: this issue may be present in other protocols, as once upon a time we did not check that an n address was already in use, so the trick (bind to "any" address and then bind to the NetDevice) was working.
Comment 3 Tommaso Pecorella 2014-05-04 09:37:05 EDT
Created attachment 1835 [details]
Tentative patch

This seems to work as intended.
I just did a quick test, no valgrind.

The regression test fails, I didn't check why.
Comment 4 Tommaso Pecorella 2014-05-04 10:25:16 EDT
Comment on attachment 1835 [details]
Tentative patch

Always check the pcap before singing victory...
Comment 5 Tommaso Pecorella 2014-05-04 10:57:00 EDT
Created attachment 1836 [details]
good patch

All tests passed.

Example program, valgrind, examples, regression... everything.
Comment 6 Tom Henderson 2014-09-12 12:41:38 EDT
I am OK with the patch but some example/regression test/documentation of how this works should also be added. 

In general, it may be nice to add an example with a trivial configuration such as:

  n0 <--- CSMA ---> n1 <--WiFi --> n2

and configure it to use different MANET routing protocols (OLSR, AODV, DSDV, DSR) via command-line argument switch, such as manet-routing-compare, and check that routing works between n0 and n2, then add the example to the regression tests multiple times, one for each option.
Comment 7 Tommaso Pecorella 2014-09-13 13:29:54 EDT
pushed in changeset:   10949:751e327b3632

I'll add an example as soon as I have the green light from the author (or I'll have made one anew).
Comment 8 Tommaso Pecorella 2016-05-06 14:28:02 EDT
For the records, the last commit didn't include all the intended changes.

Masahiro Rikiso found it and reported here: https://groups.google.com/forum/#!topic/ns-3-users/uOt_rI9azUE

Pushed the proper code in changeset:   12102:8583bcb611ef