[PATCH 3/4] nvme-fabrics: add tp8010 support

[Hannes Reinecke]

On 1/27/22 14:30, Belanger, Martin wrote:
>>>> TP8010 introduces Central Discovery Controllers (CDC) and the
>>>> Discovery Information Management (DIM) PDU, which is used to perform
>>>> explicit registration with Discovery Controllers.
>>>>
>>>> This patch defines the DIM PDU and adds logic to perform explicit
>>>> registration with DCs when registration has been requested by the
>>>> user.
>>>>
[ .. ]
>>> I'd rather like to have the TLS bits in a separate patch; they are not
>>> strictly related to this issue.
>>>
>>> And I have a rather general issue with this: Is it required to issue an
>>> explicit registration directly after sending the 'connect' command?
>>> IE do we need to have it as part of the 'connect' routine?
>>>
>>> Thing is, the linux 'connect' routine is doing several things in one go
>>> (like establishing a transport association for the admin queue, send the
>>> 'connect' command for the admin queue, create transport associations for
>>> I/O queues and send 'connect' commands on all I/O queues, _and_
>> possibly
>>> do authentication, too).
>>> If we now start overloading it with yet more functionality it becomes
>>> extremely hard to cleanup after an error, and that doesn't even mention
>>> the non-existing error reporting to userspace.
>>>
>>> Wouldn't it make more sense to delegate explicit registration to
>>> userspace (ie libnvme/nvme-cli), and leave the kernel out of it?
>>
>> It would. There is no reason what-so-ever to place all this register
>> stuff needs to live in the kernel. We have a way to passthru commands so
>> it can and should move to userspace.
> 
> I wish it could be delegated to a user-space app, and in fact that was my
> original design.
> 
> Unfortunately, while testing I realized that the kernel can autonomously
> reconnect and user-space apps are completely unaware of it.
> 
> For example, let's say the network goes down momentarily. The kernel then
> tries to reconnect. Once it successfully reconnects it doesn't tell anyone
> about it.
> 
> But let's say the kernel does send a signal to user-space on a reconnect.
> What if there is no user-space app to receive this signal? I'm thinking of
> the case where one uses nvme-cli to set up persistent connections to
> discovery controllers.  In that case there is no app to send the explicit
> registration on a re-connect.
> 
Hmm. There must be something I'm not getting.

Either we have to re-register every time if a connection drops, but then 
the patch to unregister once a connection drops is pointless.
Or we need to explicitly de-register when disconnecting, but then we 
don't need to re-register for connection failures.

Which one is it?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare at suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer