change the way e1000 is handling short VLAN frames

change the way e1000 is handling short VLAN frames

[Emil Micek]

Hello list,

I'd like to change behaviour of e1000 module when transmiting short
ethernet frames (shorter then 64 bytes) trough VLAN interface.

Current e1000 (according to our observations) first appends 4 bytes of
VLAN tag and then pads the frame to 64 bytes with zeroes if necessary
before transmiting it.

I'd like to change it so it first pads the frame to 64 B and appends the
VLAN tag afterwards, so the resulting VLAN frame would be 68 bytes
minimum.

I want to do it because C3 CMTS from arris, which is connected to our
linux box, has problems handling VLAN frames shorter then 68 bytes.

As i'm quite a newbie concerning linux sources and c programing, i'd
like to ask you if this'd be trivial or complex change and if someone
can point out relevant portions of the sources, where the necessary
changes should be done.

Regards

Emil Micek

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 09:31 +0200, Emil Micek wrote:
> Hello list,
> 
> I'd like to change behaviour of e1000 module when transmiting short
> ethernet frames (shorter then 64 bytes) trough VLAN interface.
> 
> Current e1000 (according to our observations) first appends 4 bytes of
> VLAN tag and then pads the frame to 64 bytes with zeroes if necessary
> before transmiting it.

Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
out is 68 bytes.

> I'd like to change it so it first pads the frame to 64 B and appends the
> VLAN tag afterwards, so the resulting VLAN frame would be 68 bytes
> minimum.

If you were able to figure this much out: What have you tried and what
do you still need help on? CCing some of the e1000 people(done) will
probably help you get this resolved faster.

cheers,
jamal

Re: change the way e1000 is handling short VLAN frames

[Emil Micek]

On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:
> > Current e1000 (according to our observations) first appends 4 bytes of
> > VLAN tag and then pads the frame to 64 bytes with zeroes if necessary
> > before transmiting it.
> 
> Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
> out is 68 bytes.
Are you sure about this? We did some extensive testing capturing VLAN
frames going out of our Intel PRO/1000 MT Dual Port Server Adapter
(82571EB chipset) and we definitely captured VLAN frames shorter then 68
bytes. Here is text dump of such frame:
http://www.tkrjasek.cz/mie/cmts_tests/between_linux_and_cmts.txt
(note that wireshark captures the frames without 4 bytes FCS (frame
check sequence) so the above packet is in fact 66 bytes long).

What is the right behaviour according to specification? In iee802.3,
minFrameSize is 64bytes. I've never seen any document which'd say that
VLAN frames should be 68 bytes minimum.


> > I'd like to change it so it first pads the frame to 64 B and appends the
> > VLAN tag afterwards, so the resulting VLAN frame would be 68 bytes
> > minimum.
> 
> If you were able to figure this much out: What have you tried and what
> do you still need help on? CCing some of the e1000 people(done) will
> probably help you get this resolved faster.
Well to make long story short. While testing VLAN capabilities of our C3
CMTS, we discovered that some frames are corupted after passing trough
the CMTS. We discovered, that affected frames are VLAN frames 64 - 67
bytes long. 
We tried several NIC's in our linux box mainly with e1000 driver but
also some NIC's from 3com, broadcom and Planet. All of them suffer from
this issue except NIC from Planet (RTL-8169 chipset), which pads VLAN
frames to 68 bytes. The other cards pad VLAN frames only to 64 bytes. 
We also tried to tag the traffic on some VLAN enabled switches (d-link,
edge-core, cisco catalyst) and they also pad VLAN frames to 68 bytes.

Now i'm little confused by your reply, becaouse you write, that VLAN
frames generated by e1000 should be 68 bytes minimum, which contradicts
witch our observations.

thanks for your reply
Emil

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 14:50 +0200, Emil Micek wrote:
> On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:

> > Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
> > out is 68 bytes.
> Are you sure about this? 

This is what i have always seen. Double checked with google and she gave
me many a url, example:
http://www.cisco.com/warp/public/473/741_4.html
I think you can pretty much trust cisco on something as basic as this.
Also:
http://www.techfest.com/networking/lan/ethernet2.htm

> We did some extensive testing capturing VLAN
> frames going out of our Intel PRO/1000 MT Dual Port Server Adapter
> (82571EB chipset) and we definitely captured VLAN frames shorter then 68
> bytes. Here is text dump of such frame:
> http://www.tkrjasek.cz/mie/cmts_tests/between_linux_and_cmts.txt

Do you have access to another NIC? a tg3 based one will be good to test
for hardware level tagging.

> (note that wireshark captures the frames without 4 bytes FCS (frame
> check sequence) so the above packet is in fact 66 bytes long).
> 
> What is the right behaviour according to specification? In iee802.3,
> minFrameSize is 64bytes. I've never seen any document which'd say that
> VLAN frames should be 68 bytes minimum.

Refer to above.

> Now i'm little confused by your reply, becaouse you write, that VLAN
> frames generated by e1000 should be 68 bytes minimum, which contradicts
> witch our observations.

Miscommunication ;-> I said the packets going out should be 68B
otherwise it is a bug. if e1000 is sending 64B out, it is a bug.

I think any other NIC that doesnt do hardware based vlan tags would be
good to try as well because it uses the software stack to do the
tagging.

cheers,
jamal

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 09:37 -0400, jamal wrote:
> On Fri, 2007-21-09 at 14:50 +0200, Emil Micek wrote:
> > On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:
> 
> > > Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
> > > out is 68 bytes.
> > Are you sure about this? 
> 
> This is what i have always seen. 

If the first email caused confusion - so will this ;->
I have never used e1000 for vlan tagging. My view is that if it emits
(instead of 68B) 64B packets with tags then it is buggy and needs to be
fixed. Hope that makes sense.


cheers,
jamal

Re: change the way e1000 is handling short VLAN frames

[Francesco Fondelli]

On 9/21/07, jamal <hadi@cyberus.ca> wrote:
> Hope that makes sense.
[cut]
> cheers,
> jamal

Hi all,

as far as I understand ieee docs both 64 and 68
approaches are fine...

--- Std 802.1Q-2005, 6.5.1 ---
On receipt of an M_UNITDATA.request primitive that
represents a tagged frame, the implementation is
permitted to adopt either of the following approaches
with regard to the operation of Transmit Data
Encapsulation for frames whose length would, using the
procedure as described, be less than 68 octets:
a) Use the procedure as described in 6.5.1 of IEEE
Std 802.1D. This procedure can result in
tagged frames of less than 68 octets (but at least 64 octets)
being transmitted; or
b) Include additional octets before the FCS field in order
for the transmitted frame length for such frames to be
68 octets. This procedure results in a minimum tagged
frame length of 68 octets.
When a tagged frame of less than 68 octets in length
is received on a CSMA/CD LAN segment, and is forwarded
as an untagged frame, the provisions of 6.5.1 of
IEEE Std 802.1D, result in additional octets
being included before the FCS field on transmission
in order that the transmitted frame length meets the
minimum frame size requirements of 3.2.7 in IEEE Std 802.3.
------------------------------

and

--- Std 802.1Q-2005, C.4.4.1 ---
When tagged frames are transmitted by a Bridge on an
IEEE Std 802.3 MAC, there are two permissible
approaches (7.2), as follows:
a) Keep the minimum frame size generated by the Bridge
equal to 64 octets. This implies that the
number of pad octets in a received untagged IEEE Std 802.3
frame would be reduced by up to 4 octets when that frame
was tagged;
b) Adopt a minimum tagged frame length of 68 octets.
This implies that the number of pad octets in a
received untagged IEEE Std 802.3 frame would not be adjusted
when tagging such frames; equally, if subsequently untagged,
no pad adjustment would be necessary before transmission on
IEEE 802.3/Ethernet.
------------------------------

might this problem be solved using a configurable parameter?
(default=68)

my 0.02 euro
Ciao
FF

Re: change the way e1000 is handling short VLAN frames

[Ben Greear]

jamal wrote:
> On Fri, 2007-21-09 at 09:37 -0400, jamal wrote:
>   
>> On Fri, 2007-21-09 at 14:50 +0200, Emil Micek wrote:
>>     
>>> On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:
>>>       
>>>> Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
>>>> out is 68 bytes.
>>>>         
>>> Are you sure about this? 
>>>       
>> This is what i have always seen. 
>>     
>
> If the first email caused confusion - so will this ;->
> I have never used e1000 for vlan tagging. My view is that if it emits
> (instead of 68B) 64B packets with tags then it is buggy and needs to be
> fixed. Hope that makes sense.
>   
I just re-read the spec, and a bridge *may* pad up to 68, but it is not 
required.
On page 166, it says equipment must be able to handle 64 byte minimums.

See page 22 (section 7.2) of this document:

http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf

Also, page 63, 165, 166

Thanks,
Ben

>
> cheers,
> jamal
>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   


-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 08:43 -0700, Ben Greear wrote:

> I just re-read the spec, and a bridge *may* pad up to 68, but it is not 
> required.
> On page 166, it says equipment must be able to handle 64 byte minimums.
> 
> See page 22 (section 7.2) of this document:
> 
> http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf
> 
> Also, page 63, 165, 166

Thanks for the enlightnment. 
Do we need an ethtool interface to turn off hardware accelerated vlans?
Jesse is indicating that the intel hardware can only handle the MUST but
not the SHOULD of the spec.
Actually a more basic question: Can you select one or the other mode in
the software based vlans?

cheers,
jamal

Re: change the way e1000 is handling short VLAN frames

[Ben Greear]

jamal wrote:
> On Fri, 2007-21-09 at 08:43 -0700, Ben Greear wrote:
> 
>> I just re-read the spec, and a bridge *may* pad up to 68, but it is not 
>> required.
>> On page 166, it says equipment must be able to handle 64 byte minimums.
>>
>> See page 22 (section 7.2) of this document:
>>
>> http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf
>>
>> Also, page 63, 165, 166
> 
> Thanks for the enlightnment. 
> Do we need an ethtool interface to turn off hardware accelerated vlans?
> Jesse is indicating that the intel hardware can only handle the MUST but
> not the SHOULD of the spec.
> Actually a more basic question: Can you select one or the other mode in
> the software based vlans?

It's not even clear to me that it's a Should..more like a 'May', for what
thats worth.

As for using the software stack, I don't think we are padding to 68 in
the soft vlans either...

It would be nice to have an ethtool to turn off hw vlans for other reasons,
like sniffing vlan tags, at least...

You cannot select hw v/s soft vlans...the code always uses hw-accel if it's
available, as far as I know.

Thanks,
Ben

> 
> cheers,
> jamal
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: change the way e1000 is handling short VLAN frames

[Kok, Auke]

jamal wrote:
> On Fri, 2007-21-09 at 08:43 -0700, Ben Greear wrote:
> 
>> I just re-read the spec, and a bridge *may* pad up to 68, but it is not 
>> required.
>> On page 166, it says equipment must be able to handle 64 byte minimums.
>>
>> See page 22 (section 7.2) of this document:
>>
>> http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf
>>
>> Also, page 63, 165, 166
> 
> Thanks for the enlightnment. 
> Do we need an ethtool interface to turn off hardware accelerated vlans?
> Jesse is indicating that the intel hardware can only handle the MUST but
> not the SHOULD of the spec.

I don't actually see the "SHOULD" part you are referring to. AFAIK the RX side is
fully covered and only on TX the hardware sends out 64b padded vlan packets, which
is permitted. The spec never says that it is required to send out 68b padded vlan
packets - only permits either choice.

Auke

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 14:18 -0700, Ben Greear wrote:

> As for using the software stack, I don't think we are padding to 68 in
> the soft vlans either...

Should be much easier than changing firmware or an ASIC.

> It would be nice to have an ethtool to turn off hw vlans for other reasons,
> like sniffing vlan tags, at least...

Hrm, no hardware out there will pass on tags to the stack as metadata?

On Fri, 2007-21-09 at 14:21 -0700, Kok, Auke wrote:

> I don't actually see the "SHOULD" part you are referring to. 

the "optional" part;-> MAY?

> AFAIK the RX side is fully covered 

so you can handle both 64B and 68B?

> and only on TX the hardware sends out 64b padded vlan packets, which
> is permitted. The spec never says that it is required to send out 68b padded vlan
> packets - only permits either choice.

The CISCO i pointed to seems to be only capable of 68B. So if the CISCO
sends to you, it would work, but you sending to the CISCO or to Emil's
CMTS you have challenges, no?


cheers,
jamal

Re: change the way e1000 is handling short VLAN frames

[Kok, Auke]

jamal wrote:
>> AFAIK the RX side is fully covered 
> 
> so you can handle both 64B and 68B?

I never saw any bugreports about e1000 not being able to accept vlan packets
because of this, so I'm quite certain it works OK, feel free to find me a case
where this isn't so :)

Auke

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 14:34 -0700, Kok, Auke wrote:

> I never saw any bugreports about e1000 not being able to accept vlan packets
> because of this, so I'm quite certain it works OK, feel free to find me a case
> where this isn't so :)

If you tell me it can be done on the rx, i will take your word for it;->
Emil can certainly verify it. 
The tx you certainly have issues - Look at one of the suggestions from
Chris, i think it is resolvable.

cheers,
jamal

Re: change the way e1000 is handling short VLAN frames

[Chris Leech]

On 9/21/07, jamal <hadi@cyberus.ca> wrote:
> On Fri, 2007-21-09 at 14:34 -0700, Kok, Auke wrote:
>
> > I never saw any bugreports about e1000 not being able to accept vlan packets
> > because of this, so I'm quite certain it works OK, feel free to find me a case
> > where this isn't so :)
>
> If you tell me it can be done on the rx, i will take your word for it;->
> Emil can certainly verify it.
> The tx you certainly have issues - Look at one of the suggestions from
> Chris, i think it is resolvable.

I'd say that devices that can't receive 64 bytes VLAN tagged frames
have an issue, but for the sake of interoperability and solving Emil's
problem I'm willing to discuss how a change to e1000 would work  ;-)

The simplest option is to add software small frame padding all the
time.  It won't catch software tagged frames if they were generated
somehow, but should fix the hardware tagged ones to be 68 bytes on the
wire.  If you were worried about software tagged frames then replacing
ETH_ZLEN with VLAN_ETH_ZLEN would pad all frames, VLAN or not, to 68
bytes.

Emil, this patch will probably do what you want.

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 4a22595..34e3d18 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -3284,6 +3284,9 @@ e1000_xmit_frame(struct sk_buff *skb, struct
net_device *netdev)
                return NETDEV_TX_OK;
        }

+       if (skb_padto(skb, ETH_ZLEN))
+               return NETDEV_TX_OK;
+
        /* 82571 and newer doesn't need the workaround that limited descriptor
         * length to 4kB */
        if (adapter->hw.mac_type >= e1000_82571)

Re: change the way e1000 is handling short VLAN frames

[Emil Micek]

Thanks for your replies.

We contacted Arris (manufacturer of our CMTS) about this issue with
links to relevant parts of specification about minimum size of VLAN
tagged frames and it seems they'll acknowledge the problem and fix it in
next firmware.

Meantime i tried different suggestions posted there (patch from Chris,
replacing ETH_ZLEN with VLAN_ETH_ZLEN in e1000_main.c and disabling HW
VLAN tagging in netdev->features) and combinations of them, but none of
it worked and 64B VLAN frames are still generated by our intel NIC.

I'll now consider this issue as closed on the e1000 and intel NIC side,
because there was no problem with them in the first place, and I'll
rather concentrate on resolving it on the CMTS side which doesn't act
according to specs.

Cheer

Emil.

Re: change the way e1000 is handling short VLAN frames

[Chris Leech]

On 9/21/07, jamal <hadi@cyberus.ca> wrote:
> On Fri, 2007-21-09 at 08:43 -0700, Ben Greear wrote:
>
> > I just re-read the spec, and a bridge *may* pad up to 68, but it is not
> > required.
> > On page 166, it says equipment must be able to handle 64 byte minimums.
> >
> > See page 22 (section 7.2) of this document:
> >
> > http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf
> >
> > Also, page 63, 165, 166
>
> Thanks for the enlightnment.
> Do we need an ethtool interface to turn off hardware accelerated vlans?
> Jesse is indicating that the intel hardware can only handle the MUST but
> not the SHOULD of the spec.
> Actually a more basic question: Can you select one or the other mode in
> the software based vlans?

Inserting the VLAN tag in software will not change the behavior in the
way you want anyway, short frames will still be padded to 64 bytes.
You'd have to do short packet padding in software to 68 bytes.  Or do
software padding to 64 bytes and let the hardware insert the VLAN tag
after.

Chris

Re: change the way e1000 is handling short VLAN frames

[jamal]

On Fri, 2007-21-09 at 14:27 -0700, Chris Leech wrote:

> 
> Inserting the VLAN tag in software will not change the behavior in the
> way you want anyway, short frames will still be padded to 64 bytes.
> You'd have to do short packet padding in software to 68 bytes.  Or do
> software padding to 64 bytes and let the hardware insert the VLAN tag
> after.

I like the second one because it could be done totaly in the driver and
would resolve the problem IMO. All we need after is an ethtool knob to
select between 64B or 68B txmit.

cheers,
jamal

RE: change the way e1000 is handling short VLAN frames

[Brandeburg, Jesse]

Emil Micek wrote:
> What is the right behaviour according to specification? In iee802.3,
> minFrameSize is 64bytes. I've never seen any document which'd say that
> VLAN frames should be 68 bytes minimum.

e1000 only hardware pads to 64 bytes, but if you use the vlan module and
turn off the hardware assistance of inserting tags, by unsetting
NETIF_F_HW_VLAN_* in netdev->features.
the software insertion sounds like it should work for you at the cost of
higher CPU.

>>> I'd like to change it so it first pads the frame to 64 B and
>>> appends the VLAN tag afterwards, so the resulting VLAN frame would
>>> be 68 bytes minimum.

at least for e1000 this isn't possible in the driver, while still doing
hardware vlan insertion.

Jesse