[DNSOP] Eric Rescorla's No Objection on draft-ietf-dnsop-dns-capture-format-08: (with COMMENT)

[Eric Rescorla <[e-mail]>]

Eric Rescorla has entered the following ballot position for
draft-ietf-dnsop-dns-capture-format-08: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-dnsop-dns-capture-format/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D4670


I support Ben's DISCUSS.

Am I understanding the design correctly in that you can't have
literals in the individual per-query values, but instead references to
the block tables, so you can't stream inside a block?

IMPORTANT
S 7.5.1.
>      | earliest-time    | O | A | A timestamp (2 unsigned integers,      |
>      |                  |   |   | "Timestamp") for the earliest record   |
>      |                  |   |   | in the "Block" item. The first integer |
>      |                  |   |   | is the number of seconds since the     |
>      |                  |   |   | Posix epoch ("time_t"). The second     |
>      |                  |   |   | integer is the number of ticks since   |

I don't know what a "tick" is, so you need some definitionf or this.

COMMENTS
S 7.2.
>   
>      o  The column O marks whether items in a map are optional.
>   
>         *  O - Optional.  The item may be omitted.
>   
>         *  M - Mandatory.  The item must be present.

FWIW, I think you might be happier with a mandatory "X" and optional
nothing, but that's up to you.


S 7.4.1.1.1.
>   
>      +------------------+---+---+----------------------------------------+
>      | Field            | O | T | Description                            |
>      +------------------+---+---+----------------------------------------+
>      | query-response   | M | U | Hints indicating which "QueryResponse" |
>      | -hints           |   |   | fields are omitted, see section        |

Nit: generally, you would say "indicating which fields are included"
if 1 means included.


S 7.5.3.
>      +---------------------+---+---+-------------------------------------+
>   
>   7.5.3.  "BlockTables"
>   
>      Arrays containing data referenced by individual "QueryResponse" or
>      "MalformedMessage" items in this "Block".  Each element is an array

This is not a sentence.


S 7.5.3.
>      | qrr               | O | A | Array of type "Question". Each entry  |
>      |                   |   |   | is the contents of a single question, |
>      |                   |   |   | where a question is the second or     |
>      |                   |   |   | subsequent question in a query. See   |
>      |                   |   |   | Section 7.5.3.3.                      |
>      |                   |   |   |                                       |

So if I understand this correctly:

QRR is a map of integers to question
QLIST is a map of integers to question lists, with each question list
consisting of a set of indexes int o QRR?



S 7.5.3.2.
>   
>      +--------------------+---+---+--------------------------------------+
>      | Field              | O | T | Description                          |
>      +--------------------+---+---+--------------------------------------+
>      | server-address     | O | U | The index in the item in the "ip-    |
>      | -index             |   |   | address" array of the server IP      |

So am I understanding correctly that you can't put the server address
literally in here. It has to be in the block tables?


S 7.5.3.2.
>      +--------------------+---+---+--------------------------------------+
>      | server-address     | O | U | The index in the item in the "ip-    |
>      | -index             |   |   | address" array of the server IP      |
>      |                    |   |   | address. See Section 7.5.3.          |
>      |                    |   |   |                                      |
>      | server-port        | O | U | The server port.                     |

Isn't the server port generally constant? It seems like you might save
some bits by having this attached to the server and then indixed
abvoe.



S 7.5.3.2.
>      |                    |   |   | 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       |

You might want to specify DoH


S 7.5.3.5.
>      |                    |   |   | 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.             |

Again, probably want to specify DoH.


S 17.3.
>      [18] https://github.com/dns-stats/draft-dns-capture-
>           format/blob/master/file-size-versus-block-size.svg
>   
>   Appendix A.  CDDL
>   
>      This appendix gives a CDDL [I-D.ietf-cbor-cddl] specification for

Is this a normative appendix?