(In reply to comment #0)
> 853       rreqHeader.SetOrigin (iface.GetLocal ());
> 
>    854       m_rreqIdCache.IsDuplicate (iface.GetLocal (), m_requestId);
> 
>    855 
> 
>    856       Ptr<Packet> packet = Create<Packet> ();
> 
> 
> line 854 does nothing useful.


It does have a side effect to remove this, since IsDuplicate() will add the entry to the cache.  Run manet-routing-compare with and without this line to see the difference.

(In reply to comment #1)
> (In reply to comment #0)
> > 853       rreqHeader.SetOrigin (iface.GetLocal ());
> > 
> >    854       m_rreqIdCache.IsDuplicate (iface.GetLocal (), m_requestId);
> > 
> >    855 
> > 
> >    856       Ptr<Packet> packet = Create<Packet> ();
> > 
> > 
> > line 854 does nothing useful.
> 
> 
> It does have a side effect to remove this, since IsDuplicate() will add the
> entry to the cache.  Run manet-routing-compare with and without this line to
> see the difference.

thats correct, in the context of checking bcast duplicates the return value must be checked. I also feel,the function should be renamed "CheckDupAndAdd" or similar.

Here is the side effect created.  Here is the intended initial packet flow (from src/aodv/test/aodv-regression.h):

           1       2       3       4       5
    <------|------>|       |       |       |        RREQ (orig 10.1.1.1, dst 10.1.1.5, G=1, U=1, hop=0, ID=1, org_seqno=1) src = 10.1.1.1
           |<------|------>|       |       |        RREQ (orig 10.1.1.1, dst 10.1.1.5, G=1, U=1, hop=1, ID=1, org_seqno=1) src = 10.1.1.2
           |       |<------|------>|       |        RREQ (orig 10.1.1.1, dst 10.1.1.5, G=1, U=1, hop=2, ID=1, org_seqno=1) src = 10.1.1.3
           |       |       |<------|------>|        RREQ (orig 10.1.1.1, dst 10.1.1.5, G=1, U=1, hop=3, ID=1, org_seqno=1) src = 10.1.1.4

Here is the current ns-3-dev trace for node 1.

0.000160 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 0 id 0x00000001
        dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
0.000482 IP 10.1.1.2.654 > 10.1.1.255.654:  aodv rreq 24  hops 1 id 0x00000001
        dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
0.002514 ARP, Reply 10.1.1.2 is-at 00:00:00:00:00:02, length 32
0.002913 Acknowledgment RA:00:00:00:00:00:03 
0.003059 ARP, Request who-has 10.1.1.1 (ff:ff:ff:ff:ff:ff) tell 10.1.1.2, length 32

Here is the new trace after the patch is applied:
0.000160 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 0 id 0x00000001
        dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
0.000482 IP 10.1.1.2.654 > 10.1.1.255.654:  aodv rreq 24  hops 1 id 0x00000001
        dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
0.000606 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 2 id 0x00000001
        dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
0.002514 ARP, Reply 10.1.1.2 is-at 00:00:00:00:00:02, length 32
0.002913 Acknowledgment RA:00:00:00:00:00:03 
0.003059 ARP, Request who-has 10.1.1.1 (ff:ff:ff:ff:ff:ff) tell 10.1.1.2, length
 32

Note the extra RREQ at time 0.000606.  It appears that node 0 is recirculating its own RREQ (but node 1 suppresses the retransmission) because it didn't put it in the cache when originating it.

I would be fine with renaming to "CheckDupAndAdd" as you suggest.  But I would also be fine with leaving it alone since it is documented correctly and seems to not be a bug.

At first glance I am guessing the below

1. Although the trace indicates node 0 receiving the packet, this may not imply the RREQ was processed to any great extent by AODV-RP  (Node 0 will process its own RREQs and check if they are duplicates in RecvRequest early in the function)
2. As the hop count gets bumped up , these packets could be forwards by other nodes within range.
In this case the trace might help if the MAC was displayed




(In reply to comment #3)
> Here is the side effect created.  Here is the intended initial packet flow
> (from src/aodv/test/aodv-regression.h):
> 
>            1       2       3       4       5
>     <------|------>|       |       |       |        RREQ (orig 10.1.1.1, dst
> 10.1.1.5, G=1, U=1, hop=0, ID=1, org_seqno=1) src = 10.1.1.1
>            |<------|------>|       |       |        RREQ (orig 10.1.1.1, dst
> 10.1.1.5, G=1, U=1, hop=1, ID=1, org_seqno=1) src = 10.1.1.2
>            |       |<------|------>|       |        RREQ (orig 10.1.1.1, dst
> 10.1.1.5, G=1, U=1, hop=2, ID=1, org_seqno=1) src = 10.1.1.3
>            |       |       |<------|------>|        RREQ (orig 10.1.1.1, dst
> 10.1.1.5, G=1, U=1, hop=3, ID=1, org_seqno=1) src = 10.1.1.4
> 
> Here is the current ns-3-dev trace for node 1.
> 
> 0.000160 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 0 id 0x00000001
>         dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
> 0.000482 IP 10.1.1.2.654 > 10.1.1.255.654:  aodv rreq 24  hops 1 id 0x00000001
>         dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
> 0.002514 ARP, Reply 10.1.1.2 is-at 00:00:00:00:00:02, length 32
> 0.002913 Acknowledgment RA:00:00:00:00:00:03 
> 0.003059 ARP, Request who-has 10.1.1.1 (ff:ff:ff:ff:ff:ff) tell 10.1.1.2,
> length 32
> 
> Here is the new trace after the patch is applied:
> 0.000160 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 0 id 0x00000001
>         dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
> 0.000482 IP 10.1.1.2.654 > 10.1.1.255.654:  aodv rreq 24  hops 1 id 0x00000001
>         dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
> 0.000606 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 2 id 0x00000001
>         dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1
> 0.002514 ARP, Reply 10.1.1.2 is-at 00:00:00:00:00:02, length 32
> 0.002913 Acknowledgment RA:00:00:00:00:00:03 
> 0.003059 ARP, Request who-has 10.1.1.1 (ff:ff:ff:ff:ff:ff) tell 10.1.1.2,
> length
>  32
> 
> Note the extra RREQ at time 0.000606.  It appears that node 0 is recirculating
> its own RREQ (but node 1 suppresses the retransmission) because it didn't put
> it in the cache when originating it.
> 
> I would be fine with renaming to "CheckDupAndAdd" as you suggest.  But I would
> also be fine with leaving it alone since it is documented correctly and seems
> to not be a bug.

(In reply to comment #4)

> > 0.000606 IP 10.1.1.1.654 > 10.1.1.255.654:  aodv rreq 24  hops 2 id 0x00000001
> >         dst 10.1.1.5 seq 0 src 10.1.1.1 seq 1

this trace suggests to me that node 1 is resending the same rreq (same id) that it received from node 2, after incrementing hop count from 1 to 2.  I did not try to break or log on this but it just seemed to me that a side effect of disabling IsDuplicate() when originating the packet would be that a node wouldn't be able to detect that it had already sent it.