* [PATCH] net/ipv4/proc: Avoid usage for seq_printf() when reading /proc/net/snmp
@ 2024-11-11 4:56 David Wang
2024-11-14 9:30 ` Paolo Abeni
0 siblings, 1 reply; 4+ messages in thread
From: David Wang @ 2024-11-11 4:56 UTC (permalink / raw)
To: davem, dsahern, edumazet, kuba, pabeni; +Cc: netdev, linux-kernel, David Wang
seq_printf() is costy, when reading /proc/net/snmp, profiling indicates
seq_printf() takes more than 50% samples of snmp_seq_show():
snmp_seq_show(97.751% 158722/162373)
snmp_seq_show_tcp_udp.isra.0(40.017% 63515/158722)
seq_printf(83.451% 53004/63515)
seq_write(1.170% 743/63515)
_find_next_bit(0.727% 462/63515)
...
seq_printf(24.762% 39303/158722)
snmp_seq_show_ipstats.isra.0(21.487% 34104/158722)
seq_printf(85.788% 29257/34104)
_find_next_bit(0.331% 113/34104)
seq_write(0.235% 80/34104)
...
icmpmsg_put(7.235% 11483/158722)
seq_printf(41.714% 4790/11483)
seq_write(2.630% 302/11483)
...
Time for a million rounds of stress reading /proc/net/snmp:
real 0m24.323s
user 0m0.293s
sys 0m23.679s
On average, reading /proc/net/snmp takes 0.023ms.
With this patch, extra costs of seq_printf() is avoided, and a million
rounds of reading /proc/net/snmp now takes only ~15.853s:
real 0m16.386s
user 0m0.280s
sys 0m15.853s
On average, one read takes 0.015ms, a ~40% improvement.
Signed-off-by: David Wang <00107082@163.com>
---
net/ipv4/proc.c | 116 ++++++++++++++++++++++++++++--------------------
1 file changed, 69 insertions(+), 47 deletions(-)
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 40053a02bae1..3a4586103b5e 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -315,13 +315,14 @@ static void icmpmsg_put_line(struct seq_file *seq, unsigned long *vals,
if (count) {
seq_puts(seq, "\nIcmpMsg:");
- for (j = 0; j < count; ++j)
- seq_printf(seq, " %sType%u",
- type[j] & 0x100 ? "Out" : "In",
- type[j] & 0xff);
+ for (j = 0; j < count; ++j) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, type[j] & 0x100 ? "Out" : "In");
+ seq_put_decimal_ull(seq, "Type", type[j] & 0xff);
+ }
seq_puts(seq, "\nIcmpMsg:");
for (j = 0; j < count; ++j)
- seq_printf(seq, " %lu", vals[j]);
+ seq_put_decimal_ull(seq, " ", vals[j]);
}
}
@@ -358,26 +359,35 @@ static void icmp_put(struct seq_file *seq)
atomic_long_t *ptr = net->mib.icmpmsg_statistics->mibs;
seq_puts(seq, "\nIcmp: InMsgs InErrors InCsumErrors");
- for (i = 0; icmpmibmap[i].name; i++)
- seq_printf(seq, " In%s", icmpmibmap[i].name);
+ for (i = 0; icmpmibmap[i].name; i++) {
+ seq_puts(seq, " In");
+ seq_puts(seq, icmpmibmap[i].name);
+ }
seq_puts(seq, " OutMsgs OutErrors OutRateLimitGlobal OutRateLimitHost");
+ for (i = 0; icmpmibmap[i].name; i++) {
+ seq_puts(seq, " Out");
+ seq_puts(seq, icmpmibmap[i].name);
+ }
+ seq_puts(seq, "\nIcmp:");
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_INMSGS));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_INERRORS));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_CSUMERRORS));
for (i = 0; icmpmibmap[i].name; i++)
- seq_printf(seq, " Out%s", icmpmibmap[i].name);
- seq_printf(seq, "\nIcmp: %lu %lu %lu",
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_INMSGS),
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_INERRORS),
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_CSUMERRORS));
+ seq_put_decimal_ull(seq, " ", atomic_long_read(ptr + icmpmibmap[i].index));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_OUTMSGS));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_OUTERRORS));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_RATELIMITGLOBAL));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_RATELIMITHOST));
for (i = 0; icmpmibmap[i].name; i++)
- seq_printf(seq, " %lu",
- atomic_long_read(ptr + icmpmibmap[i].index));
- seq_printf(seq, " %lu %lu %lu %lu",
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_OUTMSGS),
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_OUTERRORS),
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_RATELIMITGLOBAL),
- snmp_fold_field(net->mib.icmp_statistics, ICMP_MIB_RATELIMITHOST));
- for (i = 0; icmpmibmap[i].name; i++)
- seq_printf(seq, " %lu",
- atomic_long_read(ptr + (icmpmibmap[i].index | 0x100)));
+ seq_put_decimal_ull(seq, " ",
+ atomic_long_read(ptr + (icmpmibmap[i].index | 0x100)));
}
/*
@@ -392,8 +402,10 @@ static int snmp_seq_show_ipstats(struct seq_file *seq, void *v)
memset(buff64, 0, IPSTATS_MIB_MAX * sizeof(u64));
seq_puts(seq, "Ip: Forwarding DefaultTTL");
- for (i = 0; snmp4_ipstats_list[i].name; i++)
- seq_printf(seq, " %s", snmp4_ipstats_list[i].name);
+ for (i = 0; snmp4_ipstats_list[i].name; i++) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, snmp4_ipstats_list[i].name);
+ }
seq_printf(seq, "\nIp: %d %d",
IPV4_DEVCONF_ALL_RO(net, FORWARDING) ? 1 : 2,
@@ -404,7 +416,7 @@ static int snmp_seq_show_ipstats(struct seq_file *seq, void *v)
net->mib.ip_statistics,
offsetof(struct ipstats_mib, syncp));
for (i = 0; snmp4_ipstats_list[i].name; i++)
- seq_printf(seq, " %llu", buff64[i]);
+ seq_put_decimal_ull(seq, " ", buff64[i]);
return 0;
}
@@ -418,8 +430,10 @@ static int snmp_seq_show_tcp_udp(struct seq_file *seq, void *v)
memset(buff, 0, TCPUDP_MIB_MAX * sizeof(unsigned long));
seq_puts(seq, "\nTcp:");
- for (i = 0; snmp4_tcp_list[i].name; i++)
- seq_printf(seq, " %s", snmp4_tcp_list[i].name);
+ for (i = 0; snmp4_tcp_list[i].name; i++) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, snmp4_tcp_list[i].name);
+ }
seq_puts(seq, "\nTcp:");
snmp_get_cpu_field_batch(buff, snmp4_tcp_list,
@@ -429,7 +443,7 @@ static int snmp_seq_show_tcp_udp(struct seq_file *seq, void *v)
if (snmp4_tcp_list[i].entry == TCP_MIB_MAXCONN)
seq_printf(seq, " %ld", buff[i]);
else
- seq_printf(seq, " %lu", buff[i]);
+ seq_put_decimal_ull(seq, " ", buff[i]);
}
memset(buff, 0, TCPUDP_MIB_MAX * sizeof(unsigned long));
@@ -437,11 +451,13 @@ static int snmp_seq_show_tcp_udp(struct seq_file *seq, void *v)
snmp_get_cpu_field_batch(buff, snmp4_udp_list,
net->mib.udp_statistics);
seq_puts(seq, "\nUdp:");
- for (i = 0; snmp4_udp_list[i].name; i++)
- seq_printf(seq, " %s", snmp4_udp_list[i].name);
+ for (i = 0; snmp4_udp_list[i].name; i++) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, snmp4_udp_list[i].name);
+ }
seq_puts(seq, "\nUdp:");
for (i = 0; snmp4_udp_list[i].name; i++)
- seq_printf(seq, " %lu", buff[i]);
+ seq_put_decimal_ull(seq, " ", buff[i]);
memset(buff, 0, TCPUDP_MIB_MAX * sizeof(unsigned long));
@@ -449,11 +465,13 @@ static int snmp_seq_show_tcp_udp(struct seq_file *seq, void *v)
seq_puts(seq, "\nUdpLite:");
snmp_get_cpu_field_batch(buff, snmp4_udp_list,
net->mib.udplite_statistics);
- for (i = 0; snmp4_udp_list[i].name; i++)
- seq_printf(seq, " %s", snmp4_udp_list[i].name);
+ for (i = 0; snmp4_udp_list[i].name; i++) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, snmp4_udp_list[i].name);
+ }
seq_puts(seq, "\nUdpLite:");
for (i = 0; snmp4_udp_list[i].name; i++)
- seq_printf(seq, " %lu", buff[i]);
+ seq_put_decimal_ull(seq, " ", buff[i]);
seq_putc(seq, '\n');
return 0;
@@ -483,8 +501,10 @@ static int netstat_seq_show(struct seq_file *seq, void *v)
int i;
seq_puts(seq, "TcpExt:");
- for (i = 0; i < tcp_cnt; i++)
- seq_printf(seq, " %s", snmp4_net_list[i].name);
+ for (i = 0; i < tcp_cnt; i++) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, snmp4_net_list[i].name);
+ }
seq_puts(seq, "\nTcpExt:");
buff = kzalloc(max(tcp_cnt * sizeof(long), ip_cnt * sizeof(u64)),
@@ -493,16 +513,18 @@ static int netstat_seq_show(struct seq_file *seq, void *v)
snmp_get_cpu_field_batch(buff, snmp4_net_list,
net->mib.net_statistics);
for (i = 0; i < tcp_cnt; i++)
- seq_printf(seq, " %lu", buff[i]);
+ seq_put_decimal_ull(seq, " ", buff[i]);
} else {
for (i = 0; i < tcp_cnt; i++)
- seq_printf(seq, " %lu",
- snmp_fold_field(net->mib.net_statistics,
- snmp4_net_list[i].entry));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field(net->mib.net_statistics,
+ snmp4_net_list[i].entry));
}
seq_puts(seq, "\nIpExt:");
- for (i = 0; i < ip_cnt; i++)
- seq_printf(seq, " %s", snmp4_ipextstats_list[i].name);
+ for (i = 0; i < ip_cnt; i++) {
+ seq_putc(seq, ' ');
+ seq_puts(seq, snmp4_ipextstats_list[i].name);
+ }
seq_puts(seq, "\nIpExt:");
if (buff) {
@@ -513,13 +535,13 @@ static int netstat_seq_show(struct seq_file *seq, void *v)
net->mib.ip_statistics,
offsetof(struct ipstats_mib, syncp));
for (i = 0; i < ip_cnt; i++)
- seq_printf(seq, " %llu", buff64[i]);
+ seq_put_decimal_ull(seq, " ", buff64[i]);
} else {
for (i = 0; i < ip_cnt; i++)
- seq_printf(seq, " %llu",
- snmp_fold_field64(net->mib.ip_statistics,
- snmp4_ipextstats_list[i].entry,
- offsetof(struct ipstats_mib, syncp)));
+ seq_put_decimal_ull(seq, " ",
+ snmp_fold_field64(net->mib.ip_statistics,
+ snmp4_ipextstats_list[i].entry,
+ offsetof(struct ipstats_mib, syncp)));
}
kfree(buff);
seq_putc(seq, '\n');
--
2.39.2
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] net/ipv4/proc: Avoid usage for seq_printf() when reading /proc/net/snmp
2024-11-11 4:56 [PATCH] net/ipv4/proc: Avoid usage for seq_printf() when reading /proc/net/snmp David Wang
@ 2024-11-14 9:30 ` Paolo Abeni
2024-11-14 10:19 ` David Wang
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2024-11-14 9:30 UTC (permalink / raw)
To: David Wang, davem, dsahern, edumazet, kuba; +Cc: netdev, linux-kernel
On 11/11/24 05:56, David Wang wrote:
> seq_printf() is costy, when reading /proc/net/snmp, profiling indicates
> seq_printf() takes more than 50% samples of snmp_seq_show():
> snmp_seq_show(97.751% 158722/162373)
> snmp_seq_show_tcp_udp.isra.0(40.017% 63515/158722)
> seq_printf(83.451% 53004/63515)
> seq_write(1.170% 743/63515)
> _find_next_bit(0.727% 462/63515)
> ...
> seq_printf(24.762% 39303/158722)
> snmp_seq_show_ipstats.isra.0(21.487% 34104/158722)
> seq_printf(85.788% 29257/34104)
> _find_next_bit(0.331% 113/34104)
> seq_write(0.235% 80/34104)
> ...
> icmpmsg_put(7.235% 11483/158722)
> seq_printf(41.714% 4790/11483)
> seq_write(2.630% 302/11483)
> ...
> Time for a million rounds of stress reading /proc/net/snmp:
> real 0m24.323s
> user 0m0.293s
> sys 0m23.679s
> On average, reading /proc/net/snmp takes 0.023ms.
> With this patch, extra costs of seq_printf() is avoided, and a million
> rounds of reading /proc/net/snmp now takes only ~15.853s:
> real 0m16.386s
> user 0m0.280s
> sys 0m15.853s
> On average, one read takes 0.015ms, a ~40% improvement.
>
> Signed-off-by: David Wang <00107082@163.com>
If the user space is really concerned with snmp access performances, I
think such information should be exposed via netlink.
Still the goal of the optimization looks doubtful. The total number of
mibs domain is constant and limited (differently from the network
devices number that in specific setup can grow a lot). Stats polling
should be a low frequency operation. Why you need to optimize it?
I don't think we should accept this change, too. And a solid explanation
should be need to introduce a netlink MIB interface.
> ---
> net/ipv4/proc.c | 116 ++++++++++++++++++++++++++++--------------------
FTR you missed mptcp.
/P
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/ipv4/proc: Avoid usage for seq_printf() when reading /proc/net/snmp
2024-11-14 9:30 ` Paolo Abeni
@ 2024-11-14 10:19 ` David Wang
2024-11-14 11:16 ` Paolo Abeni
0 siblings, 1 reply; 4+ messages in thread
From: David Wang @ 2024-11-14 10:19 UTC (permalink / raw)
To: Paolo Abeni; +Cc: davem, dsahern, edumazet, kuba, netdev, linux-kernel
At 2024-11-14 17:30:34, "Paolo Abeni" <pabeni@redhat.com> wrote:
>On 11/11/24 05:56, David Wang wrote:
>> seq_printf() is costy, when reading /proc/net/snmp, profiling indicates
>> seq_printf() takes more than 50% samples of snmp_seq_show():
>> snmp_seq_show(97.751% 158722/162373)
>> snmp_seq_show_tcp_udp.isra.0(40.017% 63515/158722)
>> seq_printf(83.451% 53004/63515)
>> seq_write(1.170% 743/63515)
>> _find_next_bit(0.727% 462/63515)
>> ...
>> seq_printf(24.762% 39303/158722)
>> snmp_seq_show_ipstats.isra.0(21.487% 34104/158722)
>> seq_printf(85.788% 29257/34104)
>> _find_next_bit(0.331% 113/34104)
>> seq_write(0.235% 80/34104)
>> ...
>> icmpmsg_put(7.235% 11483/158722)
>> seq_printf(41.714% 4790/11483)
>> seq_write(2.630% 302/11483)
>> ...
>> Time for a million rounds of stress reading /proc/net/snmp:
>> real 0m24.323s
>> user 0m0.293s
>> sys 0m23.679s
>> On average, reading /proc/net/snmp takes 0.023ms.
>> With this patch, extra costs of seq_printf() is avoided, and a million
>> rounds of reading /proc/net/snmp now takes only ~15.853s:
>> real 0m16.386s
>> user 0m0.280s
>> sys 0m15.853s
>> On average, one read takes 0.015ms, a ~40% improvement.
>>
>> Signed-off-by: David Wang <00107082@163.com>
>
>If the user space is really concerned with snmp access performances, I
>think such information should be exposed via netlink.
>
>Still the goal of the optimization looks doubtful. The total number of
>mibs domain is constant and limited (differently from the network
>devices number that in specific setup can grow a lot). Stats polling
>should be a low frequency operation. Why you need to optimize it?
Well, one thing I think worth mention, optimize /proc entries can help
increase sample frequency, hence more accurate rate analysis,
for monitoring tools with a fixed/limited cpu quota.
And for /proc/net/*, the optimization would be amplified when considering network namespaces.
I think it worth to optimize.
>
>I don't think we should accept this change, too. And a solid explanation
>should be need to introduce a netlink MIB interface.
>
>> ---
>> net/ipv4/proc.c | 116 ++++++++++++++++++++++++++++--------------------
>
>FTR you missed mptcp.
>
>/P
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/ipv4/proc: Avoid usage for seq_printf() when reading /proc/net/snmp
2024-11-14 10:19 ` David Wang
@ 2024-11-14 11:16 ` Paolo Abeni
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2024-11-14 11:16 UTC (permalink / raw)
To: David Wang; +Cc: davem, dsahern, edumazet, kuba, netdev, linux-kernel
On 11/14/24 11:19, David Wang wrote:
> At 2024-11-14 17:30:34, "Paolo Abeni" <pabeni@redhat.com> wrote:
>> On 11/11/24 05:56, David Wang wrote:
>>> seq_printf() is costy, when reading /proc/net/snmp, profiling indicates
>>> seq_printf() takes more than 50% samples of snmp_seq_show():
>>> snmp_seq_show(97.751% 158722/162373)
>>> snmp_seq_show_tcp_udp.isra.0(40.017% 63515/158722)
>>> seq_printf(83.451% 53004/63515)
>>> seq_write(1.170% 743/63515)
>>> _find_next_bit(0.727% 462/63515)
>>> ...
>>> seq_printf(24.762% 39303/158722)
>>> snmp_seq_show_ipstats.isra.0(21.487% 34104/158722)
>>> seq_printf(85.788% 29257/34104)
>>> _find_next_bit(0.331% 113/34104)
>>> seq_write(0.235% 80/34104)
>>> ...
>>> icmpmsg_put(7.235% 11483/158722)
>>> seq_printf(41.714% 4790/11483)
>>> seq_write(2.630% 302/11483)
>>> ...
>>> Time for a million rounds of stress reading /proc/net/snmp:
>>> real 0m24.323s
>>> user 0m0.293s
>>> sys 0m23.679s
>>> On average, reading /proc/net/snmp takes 0.023ms.
>>> With this patch, extra costs of seq_printf() is avoided, and a million
>>> rounds of reading /proc/net/snmp now takes only ~15.853s:
>>> real 0m16.386s
>>> user 0m0.280s
>>> sys 0m15.853s
>>> On average, one read takes 0.015ms, a ~40% improvement.
>>>
>>> Signed-off-by: David Wang <00107082@163.com>
>>
>> If the user space is really concerned with snmp access performances, I
>> think such information should be exposed via netlink.
>>
>> Still the goal of the optimization looks doubtful. The total number of
>> mibs domain is constant and limited (differently from the network
>> devices number that in specific setup can grow a lot). Stats polling
>> should be a low frequency operation. Why you need to optimize it?
>
> Well, one thing I think worth mention, optimize /proc entries can help
> increase sample frequency, hence more accurate rate analysis,
> for monitoring tools with a fixed/limited cpu quota.
>
> And for /proc/net/*, the optimization would be amplified when considering network namespaces.
I guess scaling better with many namespace could be useful.
Please try to implement this interface over netlink.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-11-14 11:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-11 4:56 [PATCH] net/ipv4/proc: Avoid usage for seq_printf() when reading /proc/net/snmp David Wang
2024-11-14 9:30 ` Paolo Abeni
2024-11-14 10:19 ` David Wang
2024-11-14 11:16 ` Paolo Abeni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®