On Mon, Aug 04, 2003 at 11:08:52AM +0200, Manuel Bouyer wrote:
> On Sun, Aug 03, 2003 at 09:31:44PM +0200, Quentin Garnier wrote:
> > Well, at the time that particular check is done, VLAN capabilities aren't
> > checked for the interface either: if a frame *may* have a VLAN tag (i.e.,
> > its Ethernet type field is set to ETHERTYPE_VLAN), it gets processed. My
> > patch just does the same thing for PPPoE
> 
> At the place ETHER_MAX_FRAME() is used, yes. But it's not the only place
> where the frame length may be checked. For example, look at the use of
> ETHERCAP_VLAN_MTU in sys/dev/ic/tulip.c, or lance.c, or a few others that
> also enforce the len in hardware.

From a quick glance at the source for the use of ETHERCAP_VLAN_MTU:

ic/elinkxl.c:	interface reports an error to be ignored
ic/gem.c:	interface must be told max frame size it should accept
ic/hme.c:	interface must be told max frame size it should accept
ic/i82557.c:	interface must be told to receive bad frames, and some checks
		are not skipped when VLAN is enabled
ic/lance.c:	software sanity check of frame length
ic/tulip.c:	interface reports an error to be ignored
ic/atw.c:	interface reports an error to be ignored

pci/if_ti.c:	interface must be told max frame size it should accept
pci/if_ste.c:	interface must be told to receive large frames
pci/if_stge.c:	interface must be told max frame size it should accept

In ic/lance.c I really think the check should be removed: it's a duplicate of
what is done after in ether_input, without using the macro.

Good old cheap network cards that let the software do all the sanity checking
work :) (Well, when they last long enough to qualify as old)

> > The PPPoE client code will do the necessary sanity checks and decide if it
> > is a valid PPPoE frame. The patch just gives a chance for the frame to be
> > processed, asserting it is a valid one is the responsibility of the PPPoE
> > code.
> > 
> > I don't see how this is different from the way VLAN frames are processed,
> > and why large PPPoE frames should be handled differently.
> 
> It's not different. For now you are missing bits to have the PPPoE frames
> handled properly by some drivers.

I wonder if it is worth it, since it is very hard to test. I can add an
ETHERCAP_PPPOE_MTU for sip(4), but not for any other. Well, we still have a
lot of bits left for Ethernet capabilities, but it still feels like a
waste.

[...]
> > But anyway, we don't have to check if the NIC can receive the frame at
> > that time, because obviously it could.
> 
> ec_capenable ETHERCAP_VLAN_MTU is there to have the driver allow the nic to
> receive such frames. 

I'll do that then.

Quentin Garnier.