diff mbox series

[net] net: openvswitch: fix unwanted error log on timeout policy probing

Message ID 20240403203803.2137962-1-i.maximets@ovn.org (mailing list archive)
State Accepted
Commit 4539f91f2a801c0c028c252bffae56030cfb2cae
Delegated to: Netdev Maintainers
Headers show
Series [net] net: openvswitch: fix unwanted error log on timeout policy probing | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 943 this patch: 943
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 954 this patch: 954
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 954 this patch: 954
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-05--03-00 (tests: 951)

Commit Message

Ilya Maximets April 3, 2024, 8:38 p.m. UTC
On startup, ovs-vswitchd probes different datapath features including
support for timeout policies.  While probing, it tries to execute
certain operations with OVS_PACKET_ATTR_PROBE or OVS_FLOW_ATTR_PROBE
attributes set.  These attributes tell the openvswitch module to not
log any errors when they occur as it is expected that some of the
probes will fail.

For some reason, setting the timeout policy ignores the PROBE attribute
and logs a failure anyway.  This is causing the following kernel log
on each re-start of ovs-vswitchd:

  kernel: Failed to associated timeout policy `ovs_test_tp'

Fix that by using the same logging macro that all other messages are
using.  The message will still be printed at info level when needed
and will be rate limited, but with a net rate limiter instead of
generic printk one.

The nf_ct_set_timeout() itself will still print some info messages,
but at least this change makes logging in openvswitch module more
consistent.

Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 net/openvswitch/conntrack.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Eelco Chaudron April 4, 2024, 7:27 a.m. UTC | #1
On 3 Apr 2024, at 22:38, Ilya Maximets wrote:

> On startup, ovs-vswitchd probes different datapath features including
> support for timeout policies.  While probing, it tries to execute
> certain operations with OVS_PACKET_ATTR_PROBE or OVS_FLOW_ATTR_PROBE
> attributes set.  These attributes tell the openvswitch module to not
> log any errors when they occur as it is expected that some of the
> probes will fail.
>
> For some reason, setting the timeout policy ignores the PROBE attribute
> and logs a failure anyway.  This is causing the following kernel log
> on each re-start of ovs-vswitchd:
>
>   kernel: Failed to associated timeout policy `ovs_test_tp'
>
> Fix that by using the same logging macro that all other messages are
> using.  The message will still be printed at info level when needed
> and will be rate limited, but with a net rate limiter instead of
> generic printk one.
>
> The nf_ct_set_timeout() itself will still print some info messages,
> but at least this change makes logging in openvswitch module more
> consistent.
>
> Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

Thanks for fixing this annoying startup message! The change looks good to me.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
patchwork-bot+netdevbpf@kernel.org April 6, 2024, 6:10 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  3 Apr 2024 22:38:01 +0200 you wrote:
> On startup, ovs-vswitchd probes different datapath features including
> support for timeout policies.  While probing, it tries to execute
> certain operations with OVS_PACKET_ATTR_PROBE or OVS_FLOW_ATTR_PROBE
> attributes set.  These attributes tell the openvswitch module to not
> log any errors when they occur as it is expected that some of the
> probes will fail.
> 
> [...]

Here is the summary with links:
  - [net] net: openvswitch: fix unwanted error log on timeout policy probing
    https://git.kernel.org/netdev/net/c/4539f91f2a80

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 3019a4406ca4..74b63cdb5992 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1380,8 +1380,9 @@  int ovs_ct_copy_action(struct net *net, const struct nlattr *attr,
 	if (ct_info.timeout[0]) {
 		if (nf_ct_set_timeout(net, ct_info.ct, family, key->ip.proto,
 				      ct_info.timeout))
-			pr_info_ratelimited("Failed to associated timeout "
-					    "policy `%s'\n", ct_info.timeout);
+			OVS_NLERR(log,
+				  "Failed to associated timeout policy '%s'",
+				  ct_info.timeout);
 		else
 			ct_info.nf_ct_timeout = rcu_dereference(
 				nf_ct_timeout_find(ct_info.ct)->timeout);