The Linux Kernel Mailing List
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox