* Re: [PATCH] udp: avoid searching when no ports are available [not found] <1298633711-11924-1-git-send-email-dbaluta@ixiacom.com> @ 2011-02-25 12:06 ` Eric Dumazet 2011-02-25 16:45 ` Daniel Baluta 0 siblings, 1 reply; 3+ messages in thread From: Eric Dumazet @ 2011-02-25 12:06 UTC (permalink / raw) To: Daniel Baluta; +Cc: netdev, davem, Rohan Chitradurga Le vendredi 25 février 2011 à 13:35 +0200, Daniel Baluta a écrit : > udp_lib_get_port uses a bitmap to mark used ports. > > When no ports are available we spend a lot of time, searching > for a port while holding hslot lock. Avoid this by checking if > bitmap is full. > > > Signed-off-by: Rohan Chitradurga <rohan@ixiacom.com> > Signed-off-by: Daniel Baluta <dbaluta@ixiacom.com> > --- > net/ipv4/udp.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index d37baaa..3e3592d 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -225,6 +225,12 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum, > udp_lib_lport_inuse(net, snum, hslot, bitmap, sk, > saddr_comp, udptable->log); > > + /* avoid searching when no ports are available */ > + if (bitmap_full(bitmap, PORTS_PER_CHAIN)) { > + spin_unlock_bh(&hslot->lock); > + break; > + } > + > snum = first; > /* > * Iterate on all possible values of snum for this hash. Really ? I wonder how you got your performance numbers then. First, PORTS_PER_CHAIN is wrong here, since its value is the max possible value (256 bits) #define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) #define MAX_UDP_PORTS 65536 #define PORTS_PER_CHAIN (MAX_UDP_PORTS / UDP_HTABLE_SIZE_MIN) -> 256 As soon as your machine (and most current machines have) has enough memory, UDP hash table size is not 256, but 1024 or 2048 dmesg | grep "UDP hash" [ 1.735203] UDP hash table entries: 2048 (order: 6, 327680 bytes) So real bitmap size is 64 or 32 bits. Your call to bitmap_full() always return false. I dont like this patch. If you have special UDP needs on a small machine, just add to kernel boot param "uhash_entries=8192", so that the bitmap has 8 bits only. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] udp: avoid searching when no ports are available 2011-02-25 12:06 ` [PATCH] udp: avoid searching when no ports are available Eric Dumazet @ 2011-02-25 16:45 ` Daniel Baluta 2011-02-25 16:55 ` Eric Dumazet 0 siblings, 1 reply; 3+ messages in thread From: Daniel Baluta @ 2011-02-25 16:45 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, davem, Rohan Chitradurga On Fri, Feb 25, 2011 at 2:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le vendredi 25 février 2011 à 13:35 +0200, Daniel Baluta a écrit : >> udp_lib_get_port uses a bitmap to mark used ports. >> >> When no ports are available we spend a lot of time, searching >> for a port while holding hslot lock. Avoid this by checking if >> bitmap is full. >> >> >> Signed-off-by: Rohan Chitradurga <rohan@ixiacom.com> >> Signed-off-by: Daniel Baluta <dbaluta@ixiacom.com> >> --- >> net/ipv4/udp.c | 6 ++++++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c >> index d37baaa..3e3592d 100644 >> --- a/net/ipv4/udp.c >> +++ b/net/ipv4/udp.c >> @@ -225,6 +225,12 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum, >> udp_lib_lport_inuse(net, snum, hslot, bitmap, sk, >> saddr_comp, udptable->log); >> >> + /* avoid searching when no ports are available */ >> + if (bitmap_full(bitmap, PORTS_PER_CHAIN)) { >> + spin_unlock_bh(&hslot->lock); >> + break; >> + } >> + >> snum = first; >> /* >> * Iterate on all possible values of snum for this hash. > > Really ? I wonder how you got your performance numbers then. > > First, PORTS_PER_CHAIN is wrong here, since its value is the max > possible value (256 bits) You are right. I have been working/testing on 2.6.32 where: #define PORTS_PER_CHAIN (65536 / UDP_HTABLE_SIZE) and I thought that the latest kernel has the same meaning for PORTS_PER_CHAIN. > > #define UDP_HTABLE_SIZE_MIN (CONFIG_BASE_SMALL ? 128 : 256) > #define MAX_UDP_PORTS 65536 > #define PORTS_PER_CHAIN (MAX_UDP_PORTS / UDP_HTABLE_SIZE_MIN) -> 256 > > As soon as your machine (and most current machines have) has enough > memory, UDP hash table size is not 256, but 1024 or 2048 > > dmesg | grep "UDP hash" > [ 1.735203] UDP hash table entries: 2048 (order: 6, 327680 bytes) > > So real bitmap size is 64 or 32 bits. > > Your call to bitmap_full() always return false. I guess now, the correct bitmap size is MAX_UDP_PORTS / (udptable->mask + 1) or MAX_UDP_PORTS >> udptable->log, right? > I dont like this patch. If you have special UDP needs on a small > machine, just add to kernel boot param "uhash_entries=8192", so that the > bitmap has 8 bits only. We don't have special needs on a small machine. We just want that when when all UDP ports are exhausted, bind calls to fail faster. I will be back with tests on latest kernel. thanks, Daniel. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] udp: avoid searching when no ports are available 2011-02-25 16:45 ` Daniel Baluta @ 2011-02-25 16:55 ` Eric Dumazet 0 siblings, 0 replies; 3+ messages in thread From: Eric Dumazet @ 2011-02-25 16:55 UTC (permalink / raw) To: Daniel Baluta; +Cc: netdev, davem, Rohan Chitradurga Le vendredi 25 février 2011 à 18:45 +0200, Daniel Baluta a écrit : > I guess now, the correct bitmap size is MAX_UDP_PORTS / (udptable->mask + 1) > or MAX_UDP_PORTS >> udptable->log, right? > Yes, but using bitmap_zero(bitmap, PORTS_PER_CHAIN) is faster. It generates 4 machine instructions, movq $0x0,(%r10) movq $0x0,0x8(%r10) movq $0x0,0x10(%r10) movq $0x0,0x18(%r10) while bitmap_zero(bitmap, some_non_constant_expression) is more expensive (it calls an out of line function) > We don't have special needs on a small machine. We just want that when > when all UDP ports are exhausted, bind calls to fail faster. > > I will be back with tests on latest kernel. Hmm, please always use the latest kernel before sending patches. Thanks ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-25 16:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1298633711-11924-1-git-send-email-dbaluta@ixiacom.com>
2011-02-25 12:06 ` [PATCH] udp: avoid searching when no ports are available Eric Dumazet
2011-02-25 16:45 ` Daniel Baluta
2011-02-25 16:55 ` Eric Dumazet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox