[PATCH net] devlink: Wait longer before warning about unset port type

[PATCH net] devlink: Wait longer before warning about unset port type

[Ido Schimmel]

From: Ido Schimmel <idosch@mellanox.com>

The commit cited below causes devlink to emit a warning if a type was
not set on a devlink port for longer than 30 seconds to "prevent
misbehavior of drivers". This proved to be problematic when
unregistering the backing netdev. The flow is always:

devlink_port_type_clear()	// schedules the warning
unregister_netdev()		// blocking
devlink_port_unregister()	// cancels the warning

The call to unregister_netdev() can block for long periods of time for
various reasons: RTNL lock is contended, large amounts of configuration
to unroll following dismantle of the netdev, etc. This results in
devlink emitting a warning despite the driver behaving correctly.

In emulated environments (of future hardware) which are usually very
slow, the warning can also be emitted during port creation as more than
30 seconds can pass between the time the devlink port is registered and
when its type is set.

In addition, syzbot has hit this warning [1] 1974 times since 07/11/19
without being able to produce a reproducer. Probably because
reproduction depends on the load or other bugs (e.g., RTNL not being
released).

To prevent bogus warnings, increase the timeout to 1 hour.

[1] https://syzkaller.appspot.com/bug?id=e99b59e9c024a666c9f7450dc162a4b74d09d9cb

Fixes: 136bf27fc0e9 ("devlink: add warning in case driver does not set port type")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: syzbot+b0a18ed7b08b735d2f41@syzkaller.appspotmail.com
Reported-by: Alex Veber <alexve@mellanox.com>
Tested-by: Alex Veber <alexve@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4c63c9a4c09e..b8d698a2bf57 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6406,7 +6406,7 @@ static bool devlink_port_type_should_warn(struct devlink_port *devlink_port)
 	       devlink_port->attrs.flavour != DEVLINK_PORT_FLAVOUR_DSA;
 }
 
-#define DEVLINK_PORT_TYPE_WARN_TIMEOUT (HZ * 30)
+#define DEVLINK_PORT_TYPE_WARN_TIMEOUT (HZ * 3600)
 
 static void devlink_port_type_warn_schedule(struct devlink_port *devlink_port)
 {
-- 
2.24.1

Re: [PATCH net] devlink: Wait longer before warning about unset port type

[David Miller]

From: Ido Schimmel <idosch@idosch.org>
Date: Thu,  9 Jan 2020 19:57:41 +0200

> From: Ido Schimmel <idosch@mellanox.com>
> 
> The commit cited below causes devlink to emit a warning if a type was
> not set on a devlink port for longer than 30 seconds to "prevent
> misbehavior of drivers". This proved to be problematic when
> unregistering the backing netdev. The flow is always:
> 
> devlink_port_type_clear()	// schedules the warning
> unregister_netdev()		// blocking
> devlink_port_unregister()	// cancels the warning
> 
> The call to unregister_netdev() can block for long periods of time for
> various reasons: RTNL lock is contended, large amounts of configuration
> to unroll following dismantle of the netdev, etc. This results in
> devlink emitting a warning despite the driver behaving correctly.
> 
> In emulated environments (of future hardware) which are usually very
> slow, the warning can also be emitted during port creation as more than
> 30 seconds can pass between the time the devlink port is registered and
> when its type is set.
> 
> In addition, syzbot has hit this warning [1] 1974 times since 07/11/19
> without being able to produce a reproducer. Probably because
> reproduction depends on the load or other bugs (e.g., RTNL not being
> released).
> 
> To prevent bogus warnings, increase the timeout to 1 hour.
> 
> [1] https://syzkaller.appspot.com/bug?id=e99b59e9c024a666c9f7450dc162a4b74d09d9cb
> 
> Fixes: 136bf27fc0e9 ("devlink: add warning in case driver does not set port type")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: syzbot+b0a18ed7b08b735d2f41@syzkaller.appspotmail.com
> Reported-by: Alex Veber <alexve@mellanox.com>
> Tested-by: Alex Veber <alexve@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>

Applied and queued up for v5.3+ -stable, thanks.