Bugzilla – Bug 738
CsmaNetDevice calls ReceiveErrorModel too late
Last modified: 2010-01-05 14:15:19 EST
Overview: CsmaNetDevice::Receive is responsible for calling a ReceiveErrorModel if set. However, it removes the Ethernet header and trailer first. This means that for the same bit error rate, the packet error rate is lower than it should be, because Ethernet headers/trailers can never be corrupted. Steps to reproduce: 1) Use a simple setup that transfers packets from node A to node B. 2) Set a bit error rate. 3) Record the number of packets actually received by node B. Actual result: The packet error rate is approximately (1-BER)^x, with x being the packet size of payload + IP/UDP header in bits. Expected result: The packet error rate should be approximately (1-BER)^y, with y being the packet size of payload + IP/UDP + the 144 bits of Ethernet header/trailer.
Created attachment 653 [details] patch to fix the bug This is a small patch to fix the bug. Not sure it's the most beautiful solution, especially with the duplicated header/trailer removal in both branches, but it changes as little as possible of the code. If this is not acceptable, I can rewrite the section slightly more to eliminate the duplication. Patched code compiles, and in my testing setup, the PER now seems to match the expected rate.
(In reply to comment #1) > Created an attachment (id=653) [details] > patch to fix the bug > > This is a small patch to fix the bug. Not sure it's the most beautiful > solution, especially with the duplicated header/trailer removal in both > branches, but it changes as little as possible of the code. If this is not > acceptable, I can rewrite the section slightly more to eliminate the > duplication. > Patched code compiles, and in my testing setup, the PER now seems to match the > expected rate. I agree that this bug needs fixed. It pointed out another problem, too, which is that the m_phyRxDropTrace inconsistently gets a packet with an Ethernet header in one call above this code block, and without the header in the error model call. Attached is a patch which fixes also the drop trace issue, and removes the long else clause.
Created attachment 654 [details] revised patch
Craig, please review and push if you are OK with this.