Re: [RFC][PATCH] Avoid multi-page allocs for large mtu datagram sends

Re: [RFC][PATCH] Avoid multi-page allocs for large mtu datagram sends

[Herbert Xu]

On Wed, Apr 05, 2006 at 05:51:02PM +0000, Zach Brown wrote:
> 
> 2) I changed the final-frag test to be length + fraggap as the math later on
>    seemed to match that.. is that OK?

Yes that's a real bug introduced by a previous rework.  Could you
please split that off into a separate patch?

Thanks,
-- 
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: [RFC][PATCH] Avoid multi-page allocs for large mtu datagram sends

[Zach Brown]

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

Herbert Xu wrote:
> On Wed, Apr 05, 2006 at 05:51:02PM +0000, Zach Brown wrote:
>> 2) I changed the final-frag test to be length + fraggap as the math later on
>>    seemed to match that.. is that OK?
> 
> Yes that's a real bug introduced by a previous rework.  Could you
> please split that off into a separate patch?

Sure, here it is by itself.  I lightly tested this but didn't actually
use anything that has a non-zero trailer len, I don't think.

Did you have an opinion of the rest of the original patch?

- z



[-- Attachment #2: ip-append-data-fraggap-math-0.patch --]
[-- Type: text/x-patch, Size: 837 bytes --]


[PATCH] ip_output: account for fraggap when checking to add trailer_len

During other work I noticed that ip_append_data() seemed to be forgetting to
include the frag gap in its calculation of a fragment that consumes the rest of
the payload.  Herbert confirmed that this was a bug that snuck in during a
previous rework.

Signed-off-by: Zach Brown <zach.brown@oracle.com>

Index: 2.6.17-rc1-mm2-fraggap/net/ipv4/ip_output.c
===================================================================
--- 2.6.17-rc1-mm2-fraggap.orig/net/ipv4/ip_output.c
+++ 2.6.17-rc1-mm2-fraggap/net/ipv4/ip_output.c
@@ -904,7 +904,7 @@ alloc_new_skb:
 			 * because we have no idea what fragment will be
 			 * the last.
 			 */
-			if (datalen == length)
+			if (datalen == length + fraggap)
 				alloclen += rt->u.dst.trailer_len;
 
 			if (transhdrlen) {

Re: [RFC][PATCH] Avoid multi-page allocs for large mtu datagram sends

[Herbert Xu]

On Wed, Apr 12, 2006 at 11:22:58AM -0700, Zach Brown wrote:
> 
> Did you have an opinion of the rest of the original patch?

Sorry, I've been busy lately so I've only just started reading it.
I should have something to say by tomorrow :)
-- 
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: [RFC][PATCH] Avoid multi-page allocs for large mtu datagram sends

[Herbert Xu]

Hi Zach:

On Wed, Apr 05, 2006 at 05:51:02PM +0000, Zach Brown wrote:
>
> +			if ((rt->u.dst.dev->features&NETIF_F_SG) &&
> +			    (SKB_DATA_KMALLOC_BYTES(alloclen + datalen)
> +								> PAGE_SIZE)) {
> +				/*
> +				 * datalen is shrinking so there won't be a
> +				 * trailer, but alloclen already accounted for
> +				 * it.  use that space for payload but only if
> +				 * it wasn't the trailer_len itself that pushed
> +				 * the alloc beyond a single page.
> +				 */
> +				if (datalen - SKB_MAX_ORDER(alloclen, 0) >
> +				    rt->u.dst.trailer_len)
> +					alloclen -= rt->u.dst.trailer_len;

I think this should be done unconditionally.  First of all at this
time ESP doesn't know how to deal with this anyway so it's just going
to reallocate and linearise the whole thing.  However, when we do fix
it we can just get it to allocate the trailer in its own frag so we
don't need to worry about trailer_len at all.

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