netdev - Re: [PATCH net-next 1/2] net: transition netdev reg state earlier in run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADvbK_f8SWMxdrGSFo7=9BQMNrbB3nXqGhGv4LKa6v0keiTdsQ@mail.gmail.com>
Date:   Wed, 16 Feb 2022 12:12:09 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem <davem@...emloft.net>, network dev <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next 1/2] net: transition netdev reg state earlier in run_todo

On Wed, Feb 16, 2022 at 6:53 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> In prep for unregistering netdevs out of order move the netdev
> state validation and change outside of the loop.
>
> While at it modernize this code and use WARN() instead of
> pr_err() + dump_stack().
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  net/core/dev.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 909fb3815910..2749776e2dd2 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9906,6 +9906,7 @@ static void netdev_wait_allrefs(struct net_device *dev)
>   */
>  void netdev_run_todo(void)
>  {
> +       struct net_device *dev, *tmp;
>         struct list_head list;
>  #ifdef CONFIG_LOCKDEP
>         struct list_head unlink_list;
> @@ -9926,24 +9927,23 @@ void netdev_run_todo(void)
>
>         __rtnl_unlock();
>
> -
>         /* Wait for rcu callbacks to finish before next phase */
>         if (!list_empty(&list))
>                 rcu_barrier();
>
> -       while (!list_empty(&list)) {
> -               struct net_device *dev
> -                       = list_first_entry(&list, struct net_device, todo_list);
> -               list_del(&dev->todo_list);
> -
> +       list_for_each_entry_safe(dev, tmp, &list, todo_list) {
>                 if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
> -                       pr_err("network todo '%s' but state %d\n",
> -                              dev->name, dev->reg_state);
> -                       dump_stack();
> +                       netdev_WARN(dev, "run_todo but not unregistering\n");
> +                       list_del(&dev->todo_list);
>                         continue;
>                 }
>
>                 dev->reg_state = NETREG_UNREGISTERED;
> +       }
> +
> +       while (!list_empty(&list)) {
> +               dev = list_first_entry(&list, struct net_device, todo_list);
> +               list_del(&dev->todo_list);
>
>                 netdev_wait_allrefs(dev);
>
> --
> 2.34.1
>
Reviewed-by: Xin Long <lucien.xin@...il.com>