From: Yanjun Zhu <yanjun.zhu@linux.dev>
To: Parav Pandit <parav@nvidia.com>,
Yanjun Zhu <yanjun.zhu@linux.dev>, "jgg@ziepe.ca" <jgg@ziepe.ca>,
"leon@kernel.org" <leon@kernel.org>,
"zyjzyj2000@gmail.com" <zyjzyj2000@gmail.com>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>
Cc: Zhu Yanjun <yanjun.zhu@intel.com>
Subject: Re: RE: [PATCHv2 0/6] Fix the problem that rxe can not work in net
Date: Sun, 13 Nov 2022 18:25:47 +0800 [thread overview]
Message-ID: <b5891f03-7283-8b52-aef6-40773f31ff10@linux.dev> (raw)
In-Reply-To: <PH0PR12MB548101B6A19568A3E1FBD50ADC029@PH0PR12MB5481.namprd12.prod.outlook.com>
在 2022/11/13 12:58, Parav Pandit 写道:
> Hi Yanjun,
>
>> From: Yanjun Zhu <yanjun.zhu@linux.dev>
>> Sent: Thursday, November 10, 2022 10:38 PM
>>
>>
>> 在 2022/11/11 11:35, Parav Pandit 写道:
>>>> From: Yanjun Zhu <yanjun.zhu@linux.dev>
>>>> 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.
next prev parent reply other threads:[~2022-11-13 10:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-06 8:59 [PATCHv2 0/6] Fix the problem that rxe can not work in net yanjun.zhu
2022-10-06 8:59 ` [PATCHv2 1/6] RDMA/rxe: Creating listening sock in newlink function yanjun.zhu
2022-10-06 8:59 ` [PATCHv2 2/6] RDMA/rxe: Support more rdma links in init_net yanjun.zhu
2022-10-06 8:59 ` [PATCHv2 3/6] RDMA/nldev: Add dellink function pointer yanjun.zhu
2022-10-06 8:59 ` [PATCHv2 4/6] RDMA/rxe: Implement dellink in rxe yanjun.zhu
2022-10-06 8:59 ` [PATCHv2 5/6] RDMA/rxe: Replace global variable with sock lookup functions yanjun.zhu
2022-10-06 8:59 ` [PATCHv2 6/6] RDMA/rxe: add the support of net namespace yanjun.zhu
2022-10-19 14:56 ` [PATCHv2 0/6] Fix the problem that rxe can not work in net Yanjun Zhu
2022-11-11 2:36 ` Yanjun Zhu
2022-11-11 3:35 ` Parav Pandit
2022-11-11 3:38 ` Yanjun Zhu
2022-11-13 4:58 ` Parav Pandit
2022-11-13 10:25 ` Yanjun Zhu [this message]
2023-02-13 12:00 ` Zhu Yanjun
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=b5891f03-7283-8b52-aef6-40773f31ff10@linux.dev \
--to=yanjun.zhu@linux.dev \
--cc=davem@davemloft.net \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=parav@nvidia.com \
--cc=yanjun.zhu@intel.com \
--cc=zyjzyj2000@gmail.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;
as well as URLs for NNTP newsgroup(s).