[vortex] 802.1q VLAN problem

[Ben Greear]

Bogdan Costescu wrote:

> On Wed, 17 Apr 2002, Ben Greear wrote:
> 
> 
>>Just enabling the larger frame sizes does not mean that all of a sudden we'll
>>be using 4k packets, so I think many of your concerns relating to switch support
>>etc are not big problems.  Note that the standard VLAN setup does not increase
>>the MTU of the ethernet interface, so the max sized packet will be just 4 bytes
>>bigger than normal.
>>
> 
> I can see that our views differ: I was thinking of adding over-sized frame 
> support with the added benefit of supporting VLAN, while you are probably 
> thinking about adding VLAN support with the added benefit of supporting 
> over-sized frames. 8-)


Hehe, I like both features, so whatever motivation we have,
the end result should be good for all! :)


> 
> 
>>If there are performance tradeoffs, then just having a module variable to
>>toggle VLAN or regular support would be much better than nothing.
>>
> 
> I've taken a short look at the existing patches and it seems that the 
> "always-on" mode of accepting VLAN or over-sized frames was already 
> discussed. Probably a module option is the best solution, with the 
> default "off". Is there any common name for such a module option ?


Consider programatic driver hooks to allow the vlan code to enable
the feature run-time too?  (An IOCTL would be fine, as the vconfig
program could attempt it, maybe IOCTL_VLAN_ENABLE).  I believe Dave
added a nice clean interface for this kind of thing to 2.5, btw).


> 
> I've taken a look through the documentation and I found out that:
> 
> - Boomerang only has an "allowLargePackets" bit in MacControl register, so 
> it's all-or-nothing. I don't have documentation for older chips.


Seems a good module-load time option.


> - Cyclone has "allowLargePackets", but also has a "MaxPktSize" register 
> which defines when the oversizedFrame error occurs.


Since you can increase this by only 4 if you wish, this might be a good
feature to have default to ON.


> - Tornado has "allowLargePackets", "MaxPktSize" and goes one step further 
> with another MacControl bit (10) call "vlanOversizeEn" which if set, 
> allows packets to be 4 bytes more than "MaxPktSize" before signaling the 
> oversizedFrame error.


Even better, in this case it looks like you could enable the vlanOversizeEn
bit by default w/out any other hackery.  I can't imagine this would effect
performance, but I obviously haven't tested it to be sure...

 
> So, IMHO the easiest to add support for VLAN is Tornado where only the 
> "vlanOversizeEn" bit should be set; for Cyclone, "MaxPktSize" should be 
> set to MTU+4. As the method for Cyclone should work for both, we 
> should probably use it in both cases in order not to introduce extra 
> complexity in the code. OTOH, the driver doesn't take advantage of the MTU 
> size, it only sets the "allowLargePackets" bit when MTU > 1500, all skb
> allocations are done based on PKT_BUF_SZ, so to properly support different 
> MTU sizes more changes to the driver are needed...


Yes, I can see there would be more work to support larger (ie, 4k packets),
but the wee little 4 extra bytes for vlan shouldn't cause any problems :)

Thanks,
Ben


> 
> Any thoughts ?
> 
> 


-- 
Ben Greear <greearb@candelatech.com>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear