LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Netfilter Development Mailinglist
<netfilter-devel@vger.kernel.org>,
clameter@sgi.com, joe@perches.com, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH] netfilter: replace horrible hack with ksize()
Date: Mon, 10 Mar 2008 18:57:55 +0100 [thread overview]
Message-ID: <47D576A3.1080503@trash.net> (raw)
In-Reply-To: <47D01277.9060807@cs.helsinki.fi>
Pekka Enberg wrote:
> Hi Patrick,
>
> Patrick McHardy wrote:
>> > I think you are misunderstanding ksize() (see mm/slub.c::ksize() for
>> > example).
>>
>> The ksize() description in mm/slab.c matches exactly what netfilter
>> wants to do:
>
> Agreed.
>
> Patrick McHardy wrote:
>> The initial allocation size is calculated as max(size, min slab size)
>> and is stored as ext->alloc_size. When adding the first extension,
>
> Yes, this part is correct, however...
>
>> it allocates ext->alloc_size of memory and stores both the real amount
>> of space used (ext->len) and the actual size (ext->real_len).
>> When adding further extensions, it calculates the new total amount of
>> space needed (newlen). If that is larger than the real amount of
>> memory allocated (real_len), it reallocates.
>
> ...looking at nf_ct_ext_create() you do:
>
> *ext = kzalloc(real_len, gfp);
> ^^^^^^^^
> if (!*ext)
> return NULL;
>
> (*ext)->offset[id] = off;
> (*ext)->len = len;
> (*ext)->real_len = real_len;
> ^^^^^^^^
>
> You are storing the _object size_ (total amount of memory requested) and
> not the _buffer size_ (total amount of memory allocated). Keep in mind
> that object size < buffer size and that ksize() returns the latter.
For all length <= minimum slab size alloc_size (and thus
real_len) is equal to the buffer size. You are correct
however that your patch is fine, I somehow misread the
+ if (newlen >= ksize(ct->ext)) {
part and thought you would always compare against the
minimum slab size.
I've queued your patch and will pass it upstream after
some testing, thanks.
next prev parent reply other threads:[~2008-03-10 17:58 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-05 21:20 Pekka J Enberg
2008-03-05 21:56 ` Christoph Lameter
2008-03-05 21:57 ` Pekka Enberg
2008-03-05 22:19 ` David Miller
2008-03-06 14:03 ` Patrick McHardy
2008-03-06 14:14 ` Pekka J Enberg
2008-03-06 14:20 ` Pekka J Enberg
2008-03-06 14:35 ` Patrick McHardy
2008-03-06 14:41 ` Pekka J Enberg
2008-03-06 14:51 ` Patrick McHardy
2008-03-06 15:49 ` Pekka Enberg
2008-03-10 17:57 ` Patrick McHardy [this message]
2008-03-06 16:41 ` Christoph Lameter
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=47D576A3.1080503@trash.net \
--to=kaber@trash.net \
--cc=clameter@sgi.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=penberg@cs.helsinki.fi \
--subject='Re: [PATCH] netfilter: replace horrible hack with ksize()' \
/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
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).