From: Hannes Frederic Sowa <hannes@stressinduktion.org>
To: Benjamin Poirier <bpoirier@suse.com>, netdev@vger.kernel.org
Cc: Daniel Borkmann <daniel@iogearbox.net>,
Eric Dumazet <eric.dumazet@gmail.com>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Subject: Re: [PATCH net v2] mld, igmp: Fix reserved tailroom calculation
Date: Tue, 1 Mar 2016 11:09:53 +0100 [thread overview]
Message-ID: <56D56A71.8050400@stressinduktion.org> (raw)
In-Reply-To: <1456787013-3277-1-git-send-email-bpoirier@suse.com>
On 01.03.2016 00:03, Benjamin Poirier wrote:
> The current reserved_tailroom calculation fails to take hlen and tlen into
> account.
>
> skb:
> [__hlen__|__data____________|__tlen___|__extra__]
> ^ ^
> head skb_end_offset
>
> In this representation, hlen + data + tlen is the size passed to alloc_skb.
> "extra" is the extra space made available in __alloc_skb because of
> rounding up by kmalloc. We can reorder the representation like so:
>
> [__hlen__|__data____________|__extra__|__tlen___]
> ^ ^
> head skb_end_offset
>
> The maximum space available for ip headers and payload without
> fragmentation is min(mtu, data + extra). Therefore,
> reserved_tailroom
> = data + extra + tlen - min(mtu, data + extra)
> = skb_end_offset - hlen - min(mtu, skb_end_offset - hlen - tlen)
> = skb_tailroom - min(mtu, skb_tailroom - tlen) ; after skb_reserve(hlen)
>
> Compare the second line to the current expression:
> reserved_tailroom = skb_end_offset - min(mtu, skb_end_offset)
> and we can see that hlen and tlen are not taken into account.
>
> The min() in the third line can be expanded into:
> if mtu < skb_tailroom - tlen:
> reserved_tailroom = skb_tailroom - mtu
> else:
> reserved_tailroom = tlen
>
> Depending on hlen, tlen, mtu and the number of multicast address records,
> the current code may output skbs that have less tailroom than
> dev->needed_tailroom or it may output more skbs than needed because not all
> space available is used.
>
> Fixes: 4c672e4b ("ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs")
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
I like it, thanks a lot!
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
next prev parent reply other threads:[~2016-03-01 10:09 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-27 19:57 [PATCH] mld, igmp: Fix reserved tailroom calculation Benjamin Poirier
2016-02-29 14:57 ` Daniel Borkmann
2016-02-29 15:19 ` Benjamin Poirier
2016-02-29 15:38 ` Daniel Borkmann
2016-02-29 15:43 ` Hannes Frederic Sowa
2016-02-29 18:08 ` Benjamin Poirier
2016-02-29 18:28 ` Hannes Frederic Sowa
2016-02-29 23:03 ` [PATCH net v2] " Benjamin Poirier
2016-03-01 10:09 ` Hannes Frederic Sowa [this message]
2016-03-01 10:18 ` Daniel Borkmann
2016-03-01 16:00 ` Hannes Frederic Sowa
2016-03-03 20:42 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56D56A71.8050400@stressinduktion.org \
--to=hannes@stressinduktion.org \
--cc=bpoirier@suse.com \
--cc=daniel@iogearbox.net \
--cc=eric.dumazet@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.