Bug 2123

Created attachment 2045 [details]
Fix for TTL value and implementation of expanding ring search (RFC 3561 section 6.4)

The TTL value is updated correctly through the hops.
The protocol drops the packets according to their TTL value.
The protocol uses the expanding ring search (RFC 3561 section 6.4, Controlling Dissemination of Route Request Messages) for RREQ broadcast.
Parameters TTL_START, TTL_INCREMENT, TTL_THRESHOLD, TIMEOUT_BUFFER added to manage the TTL value during the search.
The TTL value of RREP is set to the hop field of the destination routing table for the origin.

Created attachment 2047 [details]
revised patch

The patch revision is to fix tabs Vs spaces (use spaces instead of tabs) and to fix a bug in the patch (trivial one).

However the new mechanism has an unwanted side-effect: the aodv example isn't anymore that useful: all the packets are lost. Perhaps my "fix" is wrong.
Mind to double check ?

As a side note, without the "fix" the aodv example crashes.

I tried the patch executing the manet-routing-compare in examples/routing and it works, now that I tried with the aodv example, I see that all the packets are lost. I'm checking if I find the problem.

I made some debugging on the code, and I saw that in the particular network configuration of the aodv example, the arp requests are lost, so the destination node cannot resolve the address to send the route reply. Changing some simulation parameters, like the transmission power or the distance between the nodes, some packets reach the destination. Using another network configuration, like the one in manet-routing-compare, the protocol seems to work.

Hi,

this would make sense. The problem is: why without you patch the ARP packets are received and with the patch they aren't ? The patch shouldn't change the radio range...


(In reply to Andrea Lupia from comment #4)
> I made some debugging on the code, and I saw that in the particular network
> configuration of the aodv example, the arp requests are lost, so the
> destination node cannot resolve the address to send the route reply.
> Changing some simulation parameters, like the transmission power or the
> distance between the nodes, some packets reach the destination. Using
> another network configuration, like the one in manet-routing-compare, the
> protocol seems to work.

The only reason I can think about is the different way in which the requests are spread in the network. I looked into the code and this seems to be the only difference I can find between the aodv with and without the patch. I don't think the radio range changes. Unfortunately with my actual knowledge about ns-3 I cannot find where the bug is.
(In reply to Tommaso Pecorella from comment #5)
> Hi,
> 
> this would make sense. The problem is: why without you patch the ARP packets
> are received and with the patch they aren't ? The patch shouldn't change the
> radio range...

This patch introduces some major bugs. It must *not* be used. A new patch is welcome.

Created attachment 2499 [details]
New patch

Finally I found where is the problem, so I can fix it. The attached patch includes the Expanding Ring Search and fixes the issues of the previous one.
The problem was in the RouteInput method, where broadcast packets are managed. If the TTL was >0, the packet was re-broadcasted also if it was a RREQ packet, while it is already managed by the RecvRequest method.
I put a condition where the protocol returns true if the packet is an AODV packet.
With this patch, the output of the example aodv.cc is the same as before applying the patch.

Good catch !

Just a question. In RoutingProtocol::ScheduleRreqRetry you deleted the following lines:
-  rt.IncrementRreqCnt ();
-  m_routingTable.Update (rt);

Shouldn't the routing table be updated ?

Moreover, previously we did kill line 943 "rreqHeader.SetHopCount (0);", which seems logical since it overrides the previously set value in an "if".

Last but not least, there's a potential problem in your patch. You peek a TypeHeader in RouteInput. In that point the packet doesn't have the IP header, but it DOES have the UDP header, and TypeHeader is just after the UdpHeader. I.e., you need to peek the UdpHeader first.
An even better approach is to check the Ipv4Header protocol (must be UDP), then peek the UdpHeader and check the port (look for the AODV port). That's safe against any header combination.

Cheers,

T.

PS: the handling of broadcast packets in AODV is... meh !
We should open another ticket for that point, as any broadcast message (other than AODV ones) will lead to a immediate packet flood and congestion.


(In reply to Andrea Lupia from comment #8)
> Created attachment 2499 [details]
> New patch
> 
> Finally I found where is the problem, so I can fix it. The attached patch
> includes the Expanding Ring Search and fixes the issues of the previous one.
> The problem was in the RouteInput method, where broadcast packets are
> managed. If the TTL was >0, the packet was re-broadcasted also if it was a
> RREQ packet, while it is already managed by the RecvRequest method.
> I put a condition where the protocol returns true if the packet is an AODV
> packet.
> With this patch, the output of the example aodv.cc is the same as before
> applying the patch.

Created attachment 2500 [details]
Modified patch

The lines are deleted because the RRREQ Count is incremented only when the RREQ is broadcasted using "NetDiameter" as ttl, so it is managed in the "SendRequest" method (in lines 960 and 979).
Regarding the line "rreqHeader.SetHopCount (0);", I just forgot to delete it again :).
I think your solution to control if the packet is an AODV packet is better, so check if it's as you intended in the new patch I submitted, I already tested that the behaviour is the same, so it's working. It needed 2 more headers included (udp-header.h for peeking the UDP header, udp-l4-protocol.h to get the UDP port).
For the management of broadcast packet, now I understand why the ttl of the RREQ packets is 1 in the current AODV implementation :).

Created attachment 2546 [details]
Modified patch v2

Cosmetic fixes to the previous patch by Andrea (spaces instead of tabs)

Created attachment 2547 [details]
makes variables names code style compliant

Variables names should be compliant with the coding style

Created attachment 2548 [details]
new trace files for the tests

changeset 12308:4916e2174a59
changeset 12309:4f7c7e501162
changeset 12310:1f6df76a3909
changeset 12311:67f1141352c5