linux-kernel - Re: [PATCH net-next v2 3/6] net/x25: replace x25_kill_by_device with x25_kill_by

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]

Date:   Tue, 17 Nov 2020 11:50:23 -0800
From:   Xie He <xie.he.0141@...il.com>
To:     Martin Schiller <ms@....tdt.de>
Cc:     Andrew Hendry <andrew.hendry@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Linux X25 <linux-x25@...r.kernel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2 3/6] net/x25: replace x25_kill_by_device with x25_kill_by_neigh

On Mon, Nov 16, 2020 at 6:00 AM Martin Schiller <ms@....tdt.de> wrote:
>
> Remove unnecessary function x25_kill_by_device.

> -/*
> - *     Kill all bound sockets on a dropped device.
> - */
> -static void x25_kill_by_device(struct net_device *dev)
> -{
> -       struct sock *s;
> -
> -       write_lock_bh(&x25_list_lock);
> -
> -       sk_for_each(s, &x25_list)
> -               if (x25_sk(s)->neighbour && x25_sk(s)->neighbour->dev == dev)
> -                       x25_disconnect(s, ENETUNREACH, 0, 0);
> -
> -       write_unlock_bh(&x25_list_lock);
> -}
> -
>  /*
>   *     Handle device status changes.
>   */
> @@ -273,7 +257,11 @@ static int x25_device_event(struct notifier_block *this, unsigned long event,
>                 case NETDEV_DOWN:
>                         pr_debug("X.25: got event NETDEV_DOWN for device: %s\n",
>                                  dev->name);
> -                       x25_kill_by_device(dev);
> +                       nb = x25_get_neigh(dev);
> +                       if (nb) {
> +                               x25_kill_by_neigh(nb);
> +                               x25_neigh_put(nb);
> +                       }
>                         x25_route_device_down(dev);
>                         x25_link_device_down(dev);
>                         break;

This patch might not be entirely necessary. x25_kill_by_neigh and
x25_kill_by_device are just two helper functions. One function takes
nb as the argument and the other one takes dev as the argument. But
they do almost the same things. It doesn't harm to keep both. In C++
we often have different functions with the same name doing almost the
same things.

The original code also seems to be a little more efficient than the new code.