Re: + myri10ge-build-fix.patch added to -mm tree

Re: + myri10ge-build-fix.patch added to -mm tree

[Herbert Xu]

On Tue, Jun 20, 2006 at 09:33:42PM -0700, akpm@osdl.org wrote:
>
> diff -puN drivers/net/myri10ge/myri10ge.c~myri10ge-build-fix drivers/net/myri10ge/myri10ge.c
> --- a/drivers/net/myri10ge/myri10ge.c~myri10ge-build-fix
> +++ a/drivers/net/myri10ge/myri10ge.c
> @@ -2120,7 +2120,7 @@ abort_linearize:
>  		goto drop;
>  	}
>  
> -	if (skb_linearize(skb, GFP_ATOMIC))
> +	if (skb_linearize(skb))
>  		goto drop;

Actually I think this driver (in conjunction with TSO) is buggy.
It has a maximum segment count of 12.  However TSO can send 13 pages
or more of data on i386.  If this happens it looks like this driver
will go into an infinite loop forever trying to linearize the skb.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: + myri10ge-build-fix.patch added to -mm tree

[Brice Goglin]

Herbert Xu wrote:
> On Tue, Jun 20, 2006 at 09:33:42PM -0700, akpm@osdl.org wrote:
>> diff -puN drivers/net/myri10ge/myri10ge.c~myri10ge-build-fix
>> drivers/net/myri10ge/myri10ge.c
>> --- a/drivers/net/myri10ge/myri10ge.c~myri10ge-build-fix
>> +++ a/drivers/net/myri10ge/myri10ge.c
>> @@ -2120,7 +2120,7 @@ abort_linearize:
>> goto drop;
>> }
>>
>> - if (skb_linearize(skb, GFP_ATOMIC))
>> + if (skb_linearize(skb))
>> goto drop;
>
> Actually I think this driver (in conjunction with TSO) is buggy.
> It has a maximum segment count of 12. However TSO can send 13 pages
> or more of data on i386. If this happens it looks like this driver
> will go into an infinite loop forever trying to linearize the skb.
>
> Cheers,

Actually, we use 64 segments when TSO in enabled
(MYRI10GE_MAX_SEND_DESC_TSO):

#define MYRI10GE_MAX_SEND_DESC_TSO ((65536 / 2048) * 2)

<.....>

    max_segments = MXGEFW_MAX_SEND_DESC;

#ifdef NETIF_F_TSO
if (skb->len > (dev->mtu + ETH_HLEN)) {
    mss = skb_shinfo(skb)->tso_size;
    if (mss != 0)
        max_segments = MYRI10GE_MAX_SEND_DESC_TSO;
}
#endif /*NETIF_F_TSO */


And we never try to linearize a TSO frame:

if (skb_shinfo(skb)->tso_size) {
    printk(KERN_ERR "myri10ge: %s: TSO but wanted to linearize?!?!?\n",
        mgp->dev->name);
    goto drop;
}

Brice

Re: + myri10ge-build-fix.patch added to -mm tree

[Herbert Xu]

On Wed, Jun 21, 2006 at 12:09:51PM +0000, Brice Goglin wrote:
> 
> Actually, we use 64 segments when TSO in enabled
> (MYRI10GE_MAX_SEND_DESC_TSO):
> 
> #define MYRI10GE_MAX_SEND_DESC_TSO ((65536 / 2048) * 2)

You're right, I missed that bit.  Sorry about the noise.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt