diff mbox series

[net-next,RFC] net: skip printing "link become ready" v6 msg

Message ID 20230601-net-next-skip_print_link_becomes_ready-v1-1-c13e64c14095@tessares.net (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series [net-next,RFC] net: skip printing "link become ready" v6 msg | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 4376 this patch: 4376
netdev/cc_maintainers warning 4 maintainers not CCed: maheshb@google.com maze@google.com shuah@kernel.org linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 976 this patch: 976
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4596 this patch: 4596
netdev/checkpatch warning WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Matthieu Baerts June 1, 2023, 2:34 p.m. UTC
This following message is printed in the console each time a network
device configured with an IPv6 addresses is ready to be used:

  ADDRCONF(NETDEV_CHANGE): <iface>: link becomes ready

When netns are being extensively used -- e.g. by re-creating netns with
veth to discuss with each other for testing purposes like mptcp_join.sh
selftest does -- it generates a lot of messages: more than 700 when
executing mptcp_join.sh with the latest version.

=========
== RFC ==
=========

TL;DR: can we move this message to the debug level? Or is it better with
a sysctl knob? Or something else?

When looking at commit 3c21edbd1137 ("[IPV6]: Defer IPv6 device
initialization until the link becomes ready.") which introduces this new
message, it seems it had been added to verify that the new feature was
working as expected. It could have then used a lower level than "info".

It is unclear if this message can be useful. Maybe it can be used as a
sign to know if there is something wrong, e.g. if a device is being
regularly reconfigured by accident? But even then, I don't think that
was its goal at the first place and clearly there are better ways to
monitor and diagnose such issues. Do you see any usages?

If this message is not that useful, it is probably better to simply
lower its level, similar to commit 7c62b8dd5ca8 ("net/ipv6: lower the
level of "link is not ready" messages"). If we can take this direction,
we will just need to switch from pr_info() to pr_debug().

If this message can be useful in many situations, it would be good to
have a way to turn it off because in some other situations, it floods
the logs without providing any useful input. The proposition here is to
have a new per netns sysctl knob to easily skip this specific message
when needed. If we prefer to take this direction, we will still need to
document the new knob and the modification in the MPTCP selftest should
be done in a separated commit.

Adding a new sysctl entry just for that seems a bit "heavy", maybe there
are better ways that are still easy to put in place?

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 include/net/netns/ipv6.h                        | 1 +
 net/ipv6/addrconf.c                             | 5 +++--
 net/ipv6/sysctl_net_ipv6.c                      | 9 +++++++++
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 +
 4 files changed, 14 insertions(+), 2 deletions(-)


---
base-commit: 6f4b98147b8dfcabacb19b5c6abd087af66d0049
change-id: 20230601-net-next-skip_print_link_becomes_ready-5bc2655daa24

Best regards,

Comments

Stephen Hemminger June 1, 2023, 5:37 p.m. UTC | #1
On Thu, 01 Jun 2023 16:34:36 +0200
Matthieu Baerts <matthieu.baerts@tessares.net> wrote:

> This following message is printed in the console each time a network
> device configured with an IPv6 addresses is ready to be used:
> 
>   ADDRCONF(NETDEV_CHANGE): <iface>: link becomes ready
> 
> When netns are being extensively used -- e.g. by re-creating netns with
> veth to discuss with each other for testing purposes like mptcp_join.sh
> selftest does -- it generates a lot of messages: more than 700 when
> executing mptcp_join.sh with the latest version.

Don't add yet another network nerd knob.
Just change message from pr_info to pr_debug.
David Ahern June 1, 2023, 5:43 p.m. UTC | #2
On 6/1/23 11:37 AM, Stephen Hemminger wrote:
> On Thu, 01 Jun 2023 16:34:36 +0200
> Matthieu Baerts <matthieu.baerts@tessares.net> wrote:
> 
>> This following message is printed in the console each time a network
>> device configured with an IPv6 addresses is ready to be used:
>>
>>   ADDRCONF(NETDEV_CHANGE): <iface>: link becomes ready
>>
>> When netns are being extensively used -- e.g. by re-creating netns with
>> veth to discuss with each other for testing purposes like mptcp_join.sh
>> selftest does -- it generates a lot of messages: more than 700 when
>> executing mptcp_join.sh with the latest version.
> 
> Don't add yet another network nerd knob.
> Just change message from pr_info to pr_debug.

+1
Matthieu Baerts June 2, 2023, 8:42 a.m. UTC | #3
Hi Stephen, David,

On 01/06/2023 19:43, David Ahern wrote:
> On 6/1/23 11:37 AM, Stephen Hemminger wrote:
>> On Thu, 01 Jun 2023 16:34:36 +0200
>> Matthieu Baerts <matthieu.baerts@tessares.net> wrote:
>>
>>> This following message is printed in the console each time a network
>>> device configured with an IPv6 addresses is ready to be used:
>>>
>>>   ADDRCONF(NETDEV_CHANGE): <iface>: link becomes ready
>>>
>>> When netns are being extensively used -- e.g. by re-creating netns with
>>> veth to discuss with each other for testing purposes like mptcp_join.sh
>>> selftest does -- it generates a lot of messages: more than 700 when
>>> executing mptcp_join.sh with the latest version.
>>
>> Don't add yet another network nerd knob.
>> Just change message from pr_info to pr_debug.
> 
> +1

Thank you both for your quick replies! I will send a patch doing that.

Cheers,
Matt
diff mbox series

Patch

diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 3cceb3e9320b..721abf86052f 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -56,6 +56,7 @@  struct netns_sysctl_ipv6 {
 	bool skip_notify_on_dev_down;
 	u8 fib_notify_on_flag_change;
 	u8 icmpv6_error_anycast_as_unicast;
+	bool skip_print_link_becomes_ready;
 };
 
 struct netns_ipv6 {
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3797917237d0..9cf7b4932309 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3633,8 +3633,9 @@  static int addrconf_notify(struct notifier_block *this, unsigned long event,
 				idev->if_flags |= IF_READY;
 			}
 
-			pr_info("ADDRCONF(NETDEV_CHANGE): %s: link becomes ready\n",
-				dev->name);
+			if (!net->ipv6.sysctl.skip_print_link_becomes_ready)
+				pr_info("ADDRCONF(NETDEV_CHANGE): %s: link becomes ready\n",
+					dev->name);
 
 			run_pending = 1;
 		}
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 94a0a294c6a1..c9e82377a8fa 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -213,6 +213,15 @@  static struct ctl_table ipv6_table_template[] = {
 		.proc_handler	= proc_doulongvec_minmax,
 		.extra2		= &ioam6_id_wide_max,
 	},
+	{
+		.procname	= "skip_print_link_becomes_ready",
+		.data		= &init_net.ipv6.sysctl.skip_print_link_becomes_ready,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1         = SYSCTL_ZERO,
+		.extra2         = SYSCTL_ONE,
+	},
 	{ }
 };
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index e74d3074ef90..ec7d66a0a57e 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -83,6 +83,7 @@  init_partial()
 		ip netns exec $netns sysctl -q net.mptcp.pm_type=0
 		ip netns exec $netns sysctl -q net.ipv4.conf.all.rp_filter=0
 		ip netns exec $netns sysctl -q net.ipv4.conf.default.rp_filter=0
+		ip netns exec $netns sysctl -q net.ipv6.skip_print_link_becomes_ready=1
 		if [ $checksum -eq 1 ]; then
 			ip netns exec $netns sysctl -q net.mptcp.checksum_enabled=1
 		fi