Re: Various 802.1Q VLAN driver patches. [try2]

[Andreas Ferber]

On Fri, Mar 01, 2002 at 02:09:23PM -0500, Jeff Garzik wrote:
> 
> 1) This patch is (like I mentioned earlier) for reference only, not for
> application.

Ack. Although it works like a charme in production here, and I did not
receive any problem reports about it so far. So anyone who needs this
now should apply the patch locally for the moment.

> It is really halfway in between software and hardware VLAN
> support.

Not really. The MTU setting is something which must be done for any
"dumb" card that has no real hardware VLAN support, and the only thing
Cyclone and Tornado cards care about the VlanEtherType is that they
start IP checksumming 4 octets later. They don't care a bit about the
contents of the VLAN tag.

> 2) You don't want to set_8021q_mode if VLAN is not active.  It's silly
> to activate it when VLAN is compiled as a module but no one is using
> vlans.  That's going to be THE common case, because distros will
> inevitably build the VLAN module with their stock kernel.

Well, this was discussed on the VLAN mailing list. The conclusion
there was that it will not hurt on most cards if it is enabled
unconditionally.

The only situation where it will probably hurt is if you have many
frames exceeding ethernet MTU coming in through your ethernet cable.
Those (up to FDDI size e.g. on 3COM cards without MaxPktSize support)
will not be dropped by the card but instead handed up to the network
stack and be dropped there. Are there any situations (except for
failure cases) where a lot large frames are sent over normal ethernet?

And for the 802.1q recognition of Cyclone and Tornado cards, this only
affects the IP checksumming done by the hardware. As plain IP is never
transmitted in frames with an ethernet type of 0x8100, this also will
not affect normal operation.

Anyway, if you want to enable it only if VLANs are really used on the
card, what about adding a new hook to struct net_device, which can be
used by the 802.1q core code to enable VLAN support on the card as
needed (if supported by the driver)? Or, as this is only a boolean
flag, maybe a generic feature_supported/set_feature hook pair that can
be used for other similar situations in the future?

> 3) 3c59x needs real dev->change_mtu support.  This patch provides the
> info needed to do that... but doesn't do that.

Intentionally, as this has nothing to do with VLAN support. The whole
thing about the patch was that you /don't/ have to change the physical
device MTU visible to the network stack. Otherwise the network stack
would start sending oversized frames to the ethernet, which would
render the untagged VLAN useless.

> 4) It uses magic numbers instead of ETH_DATA_LEN and ETH_HLEN

That's what the driver used before ;-)

Andreas
-- 
       Andreas Ferber - dev/consulting GmbH - Bielefeld, FRG
     ---------------------------------------------------------
         [phone] - [e-mail] - www.devcon.net