Hi Jozsef,
On Sat, Oct 27, 2018 at 06:05:43PM +0200, Jozsef Kadlecsik wrote:
> The ip_set() macro is called when either ip_set_ref_lock held only
> or no lock/nfnl mutex is held at dumping. Take this into account
> properly.
>
> Signed-off-by: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>
> ---
> net/netfilter/ipset/ip_set_core.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index 68db946..292f7c7 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -55,12 +55,27 @@ MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>");
> MODULE_DESCRIPTION("core IP set support");
> MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_IPSET);
>
> -/* When the nfnl mutex is held: */
> +/* When the nfnl mutex or ip_set_ref_lock is held: */
> #define ip_set_dereference(p) \
> - rcu_dereference_protected(p, lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET))
> + rcu_dereference_protected(p, \
> + lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET) || \
> + lockdep_is_held(&ip_set_ref_lock))
> #define ip_set(inst, id) \
> ip_set_dereference((inst)->ip_set_list)[id]
>
> +/* When the nfnl mutex is not held (dumping): */
> +static inline
> +struct ip_set * ip_set_no_mutex(struct ip_set_net *inst, ip_set_id_t id)
> +{
> + struct ip_set *set;
> +
> + rcu_read_lock();
> + set = rcu_dereference((inst)->ip_set_list)[id];
> + rcu_read_unlock();
If your goal is to calm down bogus warnings, you can use instead:
set = rcu_dereference_raw(...);
instead, in case ip_set_dump_done() is already protected under
rcu_read_lock(). Otherwise, you have to rework your rcu approach in
the dump_done path.
rcu is not useful in the way you use it above, because future
dereference of set are only valid insider the rcu read lock section.
Would you send me a follow up patch to amend this?
Thanks.
