Hi Sara

Thank you for your reply and the PRs.

Some more comments below, look for EV> 

Regards

-éric


On 06/01/2022, 14:30, "dns-privacy on behalf of Sara Dickinson" 
<[email protected] on behalf of [email protected]> wrote:

    Hi Eric, 

    Many thanks for the review. I’ve created PR 
https://github.com/huitema/dnsoquic/pull/134 with proposed updates from your 
comments.

    > On 10 Dec 2021, at 06:42, Eric Vyncke (evyncke) 
<[email protected]> wrote:
    > 
    > Dear authors,
    > 
    > In the revised I-D, please also consider Martin Duke (the transport AD)'s 
WGLC comments, see the thread at
    > 
https://mailarchive.ietf.org/arch/msg/dns-privacy/F35q1jpUEPPqaq5gYVM8aYFoQUQ/
    > 
    > Especially around section 10.2 and the demux of DTLS & QUIC if both 
protocols run on the same port. Martin has suggested some text for this part, 
let's use it and remove any potential friction in the publication process.

    PR for this approved and merged (thanks Martin!)

    > 
    > Regards
    > 
    > -éric
    > 
    > 
    > 
    > On 09/12/2021, 14:40, "Eric Vyncke (evyncke)" <[email protected]> wrote:
    > 
    >   As usual, when a document is submitted for publication, as the 
responsible AD, I do my own review of the draft before going to the IETF Last 
Call.
    > 
    >   First, thank you Brian for a detailed doc shepherd write-up. Your 
points about the 'down-ref' are valid and for now I would leave them like they 
are.

    Yes - thank you Brian. 

    FYI: 
    - PR https://github.com/huitema/dnsoquic/pull/135 includes a change to make 
RFC8094 informative and 
    - PR https://github.com/huitema/dnsoquic/pull/132 provides updated text 
based on Alex’s review also making RFC8467 informative.

    > 
    >   Thanks, as well to the WG and the authors Christian, Sara, and Alisson 
for their work. The document is well written and easy to read: congratulations !
    > 
    >   Please find below my review comments:
    > 
    >   Abstract: should it state where DoQ could be used (i.e., between which 
DNS functions notably by citing XFR) ?

    Yes - good point. I’ve added text.

    > 
    >   Section 1: s/ this document is intended to specify/ this document 
specifies/ ?
    > 
    >   Section 2: please use the real template from BCP14 (section 2 of RFC 
8174)

    Ack to both. 

    > 
    >   Section 5: should there be a sentence "this section is normative" to be 
consistent with a similar sentence in section 4 ?

    Would it be better to clarify the text in section 4?

    “Whilst all other sections in this document are normative, this section is 
informative in nature."

EV> good suggestion, this is better with your text above

    > 
    >   Section 5.1.2: isn't the 3rd § obvious ? Hence could it be removed ?

    Yes - I suppose it is an implementation detail, removed.

    > Unsure whether there is value in the last §, especially as it appears to 
contradict the previous ones.

    This was added in response to having read *multiple* papers/presentations 
on DoT vs DoH where a statement like ‘DoT must run on port 853 whereas DoH runs 
on port 443’ is made regarding deployment obstacles for DoT. For a wider 
audience it does seem useful to point out that 443 is a possibility for DoQ 
deployments.

EV> fair enough

    > 
    >   Section 5.3: should DOQ_REQUEST_CANCELLED also be available when the 
server wants to close a transaction (e.g., when there are multiple 
responses/XFR) ?

    Well, section 5.3.2 really deals with the server side having issues and 
using a stream reset + DOQ_INTERNAL_ERROR to signal this. I’m not sure when a 
server would choose to send DOQ_REQUEST_CANCELLED as the error here as opposed 
to DOQ_INTERNAL_ERROR since the only reason to not send all the responses would 
be an error of some kind (even for XFR)… or is there another use case I’m 
missing?

EV> you are not missing anything, my bad on my comment :-O

    > 
    >   Section 6.3: contains two SHOULD, the general expectation is that the 
document describes when it is acceptable not to follow the SHOULD. Or perhaps 
should RECOMMENDED be used ?

    The kind of reasons I always assume are behind these kind of SHOULDs are: 
is an API for this exposed, does it increase code complexity too much or does 
it add too much performance overhead, etc. I’m not sure we have a specific case 
for section 6.3 but Christian may be able to point to one.

EV> you and Christian will have the final say on this comment.

    > 
    >   Section 6.4: same comment as above and also in other places.

    6.4 is proposed to change to a MUST for the use of padding in PR 
https://github.com/huitema/dnsoquic/pull/132. The main reason I can see for not 
using a QUIC padding API if it existed would be code complexity e.g. the 
implementation may choose to re-use padding logic already implemented in the 
DNS layer for DoT/DoH. I can add text about this if you think it is useful?

EV> adding some text about this would be helpful IMHO but not mandatory

    > 
    >   Section 6.7: should this document formally update RFC 1995, 5936, 7766 
? I think so as it is similar to RFC 9103, which updates those 3 RFCs.

    Good question but I don’t think so - all of those documents are specific to 
DNS over TCP. This document doesn’t define any new behaviour for XFR that 
changes how XFR over TCP behaves (which RFC9103 did) nor does it update 
anything in RFC7766.

EV> OK, let's wait and see whether IETF Last Call / IESG evaluation bring this 
topic back

    > 
    >   Section 7.1: s/ To our knowledge/To the authors' knowledge/ ?

    Agreed.

    >   Also in " it performs well compared" does it mean "better" or "similar" 
?

    Adguard haven’t published exact data yet but did say in a presentation  “it 
seems that…. it does provide advantage over DoH in cellular data networks, as 
expected” and their user feedback "ranges from very positive to neutral”.  I’m 
reluctant to declare it ‘better’ without more raw data….

EV> in this case "better" would be wrong indeed but doesn't "well" imply 'good' 
? I.e., "similar" could be better ? On this one, you are the native English 
speaker so I let you decide ;-)

    > 
    >   Section 9.3: " due to the prevalence of NAT" is not the only cause as 
IPv6 nodes often change their IPv6 addresses. Please extend the text here as 
the DoQ may not have an API to check whether the IPv6 address has changed.

    Text added. 

    > 
    >   Finally, while I am not a native English speaker (but I relied on the 
Microsoft Word spell-checker on the attached document), I think that there are 
some nits in the text:
    >   - Missing a lot of '-' in some constructions: "general purpose 
transport", "server initiated transactions", "long term state ", ...
    >   - "however" should be followed by a comma
    >   - No "n" in "an" in " an unidirectional QUIC stream"
    >   - "acknowledgeemnts"
    >   - "Provisonal"

    All fixed, I hope (but please check as I had to use a different checker 
tool).

    Best regards

    Sara.


    _______________________________________________
    dns-privacy mailing list
    [email protected]
    https://www.ietf.org/mailman/listinfo/dns-privacy

_______________________________________________
dns-privacy mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/dns-privacy

Reply via email to