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(-)
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 is doing, it generates a lot of messages: more than 700 with
the latest version.
This message can be useful in many situations but in some, it floods the
logs without providing any useful input. The proposition here is to have
a new sysctl knob to easily skip this specific message.
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(-)
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 0044d87556dd..27179e9175be 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -84,6 +84,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 2>/dev/null || true
if [ $checksum -eq 1 ]; then
ip netns exec $netns sysctl -q net.mptcp.checksum_enabled=1
fi
---
base-commit: 194dd0efe579cb5d3a746d248b3476f4b3fc0b48
change-id: 20230522-mptcp-skip_print_link_becomes_ready-af50c71a9daa
Best regards,
--
Matthieu Baerts <matthieu.baerts@tessares.net>On Mon, 22 May 2023, Matthieu Baerts 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 is doing, it generates a lot of messages: more than 700 with
> the latest version.
>
> This message can be useful in many situations but in some, it floods the
> logs without providing any useful input. The proposition here is to have
> a new sysctl knob to easily skip this specific message.
>
> 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(-)
>
Hi Matthieu -
Using a sysctl for this seems like overkill. I noticed a similar change
for ADDRCONF(NETDEV_UP) that changed a pr_info() to pr_debug():
7c62b8dd5ca89dabd9f455d19e663bad60951bd5
Maybe that's more suitable?
If you did want to keep pursuing the sysctl, the patch would need to
update Documentation/networking/ip-sysctl.rst
Thanks,
Mat
> 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 0044d87556dd..27179e9175be 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -84,6 +84,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 2>/dev/null || true
> if [ $checksum -eq 1 ]; then
> ip netns exec $netns sysctl -q net.mptcp.checksum_enabled=1
> fi
>
> ---
> base-commit: 194dd0efe579cb5d3a746d248b3476f4b3fc0b48
> change-id: 20230522-mptcp-skip_print_link_becomes_ready-af50c71a9daa
>
> Best regards,
> --
> Matthieu Baerts <matthieu.baerts@tessares.net>
>
>
>
Hi Mat, Paolo, @Mat: Thank you for having reviewed this patch! @Paolo: I have a question for you below if you don't mind. On 24/05/2023 02:20, Mat Martineau wrote: > On Mon, 22 May 2023, Matthieu Baerts 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 is doing, it generates a lot of messages: more than 700 with >> the latest version. >> >> This message can be useful in many situations but in some, it floods the >> logs without providing any useful input. The proposition here is to have >> a new sysctl knob to easily skip this specific message. >> >> 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(-) >> > > Hi Matthieu - > > Using a sysctl for this seems like overkill. I noticed a similar change > for ADDRCONF(NETDEV_UP) that changed a pr_info() to pr_debug(): > > 7c62b8dd5ca89dabd9f455d19e663bad60951bd5 > > Maybe that's more suitable? I don't know. I mean: I don't even know if this message is that useful. I quickly search for this message on the Internet and I found bug reports because this message was abnormally printed. So in these cases, it was a sign something else was wrong. Maybe the case in most use cases? Even if there are other (better) ways to track issues with network devices... But yes, pr_debug() would be easier :) The commit you mentioned also talk about the message I want to mute: (...) it seems to be more interesting to hear when the addrconf actually start (...) (but it doesn't say if it is that useful) @Paolo: should I ask on netdev which direction to take? Or send this patch as a RFC (and mention the doc is missing and the modification on the MPTCP side will be done in a separated commit + the pr_debug() possibility)? > If you did want to keep pursuing the sysctl, the patch would need to > update Documentation/networking/ip-sysctl.rst Good point, I forgot that! Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
© 2016 - 2024 Red Hat, Inc.