[PATCH] sky2: jumbo frame regression fix

[PATCH] sky2: jumbo frame regression fix

[Stephen Hemminger]

Remove unneeded check that caused problems with jumbo frame sizes.
The check was recently added and is wrong.
When using jumbo frames the sky2 driver does fragmentation, so
rx_data_size is less than mtu.

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/sky2.c	2007-10-02 17:56:31.000000000 -0700
+++ b/drivers/net/sky2.c	2007-10-02 17:58:56.000000000 -0700
@@ -2163,9 +2163,6 @@ static struct sk_buff *sky2_receive(stru
 	sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
 	prefetch(sky2->rx_ring + sky2->rx_next);
 
-	if (length < ETH_ZLEN || length > sky2->rx_data_size)
-		goto len_error;
-
 	/* This chip has hardware problems that generates bogus status.
 	 * So do only marginal checking and expect higher level protocols
 	 * to handle crap frames.

Re: [PATCH] sky2: jumbo frame regression fix

[Jeff Garzik]

Stephen Hemminger wrote:
> Remove unneeded check that caused problems with jumbo frame sizes.
> The check was recently added and is wrong.
> When using jumbo frames the sky2 driver does fragmentation, so
> rx_data_size is less than mtu.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> --- a/drivers/net/sky2.c	2007-10-02 17:56:31.000000000 -0700
> +++ b/drivers/net/sky2.c	2007-10-02 17:58:56.000000000 -0700
> @@ -2163,9 +2163,6 @@ static struct sk_buff *sky2_receive(stru
>  	sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
>  	prefetch(sky2->rx_ring + sky2->rx_next);
>  
> -	if (length < ETH_ZLEN || length > sky2->rx_data_size)
> -		goto len_error;
> -

2.6.23?  2.6.24?  enquiring minds want to know...

Re: [PATCH] sky2: jumbo frame regression fix

[Stephen Hemminger]

On Tue, 02 Oct 2007 21:07:22 -0400
Jeff Garzik <jgarzik@pobox.com> wrote:

> Stephen Hemminger wrote:
> > Remove unneeded check that caused problems with jumbo frame sizes.
> > The check was recently added and is wrong.
> > When using jumbo frames the sky2 driver does fragmentation, so
> > rx_data_size is less than mtu.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> > 
> > --- a/drivers/net/sky2.c	2007-10-02 17:56:31.000000000 -0700
> > +++ b/drivers/net/sky2.c	2007-10-02 17:58:56.000000000 -0700
> > @@ -2163,9 +2163,6 @@ static struct sk_buff *sky2_receive(stru
> >  	sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
> >  	prefetch(sky2->rx_ring + sky2->rx_next);
> >  
> > -	if (length < ETH_ZLEN || length > sky2->rx_data_size)
> > -		goto len_error;
> > -
> 
> 2.6.23?  2.6.24?  enquiring minds want to know...

2.6.23, since it is a regression

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] sky2: jumbo frame regression fix

[Jeff Garzik]

Stephen Hemminger wrote:
> On Tue, 02 Oct 2007 21:07:22 -0400
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>> Stephen Hemminger wrote:
>>> Remove unneeded check that caused problems with jumbo frame sizes.
>>> The check was recently added and is wrong.
>>> When using jumbo frames the sky2 driver does fragmentation, so
>>> rx_data_size is less than mtu.
>>>
>>> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>>>
>>> --- a/drivers/net/sky2.c	2007-10-02 17:56:31.000000000 -0700
>>> +++ b/drivers/net/sky2.c	2007-10-02 17:58:56.000000000 -0700
>>> @@ -2163,9 +2163,6 @@ static struct sk_buff *sky2_receive(stru
>>>  	sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
>>>  	prefetch(sky2->rx_ring + sky2->rx_next);
>>>  
>>> -	if (length < ETH_ZLEN || length > sky2->rx_data_size)
>>> -		goto len_error;
>>> -
>> 2.6.23?  2.6.24?  enquiring minds want to know...
> 
> 2.6.23, since it is a regression

You can have regressions in behavior in net-2.6.24.git, too.  _Please_ 
be specific about where you want your patches to go.  Thanks.

	Jeff

Re: [PATCH] sky2: jumbo frame regression fix

[Ian Kumlien]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> Remove unneeded check that caused problems with jumbo frame sizes.
> The check was recently added and is wrong.
> When using jumbo frames the sky2 driver does fragmentation, so
> rx_data_size is less than mtu.

Confirmed working.

Now running with 9k mtu with no errors, =)

It also seems that the FIFO bug was the one that affected me before,
damn odd race that one.

> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
Tested-by: Ian Kumlien <pomac@vapor.com>

(if that tag exists now)

Btw, Sorry but all mail directly to you will be blocked. I have yet to
fix the relaying properly with isp:s blocking port 25 etc so for some of
you this mail will only show up on the ML.

> --- a/drivers/net/sky2.c	2007-10-02 17:56:31.000000000 -0700
> +++ b/drivers/net/sky2.c	2007-10-02 17:58:56.000000000 -0700
> @@ -2163,9 +2163,6 @@ static struct sk_buff *sky2_receive(stru
>  	sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
>  	prefetch(sky2->rx_ring + sky2->rx_next);
>  
> -	if (length < ETH_ZLEN || length > sky2->rx_data_size)
> -		goto len_error;
> -
>  	/* This chip has hardware problems that generates bogus status.
>  	 * So do only marginal checking and expect higher level protocols
>  	 * to handle crap frames.
-- 
Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] sky2: jumbo frame regression fix

[Stephen Hemminger]

On Wed, 03 Oct 2007 03:34:34 +0200
Ian Kumlien <pomac@vapor.com> wrote:

> On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> > Remove unneeded check that caused problems with jumbo frame sizes.
> > The check was recently added and is wrong.
> > When using jumbo frames the sky2 driver does fragmentation, so
> > rx_data_size is less than mtu.
> 
> Confirmed working.
> 
> Now running with 9k mtu with no errors, =)
> 
> It also seems that the FIFO bug was the one that affected me before,
> damn odd race that one.

Does the workaround (forced reset work). Ian, you are the first person to
report triggering it.  I haven't found a way to make it happen.
What combination of flow control and speeds are you using?


-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH] sky2: jumbo frame regression fix

[Ian Kumlien]

[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]

On tis, 2007-10-02 at 21:59 -0700, Stephen Hemminger wrote:
> On Wed, 03 Oct 2007 03:34:34 +0200
> Ian Kumlien <pomac@vapor.com> wrote:
> 
> > On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> > > Remove unneeded check that caused problems with jumbo frame sizes.
> > > The check was recently added and is wrong.
> > > When using jumbo frames the sky2 driver does fragmentation, so
> > > rx_data_size is less than mtu.
> > 
> > Confirmed working.
> > 
> > Now running with 9k mtu with no errors, =)
> > 
> > It also seems that the FIFO bug was the one that affected me before,
> > damn odd race that one.
> 
> Does the workaround (forced reset work). Ian, you are the first person to
> report triggering it.  I haven't found a way to make it happen.
> What combination of flow control and speeds are you using?

Yes it works, it's the problem i had all along =)

As to how to make it happen thats a bit harder...
To me it seems like it's a combination of several connections and
somewhat high bandwidth but you have to send data for it to happen...

To me it usually happens when seeding files via Bittorrent, but it seems
like it has to be somewhat special circumstances to actually trigger it.

I use jumbo frames, my lan is gigabit, to my firewall. From the firewall
it's common 1500 mtu 100mbit and i doubt that this has anything to do
with it (if it's not a 'number of frames that can be stored' problem and
thus the mtu limits it to a really small value making it easier to
trigger)

Well, thats my thoughts atleast but then i just got up after having
slept 5 hours, so =)

-- 
Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] sky2: jumbo frame regression fix

[iank]

On Tue, Oct 02, 2007 at 09:59:14PM -0700, Stephen Hemminger wrote:
> On Wed, 03 Oct 2007 03:34:34 +0200
> Ian Kumlien <pomac@vapor.com> wrote:
> 
> > On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> > > Remove unneeded check that caused problems with jumbo frame sizes.
> > > The check was recently added and is wrong.
> > > When using jumbo frames the sky2 driver does fragmentation, so
> > > rx_data_size is less than mtu.
> > 
> > Confirmed working.
> > 
> > Now running with 9k mtu with no errors, =)
> > 
> > It also seems that the FIFO bug was the one that affected me before,
> > damn odd race that one.
> 
> Does the workaround (forced reset work). Ian, you are the first person to
> report triggering it.  I haven't found a way to make it happen.
> What combination of flow control and speeds are you using?

I forgot to add, last time was -rc8-git2 or 3 and using Westwood flow
control.

> -- 
> Stephen Hemminger <shemminger@linux-foundation.org>

[SKY2] Problems (2.6.24-rc3-git1)

[Ian Kumlien]

[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]

Hi,

A little while ago, something went horribly wrong.

I could still use my mouse and the desktop was still alive more or
less... everything using networking was dead AND the keyboard was
dead... So i composed commands using existing text on the screen.

The device:
sky2 0000:02:00.0: v1.20 addr 0xdbffc000 irq 17 Yukon-EC (0xb6) rev 2
sky2 0000:02:00.0: PCI Express Advanced Error Reporting not configured or MMCONFIG problem?
sky2 0000:02:00.0: No interrupt generated using MSI, switching to INTx mode.
sky2 eth0: addr 00:15:f2:aa:8b:3e

From dmesg:
sky2 eth0: hung mac 124:39 fifo 195 (185:180)
sky2 eth0: receiver hang detected
sky2 eth0: disabling interface
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442
NETDEV WATCHDOG: eth0: transmit timed out
sky2 eth0: tx timeout
sky2 eth0: transmit ring 442 .. 461 report=442 done=442

And it continues until i press the reset button.


-- 
Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] sky2: jumbo frame regression fix

[Bill Davidsen]

Ian Kumlien wrote:
> On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
>> Remove unneeded check that caused problems with jumbo frame sizes.
>> The check was recently added and is wrong.
>> When using jumbo frames the sky2 driver does fragmentation, so
>> rx_data_size is less than mtu.
> 
> Confirmed working.
> 
> Now running with 9k mtu with no errors, =)

Have you verified that you are actually getting jumbo packets out of the 
NIC? I had one machine which did standard packets silently using sky2 
and jumbo using sk98lin. I was looking for something else with tcpdump 
and got one of those WTF moments when I saw all the tiny packets.

-- 
Bill Davidsen <davidsen@tmr.com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot

Re: [PATCH] sky2: jumbo frame regression fix

[Ian Kumlien]

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

On ons, 2007-10-03 at 14:04 -0400, Bill Davidsen wrote:
> Ian Kumlien wrote:
> > On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> >> Remove unneeded check that caused problems with jumbo frame sizes.
> >> The check was recently added and is wrong.
> >> When using jumbo frames the sky2 driver does fragmentation, so
> >> rx_data_size is less than mtu.
> > 
> > Confirmed working.
> > 
> > Now running with 9k mtu with no errors, =)
> 
> Have you verified that you are actually getting jumbo packets out of the 
> NIC? I had one machine which did standard packets silently using sky2 
> and jumbo using sk98lin. I was looking for something else with tcpdump 
> and got one of those WTF moments when I saw all the tiny packets.

20:27:06.542461 IP pi.local > blue.local: ICMP echo request, id 27173, seq 42, length 8008
20:27:06.543136 IP blue.local > pi.local: ICMP echo reply, id 27173, seq 42, length 8008

That should solve it for us, right? =)

-- 
Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] sky2: jumbo frame regression fix

[Jeff Garzik]

Stephen Hemminger wrote:
> Remove unneeded check that caused problems with jumbo frame sizes.
> The check was recently added and is wrong.
> When using jumbo frames the sky2 driver does fragmentation, so
> rx_data_size is less than mtu.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

applied