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

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

On 5/16/22 7:49 PM, Jason Gunthorpe wrote:
> On Mon, May 16, 2022 at 11:15:32AM +0800, Cheng Xu wrote:
>>
>>
>> On 5/10/22 9:17 PM, Jason Gunthorpe wrote:
>>> On Thu, Apr 21, 2022 at 03:17:45PM +0800, Cheng Xu wrote:
>>>
>>>> +static struct rdma_link_ops erdma_link_ops = {
>>>> +	.type = "erdma",
>>>> +	.newlink = erdma_newlink,
>>>> +};
>>>
>>> Why is there still a newlink?
>>>
>>> Jason
>>
>>
>> Yeah, I remember your suggestion that the ibdev should keep the same
>> lifecycle with its PCI device, and so does it now.
>>
>> The initialization flow for erdma now:
>>       probe:
>>         - Hardware specified initialization
>>         - IB device initialization
>>         - Calling ib_register_device with ibdev->netdev == NULL
>>
>> After probe, The ib device has been registered, but we left it in
>> invalid state.
>> To fully complete the initialization, we should set the ibdev->netdev.
>>
>> And the newlink command in erdma only do one thing now: set the
>> ibdev->netdev if the rule matches, and it is uncorrelated with the
>> ib device lifecycle.
> 
> This is not what newlink is for, it can't be used like this
> 
> Jason


[A typo in previous reply, so I re-edit it]

Yeah, the newlink is only used by siw and rxe to control their ib
device lifecycle now.

But for the usage of newlink, it is triggered by 'rdma link add'
command. In my opinion, the 'rdma link' command set is all about
display (rdma link show) or management of the link relationship between
ib device and net device (rdma link add/del). And what erdma needs is
setting up the link between ib device and net device.

So, though the current usage is controling the lifecycle of siw/rxe, but
for the command's naming and connotation, should it be OK to expand the
usage of newlink to meet our requirement?

Or, Is it ok that erdma registers a net notifier in our module to handle
this?

Thanks,
Cheng Xu