[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20241107223048.17156-1-kuniyu@amazon.com>
Date: Thu, 7 Nov 2024 14:30:48 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <gnaaman@...venets.com>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
<horms@...nel.org>, <kuba@...nel.org>, <netdev@...r.kernel.org>,
<pabeni@...hat.com>
Subject: Re: [PATCH net-next] Avoid traversing addrconf hash on ifdown
From: Gilad Naaman <gnaaman@...venets.com>
Date: Thu, 7 Nov 2024 16:13:23 +0000
> struct inet6_dev already has a list of addresses owned by the device,
> enabling us to traverse this much shorter list, instead of scanning
> the entire hash-table.
>
> Signed-off-by: Gilad Naaman <gnaaman@...venets.com>
> ---
> net/ipv6/addrconf.c | 36 ++++++++++++++++--------------------
> 1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index d0a99710d65d..9c57c993e1ec 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3846,12 +3846,12 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister)
> {
> unsigned long event = unregister ? NETDEV_UNREGISTER : NETDEV_DOWN;
> struct net *net = dev_net(dev);
> - struct inet6_dev *idev;
> struct inet6_ifaddr *ifa;
> LIST_HEAD(tmp_addr_list);
> + struct inet6_dev *idev;
> bool keep_addr = false;
> bool was_ready;
> - int state, i;
> + int state;
>
> ASSERT_RTNL();
>
> @@ -3890,28 +3890,24 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister)
> }
>
> /* Step 2: clear hash table */
> - for (i = 0; i < IN6_ADDR_HSIZE; i++) {
> - struct hlist_head *h = &net->ipv6.inet6_addr_lst[i];
> + read_lock_bh(&idev->lock);
> + spin_lock_bh(&net->ipv6.addrconf_hash_lock);
No need to nest _bh.
>
> - spin_lock_bh(&net->ipv6.addrconf_hash_lock);
> -restart:
> - hlist_for_each_entry_rcu(ifa, h, addr_lst) {
> - if (ifa->idev == idev) {
> - addrconf_del_dad_work(ifa);
> - /* combined flag + permanent flag decide if
> - * address is retained on a down event
> - */
> - if (!keep_addr ||
> - !(ifa->flags & IFA_F_PERMANENT) ||
> - addr_is_local(&ifa->addr)) {
> - hlist_del_init_rcu(&ifa->addr_lst);
> - goto restart;
> - }
> - }
> + list_for_each_entry(ifa, &idev->addr_list, if_list) {
> + addrconf_del_dad_work(ifa);
while at it, please add newline here
> + /* combined flag + permanent flag decide if
> + * address is retained on a down event
> + */
> + if (!keep_addr ||
> + !(ifa->flags & IFA_F_PERMANENT) ||
> + addr_is_local(&ifa->addr)) {
> + hlist_del_init_rcu(&ifa->addr_lst);
> }
and remove unnecessary {}.
> - spin_unlock_bh(&net->ipv6.addrconf_hash_lock);
> }
>
> + spin_unlock_bh(&net->ipv6.addrconf_hash_lock);
> + read_unlock_bh(&idev->lock);
> +
> write_lock_bh(&idev->lock);
>
> addrconf_del_rs_timer(idev);
> --
> 2.34.1
>
Powered by blists - more mailing lists