Hi Sara, On Wed, Nov 21, 2018, at 2:36 PM, Sara Dickinson wrote: >> On 19 Nov 2018, at 20:37, Alexey Melnikov >> <aamelni...@fastmail.fm> wrote:> > Hi Alexey, > > Thanks for the review. > >> >> Alexey Melnikov has entered the following ballot position for >> draft-ietf-dnsop-dns-capture-format-08: Discuss >> --------------------------------------------------------------- >> ------->> DISCUSS: >> --------------------------------------------------------------- >> ------->> >> Thank you for this document, it is a useful contribution to RFC >> series. I>> enjoyed reading it. >> >> I have a small list of issues that is hopefully easy to fix: >> >> 1) >> >> In 7.4.2: >> >> | filter | O | T | "tcpdump" [pcap] style filter >> | for |>> | | | | input. >> | | | | |>> >> This makes the [pcap] reference Normative. If you don't want to do >> that, please>> fully specify syntax in this document. > > Is that true if it is an optional field? Yes, optionallity of a field doesn't make its full specification optional. > >> >> 2) I believe [I-D.ietf-cbor-cddl] reference needs to be Normative due >> to use of>> CDDL in Appendix A. If you don't think CDDL is normative, you >> need >> to state>> that in Appendix A. > > Yes indeed - it should be normative, will fix. > > >> >> --------------------------------------------------------------- >> ------->> COMMENT: >> --------------------------------------------------------------- >> ------->> >> Was CDDL in Appendix A validated with a tool? This information is >> missing from>> the shepherding write-up. > > We (the authors) have used the CDDL tool described on > http://cbor.io/tools.html to read the CDDL in Appendix A and ensured > an example instance can be created.> > Did you have some other validation tool in mind? No, this is fine. I just wanted to know.
> >> >> 6.2.3. Storage flags >> >> The Storage Parameters also contains optional fields holding >> details>> of the sampling method used and the anonymisation method >> used. >> It is>> RECOMMENDED these fields contain URIs pointing to resources >> describing the methods used. >> >> Please add a Normative Reference for URI spec here (RFC 3986). > > Yes, will do. > >> >> 7.5.3.2. "QueryResponseSignature" >> >> | qr-transport-flags | O | U | Bit flags describing the >> | transport |>> | | | | used to service the >> query. >> | | | | |>> | | | | Bit >> 0. IP version. 0 if IPv4, 1 if >> | | | | |>> | | | | IPv6 >> | | | | |>> | | | | Bit >> 1-4. Transport. 4 bit unsigned >> | | | | |>> | | | | >> value where 0 = UDP, 1 = TCP, 2 = >> | | | | |>> | | | | TLS, >> 3 = DTLS. Values 4-15 are >> | | | | |>> | | | | >> reserved for future use. >> | | | | |>> | | | | Bit >> 5. 1 if trailing bytes in query >> | | | | |>> | | | | >> packet. See Section 11.2. >> | | | | |>> >> Would something like DoH appear as a separate transport choice? > > No, we need to add DoH to this list (it didn’t exist when we started > the draft!).> >> >> How can new values be added to the list if there are no IANA >> Considerations?> > As described in response to the DISCUSS on this issue we propose IANA > create a C-DNS registry with> subregistries with keys for each of the > different maps used in C-DNS.> New entries in these subregistries would > follow Expert Review Ok, great. > >> >> 7.5.3.5. "MalformedMessageData" >> >> | | | | | >> | mm-transport-flags | O | U | Bit flags describing the >> | transport |>> | | | | used to service the >> query. Bit 0 is >> | | | | |>> | | | | the >> least significant bit. >> | | | | |>> | | | | Bit >> 0. IP version. 0 if IPv4, 1 if >> | | | | |>> | | | | IPv6 >> | | | | |>> | | | | Bit >> 1-4. Transport. 4 bit unsigned >> | | | | |>> | | | | >> value where 0 = UDP, 1 = TCP, 2 = >> | | | | |>> | | | | TLS, >> 3 = DTLS. Values 4-15 are >> | | | | |>> | | | | >> reserved for future use. >> | | | | |>> >> If the above bits supposed to be the same as for qr-transport-flags,>> maybe >> it is worth defining them once and referencing in all relevant >> places?> > The qr-transport-flags and mm-transport-flags are different in that > the qr-transport-flags include Bit 5, the trailing bytes indicator.> > In the CDDL a base ’TransportFlags’ type is defined and then > > mm-transport-flags => TransportFlags, > > qr-transport-flags => QueryResponseTransportFlags, > > QueryResponseTransportFlagValues = &( > query-trailingdata : 5, > ) / TransportFlagValues > QueryResponseTransportFlags = uint .bits > QueryResponseTransportFlagValues> > We can add some text to the table descriptions in sections 7.5.3.2 and > 7.5.3.5 to clarify the relationship.Thank you for your clarification. Your > explanation is sufficient. > >> >> 7.6. "QueryResponse" >> | query-size | O | U | DNS query message size >> | (see |>> | | | | below). >> | | | | |>> | | | | >> | >> | response-size | O | U | DNS query message size >> | (see |>> | | | | below). >> | | | | |>> >> I think "DNS response message size" for response-size. >> >> It is odd to have RFC 2119 language in B.2, which is itself >> informative.> > Good catch :-) > > Many thanks > > Sara.
_______________________________________________ DNSOP mailing list DNSOP@ietf.org https://www.ietf.org/mailman/listinfo/dnsop
