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
signature.asc
Description: Message signed with OpenPGP using GPGMail
------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________ Nbd-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
