[RFC v1 0/3] Unifying fabrics drivers

Sagi Grimberg sagi at grimberg.me
Wed Mar 8 03:38:18 PST 2023



On 3/8/23 00:09, James Smart wrote:
> On 3/7/2023 4:28 AM, Daniel Wagner wrote:
>> On Tue, Mar 07, 2023 at 11:26:12AM +0200, Sagi Grimberg wrote:
>>> I think we should make all transports to unify setup/teardown sequences
>>> (i.e. including pcie and fc). Otherwise we are not gaining much.
>>
>> Not sure about pcie but I haven't really looked at it. The state 
>> machine is
>> really handling all the fabrics specific queue handling.
>>
>> Ideally, fc would use the same state machine. Though the current code 
>> base is
>> differs quiet significantly but I can try to convert it too.
> 
> I'll certainly help for FC.
> 
> The 2 main hurdles are our differences around:
> a) the new flag - which I replaced in the fc calling setup and a state 
> flag. Shouldn't be much of an issue. We just need to look at when/where 
> the tagsets are allocated (and embedding into admin queue setup isn't 
> necessarily best).

Does fc create an I/O tagset on every reconnect? Doesn't appear
that it is removed on every disconnect... I'm a bit confused.

> b) the init_ctrl - I don't have the initial connect inline to the 
> init_ctrl call - I push it out to the normal "reconnect" path so that 
> everything, initial connect and all reconnects, use the same 
> routine/work element. Also says the transport will retry the initial 
> connect if there's a failure on the initial attempt. To keep the app 
> happy, init_ctrl will wait for the 1st connect attempt to finish before 
> returning.   Don't know how rdma/tcp feels about it, but I found it a 
> much better solution and cleanup became cleaner.

Don't see a major difference, I personally find the initial setup
different than a reconnect so its simpler to do them separately.

Can these two differences change for unification of the transports?

I would also like to get rid of the new flag, and just break these
sequences to different routines.



More information about the Linux-nvme mailing list