From: Ido Schimmel <idosch@idosch•org>
To: David Ahern <dsahern@gmail•com>
Cc: netdev@vger•kernel.org, jiri@mellanox•com, idosch@mellanox•com,
kjlx@templeofstupid•com, davem@davemloft•net,
yoshfuji@linux-ipv6•org
Subject: Re: [PATCH net-next 1/5] ipv6: addrconf: cleanup locking in ipv6_add_addr
Date: Sun, 15 Oct 2017 10:50:52 +0300 [thread overview]
Message-ID: <20171015075052.GA10604@shredder.mtl.com> (raw)
In-Reply-To: <1507935733-18950-2-git-send-email-dsahern@gmail.com>
On Fri, Oct 13, 2017 at 04:02:09PM -0700, David Ahern wrote:
> ipv6_add_addr is called in process context with rtnl lock held
> (e.g., manual config of an address) or during softirq processing
> (e.g., autoconf and address from a router advertisement).
>
> Currently, ipv6_add_addr calls rcu_read_lock_bh shortly after entry
> and does not call unlock until exit, minus the call around the address
> validator notifier. Similarly, addrconf_hash_lock is taken after the
> validator notifier and held until exit. This forces the allocation of
> inet6_ifaddr to always be atomic.
>
> Refactor ipv6_add_addr as follows:
> 1. add an input boolean to discriminate the call path (process context
> or softirq). This new flag controls whether the alloc can be done
> with GFP_KERNEL or GFP_ATOMIC.
>
> 2. Move the rcu_read_lock_bh and unlock calls only around functions that
> do rcu updates.
>
> 3. Remove the in6_dev_hold and put added by 3ad7d2468f79f ("Ipvlan should
> return an error when an address is already in use."). This was done
> presumably because rcu_read_unlock_bh needs to be called before calling
> the validator. Since rcu_read_lock is not needed before the validator
> runs revert the hold and put added by 3ad7d2468f79f and only do the
> hold when setting ifp->idev.
>
> 4. move duplicate address check and insertion of new address in the global
> address hash into a helper. The helper is called after an ifa is
> allocated and filled in.
>
> This allows the ifa for manually configured addresses to be done with
> GFP_KERNEL and reduces the overall amount of time with rcu_read_lock held
> and hash table spinlock held.
>
> Signed-off-by: David Ahern <dsahern@gmail•com>
[...]
> @@ -1073,21 +1085,19 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
>
> in6_ifa_hold(ifa);
> write_unlock(&idev->lock);
> -out2:
> +
> rcu_read_unlock_bh();
>
> - if (likely(err == 0))
> - inet6addr_notifier_call_chain(NETDEV_UP, ifa);
> - else {
> + inet6addr_notifier_call_chain(NETDEV_UP, ifa);
> +out:
> + if (unlikely(err < 0)) {
> + if (rt)
> + ip6_rt_put(rt);
I believe 'rt' needs to be set to NULL after addrconf_dst_alloc()
fails.
> kfree(ifa);
> - in6_dev_put(idev);
> ifa = ERR_PTR(err);
> }
>
> return ifa;
> -out:
> - spin_unlock(&addrconf_hash_lock);
> - goto out2;
> }
next prev parent reply other threads:[~2017-10-15 7:50 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-13 23:02 [PATCH net-next 0/5] mlxsw: spectrum_router: Add extack messages for RIF and VRF overflow David Ahern
2017-10-13 23:02 ` [PATCH net-next 1/5] ipv6: addrconf: cleanup locking in ipv6_add_addr David Ahern
2017-10-15 7:50 ` Ido Schimmel [this message]
2017-10-15 15:24 ` David Ahern
2017-10-15 15:59 ` Ido Schimmel
2017-10-15 16:03 ` David Ahern
2017-10-13 23:02 ` [PATCH net-next 2/5] net: ipv6: Make inet6addr_validator a blocking notifier David Ahern
2017-10-15 7:53 ` Ido Schimmel
2017-10-15 16:49 ` [PATCH] ipv6: addrconf: Use normal debugging style Joe Perches
2017-10-16 20:14 ` David Miller
2017-10-13 23:02 ` [PATCH net-next 3/5] net: Add extack to validator_info structs used for address notifier David Ahern
2017-10-13 23:02 ` [PATCH net-next 4/5] mlxsw: spectrum: router: Add support for address validator notifier David Ahern
2017-10-15 8:36 ` Ido Schimmel
2017-10-13 23:02 ` [PATCH net-next 5/5] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171015075052.GA10604@shredder.mtl.com \
--to=idosch@idosch$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=dsahern@gmail$(echo .)com \
--cc=idosch@mellanox$(echo .)com \
--cc=jiri@mellanox$(echo .)com \
--cc=kjlx@templeofstupid$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=yoshfuji@linux-ipv6$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox