Re: Genart telechat review of draft-ietf-dnsop-session-signal-11

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

 



In line. The general point is that the document should be clear to readers who understand the space but do not live it at the detail of those who authored it.

Joel

On 7/5/18 6:13 PM, Ted Lemon wrote:
Joel, it's immaterial whether the DSO engine responds in time or not. If it responds in time, the ack and the response will be combined; if it does not, then Nagle's algorithm will ensure that the ack goes out, and the response will go out in a later packet.   Either outcome is fine. There is no need to caution the implementor that they should ensure a quick response—if they don't care to get the response out in 200ms, they obviously don't care about performance, and that's perfectly fine.   It is absolutely /not/ a requirement that they do so.   The point of this text in the document is to inform implementors that /do/ care about performance about the interaction between Nagle and DA.

The text says that combining will happen. I am well aware that if the processing is in time, that is normal processing. But that is not what the text says. It would take about a sentence to fix the text to match what you describe above.


We looked at your comment about middleboxes and couldn't figure out what problem you are trying to solve here.   If a middlebox is not DSO-aware, it's going to prevent DSO from working (which would be correct), or else forward it unchanged (which would also be correct).   The text is an admonishment for implementations that are DSO-aware.   If an implementation is not DSO-aware, then adding text to instruct the implementor, who presumably will not read it, doesn't make sense.

The text says that middleboxes MUST NOT blindly forward DSO messages. Your text above says that actually it doesn't matter, and no, existing middleboxes will not comply with this MUST NOT. A MUST NOT in a document is generally a statement that things will break if they behave wrong. Thus, what the document says, combined with your behavioral description, is that existing middleboxes will break DSO. I doubt that (which is why I consider this minor rather than major.) Please fix the text to either not require changes to existing middleboxes or to explain to readers why and how existing middlebox will comply with the MUST NOT.



Your proposed change to 5.2.2 seems fine to me—I don't remember what happened with that.

On Thu, Jul 5, 2018 at 5:48 PM, Joel Halpern <jmh@xxxxxxxxxxxxxxx <mailto:jmh@xxxxxxxxxxxxxxx>> wrote:

    Reviewer: Joel Halpern
    Review result: Ready with Nits

    I am the assigned Gen-ART reviewer for this draft. The General Area
    Review Team (Gen-ART) reviews all IETF documents being processed
    by the IESG for the IETF Chair. Please wait for direction from your
    document shepherd or AD before posting a new version of the draft.

    For more information, please see the FAQ at

    <https://trac.ietf.org/trac/gen/wiki/GenArtfaq
    <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>>.

    Document: draft-ietf-dnsop-session-signal-11
    Reviewer: Joel Halpern
    Review Date: 2018-07-05
    IETF LC End Date: 2018-06-25
    IESG Telechat date: 2018-08-02

    Summary: This document is ready for publication as a Proposed
    Standard RFC

    Some of my earlier comments have been addressed.  It appears that an
    effort was
    made to address more, but I was apparently unclear.  I have copied
    the comments
    that seem to still apply, with elaboration.  If I am still unclear,
    please
    contact me.

    Major issues: N/A

    Minor issues:
         Section 5.1.3 places some requirements on application level
    middleboxes,
         and includes a very clear explanation of why it places these
    requirements.
         While it may be "obvious" to one who lives and breathes DNS, I
    think it
         would help to explain why the usual operation of an existing
    middlebox will
         (typically? always?  inherently?) meet this requirement.  To
    rephrase, the
         text says things like "the middlebox MUST NOT blindly forward
    DSO messages
         in either direction." Apparently, somehow, the existing world
    middleboxes
         will do comply with this.  How?

         The third and fourth paragraphs of section 5.2.2 do not talk
    about optional
         additional TLVs.  It would be helpful if the document stated
    that in
         addition to those additional TLVs required by the primary TLV,
    other TLVs
         may be included based on their individual definition,
    independent of the
         definition of the primary TLV.  (Both the Encryption padding
    and the delay
         retry TLVs may be included in suitable messages without being
    called out in
         the definition of the primary TLVs.)
         An effort appears to have been made to address this, which
    suggests I was
         unclear.  The text says:
             A DSO response message may contain no TLVs, or it may be
    specified to
             contain one or more TLVs appropriate to the information being
             communicated.
         The definition of the specific response messages does not
    discuss the
         encryption padding or delay response TLVs.  They are clearly
    intended to
         be allowed.  So can we tune the text to make that clear.  I
    think the
         intention is that the specification of the response message
    indicates which
         TVLs are required, and that others are allowed.  So say that.

    Nits/editorial comments:
         Section 5.4 talks about by default the TCP data ack and the DSO
    reply
         message being combined.  Doesn't this depend upon the
    responsiveness of the
         DSO engine?  Is there an implicit assumption about such
    timeliness (sub 200
         ms)?
         I suspect from the lack of comment on this that I am missing
    something
         obvious?







[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux