All of lore.kernel.org
 help / color / mirror / Atom feed
From: luoxuanqiang <xuanqiang.luo@linux.dev>
To: Paolo Abeni <pabeni@redhat.com>, Ido Schimmel <idosch@nvidia.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	David Ahern <dsahern@kernel.org>, Simon Horman <horms@kernel.org>,
	Kuniyuki Iwashima <kuniyu@google.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Subject: Re: [PATCH net-next v2] ipv4: hold a consistent view of rt->dst.dev under RCU
Date: Tue, 7 Jul 2026 18:41:28 +0800	[thread overview]
Message-ID: <ca14985b-1ed9-4242-a098-df52c2651135@linux.dev> (raw)
In-Reply-To: <ef3a91ba-f4c5-4d89-a88b-465b5b7fab12@redhat.com>


在 2026/7/7 17:02, Paolo Abeni 写道:
> On 7/7/26 3:26 AM, luoxuanqiang wrote:
>> 在 2026/7/6 00:46, Ido Schimmel 写道:
>>> On Wed, Jul 01, 2026 at 11:24:34AM +0800,xuanqiang.luo@linux.dev wrote:
>>>> From: Xuanqiang Luo<luoxuanqiang@kylinos.cn>
>>>>
>>>> rt_flush_dev() walks the per-CPU uncached route list and rewrites
>>>> rt->dst.dev in-place to blackhole_netdev under spin_lock_bh().
>>>> This lock does not exclude RCU readers, which may load rt->dst.dev
>>>> multiple times within a single rcu_read_lock() region.
>>>>
>>>> ip_rt_send_redirect() is a typical example: it reads rt->dst.dev
>>>> three times to obtain in_dev, the L3 master ifindex, and net.
>>>> A concurrent device unregistration can repoint rt->dst.dev to
>>>> blackhole_netdev between those reads, making the reader combine
>>>> state from two different net_devices — for instance, an in_dev
>>>> from the real device but a netns and peer lookup from the blackhole
>>>> device.  ip_rt_get_source() has the same problem: it reads
>>>> rt->dst.dev four times to obtain the output ifindex, the netns,
>>>> and the source address, so a concurrent flush can cause the source
>>>> selection to mix state from different devices.
>>> Why only change ip_rt_send_redirect() and ip_rt_get_source() when the
>>> patch is titled "ipv4: hold a consistent view of rt->dst.dev under RCU"?
>>> What is the criterion?
>> Thanks! You are right, the subject is too broad. I will make them
>> more accurate in the next version.
>>
>>>> Take a single dst_dev_rcu() snapshot of rt->dst.dev at the start
>>>> of each affected RCU reader and use that snapshot throughout, so
>>>> concurrent flushes cannot cause mid-function inconsistency.
>>>> Publish the in-place write in rt_flush_dev() with rcu_assign_pointer()
>>>> to match the readers.
>>> The rt_flush_dev() change should be a separate change. Note that
>>> dst_dev_put() was already converted to use rcu_assign_pointer().
>>>
>> I will split the rt_flush_dev() change into a separate patch.
>>
>>>> Fixes: caacf05e5ad1a ("ipv4: Properly purge netdev references on uncached routes.")
>>> Please remove the Fixes tag given you are targeting net-next.
>> Just to clarify: is the suggestion to drop the Fixes tag here solely
>> because this patch is targeted at net-next? Or are there any other
>> reasons?
> Generally speaking, yes: net-next patches should not include a fixes tag
> unless the blamed commit is on net-next only.
>
> More specifically, this patch is really a behavior improvement and not a
> vertical fix, as such we want to avoid it propagating on stable trees,
> as the fixes tag sometimes does.
>
> /P
>
Many thanks for the detailed explanation!


  reply	other threads:[~2026-07-07 10:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01  3:24 [PATCH net-next v2] ipv4: hold a consistent view of rt->dst.dev under RCU xuanqiang.luo
2026-07-05 16:46 ` Ido Schimmel
2026-07-07  1:26   ` luoxuanqiang
2026-07-07  9:02     ` Paolo Abeni
2026-07-07 10:41       ` luoxuanqiang [this message]
  -- strict thread matches above, loose matches on Subject: below --
2026-06-30  9:42 [PATCH net-next v1] " xuanqiang.luo
2026-07-01  3:16 ` [PATCH net-next v2] " xuanqiang.luo

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=ca14985b-1ed9-4242-a098-df52c2651135@linux.dev \
    --to=xuanqiang.luo@linux.dev \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luoxuanqiang@kylinos.cn \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.