> 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? > > 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 <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? > > 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 > > 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. > > 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