From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-173.mta0.migadu.com (out-173.mta0.migadu.com [91.218.175.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 11E282D47F1 for ; Tue, 7 Jul 2026 01:26:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.173 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783387602; cv=none; b=kE5XIGBCZhiXIPVs5Z2oycXwYqWNn8i5S9xDs1G8HaYlfFNr0PzJTHOvMYk88jMaobUwYGO7LSUvKgw1zVNM64DNxnziDKgzvmU3z0OXSzKUyqfNnQvRPffMbPhiBAGsZ20P+6MpNGcijeIqfFu7PHfhKSiLgJdSiGxceVUS3LQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783387602; c=relaxed/simple; bh=rypkywCllkRCtt7lrzaFfAjRvYVR/iBmoYtPGMl+Hms=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=msv22JztYjA+M9FBiQnrJNgwmbleD7qV2KYusKccSLnEeWyzpUvCv8PBvkla+VPiGO9OyYzYjBtrrN5C81c9u0VMDwlDIwINO9eplSESXepsKl5OyVJ1LtZjIMbhlxm7kMjBnnXWcRmJydDt88DXGR224gzQOzkWKc3Bqsc9rLk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=NLpehEzj; arc=none smtp.client-ip=91.218.175.173 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="NLpehEzj" Message-ID: <5e2119f1-3c1c-43eb-984e-a8eeab2896e0@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1783387597; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QOhHJ1dEBTNHFcbH9VoW95mNmVv3gt+0b86uohL6/ko=; b=NLpehEzjCglzQx+nvZdL1ZNVCLZDZoJdJq7jhEjuPkZangZaof5yjCXsyJ+ChqF6Cp/oS/ uP6T7WxFcbSBuq61400mGJ5AP7270TSqRT62RqwObsREa/ldagd/m9nyad9A5Im+GmQXp/ WapcEt534CmmlT9lKJSVR9NEXH3A+1E= Date: Tue, 7 Jul 2026 09:26:06 +0800 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: luoxuanqiang Subject: Re: [PATCH net-next v2] ipv4: hold a consistent view of rt->dst.dev under RCU To: Ido Schimmel Cc: "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , David Ahern , Simon Horman , Kuniyuki Iwashima , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Xuanqiang Luo References: <20260701032434.17500-1-xuanqiang.luo@linux.dev> <20260705164649.GA295849@shredder> In-Reply-To: <20260705164649.GA295849@shredder> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 在 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 >> >> 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? >> Signed-off-by: Xuanqiang Luo >> --- >> v2: >> - Use dst_dev_rcu() and dev_net_rcu() for the RCU readers. >> - Use rcu_assign_pointer() when publishing the uncached route device >> replacement. >> - Slightly adjust the commit message wording because this issue was found >> by inspection, not from an observed user-visible failure. >> >> v1:https://lore.kernel.org/all/20260630094250.29386-1-xuanqiang.luo@linux.dev/ >> >> net/ipv4/route.c | 29 +++++++++++++++++------------ >> 1 file changed, 17 insertions(+), 12 deletions(-) >> >> diff --git a/net/ipv4/route.c b/net/ipv4/route.c >> index 3f3de5164d6e5..57f38467e6d0c 100644 >> --- a/net/ipv4/route.c >> +++ b/net/ipv4/route.c >> @@ -873,6 +873,7 @@ static void ipv4_negative_advice(struct sock *sk, >> void ip_rt_send_redirect(struct sk_buff *skb) >> { >> struct rtable *rt = skb_rtable(skb); >> + struct net_device *dev; > https://docs.kernel.org/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs > > Same in other places. Thanks for pointing this out! I will fix the local variable ordering in the next version.