Patchwork [net-next,2/4] net: increase frag hash size

login
register
mail settings
Submitter Jesper Dangaard Brouer
Date April 24, 2013, 3:48 p.m.
Message ID <20130424154822.16883.93014.stgit@dragon>
Download mbox | patch
Permalink /patch/239241/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Jesper Dangaard Brouer - April 24, 2013, 3:48 p.m.
Increase fragmentation hash bucket size to 1024 from old 64 elems.

After we increased the frag mem limits (in commit v3.8-rc3-503-gc2a9366)
the hash size of 64 elements is simply too small.  Also considering
the mem limit is per netns and the hash table is shared for all netns.

For the embedded people, note that this increase will change the hash
table/array from using approx 1 Kbytes to 16 Kbytes.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 include/net/inet_frag.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - April 24, 2013, 10:09 p.m.
Hello.

On 24-04-2013 19:48, Jesper Dangaard Brouer wrote:

> Increase fragmentation hash bucket size to 1024 from old 64 elems.

> After we increased the frag mem limits (in commit v3.8-rc3-503-gc2a9366)

    This is not a commit ID, commit ID is SHA1 key alone. And you should also 
specify that commit's summary line with it, in parens.

> the hash size of 64 elements is simply too small.  Also considering
> the mem limit is per netns and the hash table is shared for all netns.

> For the embedded people, note that this increase will change the hash
> table/array from using approx 1 Kbytes to 16 Kbytes.

> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - April 24, 2013, 11:48 p.m.
On Wed, 2013-04-24 at 17:48 +0200, Jesper Dangaard Brouer wrote:
> Increase fragmentation hash bucket size to 1024 from old 64 elems.
> 
> After we increased the frag mem limits (in commit v3.8-rc3-503-gc2a9366)
> the hash size of 64 elements is simply too small.  Also considering
> the mem limit is per netns and the hash table is shared for all netns.
> 
> For the embedded people, note that this increase will change the hash
> table/array from using approx 1 Kbytes to 16 Kbytes.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> 
>  include/net/inet_frag.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index eb1d6ee..4e15856 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -41,7 +41,7 @@ struct inet_frag_queue {
>  	struct netns_frags	*net;
>  };
>  
> -#define INETFRAGS_HASHSZ		64
> +#define INETFRAGS_HASHSZ	1024

Acked-by: Eric Dumazet <edumazet@google.com>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Frederic Sowa - April 25, 2013, 3:26 a.m.
On Wed, Apr 24, 2013 at 05:48:31PM +0200, Jesper Dangaard Brouer wrote:
> Increase fragmentation hash bucket size to 1024 from old 64 elems.
> 
> After we increased the frag mem limits (in commit v3.8-rc3-503-gc2a9366)
> the hash size of 64 elements is simply too small.  Also considering
> the mem limit is per netns and the hash table is shared for all netns.
> 
> For the embedded people, note that this increase will change the hash
> table/array from using approx 1 Kbytes to 16 Kbytes.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

If this change goes in and patch 1 does not get applied I do think we
can even lower the INETFRAGS_MAXDEPTH a bit (altough I don't see a need
for that because of stability issues)? It would just decrease latency
a bit in case someone attacks the hash buckets directly.

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesper Dangaard Brouer - April 25, 2013, 10:13 a.m.
On Thu, 2013-04-25 at 02:09 +0400, Sergei Shtylyov wrote:
> On 24-04-2013 19:48, Jesper Dangaard Brouer wrote:
> 
> > Increase fragmentation hash bucket size to 1024 from old 64 elems.
> 
> > After we increased the frag mem limits (in commit v3.8-rc3-503-gc2a9366)
> 
>     This is not a commit ID, commit ID is SHA1 key alone. And you should also 
> specify that commit's summary line with it, in parens.

Did you know that you can use the string "v3.8-rc3-503-gc2a9366" like:
 git show v3.8-rc3-503-gc2a9366
That's why I like this kind of commit ID better, as it also tell the
reader what approx version this patch were in.

But you are right about the title in parentheses...

Dave, do you want me to resubmit this, with nitpicked commit message?

--JEsper

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - April 25, 2013, 12:13 p.m.
Hello.

On 25-04-2013 14:13, Jesper Dangaard Brouer wrote:

>>> Increase fragmentation hash bucket size to 1024 from old 64 elems.

>>> After we increased the frag mem limits (in commit v3.8-rc3-503-gc2a9366)

>>      This is not a commit ID, commit ID is SHA1 key alone. And you should also
>> specify that commit's summary line with it, in parens.

> Did you know that you can use the string "v3.8-rc3-503-gc2a9366" like:
>   git show v3.8-rc3-503-gc2a9366

    No, I didn't. I only know such string is a result of the command 'git 
describe' (if I don't mistake).

> That's why I like this kind of commit ID better, as it also tell the
> reader what approx version this patch were in.

   Understood. Everybody else is using SHA1 though for which gitweb certainly 
could generate a coimmit link if it encountered SHA1 in the changelog. Now, 
with the switch of kernel.org to cgit though, this ability seems to be gone.
That's why (in part) I don't think using cgit was such a good idea... I want 
the old gitweb back!

> --JEsper

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 25, 2013, 7:11 p.m.
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Thu, 25 Apr 2013 12:13:15 +0200

> Did you know that you can use the string "v3.8-rc3-503-gc2a9366" like:
>  git show v3.8-rc3-503-gc2a9366
> That's why I like this kind of commit ID better, as it also tell the
> reader what approx version this patch were in.

Don't invent your own conventions without discussing it with other
developers first.

Being different in style from what other people are doing hurts all
by itself, because people have to interpret and read your commit
messages differently.

> Dave, do you want me to resubmit this, with nitpicked commit message?

Of course.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index eb1d6ee..4e15856 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -41,7 +41,7 @@  struct inet_frag_queue {
 	struct netns_frags	*net;
 };
 
-#define INETFRAGS_HASHSZ		64
+#define INETFRAGS_HASHSZ	1024
 
 struct inet_frag_bucket {
 	struct hlist_head	chain;