Re: [PATCH v2 4/5] nvme-fc: Make initial connect attempt synchronous

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/12/23 00:47, James Smart wrote:
On 7/6/2023 5:07 AM, Daniel Wagner wrote:
Hi James,

On Sat, Jul 01, 2023 at 05:11:11AM -0700, James Smart wrote:
As much as you want to make this change to make transports "similar", I am dead set against it unless you are completing a long qualification of the change on real FC hardware and FC-NVME devices. There is probably 1.5 yrs of testing of different race conditions that drove this change. You cannot declare success from a simplistic toy tool such as fcloop for validation.

The original issues exist, probably have even morphed given the time from the original change, and this will seriously disrupt the transport and any
downstream releases.  So I have a very strong NACK on this change.

Yes - things such as the connect failure results are difficult to return
back to nvme-cli. I have had many gripes about the nvme-cli's behavior over
the years, especially on negative cases due to race conditions which
required retries. It still fails this miserably.  The async reconnect path
solved many of these issues for fc.

For the auth failure, how do we deal with things if auth fails over time as reconnects fail due to a credential changes ?  I would think commonality of
this behavior drives part of the choice.

Alright, what do you think about the idea to introduce a new '--sync' option to nvme-cli which forwards this info to the kernel that we want to wait for the initial connect to succeed or fail? Obviously, this needs to handle signals too.

 From what I understood this is also what Ewan would like to have
To me this is not sync vs non-sync option, it's a max_reconnects value tested for in nvmf_should_reconnect(). Which, if set to 0 (or 1), should fail if the initial connect fails.

Well, this is more a technical detail while we continue to harp about 'sync' vs 'non-sync'.
Currently all instances of ->create_ctrl() are running asynchronously,
ie ->create_ctrl() returns a 'ctrl' object which is still in the process
of establishing the connection.
(And there it doesn't really matter whether it's FC or TCP/RDMA; FC is kicking of a workqueue for the 'reconnect' call, whereas TCP/RDMA is creating the association and issues the actual 'connect' NVMe SQE via
an I/O workqueue; net result is identical).
And when we talk about 'sync' connect we are planning to _wait_ until
this asynchronous operation reaches a steady state, ie either after the connect attempts succeeded or after the connect retries are exhausted.

And yes, we _are_ aware that this might be a quite long time.

Right now max_reconnects is calculated by the ctrl_loss_tmo and reconnect_delay. So there's already a way via the cli to make sure there's only 1 connect attempt. I wouldn't mind seeing an exact cli option that sets it to 1 connection attempt w/o the user calculation and 2 value specification.

Again, we do _not_ propose to change any of the default settings.
The 'sync' option will not modify the reconnect settings, it will just wait until a steady state it reached.

I also assume that this is not something that would be set by default in the auto-connect scripts or automated cli startup scripts.

You assume correctly. That's why it'll be an additional option.

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux