inet: add bound ports statistic
Subsystems (3)
PATCH 1/1 inet: add bound ports statistic
This adds a number of bound ports in a network namespace which is a useful for socket summary (ss) command. It adds one additional field onto /proc/net/sockstat.
Since collecting these kind of counters can be sensitive for large machines, the impact is placed on the reading side which will be much rarer.
--- include/net/inet_connection_sock.h | 2 +- net/ipv4/inet_connection_sock.c | 22 ++++++++++++++++++++++ net/ipv4/proc.c | 5 +++-- 3 files changed, 26 insertions(+), 3 deletions(-)
include/net/inet_connection_sock.h +1 -1
@@ -265,7 +265,7 @@ inet_csk_rto_backoff(const struct inet_connection_sock *icsk, }struct sock *inet_csk_accept (struct sock *sk,int flags,int *err,bool kern); - +unsigned int inet_csk_count_ports (struct net *net,struct proto *prot);int inet_csk_get_port (struct sock *sk,unsigned short snum); struct dst_entry *inet_csk_route_req(const struct sock *sk, struct flowi4 *fl4,
net/ipv4/inet_connection_sock.c +22 -0
@@ -424,6 +424,28 @@ static int inet_csk_wait_for_connect(struct sock *sk, long timeo)return err; } +/* Count how many any entries are in the bind hash table */ +unsigned int inet_csk_count_ports (struct net *net,struct proto *prot) +{ +struct inet_hashinfo *hinfo = prot->h.hashinfo; +struct inet_bind_hashbucket *head; +struct inet_bind_bucket *tb; +unsigned int i, ports =0 ; + +for (i =0 ; i < hinfo->bhash_size; i++) { + head = &hinfo->bhash[i]; + +spin_lock_bh (&head->lock); +inet_bind_bucket_for_each (tb, &head->chain) { +if (net_eq (ib_net (tb), net)) + ++ports; + } +spin_unlock_bh (&head->lock); + } + +return ports; +} +/* * This will accept the next outstanding connection. */
net/ipv4/proc.c +3 -2
@@ -61,10 +61,11 @@ static int sockstat_seq_show(struct seq_file *seq, void *v) sockets =proto_sockets_allocated_sum_positive (&tcp_prot);socket_seq_show (seq); -seq_printf (seq,"TCP: inuse %d orphan %d tw %d alloc %d mem %ld\n" , +seq_printf (seq,"TCP: inuse %d orphan %d tw %d alloc %d mem %ld ports %u\n" ,sock_prot_inuse_get (net, &tcp_prot), orphans,atomic_read (&net->ipv4.tcp_death_row.tw_count), sockets, -proto_memory_allocated (&tcp_prot)); +proto_memory_allocated (&tcp_prot), +inet_csk_count_ports (net, &tcp_prot));seq_printf (seq,"UDP: inuse %d mem %ld\n" ,sock_prot_inuse_get (net, &udp_prot),proto_memory_allocated (&udp_prot));
Signature · 1 line
2.16.1
Quoted 33 lines · Stephen Hemminger
This adds a number of bound ports in a network namespace which is a useful for socket summary (ss) command. It adds one additional field onto
/proc/net/sockstat.Since collecting these kind of counters can be sensitive for large machines, the impact is placed on the reading side which will be much rarer.
Signed-off-by Stephen Hemminger <[email protected]>--- include/net/inet_connection_sock.h | 2 +- net/ipv4/inet_connection_sock.c | 22 ++++++++++++++++++++++ net/ipv4/proc.c | 5 +++-- 3 files changed, 26 insertions(+), 3 deletions(-)include/net/inet_connection_sock.h +1
-1@@ -265,7 +265,7 @@ inet_csk_rto_backoff(const struct inet_connection_sock *icsk, }struct sock *inet_csk_accept (struct sock *sk,int flags,int *err,bool kern); - +unsigned int inet_csk_count_ports (struct net *net,struct proto *prot);int inet_csk_get_port (struct sock *sk,unsigned short snum); struct dst_entry *inet_csk_route_req(const struct sock *sk, struct flowi4 *fl4,net/ipv4/inet_connection_sock.c +0
-0
net/ipv4/inet_connection_sock.c static int inet_csk_wait_for_connect(struct sock *sk, long timeo) · line 427
Quoted 4 lines
net/ipv4/inet_connection_sock.c +1
-0@@ -424,6 +424,28 @@ static int inet_csk_wait_for_connect(struct sock *sk, long timeo)return err; } +/* Count how many any entries are in the bind hash table */
How useful it is to report this information ?
Given REUSEADDR and REUSEPORT, I really wonder what can be derived from this counter.
It seems its semantic is weak.
David Miller
maintainerFrom: Eric Dumazet <[email protected]>
Date: Wed, 28 Feb 2018 18:28:02 -0800
Quoted 6 lines
How useful it is to report this information ?
Given REUSEADDR and REUSEPORT, I really wonder what can be derived from this counter.
It seems its semantic is weak.
To me none of this really matters.
What matters is that iproute2 reported this via slabinfo for longer than a decade.
It broke recently when SLAB started merging caches just like SLUB always did.
So alternative to Stephen's patch is turning off SLAB cache merging for certain networking caches, which is definitely a non-starter.
I have directed Stephen to handle the situation in this way.
Quoted 10 lines · David Miller
From: Eric Dumazet <[email protected]>
Date: Wed, 28 Feb 2018 18:28:02 -0800Quoted 6 lines
How useful it is to report this information ?
Given REUSEADDR and REUSEPORT, I really wonder what can be derived from this counter.
It seems its semantic is weak.
Quoted 7 lines
To me none of this really matters.
What matters is that iproute2 reported this via slabinfo for longer than a decade.
It broke recently when SLAB started merging caches just like SLUB always did.
Linus himself removed some info that was much more useful in commit a5ad88ce8c7fae7d ("mm: get rid of 'vmalloc_info' from /proc/meminfo")
# egrep "VmallocUsed|VmallocChunk" /proc/meminfo VmallocUsed: 0 kB VmallocChunk: 0 kB
So I vote for not re-adding another loop in the kernel with no preemption point.
Simply taking spinlocks like Stephen did is going to slow down the other threads, lets face it.
This implementation has a high cost, and provides something that made no sense in the first place.
Quoted 28 lines · On Wed, 28 Feb 2018 20:28:15 -0800 Eric Dumazet <[email protected]> wrote:
Quoted 10 lines · David Miller
From: Eric Dumazet <[email protected]>
Date: Wed, 28 Feb 2018 18:28:02 -0800Quoted 6 lines
How useful it is to report this information ?
Given REUSEADDR and REUSEPORT, I really wonder what can be derived from this counter.
It seems its semantic is weak.
Quoted 7 lines
To me none of this really matters.
What matters is that iproute2 reported this via slabinfo for longer than a decade.
It broke recently when SLAB started merging caches just like SLUB always did.
Linus himself removed some info that was much more useful in commit
a5ad88ce8c7fae7d("mm: get rid of 'vmalloc_info' from/proc/meminfo")# egrep "VmallocUsed|VmallocChunk" /proc/meminfo VmallocUsed: 0 kB VmallocChunk: 0 kB
Quoted 9 lines
So I vote for not re-adding another loop in the kernel with no preemption point.
Simply taking spinlocks like Stephen did is going to slow down the other threads, lets face it.
This implementation has a high cost, and provides something that made no sense in the first place.
I went through a several possible alternatives.
1. Add a counter in the hash bucket head (like listen already has). But not namespace aware
2. Add a percpu counter in network namespace (new struct tcp_netns) Logical and adds place to move tcp open sockets as well. But more expensive and several places in code don't have easy access to namespace.
3. Counting entries in userspace; defeats the purpose of -s flag.
Agree it is not an urgent statistic, it is just it got broken; willing to just drop it.
What about adding cond_resched between buckets like other places do?
Quoted 8 lines · Stephen Hemminger
I went through a several possible alternatives.
1. Add a counter in the hash bucket head (like listen already has). But not namespace aware
2. Add a percpu counter in network namespace (new structtcp_netns) Logical and adds place to move tcp open sockets as well. But more expensive and several places in code don't have easy access to namespace.
__this_cpu_inc() should not be very expensive.
We always have access to socket namespace when inserting/deleting a bind entry.
Since we take the spinlock at that times, inserting __this_cpu_inc(sock_net(sk)->ipv4.bind_counter) would add few instructions.
Quoted 4 lines
3. Counting entries in userspace; defeats the purpose of -s flag.
Agree it is not an urgent statistic, it is just it got broken; willing to just drop it.
Not counting the fact that it has really no purpose.
Quoted 2 lines
What about adding
cond_reschedbetween buckets like other places do?
This still is dirtying at least 1MB of memory and performs ~64K atomic operations.
Waste of cpu caches and memory bus.
If we consider converting the whole /proc/net/socketstat to provide a netns view only, we might use a percpu structure holding all TCP socket counters, to make sure no more than one cache line per cpu is used while folding all the cpus counters.