Bug 1787

Summary:	Runtime error when using AnimationInterface::EnablePacketMetadata() to fetch metadata of CSMA packet
Product:	ns-3	Reporter:	Min-Cheng Chan <rascov>
Component:	tap-bridge	Assignee:	Tom Goff <tgoff>
Status:	RESOLVED FIXED
Severity:	normal	CC:	ns-bugs, rascov, tomh, tommaso.pecorella
Priority:	P5
Version:	ns-3.18
Hardware:	All
OS:	Linux
Attachments:	Patch file to tap-bridge.cc Patch file to tap-bridge.h Pcap file which demonstrates the problem proposed fix problem example

Description Min-Cheng Chan 2013-10-29 00:46:04 UTC

There is a runtime error when using AnimationInterface::EnablePacketMetadata() to fetch metadata of CSMA packet.

The error message is
assert failed. cond="m_current >= delta", file=../src/network/model/buffer.h, line=686
terminate called without an active exception

and the backtrace list is
ns3::Buffer::Iterator::Prev at src/network/model/buffer.h:686
ns3::EthernetTrailer::Deserialize at src/network/utils/ethernet-trailer.cc:148
ns3::Packet::Print at src/network/model/packet.cc:488
ns3::AnimationInterface::GetPacketMetadata at src/netanim/model/animation-interface.cc:1570
ns3::AnimationInterface::CsmaMacRxTrace at src/netanim/model/animation-interface.cc:1677

Comment 1 Min-Cheng Chan 2013-11-04 13:49:10 UTC

Created attachment 1694 [details]
Patch file to tap-bridge.cc

Comment 2 Min-Cheng Chan 2013-11-04 13:49:37 UTC

Created attachment 1695 [details]
Patch file to tap-bridge.h

Comment 3 Min-Cheng Chan 2013-11-04 13:53:10 UTC

Created attachment 1696 [details]
Pcap file which demonstrates the problem

Comment 4 Min-Cheng Chan 2013-11-04 13:58:56 UTC

We check the ns3::Packet::Print function in packet.cc and find out the return item of PacketMetadata::ItemIterator::Next 
function is wrong. So we print "extraItem.fragmentEnd", "extraItem.fragmentStart", "before m_offset" that is the size while 
process enter Next function and "after m_offset" that is the size after add (extraItem.fragmentEnd - extraItem.fragmentStart).
The results is as follow:

  extraItem.fragmentEnd = 14, extraItem.fragmentStart = 0, before m_offset = 0, after m_offset = 14
  extraItem.fragmentEnd = 90, extraItem.fragmentStart = 0, before m_offset = 14, after m_offset = 104
  extraItem.fragmentEnd = 4, extraItem.fragmentStart = 0, before m_offset = 104, after m_offset = 108

We fetch the information of a Ethernet packet by Wireshark(as attachment csma-2-4.pcap), the total size of the packet is 94(14+76+4) bytes.
And then we find out there is a 14-bytes difference between extraItem.fragmentEnd of second line and payload size.
This is because the packet obtained from Create<Packet>(buf, len) at tap-bridge.cc:739 is packed directly with Ethernet header(14 bytes) and payload(76 bytes), so it only has one smallitem.
However in Filter function at tap-bridge.cc:842, the packet need RemoveHeader, but item.size != size at packet-metadata.cc:709 is occurred.

We fixed the bug of Filter function and ForwardToBridgedDevice function at tap-bridge.cc and tap-bridge.h.
Attached please find the patches of tap-bridge.cc and tap-bridge.h.

Comment 5 Tom Goff 2013-12-31 22:45:10 UTC

Created attachment 1750 [details]
proposed fix

I think the underlying issue is the use of Packet::RemoveHeader() in
TapBridge::Filter().

RemoveHeader() assumes a corresponding header metadata item exists.
In this case it does not and the result is that the packet size gets
updated appropriately but the payload meta data item is unchanged.

I suggest the attached patch to avoid using RemoveHeader().

Comment 6 Tommaso Pecorella 2014-01-01 07:59:10 UTC

I don't think the problem is there.

The lines just above are already checking that the packet is long enough to be decoded. Hence, removing the header should be safe.

I don't have a system to test it here, but I have to pinpoint that the example PCAP contains an ICMPv6 packet.

Question is: isn't it a problem related to IPv6 support by TapBridge and/or AnimationInterface ?

Comment 7 Tom Goff 2014-01-01 11:21:53 UTC

Created attachment 1751 [details]
problem example

As I said, I think this problem is related to packet metadata.  See the
attachment for a simplified example.

Comment 8 Tommaso Pecorella 2014-01-05 07:09:17 UTC

(In reply to Tom Goff from comment #7)
> Created attachment 1751 [details]
> problem example
> 
> As I said, I think this problem is related to packet metadata.  See the
> attachment for a simplified example.

You're right. Not haven the metadata is definitely not helping.

+1 to apply your patch, but I'd double check if the problem persist.

I checked the TapBridge and its IPv6 support shouldn't be a problem (if I'm right, it's just used to give an IP address to the interface).

Comment 9 Tommaso Pecorella 2014-02-20 16:38:01 UTC

Pushed in changeset: 10619:78c44da5cc94