Eric, Agree with the nits - many of them were from the mailing list message which of course I then didn't check before copying into the commit message. Re the substance: >> +* Transmission mode can be entered (by the client sending >> + `NBD_OPT_EXPORT_NAME` or by the server responding to an >> + `NBD_OPT_GO` with `NBD_REP_ACK`). This is documented > > s/ACK/SERVER/ yep > (although I may bite the bullet and create a new NBD_REP_INFO if we want > the name to be optional, since NBD_OPT_[INFO/GO] is still experimental, > as part of my rework on block size information) indeed, but that's orthogonal. >> +A client MAY use a soft disconnect to terminate the session >> +whenever it wishes, provided that there are no outstanding >> +replies to options. > > Why the disclaimer on no outstanding replies? See reply to Wouter who made the same point. Let's handle that there. >> +terminate the session. In the client's case, if it wishes to >> +do so it MUST use soft disconnect. In the server's case it >> +MUST (save where set out above) simply error inbound options until >> +the client gets the hint that it is unwelcome. > > so basically wait for either the client to give up and close first, or > for the client to do something that is provably in violation of a MUST > in the protocol so the server can close the connection. Can a malicious > client abuse this requirement to tie up a server as a denial of service? Good point. I think we should give the server the right to disconnect in a DoS situation. This is a bit like DoS protection for TCP violating the TCP spec though. >> +On a server shutdown, the server SHOULD wait for inflight >> +requests to be serviced prior to initiating a hard disconnect. > > Maybe a mention that the server MAY use error replies to speed up the > processing of those requests, even if the command would normally succeed > if termination weren't pending? +1, and as Wouter suggested, use a different reply. >> +The client MAY issue a soft disconnect at any time, but >> +MUST wait until there are no inflight requests first. > > Why MUST and not SHOULD? Didn't Wouter have an example of a client that > batches up its entire request sequence, including NBD_CMD_DISC, and > sends that in bulk before waiting for any server replies? I thought the > goal was that the server MUST NOT react to NBD_CMD_DISC until all other > pending requests have been dealt with, but don't necessarily see the > reason why the client MUST NOT send NBD_CMD_DISC while requests are > inflight. The issue is that the server MAY process requests out of order. I thus think such a client is foolhardy as the server MAY process the NBD_CMD_DISC first. It depends whether that 'processing' includes 'waiting for all the other commands'. Again, see my reply to Wouter - this is definitely an area we need to sort out. I agree though that MUST is too strong. I think I perhaps I'd say it may send it if it wishes, but should remember the server can process replies out of order. >> - `NBD_OPT_ABORT` (2) >> >> - The client desires to abort the negotiation and close the >> - connection. >> + The client desires to abort the negotiation and terminate the >> + session. The server MUST reply with `NBD_REP_ACK`. > > Maybe explicitly mention that the client MAY disconnect immediately > rather than waiting to receive the response? I'm saying the client MUST wait. But if it doesn't (meaning only clients will be non-conformant) nothing is lost, particular as this is only at option haggling stage. So it's more (as Wouter said) "be aware that there may old non-compliant clients that will not wait". -- Alex Bligh
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail