PR/39203 CVS commit: src/sys/net

[Martin Husemann]

src/sys/net: if_ether.h
PR/39203: Paul Ripke: PPPoE issues with broken MTU/MRU implementations
Allow larger frames for systems that don't negotiate MTU/MRU properly.

I don't object this change, but it does look kinda wrong.

I wondered if the whole ETHER_MAX_FRAME() check in ether_input is not a bogus
workaround for some old (maybe even long fixed) bugs in chipset drivers.
Shouldn't we leave the decision "I did receive this frame OK" to the driver
(modulo fcs if not checked in hardware)?

If we did receive the frame ok, why should we drop it?

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Christos Zoulas]

On Jul 25, 5:35pm, ***@duskware.de (Martin Husemann) wrote:
-- Subject: Re: PR/39203 CVS commit: src/sys/net

| On Fri, Jul 25, 2008 at 03:15:03PM +0000, Christos Zoulas wrote:
| > Modified Files:
| > src/sys/net: if_ether.h
| >
| > Log Message:
| > PR/39203: Paul Ripke: PPPoE issues with broken MTU/MRU implementations
| > Allow larger frames for systems that don't negotiate MTU/MRU properly.
|
| I don't object this change, but it does look kinda wrong.
|
| I wondered if the whole ETHER_MAX_FRAME() check in ether_input is not a bogus
| workaround for some old (maybe even long fixed) bugs in chipset drivers.
| Shouldn't we leave the decision "I did receive this frame OK" to the driver
| (modulo fcs if not checked in hardware)?

Yes, but it is also a cheap belt and suspenders test for ethernet health...

| If we did receive the frame ok, why should we drop it?

Are there parts in the system that make assumptions about maximum packet size?

christos

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Quentin Garnier]

Post by Martin Husemann

src/sys/net: if_ether.h
PR/39203: Paul Ripke: PPPoE issues with broken MTU/MRU implementations
Allow larger frames for systems that don't negotiate MTU/MRU properly.

I don't object this change, but it does look kinda wrong.
I wondered if the whole ETHER_MAX_FRAME() check in ether_input is not a bogus
workaround for some old (maybe even long fixed) bugs in chipset drivers.
Shouldn't we leave the decision "I did receive this frame OK" to the driver
(modulo fcs if not checked in hardware)?
If we did receive the frame ok, why should we drop it?

The NIC might have simply truncated it. I'm pretty sure I've seen it
happen in that context, without specific notification to the driver.

It might not be worse to let a truncated frame through the upper layers,
but the point is that you shouldn't have received such a frame in the
first place.

My initial implementation was smarter because it let the NIC driver
confirm it is able and willing to receive such frames.

[Martin Husemann]

Post by Quentin Garnier
The NIC might have simply truncated it. I'm pretty sure I've seen it
happen in that context, without specific notification to the driver.

[..]

Post by Quentin Garnier
My initial implementation was smarter because it let the NIC driver
confirm it is able and willing to receive such frames.

Wouldn't it bet better to have the driver detect this (if possible) and
not pass the frame to ether_input at all? If the hardware does not tell
the driver, it should still know the maximum size and do this check
better.

I don't see how the ethertype relates to this check - if at all the vlan
or jumbo frame capabilities of the NIC should count.

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Quentin Garnier]

Post by Martin Husemann

Post by Quentin Garnier
The NIC might have simply truncated it. I'm pretty sure I've seen it
happen in that context, without specific notification to the driver.

[..]

Post by Quentin Garnier
My initial implementation was smarter because it let the NIC driver
confirm it is able and willing to receive such frames.

Wouldn't it bet better to have the driver detect this (if possible) and
not pass the frame to ether_input at all? If the hardware does not tell
the driver, it should still know the maximum size and do this check
better.

How can the driver tell if a frame was truncated or not, if the hardware
doesn't indicate it? I'm pretty sure I saw it with sip(4), but it might
have been another.

Post by Martin Husemann
I don't see how the ethertype relates to this check - if at all the vlan
or jumbo frame capabilities of the NIC should count.

It relates because the check is about whether or not the system *should*
have received such a packet, not whether or not it was able to (rather
obviously it was able to receive it).

That said, I'm all in favour of dropping the check completely.

[Martin Husemann]

Post by Quentin Garnier
How can the driver tell if a frame was truncated or not, if the hardware
doesn't indicate it? I'm pretty sure I saw it with sip(4), but it might
have been another.

By checking the size, unless it knows the hardare can reliably receive
larger frames (or will tell it about truncation). This is just moving
the check.

Post by Quentin Garnier
It relates because the check is about whether or not the system *should*
have received such a packet, not whether or not it was able to (rather
obviously it was able to receive it).

I'm not sure I follow how a (broken) pppoe peer means the system should
have been able to receive the packet. How do we know the frame is not
truncated?

Martin

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Quentin Garnier]

Post by Martin Husemann

Post by Quentin Garnier
How can the driver tell if a frame was truncated or not, if the hardware
doesn't indicate it? I'm pretty sure I saw it with sip(4), but it might
have been another.

By checking the size, unless it knows the hardare can reliably receive
larger frames (or will tell it about truncation). This is just moving
the check.

No, it can't tell if the frame was truncated. That's not what that
check does. There is no payload length information in the Ethernet
header, so you can only tell if a frame is truncated by checking the
FCS (if you happen to have it) or by inspecting the contents of the
payload. Either way it's expensive so there's no point in doing it.

Post by Martin Husemann

Post by Quentin Garnier
It relates because the check is about whether or not the system *should*
have received such a packet, not whether or not it was able to (rather
obviously it was able to receive it).

I'm not sure I follow how a (broken) pppoe peer means the system should
have been able to receive the packet. How do we know the frame is not
truncated?

You have a point. The PPPoE peer is broken but it's the kind of
brokenness you really want in a PPPoE peer :-) [Well, at least in an
environment where the ISP only sees PPPoA because the telephony operator
has an Ethernet-ATM bridge and the ISP can't tell if a client is oE or
oA, and thus cannot lower the MTU on the PPP session server because it
would be unfair to all oA users. That happened to be my situation at
the time, I don't know if it exactly matches Ripke's.]

The check is not very good anyway: even if we consider only the VLAN
case, it will let packet through whether or not the system plays with
VLANs or not. There are other situations where it doesn't really make
sense, too.