A couple of comments about this patch, and then some questions.

(In reply to comment #0)
> For certain multicast packets, the 'lcb' callback may be NULL. 
> aodv::RouteInput may call these NULL callbacks.
> 
> Quick Patch (ns-3.10):
> 
> --- aodv-routing-protocol.cc    2011-01-24 11:38:02.000000000 -0500
> +++ aodv-routing-protocol.cc.new        2011-01-24 11:40:32.000000000 -0500
> @@ -401,7 +401,10 @@
>              UpdateRouteLifeTime (origin, ActiveRouteTimeout);
>              NS_LOG_LOGIC ("Broadcast local delivery to " << iface.GetLocal
> ());
>              Ptr<Packet> packet = p->Copy ();
> -            lcb (p, header, iif);
> +            if (lcb.IsNull() == false)
> +              {
> +                lcb (p, header, iif);
> +              }

In general, I agree to check for a non-null callback.  However, I would also suggest to move the packet copy inside the conditional, and perhaps also to move the log statement in there (perhaps also adding an else clause with a different log statement).

>              if (!EnableBroadcast)
>                {
>                  return true;
> @@ -439,7 +442,19 @@
>            m_nb.Update (toOrigin.GetNextHop (), ActiveRouteTimeout);
>          }
>        NS_LOG_LOGIC ("Unicast local delivery to " << dst);
> -      lcb (p, header, iif);
> +      if (lcb.IsNull() == false)
> +        {
> +          lcb (p, header, iif);
> +        }

Again, a log statement on the else clause may be helpful.

> +      else
> +        {
> +          // The local delivery callback is null.  This may be a multicast
> +          // or broadcast packet, so return false so that another

How can it be a multicast or broadcast packet in this branch of the code?
 
> +          // multicast routing protocol can handle it.  It should be possible
> +          // to extend this to explicitly check whether it is a unicast
> +          // packet, and invoke the error callback if so
> +          return false;
> +        }
>        return true;
>      }

My other question is why not check for multicast before hitting this patch, and return false?  

  // Duplicate of own packet
  if (IsMyOwnAddress (origin))
    return true;

+ // AODV is not a multicast routing protocol
+ if (dst.IsMulticast ())
+   {
+      return false; 
+   }

  // Broadcast local delivery/forwarding

> In general, I agree to check for a non-null callback.  However, I would also
> suggest to move the packet copy inside the conditional, and perhaps also to
> move the log statement in there (perhaps also adding an else clause with a
> different log statement).

Agree completely!

> 
> How can it be a multicast or broadcast packet in this branch of the code?

For me, this was getting called via IpvXListRouting::RouteInput().  That should deliver the multicast packet locally if the local node is a member of the multicast group.  It then calls other routing protocols in the list to forward if necessary, but it NULLs out the LocalDeliverCallback so that it doesn't get delivered locally again.


> My other question is why not check for multicast before hitting this patch, and
> return false?  
>
> 
>   // Duplicate of own packet
>   if (IsMyOwnAddress (origin))
>     return true;
> 
> + // AODV is not a multicast routing protocol
> + if (dst.IsMulticast ())
> +   {
> +      return false; 
> +   }
> 
>   // Broadcast local delivery/forwarding


Hmmm...  This looks even better.  This should allow it to fall through to another routing protocol (static?) that would forward the multicast packet.

The patch I originally submitted on this bug was a 'copy' of what had been done for OLSR a while back.  Whatever is done here for AODV should probably be done for OLSR as well. 

Thanks!!

patched for AODV in changeset: c3c23b4f49be

similar fix could be applied to DSDV; patch attached

Created attachment 1066 [details]
patch to fix

changeset: 075e6cbaa9d9