[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e9d342ac-75da-4efc-98e3-67bf43bc7487@redhat.com>
Date: Sun, 28 Dec 2025 10:46:39 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Dmitry Skorodumov <dskr99@...il.com>, netdev@...r.kernel.org,
Jakub Kicinski <kuba@...nel.org>,
Dmitry Skorodumov <skorodumov.dmitry@...wei.com>,
Xiao Liang <shaw.leon@...il.com>, Kuniyuki Iwashima <kuniyu@...gle.com>,
Julian Vetter <julian@...er-limits.org>, Guillaume Nault
<gnault@...hat.com>, Ido Schimmel <idosch@...dia.com>,
Eric Dumazet <edumazet@...gle.com>, Stanislav Fomichev <sdf@...ichev.me>,
Etienne Champetier <champetier.etienne@...il.com>,
"David S. Miller" <davem@...emloft.net>, linux-kernel@...r.kernel.org
Cc: Andrew Lunn <andrew+netdev@...n.ch>
Subject: Re: [PATCH net 1/2] ipvlan: Make the addrs_lock be per port
On 12/25/25 7:55 PM, Dmitry Skorodumov wrote:
> Make the addrs_lock be per port, not per ipvlan dev.
>
> Initial code seems to be written in the assumption,
> that any address change must occur under RTNL.
> But it is not so for the case of IPv6. So
>
> 1) Introduce per-port addrs_lock.
>
> 2) It was needed to fix places where it was forgotten
> to take lock (ipvlan_open/ipvlan_close)
>
> This appears to be a very minor problem though.
> Since it's highly unlikely that ipvlan_add_addr() will
> be called on 2 CPU simultaneously. But nevertheless,
> this could cause:
>
> 1) False-negative of ipvlan_addr_busy(): one interface
> iterated through all port->ipvlans + ipvlan->addrs
> under some ipvlan spinlock, and another added IP
> under its own lock. Though this is only possible
> for IPv6, since looks like only ipvlan_addr6_event() can be
> called without rtnl_lock.
>
> 2) Race since ipvlan_ht_addr_add(port) is called under
> different ipvlan->addrs_lock locks
>
> This should not affect performance, since add/remove IP
> is a rare situation and spinlock is not taken on fast
> paths.
>
> Fixes: 8230819494b3 ("ipvlan: use per device spinlock to protect addrs list updates")
> Signed-off-by: Dmitry Skorodumov <skorodumov.dmitry@...wei.com>
> CC: Paolo Abeni <pabeni@...hat.com>
Not so minor process nits: you should include the revision number in the
subj prefix, and you should include the main changes vs the previous
revision (and possibly even link to the previous revisions after the
'---' separator, it will help reviewers greatly.
Patch contents LGTM.
/P
Powered by blists - more mailing lists