From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 493DEC43217 for ; Sun, 13 Nov 2022 10:26:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235268AbiKMK0D (ORCPT ); Sun, 13 Nov 2022 05:26:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38192 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232014AbiKMK0A (ORCPT ); Sun, 13 Nov 2022 05:26:00 -0500 Received: from out2.migadu.com (out2.migadu.com [188.165.223.204]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3E6ED11834; Sun, 13 Nov 2022 02:25:58 -0800 (PST) Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1668335157; 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=7Ix5CiIju4tuYtmb+hMgKgKfn6aGjoc/cdFjHgvGIp4=; b=kbkrA8sqbm48Wun+YYGfJh4HyZmFW9/V4pmgE4scxjhsWRa7N+WidivUB22QD1hdRnrBMG wcF+H/3rPO5JyC1VePsEpjmol6JjZgvPkOuBLMnKQ/E99NIRp2vHt9d+TTQQ/tXKm4KKda j/Ue/YUtH5gL9IeWE/G1P48euC0FK44= Date: Sun, 13 Nov 2022 18:25:47 +0800 MIME-Version: 1.0 Subject: Re: RE: [PATCHv2 0/6] Fix the problem that rxe can not work in net To: Parav Pandit , Yanjun Zhu , "jgg@ziepe.ca" , "leon@kernel.org" , "zyjzyj2000@gmail.com" , "linux-rdma@vger.kernel.org" , "netdev@vger.kernel.org" , "davem@davemloft.net" Cc: Zhu Yanjun References: <20221006085921.1323148-1-yanjun.zhu@linux.dev> <204f1ef4-77b1-7d4b-4953-00a99ce83be4@linux.dev> <25767d73-c7fc-4831-4a45-337764430fe7@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yanjun Zhu In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 在 2022/11/13 12:58, Parav Pandit 写道: > Hi Yanjun, > >> From: Yanjun Zhu >> Sent: Thursday, November 10, 2022 10:38 PM >> >> >> 在 2022/11/11 11:35, Parav Pandit 写道: >>>> From: Yanjun Zhu >>>> Sent: Thursday, November 10, 2022 9:37 PM >>> >>>> Can you help to review these patches? >>> I will try to review it before 13th. > > I did a brief review of patch set. > I didn’t go line by line for each patch; hence I give lumped comments here for overall series. > > 1. Add example and test results in below test flow in exclusive mode in cover letter. > # ip netns exec net1 rdma link add rxe1 type rxe netdev eno3 > # ip netns del net0 > Verify that rdma device rxe1 is deleted. Hi, Parav Thanks a lot. I will add this example to the cover letter. And I confirm that rdma device rxe1 is deleted after the command "ip netns del net0". I will delve into the following comments. Thanks and Regards, Zhu Yanjun > > 2. Usage of dev_net() in rxe_setup_udp_tunnel() is unsafe. > This is because when rxe_setup_udp_tunnel() is executed, net ns of netdev can change. > This needs to be synchronized with per net notifier register_pernet_subsys() of exit or exit_batch. > This notifiers callback should be added to rxe module. > > 3. You need to set bind_ifindex of udp config to the netdev given in newlink in rxe_setup_udp_tunnel. > Should be a separate pre-patch to ensure that close and right relation to udp socket with netdev in a given netns. > > 4. Rearrange series to implement delete link as separate series from net ns securing series. > They are unrelated. Current delink series may have use after free accesses. Those needs to be guarded in likely larger series. > > 5. udp tunnel must shutdown synchronously when rdma link del is done. > This means any new packet arriving after this point, will be dropped. > Any existing packet handling present is flushed. > From your cover letter description, it appears that sock deletion is refcount based and above semantics is not ensured. > > 6. In patch 5, rxe_get_dev_from_net() can return NULL, hence l_sk6 check can be unsafe. Please add check for rdev null before rdev->l_sk6 check. > > 7. In patch 5, I didn't fully inspect, but seems like call to rxe_find_route4() is not rcu safe. > Hence, extension of dev_net() in rxe_find_route4() doesn't look secure. > Accessing sock_net() is more accurate, because at this layer, it is processing packets at socket layer.