On Fri, Feb 01, 2008 at 07:15:52PM -0700, Stephen Fisher wrote:
> We have bug #2202 open for a problem where Wireshark is reporting an
> incorrect PPPoE payload length (as read from the PPPoE protocol data)
> when it is compared to the entire tvb length. The problem is that there
> is an Ethernet trailer at the end that isn't being recognized as an
> Ethernet trailer. One fix I have come up with is to call the
> tvb_set_reported_length() function to "chop off" the Ethernet trailer so
> the Ethernet dissector recognizes it as such. However, this would
> defeat the check of PPPoE payload length vs. actual length available.
> Can anyone think of a better fix?
I wanted to suggest to treat the case where the ethernet frame was
padded (to make it 64 bytes) differently, but then I saw that it
should already do that:
/*
* The only reason why the payload length from the header
* should differ from the remaining data in the packet
* would be if the total packet length, including Ethernet
* CRC, were < 64 bytes, so that padding was required.
*
* That means that you have 14 bytes of Ethernet header,
* 4 bytes of FCS, and fewer than 46 bytes of PPPoE packet.
*
* If that's not the case, we report a difference between
* the payload length in the packet, and the amount of
* data following the PPPoE header, as an error.
*/
However, the code has a one-off error in it:
if (tvb_reported_length(tvb) >= 46 &&
reported_payload_length != actual_payload_length) {
proto_item_append_text(ti, " [incorrect, should be %u]",
actual_payload_length);
expert_add_info_format(pinfo, ti, PI_MALFORMED,
PI_WARN, "Possible bad payload length %u != %u",
reported_payload_length, actual_payload_length);
}
In case of a minimal length ethernet frame, the reported_length
is actually 46, so the ">=" should have been a ">".
I'll check in a fix as I have it ready anyways... Sorry to steal
your project for today ;-)
Cheers,
Sake
