Re: [PATCH for-next v7 10/12] RDMA/erdma: Add the erdma module

[Cheng Xu <chengyou@linux.alibaba.com>]

On 5/17/22 1:31 AM, Jason Gunthorpe wrote:
> On Mon, May 16, 2022 at 11:14:24PM +0800, Cheng Xu wrote:
>>>> I think this twice, use a net notifier of module can not solve this, due
>>>> to the leak of all ib devices information in erdma module.
>>>
>>> I don't understand what this means
>> At first I want to register a global net notifier in our module, and link
>> the ib device and net device if matches in the callback. But in the
>> callback, we can not get the ib devices registered by erdma, because our
>> driver module does not maintain the ib device list. This is not the
>> right way.
> 
> The core has the device list, if you need to search it then you need
> to add a new searching function
> 

I had a mistake, instead of what I said, erdma should search net devices
in a wq to find the matched net device. Not search ib devices in net
notifier callback.

I think is should be fine.

>>
>>>> The solution may be simple: register net notifier per device in probe,
>>>> and call ib_device_set_netdev if matches in the notifier callback.
>>>
>>> That sounds wrong
>>>
>>
>> OK. So using rdma link command to handle the link relationship manually
>> is still a better choice？
> 
> No
>   
>> Actually, erdma looks like a hardware implementation of siw/rxe (more
>> likes siw, because erdma uses iWarp, and have similar CM
>> implementation): the ethernet functionality is provided by existed net
>> device, no matter what kind of the net device (due to virtio-net is
>> de-facto in cloud virtualization, erdma is binded to virtio-net in
>> practice). Due to this similarity, siw/rxe use 'rdma link' to link the
>> ib device and net device (including create ib device), Does it sound
>> reasonable that erdma use it to link the net deivce?
> 
> It is not like siw because it cannot run on any netdevice, it can only
> run on the signle netdevice the HW is linked to in the background.
> 

Yeah, you are exactly right, I omitted this detail in the last
reply.

Thanks,
Cheng Xu

> Jason