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

[Dmitry Vyukov]

On Thu, Mar 25, 2021 at 9:46 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Thu, Mar 25, 2021 at 8:39 AM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> >
> > On Wed, Mar 24, 2021 at 10:40 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> > >
> > > On Tue, Mar 23, 2021 at 7:49 AM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> > > >
> > > > 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@xxxxxxxxxx>
> > > > Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=211877
> > > > Cc: netdev@xxxxxxxxxxxxxxx
> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > >
> > > > ---
> > > > Changes since v1:
> > > >  - use sysctl instead of a config
> > > > ---
> > >
> > > >         },
> > > > +       {
> > > > +               .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".