Bug 2416

Summary: "The capture file appears to have been cut short in the middle of a packet."
Product: ns-3 Reporter: Matthieu Coudron <mattator>
Component: networkAssignee: ns-bugs <ns-bugs>
Status: NEW ---    
Severity: enhancement CC: mattator
Priority: P5    
Version: ns-3.25   
Hardware: PC   
OS: Linux   

Description Matthieu Coudron 2016-05-19 12:18:37 EDT
Wireshark complains from time to time when opening pcap generated by ns3.
I used https://f00l.de/pcapfix/ (available in ubuntu repos) to find the problem:
====
    $~/pcapfix/pcapfix ~/ns3off2/mptcp-multi-0-1.pcap -v

    [+] Packet #1479 at position 273354 (1 | 552762 | 50 | 50).
    [+] Packet #1480 at position 273420 (1 | 552770 | 50 | 50).
    [+] Packet #1481 at position 273486 (1 | 552780 | 50 | 50).
    [+] Packet #1482 at position 273552 (1 | 552788 | 50 | 50).
    [+] Packet #1483 at position 273618 (1 | 552799 | 50 | 50).
    [+] Packet #1484 at position 273684 (1 | 552808 | 50 | 50).
    [+] Packet #1485 at position 273750 (1 | 552823 | 50 | 50).
    [+] Packet #1486 at position 273816 (1 | 552832 | 50 | 50).
    [+] Packet #1487 at position 273882 (1 | 552848 | 50 | 50).
    [+] Packet #1488 at position 273948 (1 | 552859 | 50 | 50).
    [+] Packet #1489 at position 274014 (1 | 552881 | 50 | 50).
    [-] LAST PACKET MISMATCH (1 | 552906 | 50 | 50)
    [+] CORRECTED Packet #1490 at position 274080 (1 | 552906 | 58 | 58).
    [+] Packet #1490 at position 274080 (1 | 552906 | 58 | 58).
    [*] Wrote 1490 packets to file.
====
The numbers in (1 | 552906 | 50 | 50) are Second/MicroSecond, includedLen, originalLen.
pcapfix change the length to match the size of the file. I checked what size this packet should be and it should be indeed 50, which means the extra bytes are written by ns3 because of a bug.
I tracked it down to "void Buffer::CopyData (std::ostream *os, uint32_t size) const". I noticed it was a (inexact) copy of "uint32_t 
Buffer::CopyData (uint8_t *buffer, uint32_t size) const"
hence as a quickfix, I changed the function to
===
uint32_t
Buffer::CopyData (std::ostream *os, uint32_t size) const
{
  NS_LOG_FUNCTION (this << &os << size);
  uint8_t *temp = new uint8_t[size];
  uint32_t written = CopyData (temp, size);
  os->write ((const char*)(temp), written);
  delete temp;
  return written;
}
===
and now the problem seems to have disappeared. We should avoid having a copied function, a better fix might be to pass a lambda or a boolean to decide if CopyData should write to an ostream or an array.
Comment 1 Matthieu Coudron 2016-05-19 12:20:31 EDT
s/change the length/changes the length from 50 to 58