Bug 1304

Created attachment 1276 [details]
Patch file generated for ipv4-l3-protocol.cc

Just run the aodv.cc under AODV example can see the results

This is a bit more complex than I thought. If you add these lines to Ipv4L3Protocol::Send

  Ipv4PacketInfoTag infoTag;
  if( packet->RemovePacketTag(infoTag) )
    {
      std::cout << "tag was already present " << infoTag.GetAddress() << std::endl;;
    }
  infoTag.SetAddress(GetAddress (1, 0).GetLocal ()); 
  packet->AddPacketTag(infoTag); 
  std::cout << "added " << GetAddress (1, 0).GetLocal () << std::endl;
  std::cout << "added(2) " << infoTag.GetAddress() << std::endl;

  std::cout << "Tags: ";
  packet->PrintPacketTags(std::cout);
  std::cout << std::endl;

 you'll get this:
added 10.0.0.1
added(2) 10.0.0.1
Tags: Ipv4 PKTINFO [DestAddr: 102.102.102.102, Local Address:102.102.102.102, RecvIf:0, TTL:0]  assert failed. cond="item.GetTypeId ().HasConstructor ()", file=../src/network/model/packet.cc, line=871

Now, this is the issue's origin. Mind that Ipv4PacketInfoTag constructor is perfectly fine, so for its TypeId metadata.

I'm not a Tag expert, so I'll leave the resolution to the experts. However I think it's a quite huge bug. and it can be the bell ringing for something bigger.

I think we should raise the priority of this.

T.

Hi,

    For the bug I mentioned,  It's a tiny bug in Deserilize(). 
in file ipv4-packet-info-tag.cc in internet folder. 
in line 124 and 126 
instead of 
  m_addr.Deserialize (buf); 
  m_spec_dst.Deserialize (buf); 
it should be : 
m_addr = Ipv4Address::Deserialize (buf); 
m_spec_dst = Ipv4Address::Deserialize (buf); 

Then it will work fine. however, for your case, I don't know whether it is the same bug..


(In reply to comment #3)
> This is a bit more complex than I thought. If you add these lines to
> Ipv4L3Protocol::Send
> 
>   Ipv4PacketInfoTag infoTag;
>   if( packet->RemovePacketTag(infoTag) )
>     {
>       std::cout << "tag was already present " << infoTag.GetAddress() <<
> std::endl;;
>     }
>   infoTag.SetAddress(GetAddress (1, 0).GetLocal ()); 
>   packet->AddPacketTag(infoTag); 
>   std::cout << "added " << GetAddress (1, 0).GetLocal () << std::endl;
>   std::cout << "added(2) " << infoTag.GetAddress() << std::endl;
> 
>   std::cout << "Tags: ";
>   packet->PrintPacketTags(std::cout);
>   std::cout << std::endl;
> 
>  you'll get this:
> added 10.0.0.1
> added(2) 10.0.0.1
> Tags: Ipv4 PKTINFO [DestAddr: 102.102.102.102, Local Address:102.102.102.102,
> RecvIf:0, TTL:0]  assert failed. cond="item.GetTypeId ().HasConstructor ()",
> file=../src/network/model/packet.cc, line=871
> 
> Now, this is the issue's origin. Mind that Ipv4PacketInfoTag constructor is
> perfectly fine, so for its TypeId metadata.
> 
> I'm not a Tag expert, so I'll leave the resolution to the experts. However I
> think it's a quite huge bug. and it can be the bell ringing for something
> bigger.
> 
> I think we should raise the priority of this.
> 
> T.

Hi,

you're damn right, it's a bug.

And this raise another issue:
tommaso:ns-3-dev pecos$ grep -r Deserialize src/*/*/*.h | grep static
src/network/utils/ipv4-address.h:  static Ipv4Address Deserialize (const uint8_t buf[4]);
src/network/utils/ipv6-address.h:  static Ipv6Address Deserialize (const uint8_t buf[16]);
src/network/utils/packetbb.h:  static Ptr<PbbMessage> DeserializeMessage (Buffer::Iterator &start);

WHY those 3 Deserializes are static ?!?!?!

I'll check better later tonight, but in my opinion they should NOT be static, as all the other Deserialize(s) are not. So those 3 are a bug source (the user will use them as "normal" but they don't change the instance status.

Design bug.

BTW, the vary same bug (or similar ones) might be scattered around. I'll check those as well and I'll do a global patch.


T.

Fixed with changeset:   7780 - 819d9c315765

Leaving it open, downgrading the priority and changing the name as I don't like the idea of Deserialize static functions.

T.