netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ip_gre: include route header_len in max_headroom calculation
@ 2010-03-20 12:27 Timo Teras
  2010-03-20 12:37 ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Timo Teras @ 2010-03-20 12:27 UTC (permalink / raw)
  To: netdev; +Cc: Timo Teras, Herbert Xu

Taking route's header_len into account, and updating gre device
needed_headroom will give better hints on upper bound of required
headroom. This is useful if the gre traffic is xfrm'ed.

Signed-off-by: Timo Teras <timo.teras@iki.fi>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
 net/ipv4/ip_gre.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

This was earlier discussed in netdev thread:
  http://marc.info/?t=124470870200004&r=1&w=2

I just never got to writing the patch until now.

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index f47c9f7..f78402d 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -810,11 +810,13 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
 			tunnel->err_count = 0;
 	}
 
-	max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen;
+	max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen + rt->u.dst.header_len;
 
 	if (skb_headroom(skb) < max_headroom || skb_shared(skb)||
 	    (skb_cloned(skb) && !skb_clone_writable(skb, 0))) {
 		struct sk_buff *new_skb = skb_realloc_headroom(skb, max_headroom);
+		if (max_headroom > dev->needed_headroom)
+			dev->needed_headroom = max_headroom;
 		if (!new_skb) {
 			ip_rt_put(rt);
 			txq->tx_dropped++;
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ip_gre: include route header_len in max_headroom calculation
  2010-03-20 12:27 [PATCH] ip_gre: include route header_len in max_headroom calculation Timo Teras
@ 2010-03-20 12:37 ` Herbert Xu
  2010-03-20 12:43   ` Timo Teräs
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2010-03-20 12:37 UTC (permalink / raw)
  To: Timo Teras; +Cc: netdev

On Sat, Mar 20, 2010 at 02:27:58PM +0200, Timo Teras wrote:
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index f47c9f7..f78402d 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -810,11 +810,13 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
>  			tunnel->err_count = 0;
>  	}
>  
> -	max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen;
> +	max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen + rt->u.dst.header_len;
>  
>  	if (skb_headroom(skb) < max_headroom || skb_shared(skb)||
>  	    (skb_cloned(skb) && !skb_clone_writable(skb, 0))) {
>  		struct sk_buff *new_skb = skb_realloc_headroom(skb, max_headroom);
> +		if (max_headroom > dev->needed_headroom)
> +			dev->needed_headroom = max_headroom;

Are you sure about this? LL_RESERVED_SPACE already includes
dev->needed_headroom so won't this get bigger each time?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ip_gre: include route header_len in max_headroom calculation
  2010-03-20 12:37 ` Herbert Xu
@ 2010-03-20 12:43   ` Timo Teräs
  2010-03-20 14:47     ` Timo Teräs
  0 siblings, 1 reply; 6+ messages in thread
From: Timo Teräs @ 2010-03-20 12:43 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev

Herbert Xu wrote:
> On Sat, Mar 20, 2010 at 02:27:58PM +0200, Timo Teras wrote:
>> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> index f47c9f7..f78402d 100644
>> --- a/net/ipv4/ip_gre.c
>> +++ b/net/ipv4/ip_gre.c
>> @@ -810,11 +810,13 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
>>  			tunnel->err_count = 0;
>>  	}
>>  
>> -	max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen;
>> +	max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen + rt->u.dst.header_len;
>>  
>>  	if (skb_headroom(skb) < max_headroom || skb_shared(skb)||
>>  	    (skb_cloned(skb) && !skb_clone_writable(skb, 0))) {
>>  		struct sk_buff *new_skb = skb_realloc_headroom(skb, max_headroom);
>> +		if (max_headroom > dev->needed_headroom)
>> +			dev->needed_headroom = max_headroom;
> 
> Are you sure about this? LL_RESERVED_SPACE already includes
> dev->needed_headroom so won't this get bigger each time?

Whoops. Must've been after-midnight issue. And not noticed since the
max_headroom won't change many times.

dev->needed_headroom should be compared against gre_hlen+rt->u.dst.header_len.
I'll send a fixed patch in a jiffy.

- timo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ip_gre: include route header_len in max_headroom calculation
  2010-03-20 12:43   ` Timo Teräs
@ 2010-03-20 14:47     ` Timo Teräs
  2010-03-20 15:00       ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Timo Teräs @ 2010-03-20 14:47 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev

Timo Teräs wrote:
> Herbert Xu wrote:
>> On Sat, Mar 20, 2010 at 02:27:58PM +0200, Timo Teras wrote:
>>> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>>> index f47c9f7..f78402d 100644
>>> --- a/net/ipv4/ip_gre.c
>>> +++ b/net/ipv4/ip_gre.c
>>> @@ -810,11 +810,13 @@ static netdev_tx_t ipgre_tunnel_xmit(struct 
>>> sk_buff *skb, struct net_device *dev
>>>              tunnel->err_count = 0;
>>>      }
>>>  
>>> -    max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen;
>>> +    max_headroom = LL_RESERVED_SPACE(tdev) + gre_hlen + 
>>> rt->u.dst.header_len;
>>>  
>>>      if (skb_headroom(skb) < max_headroom || skb_shared(skb)||
>>>          (skb_cloned(skb) && !skb_clone_writable(skb, 0))) {
>>>          struct sk_buff *new_skb = skb_realloc_headroom(skb, 
>>> max_headroom);
>>> +        if (max_headroom > dev->needed_headroom)
>>> +            dev->needed_headroom = max_headroom;
>>
>> Are you sure about this? LL_RESERVED_SPACE already includes
>> dev->needed_headroom so won't this get bigger each time?
> 
> Whoops. Must've been after-midnight issue. And not noticed since the
> max_headroom won't change many times.
> 
> dev->needed_headroom should be compared against 
> gre_hlen+rt->u.dst.header_len.
> I'll send a fixed patch in a jiffy.

Actually, isn't the above right?

max_headroom is calculated with LL_RESERVED_SPACE of the tdev, which
is the interface to which the gre packet is being sent to, not the
gre interface. Thus, max_headroom won't include gre devices
previous needed_headroom.

- Timo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ip_gre: include route header_len in max_headroom calculation
  2010-03-20 14:47     ` Timo Teräs
@ 2010-03-20 15:00       ` Herbert Xu
  2010-03-22  4:26         ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2010-03-20 15:00 UTC (permalink / raw)
  To: Timo Teräs; +Cc: netdev

On Sat, Mar 20, 2010 at 04:47:28PM +0200, Timo Teräs wrote:
>
> Actually, isn't the above right?
>
> max_headroom is calculated with LL_RESERVED_SPACE of the tdev, which
> is the interface to which the gre packet is being sent to, not the
> gre interface. Thus, max_headroom won't include gre devices
> previous needed_headroom.

Indeed you're right.  Sorry for the confusion.

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ip_gre: include route header_len in max_headroom calculation
  2010-03-20 15:00       ` Herbert Xu
@ 2010-03-22  4:26         ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2010-03-22  4:26 UTC (permalink / raw)
  To: herbert; +Cc: timo.teras, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sat, 20 Mar 2010 23:00:49 +0800

> On Sat, Mar 20, 2010 at 04:47:28PM +0200, Timo Teräs wrote:
>>
>> Actually, isn't the above right?
>>
>> max_headroom is calculated with LL_RESERVED_SPACE of the tdev, which
>> is the interface to which the gre packet is being sent to, not the
>> gre interface. Thus, max_headroom won't include gre devices
>> previous needed_headroom.
> 
> Indeed you're right.  Sorry for the confusion.
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks everyone.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-03-22  4:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-20 12:27 [PATCH] ip_gre: include route header_len in max_headroom calculation Timo Teras
2010-03-20 12:37 ` Herbert Xu
2010-03-20 12:43   ` Timo Teräs
2010-03-20 14:47     ` Timo Teräs
2010-03-20 15:00       ` Herbert Xu
2010-03-22  4:26         ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).