Bug 2063

Summary:	FdNetDevice::SendFrom() Assert does not take into account the size of the headers.
Product:	ns-3	Reporter:	Rubén Martínez <rmartinez>
Component:	fd-net-device	Assignee:	alina <aquereilhac>
Status:	RESOLVED FIXED
Severity:	minor	CC:	ns-bugs, tomh
Priority:	P5
Version:	ns-3-dev
Hardware:	PC
OS:	Linux

Description Rubén Martínez 2015-02-13 05:46:37 UTC

This assertion is failing:

Line  543:   NS_ASSERT_MSG (packet->GetSize () <= m_mtu, FdNetDevice::SendFrom(): Packet too big " << packet->GetSize ());

Due to the added Ethernet header in:

Line 529:   packet->AddHeader (header);

I would suggest to move it before the addition of the header or to change the condition to include it:

 NS_ASSERT_MSG (packet->GetSize () <= m_mtu + header.GetSerializedSize(), "FdNetDevice::SendFrom(): Packet too big " << packet->GetSize ());

Comment 1 Tom Henderson 2015-02-13 16:08:51 UTC

This popped up during some emulation testing; marking as "Patch Pending" since the trivial patch options are in the bug report itself.

Any preference in how to fix this?  Any idea why this didn't trigger earlier?  (surely there have been some 1500 byte frames submitted to this device in the past)

Comment 2 Tom Henderson 2015-02-16 16:44:44 UTC

There is a related problem involving the reading of frames that are greater than the MTU.

FdNetDevice::SetBufferSize() will set the m_bufferSize to the MTU:

  m_fdReader->SetBufferSize (m_mtu);

later, FdNetDeviceFdReader::DoRead () will use this value to set the maximum buffer length to read from the device.

This is before the Ethernet header is removed in FdNetDevice::ForwardUp().

So, what I was observing is that incoming packets that were above 1500 bytes would be truncated.  e.g. a frame of 1504 bytes, which has 1490 bytes of IP datagram, 1470 bytes of IP payload, 1462 bytes of UDP payload.  The UDP checksum was omitting the last four bytes of payload and failing.

This worked around the issue for me, allowing up to 1522 bytes to be read from the device for a 1500 byte MTU:

-  m_fdReader->SetBufferSize(m_mtu);
+  m_fdReader->SetBufferSize (m_mtu + 22);

Again, I'm surprised that this has not been reported before; it looks like possibly a serious bug.

Comment 3 alina 2015-02-16 22:37:19 UTC

The suggested fixes (m_mtu+22 and moving assert) are now committed to ns-3-dev in changeset c97ef876d98c.

The reason why these errors might not have been reported before is possibly that the module is not being used much. It is likely that the only use given to the FdNetDevice so far involves exchanging traffic with small payloads. Although it is strange that the tests with the onoff application and large packet size did not failed. I will investigate that.

Comment 4 Tom Henderson 2015-02-23 15:56:33 UTC

leaving open in case any additional tests by Alina find anything, but this should now be resolved (will close if nothing found shortly).

Will patch this in ns-3.22 also if a minor release is made.

Comment 5 alina 2015-03-19 10:31:12 UTC

The examples in the fd-net-device module did not reveal this bug because none of them used UDP to send large packets. Examples like fd2fd-onoff and fd-emu-onoff used only TCP with the default TCP MSS of 536, so packet->GetSize () in the SendFrom was never bigger than m_mtu. I changed the fd2fd-onoff example to use UDP by default and added it to module tests (changeset:   27a4e883d58f). I think this bug report can be closed now. I did not find further problems while testing.