Re: [PATCH v2] net: make unregister netdev warning timeout configurable

On Thu, Mar 25, 2021 at 9:46 AM Eric Dumazet [off-list ref] wrote:

On Thu, Mar 25, 2021 at 8:39 AM Dmitry Vyukov [off-list ref] wrote:

quoted

On Wed, Mar 24, 2021 at 10:40 AM Eric Dumazet [off-list ref] wrote:

quoted

On Tue, Mar 23, 2021 at 7:49 AM Dmitry Vyukov [off-list ref] wrote:

quoted

netdev_wait_allrefs() issues a warning if refcount does not drop to 0
after 10 seconds. While 10 second wait generally should not happen
under normal workload in normal environment, it seems to fire falsely
very often during fuzzing and/or in qemu emulation (~10x slower).
At least it's not possible to understand if it's really a false
positive or not. Automated testing generally bumps all timeouts
to very high values to avoid flake failures.
Add net.core.netdev_unregister_timeout_secs sysctl to make
the timeout configurable for automated testing systems.
Lowering the timeout may also be useful for e.g. manual bisection.
The default value matches the current behavior.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=211877
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Changes since v1:
 - use sysctl instead of a config
---

quoted

        },
+       {
+               .procname       = "netdev_unregister_timeout_secs",
+               .data           = &netdev_unregister_timeout_secs,
+               .maxlen         = sizeof(unsigned int),
+               .mode           = 0644,
+               .proc_handler   = proc_dointvec_minmax,
+               .extra1         = SYSCTL_ZERO,
+               .extra2         = &int_3600,
+       },
        { }
 };

If we allow the sysctl to be 0, then we risk a flood of pr_emerg()
(one per jiffy ?)

My reasoning was that it's up to the user. Some spammy output on the
console for rare events is probably not the worst way how root can
misconfigure the kernel :)
It allows one to check (more or less) if we are reaching
unregister_netdevice with non-zero refcount, which may be useful for
some debugging maybe.
But I don't mind changing it to 1 (or 5) if you prefer. On syzbot we
only want to increase it.

Yes, please use a lower limit of one to avoid spurious reports.

This commit is already in net-next and net-next is not rebased, right?
I sent a separate fix as "net: change netdev_unregister_timeout_secs
min value to 1".