* [PATCH net-next] ndisc: Remove unused space at tail of skb for ndisc messages. (TAKE 3)
@ 2013-01-04 13:58 YOSHIFUJI Hideaki
2013-01-04 16:00 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: YOSHIFUJI Hideaki @ 2013-01-04 13:58 UTC (permalink / raw)
To: David Miller, Eric Dumazet, netdev, YOSHIFUJI Hideaki
Currently, the size of skb allocated for NDISC is MAX_HEADER +
LL_RESERVED_SPACE(dev) + packet length + dev->needed_tailroom,
but only LL_RESERVED_SPACE(dev) bytes is "reserved" for headers.
As a result, the skb looks like this (after construction of the
message):
head data tail end
+--------------------------------------------------------------+
+ | | | |
+--------------------------------------------------------------+
|<-hlen---->|<---ipv6 packet------>|<--tlen-->|<--MAX_HEADER-->|
=LL_ = dev
RESERVED_ ->needed_
SPACE(dev) tailroom
As the name implies, "MAX_HEADER" is used for headers, and should
be "reserved" in prior to packet construction. Or, if some space
is really required at the tail of ther skb, it should be
explicitly documented.
We have several option after construction of NDISC message:
Option 1:
head data tail end
+---------------------------------------------+
+ | | |
+---------------------------------------------+
|<-hlen---->|<---ipv6 packet------>|<--tlen-->|
=LL_ = dev
RESERVED_ ->needed_
SPACE(dev) tailroom
Option 2:
head data tail end
+--------------------------------------------------+
+ | | |
+--------------------------------------------------+
|<--MAX_HEADER-->|<---ipv6 packet------>|<--tlen-->|
= dev
->needed_
tailroom
Option 3:
head data tail end
+--------------------------------------------------------------+
+ | | | |
+--------------------------------------------------------------+
|<--MAX_HEADER-->|<-hlen---->|<---ipv6 packet------>|<--tlen-->|
=LL_ = dev
RESERVED_ ->needed_
SPACE(dev) tailroom
Our tunnel drivers try expanding headroom and the space for tunnel
encapsulation was not a mandatory space -- so we are not seeing
bugs here --, but just for optimization for performance critial
situations.
Since NDISC messages are not performance critical unlike TCP,
and as we know outgoing device, LL_RESERVED_SPACE(dev) should be
just enough for the device in most (if not all) cases:
LL_RESERVED_SPACE(dev) <= LL_MAX_HEADER <= MAX_HEADER
Note that LL_RESERVED_SPACE(dev) is also enough for NDISC over
SIT (e.g., ISATAP).
So, I think Option 1 is just fine here.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
net/ipv6/ndisc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 6574175..4c4ccf7 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -395,7 +395,7 @@ static struct sk_buff *ndisc_build_skb(struct net_device *dev,
len += ndisc_opt_addr_space(dev);
skb = sock_alloc_send_skb(sk,
- (MAX_HEADER + sizeof(struct ipv6hdr) +
+ (sizeof(struct ipv6hdr) +
len + hlen + tlen),
1, &err);
if (!skb) {
@@ -1439,7 +1439,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
hlen = LL_RESERVED_SPACE(dev);
tlen = dev->needed_tailroom;
buff = sock_alloc_send_skb(sk,
- (MAX_HEADER + sizeof(struct ipv6hdr) +
+ (sizeof(struct ipv6hdr) +
len + hlen + tlen),
1, &err);
if (buff == NULL) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] ndisc: Remove unused space at tail of skb for ndisc messages. (TAKE 3)
2013-01-04 13:58 [PATCH net-next] ndisc: Remove unused space at tail of skb for ndisc messages. (TAKE 3) YOSHIFUJI Hideaki
@ 2013-01-04 16:00 ` Eric Dumazet
2013-01-04 23:17 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2013-01-04 16:00 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: David Miller, netdev
On Fri, 2013-01-04 at 22:58 +0900, YOSHIFUJI Hideaki wrote:
> Currently, the size of skb allocated for NDISC is MAX_HEADER +
> LL_RESERVED_SPACE(dev) + packet length + dev->needed_tailroom,
> but only LL_RESERVED_SPACE(dev) bytes is "reserved" for headers.
> As a result, the skb looks like this (after construction of the
> message):
>
> head data tail end
> +--------------------------------------------------------------+
> + | | | |
> +--------------------------------------------------------------+
> |<-hlen---->|<---ipv6 packet------>|<--tlen-->|<--MAX_HEADER-->|
> =LL_ = dev
> RESERVED_ ->needed_
> SPACE(dev) tailroom
>
> As the name implies, "MAX_HEADER" is used for headers, and should
> be "reserved" in prior to packet construction. Or, if some space
> is really required at the tail of ther skb, it should be
> explicitly documented.
>
> We have several option after construction of NDISC message:
>
> Option 1:
>
> head data tail end
> +---------------------------------------------+
> + | | |
> +---------------------------------------------+
> |<-hlen---->|<---ipv6 packet------>|<--tlen-->|
> =LL_ = dev
> RESERVED_ ->needed_
> SPACE(dev) tailroom
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] ndisc: Remove unused space at tail of skb for ndisc messages. (TAKE 3)
2013-01-04 16:00 ` Eric Dumazet
@ 2013-01-04 23:17 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2013-01-04 23:17 UTC (permalink / raw)
To: eric.dumazet; +Cc: yoshfuji, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 04 Jan 2013 08:00:19 -0800
> On Fri, 2013-01-04 at 22:58 +0900, YOSHIFUJI Hideaki wrote:
>> Currently, the size of skb allocated for NDISC is MAX_HEADER +
>> LL_RESERVED_SPACE(dev) + packet length + dev->needed_tailroom,
>> but only LL_RESERVED_SPACE(dev) bytes is "reserved" for headers.
>> As a result, the skb looks like this (after construction of the
>> message):
>>
>> head data tail end
>> +--------------------------------------------------------------+
>> + | | | |
>> +--------------------------------------------------------------+
>> |<-hlen---->|<---ipv6 packet------>|<--tlen-->|<--MAX_HEADER-->|
>> =LL_ = dev
>> RESERVED_ ->needed_
>> SPACE(dev) tailroom
>>
>> As the name implies, "MAX_HEADER" is used for headers, and should
>> be "reserved" in prior to packet construction. Or, if some space
>> is really required at the tail of ther skb, it should be
>> explicitly documented.
>>
>> We have several option after construction of NDISC message:
>>
>> Option 1:
>>
>> head data tail end
>> +---------------------------------------------+
>> + | | |
>> +---------------------------------------------+
>> |<-hlen---->|<---ipv6 packet------>|<--tlen-->|
>> =LL_ = dev
>> RESERVED_ ->needed_
>> SPACE(dev) tailroom
>
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied, thanks for your persistence.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-04 23:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-04 13:58 [PATCH net-next] ndisc: Remove unused space at tail of skb for ndisc messages. (TAKE 3) YOSHIFUJI Hideaki
2013-01-04 16:00 ` Eric Dumazet
2013-01-04 23:17 ` 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).