Messages in this thread | | | Date | Wed, 10 Apr 2024 12:06:17 +0200 | From | Daniel Wagner <> | Subject | Re: [PATCH v5 3/6] nvme-tcp: short-circuit reconnect retries |
| |
On Tue, Apr 09, 2024 at 11:40:03PM +0300, Sagi Grimberg wrote: > > +/* > > + * Evaluate the status information returned by the LLDD > > We usually say transport, less so LLDD.
Updated the documentation accordingly.
> > + * 1) The LLDD reports an negative status. There was an error (e.g. no > > + * memory) on the host side and thus abort the operation. > > + * Note, there are exception such as ENOTCONN which is > > + * not an internal driver error, thus we filter these errors > > + * out and retry later. > > What is this ENOTCONN here? From what I see its just a random > escape value to bypass the status check. I think that 0 would do the same > wouldn't it?
ENOTCONN is issued by the reset handler. Sure, 0 will work as well, so if the consense is that the reset handler should do that. see below.
> > - if (nvmf_should_reconnect(ctrl)) { > > + if (recon && nvmf_should_reconnect(ctrl)) { > > its weird to have a condition that checks _and_ condition > of two different "should reconnect" conditions. > > Why don't we simply pass the status to nvmf_should_reconnect() > and evaluate there?
Sure, makes sense.
> > static void nvme_tcp_error_recovery_work(struct work_struct *work) > > @@ -2295,7 +2300,7 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work) > > return; > > } > > - nvme_tcp_reconnect_or_remove(ctrl); > > + nvme_tcp_reconnect_or_remove(ctrl, -ENOTCONN);
Here is the source of ENOTCONN.
| |