There are a couple of issues in the nvmeof spec that conflict with removing the
guarantee - namely - can a value other than 1 (or 0) be returned from
the connect fabric command for sqhd ?  

I think the Connect command response tells the host exactly whether it's 1 or 0.

   Figure 21: Connect Response
   SQ Head Pointer (SQHD): Indicates the current Submission Queue Head pointer for the associated Submission Queue.
  [Fabrics 1.0]

 It can be argued that our implementation today, which returns 1, which assumes an increment after
 connect at idx 0 is actually incorrect to the spec as the queue is to be created as part of connect and
 it is to be "empty" with sqhd=sqtail=0.

I agree that this is an assumption, and I think a number of problems arise from the fact that what we have today
did not implement the spec when it comes to the Send Q head pointer.

A poor mans implementation, which ignores this initial connect response issue in the spec, and tries to avoid dedicated
resources, would live with allowing a storm of 1 sqsize worth per queue, expecting that it won't really happen in a complete 
burst, and sqhd can be controlled going back to the host such that further io can't be sent beyond 1 sqsizes's worth unless
the controller moves sqhd.

Yes,  NVMe allows the controller to support a logical queue depth (MAXCMD) and a physical
queue depth (CAP.MQES).  So if proper support for the SQHD updates can be implemented, it
will solve a number of "resource problems" with both Fabrics and PCI devices.  There is no need
for the controller to allocate a huge MQES simply to support a large logical queue depth.  In fact, 
the minimum IO queue depth (MEQS) is 2.  

So, as James describes above,  it should be possible for controllers out there to reduce the resources
needed to provision NVMe queues, without reducing their actual queue depth.   But it all depends
upon properly supporting the SQHD field in the CQE.   

quoted

See Hannes' patch description, he talks about QoS, not a resource
 problem.

So, the patch description may be a misnomer.  This is an RFC, and I think the real
Issue here is our current treatment of the SQHD.  QOS is just one use case that may
benefit from properly supporting the queue mechanics specified by the NVMe specification.

We argued about this at Vault, and there needs to be some clarification w.r.t. NVMe spec., but
assuming the assertion that the implementation we have today is not compliant with the NVMe
spec, I think this RFC is an important patch.
  

quoted

 I am arguing that rate-limiting a specific host can be done today.

I  disagree. It can't, because it was never implemented.  I know this was the idea, at least with FC, 
but there is no end to end credit management system that can be used to solve this problem
in any of the transports.  So this means all of IOs, and all of the problems, end up in the controller.

quoted

quoted

Current nvme/nvmeof model requires memory for entire sq size (well maybe
sqsize -1) so that the host can send a sqe and there's a place to put it
on the target.

Actually, I don't think this is true.  As mentioned above, the current NVMe
spec allows the controller to support a huge logical queue size with a small
sq size. 
 

quoted

That is not the case for either TCP or RDMA. TCP can not read from the
socket, 

Right, and that's exactly the kind of hack that leads to IO timeouts and
controller resets.  This is not an effective way to support  rate limiting or QOS. 

quoted

and  RDMA can not post a receive buffer, which will cause the
HW to issue rnr nak (receive not ready), and the host would be
effectively throttled. 

I disagree.  This RNR/NACK solution assumes SRQ, and it is not
a viable solution for QOS. It suffers from the same problems that
dropping IP packets on the floor does.  There is no way for
the host to figure out what the problem is and you just end up
with IO timeouts and controller resets.

quoted

So there are no such memory requirement (not for PCIe either).

If you are saying that the PCI controller can run with a send queue
of 2, I agree.  But to make that solution work, the host needs to 
support SQHD update correctly.

     Figure 69: Offset 0h: CAP – Controller Capabilities
     Maximum Queue Entries Supported (MQES): This field indicates the maximum individual queue size that the controller supports.
     For NVMe over PCIe implementations, this value applies to the I/O Submission Queues and I/O Completion Queues that the host creates.
     For NVMe over Fabrics implementations, this value applies to only the I/O Submission Queues that the host creates. 

 This is a 0’s based value. The minimum value is 1h, indicating two entries.

     4.1 Submission Queue & Completion Queue Definition
     The controller uses the SQ Head Pointer (SQHD) field in Completion Queue entries to communicate new values of the Submission Queue Head 
     Pointer to the host. A new SQHD value indicates that Submission Queue entries have been consumed, but does not indicate either execution or
     completion of any command. Refer to section 4.6.
     [NVMe 1.4]

Ultimately this all boils down to who's queue is going hold the IO?   The assertion is, it's better to 
hold the IOs on the host's queue than in the controller's queue.  And the assertion is that the NVMe
protocol provided a mechanism to do that.  So we want to use it.

Otherwise NVMe is really no better that SCSI, SATA, SAS or any other protocol we've worked on over
the last 20 years.

/John

On 2/26/20, 7:52 PM, "Sagi Grimberg" [off-list ref] wrote:    
    
    >>> As per NVMe-oF spec sq flow control is actually mandatory, and we should
    >>> be implementing it to avoid the controller to return a fatal status
    >>> error, and try to play nicely with controllers using sq flow control
    >>> to implement QoS.
    >>
    >> Hannes,
    >>
    >> Can you please clarify why the individual transports aren't sufficient
    >> for this QoS feature you are talking about?
    >
    >
    > It's all about allocation of resources - whether mandated or not.
    
    See Hannes' patch description, he talks about QoS, not a resource
    problem. I am arguing that rate-limiting a specific host can be done
    today.
    
    > Current nvme/nvmeof model requires memory for entire sq size (well maybe
    > sqsize -1) so that the host can send a sqe and there's a place to put it
    > on the target.
    
    That is not the case for either TCP or RDMA. TCP can not read from the
    socket, and RDMA can not post a receive buffer, which will cause the
    HW to issue rnr nak (receive not ready), and the host would be
    effectively throttled. So there are no such memory requirement (not for
    PCIe either).
    
    > For a transport like FC, where there isn't dedicated
    > memory allocated for the sq and each sqe arrives independently, there's
    > a desire to guarantee only a small number of elements, then let the
    > controller adjust sqhd up to open more of a window to allow more
    > commands from a particular host.
    
    I can have shared resources with both RDMA, TCP (and PCIe) and
    implementing what you are saying is perfectly achievable.
    
    > When the subsystem is managing
    > multiple controllers to perhaps many hosts, they may be all sharing the
    > same set of receive buffers.
    
    This is not specific to FC, see nvmet-rdma usage of srq for example, and
    nvmet-tcp can have the same concept.
    
    This is becoming a resource constrained issue which is different from
    the patch description.
    
    > The controller would like to control the
    > ebb and flow of what controllers/what queues use the available resources
    > at any particular point in time based on host/controller QOS settings.
    >
    > As most arrays are limited in the amount of memory they can dedicate to
    > receive buffers/queues and as the rules to date require guaranteed
    > allocation of sq size, the controllers have to pick how much they scale
    > - how many associations/controllers vs how many queues per controller vs
    > sq size per queue that they allow.
    
    They should do that regardless, there is no point allowing lots of
    queues if effectively the array supports very little right?
    
    > The disablement of sqflow control
    > doesn't help the "guarantee", it actually confirms it. There are a
    > couple of issues in the nvmeof spec that conflict with removing the
    > guarantee - namely - can a value other than 1 (or 0) be returned from
    > the connect fabric command for sqhd ?  It can be argued that our
    > implementation today, which returns 1, which assumes an increment after
    > connect at idx 0 is actually incorrect to the spec as the queue is to be
    > created as part of connect and it is to be "empty" with
    > sqhd=sqtail=0.     A poor mans implementation, which ignores this
    > initial connect response issue in the spec, and tries to avoid dedicated
    > resources, would live with allowing a storm of 1 sqsize worth per queue,
    > expecting that it won't really happen in a complete burst, and sqhd can
    > be controlled going back to the host such that further io can't be sent
    > beyond 1 sqsizes's worth unless the controller moves sqhd.
    
    You lost me a bit, is the issue supporting QD=1 from a large number of
    queues?
    
    >> If we look at the transports landscape, each transport has a credit
    >> mechanism that can throttle bulk data transfers. In FC exchanges the
    >> target is in control pulling data from the host with xfer_ready,
    >> In RDMA the target decides when to issue rdma_read, and in TCP the
    >> target decides when to issue R2T.
    >
    > it's not data flow, it's reception of io commands vs sq.
    >
    >>
    >> These are all credits that give the control to the target to
    >> back-pressure the host. Now if the target doesn't want the host to send
    >> more commands, it can throttle sending completions thus controlling the
    >> pace.
    >
    > that doesn't help the system. It only makes the io look like it takes a
    > lot longer to complete.
    
    Exactly, they will take longer to complete, meaning the host won't issue
    more. Even if the host is not issuing the command, it is still
    effectively taking the same time to complete.
    
    > At some point, the completion has to go back.
    
    Yes.
    
    >> I must say that returning BLK_STS_RESOURCE for host managed SQ_HEAD is a
    >> bit awkward in my mind, but that just one's opinion, what do others have
    >> to say?
    >
    > it isn't a host-managed SQ_HEAD. it's a real implementation of
    > acknowledging the sq flow control that was originally spec'd.  We've
    > always ignored this in lieu of cheating and setting the blk-mq request
    > count, and hoped as we never sent less than a queue's worth there was
    > never a reason for the controller to need to not increment sqhd on a
    > 1by1 basis.    But the other issue with this is - we're wasting lots of
    > sq space on the controller. We have 4, 10, 100 sqs but only send 1 sq's
    > worth of io ?  why would a controller ever want to support a high queue
    > count except for a back-to-back attachment to a fixed number of hosts (1
    > ?).
    
    I agree, the controller can tell the host how many queues and max
    queue depth, it can prevent the host from allocating lots of long
    queues.
    
    > That's not reasonable for a real SAN device - so we're forcing them
    > into small queue counts and small sq sizes so they don't waste memory
    > yet can handle bunches of associations.
    
    Sounds like this makes sense.
    
    > So in addition to this sqhd
    > tracking that Hannes was proposing, for FC at least, as sq's are
    > logical,
    
    You describe this sqhd throttling as something that will happen a lot,
    and in normal working points, so returning BLK_STS_RESOURCE having
    blk-mq retrying over and over doesn't seem like an adequate solution
    in my mind...
    
    > I was also going to look into supporting more than 1 sq's worth
    > of ios.
    
    What do you mean? where is more than 1 sq's worth of ios not supported?
    You talking about for a single queue? Maybe I'm missing something...
    

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme