Re: [DNSOP] Benjamin Kaduk's Discuss on draft-ietf-dnsop-dns-capture-format-08: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Thu, Nov 29, 2018 at 03:43:20PM +0000, Sara Dickinson wrote:
> 
> > On 24 Nov 2018, at 03:58, Benjamin Kaduk <[e-mail] <mailto:[e-mail]>> wrote:
> > 
> > On Thu, Nov 22, 2018 at 12:01:00PM +0000, Sara Dickinson wrote:
> >>> 
> >>> Section 7.4.1.1.1
> >>> 
> >>> Am I parsing the "query-response-hints" text correctly to say that a bit is
> >>> set in the bitmap if the corresponding field is recorded (if present) by
> >>> the collecting implementation?  The causality of "if the field is omitted
> >>> the bit is unset" goes in a direction that is not what I expected.
> >>> (Similarly for the other fields in this table.)
> >> 
> >> ekr picked up on the same point - as responded to him:
> >> 
> >> "The issue is that if the bit is set the field might still be missing because although the configuration was set to collect it the data wasn’t available to the encoder from some other reason. However when the bit is not set it means that the data will definitely not be present because the collector is configured not to collect it. 
> >> 
> >> We do discuss this problem in section 6.2.1 - perhaps a reference in the table back to that discussion is what is needed?”
> >> 
> >> Looking again I think a slight update to the text in 6.2.1 might help too:
> >> 
> >> OLD:
> >> “The Storage Parameters therefore also contains a Storage Hints item
> >>  which specifies which items the encoder of the file omits from the
> >>  stored data."
> >> 
> >> NEW: “The Storage Parameters therefore also contains a Storage Hints item
> >>  which specifies which items the encoder of the file omits from the
> >>  stored data and will therefore never be present. (This approach is taken 
> >> because a flag that indicated which items were included for collection would 
> >> not guarantee that the item was present, only that it might be.) "
> > 
> > This text helps, but I think it is not quite what I was going after -- that
> > is, when I think of a "hint" that feels like something active and that
> > would be indicated by setting a bit to one.  In this design, the hints
> > about what are *omitted* are the bits that are *zero*, which is
> > counter-intuitive, at least to me.  So maybe we could say (in 7.4.1.1.1, in
> > addition to your suggested change in 6.2.1):
> > 
> > Hints indicating which "QueryResponse" fields are candidates for capture or
> > omitted, see section 7.6.  If a bit is unset, that field is omitted from
> > the capture.
> 
> Ah, ok I see the confusion now and yes - this text improves the draft - thank you!
> 
> > 
> >> 
> >>> 
> >>> Section 7.4.2
> >>> 
> >>> Do we need a reference for "promiscuous mode”?
> >> 
> >> Promiscuous mode is discussed on the main PCAP manpage…. Hopefully a way
> >> will be found to address the question of a suitable reference format for
> >> PCAP material.
> >> 
> >>> 
> >>> Just to check: in "server-addresses", I just infer the IP version from the
> >>> length of the byte string?
> >> 
> >> As mentioned in the DISCUSS response, we probably need to make the transport flags mandatory.
> >> 
> >>> 
> >>> Do we need to say more about where the vlan-ids identifiers are taken from?
> >> 
> >> Suggest: 
> >> 
> >> OLD: “ | vlan-ids         | O | A | Array of identifiers (of type unsigned |
> >>  |                  |   |   | integer) of VLANs selected for         |
> >>  |                  |   |   | collection. “
> >> 
> >> NEW: “ | vlan-ids         | O | A | User specified array of identifiers (of type unsigned |
> >>  |                  |   |   | integer) of VLANs  [IEEE 802.1Q] selected for         |
> >>  |                  |   |   | collection.  "
> > 
> > It seems likely to me that we want to say that the actual VLAN ID values
> > are only unique within an administrative domain.
> 
> OK - yes, makes sense.
> 
> > 
> >>> 
> >>> Is the "generator-id" string intended to only be human readable?  Only
> >>> within a specific (administrative) context?
> >> 
> >> The generator ID is intended only to identify the collecting
> >> application. Specifying that it is human-readable (if present) seems a
> >> good idea. Would this be sufficient?
> >> 
> >> OLD: "String identifying the collection method.”
> >> NEW: “User specified human-readable string identifying the collection method."
> > 
> > Does "user-specified" mean that only the user is responsible for reading it
> > later (or would we want it to make sense even when the data is conveyed to
> > some other party)?
> 
> Yes - that’s correct. Maybe 'implementation specific' is better?

I think that's more explicit about what scope we should expect.
(But of course this is all in the non-blocking comment section, so your
judgment takes precedence over mine.)

> > If so, this would be enough for to address my comment, but then Ben's
> > comment about internationalization concerns would come into play.
> 
> Sorry - I missed that comment - could you clarify? I’m not sure how I see this is any different to any other (unicode) text string used in CBOR?

It's my turn to apologize; I am probably thinking of a different document.
I'll try to double-check, but it should be safe to assume there's not an
issue here.

> > 
> >>> 
> >>> Section 7.5.1
> >>> 
> >>> Does "earliest-time" include leap seconds?
> >> 
> >> Thanks for noticing this…after digging into it…
> >> 
> >> The description specifies the number of seconds to be the
> >> number of seconds since the POSIX epoch ("time_t"). POSIX requires that
> >> leap seconds be omitted from reported time, and all days are defined as
> >> having 86,400 seconds. This means that a POSIX timestamp can be
> >> ambiguous and refer to either of the last 2 seconds of a day containing
> >> a leap second (who knew time could stand still in POSIX world - aargh?!) 
> >> 
> >> However, libpcap (for example) can only provide POSIX timestamps for 
> >> packets as far as we can see… 
> >> 
> >> Do you think we should just document this as a limitation or do you have 
> >> another option in mind?
> > 
> > To be honest, I was only expecting "number of seconds since the POSIX epoch
> > ("time_t", excluding leap seconds)" or "number of seconds since the POSIX
> > epoch ("time_t", including leap seconds)".  My concern is just that we
> > state how to interpret the number in this field; choosing whichever case
> > the common API provides is fine, and we don't need to document it as a
> > limitation at all.  If someone needs to convert between TAI and UTC, we
> > give them enough information so that they can do it, but otherwise it's not
> > our problem.
> 
> We are happy with just adding the ‘excluding leap seconds’ qualifier here if that work for you :-)

That works great for me :)

Many thanks for these updates,

Benjamin

> <snip>
> 
> > 
> >>> 
> >>> Section 7.7
> >>> 
> >>> How is the value of the "ae-code" determined?
> >> 
> >> "ae-code" is intended to hold the ICMP or ICMPv6 code. We suggest making
> >> this clearer:
> >> 
> >> OLD: "A code relating to the event."
> >> NEW: "A code relating to the event. For ICMP or ICMPv6 events, this
> >> should be the ICMP [RFC792] or ICMPv6 [ RFC4443] code."
> > 
> > I think we need to say that the contents are undefined (or only locally
> > defined) in other cases.  But this new text is a big step forward, thanks!
> 
> Right, I see the point now. We’ll add that. 
> 
> Thanks and regards
> 
> Sara. 
> 
> 
> 
>