Skip to main content

Last Call Review of draft-ietf-cdni-logging-15
review-ietf-cdni-logging-15-genart-lc-thomson-2015-02-12-00

Request Review of draft-ietf-cdni-logging
Requested revision No specific revision (document currently at 27)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2015-02-18
Requested 2015-02-04
Authors François Le Faucheur , Gilles Bertrand , Iuniana Oprescu , Roy Peterkofsky
I-D last updated 2015-02-12
Completed reviews Genart Last Call review of -15 by Martin Thomson (diff)
Opsdir Early review of -11 by David Harrington (diff)
Opsdir Telechat review of -18 by Benoît Claise (diff)
Secdir Last Call review of -15 by Klaas Wierenga (diff)
Assignment Reviewer Martin Thomson
State Completed
Request Last Call review on draft-ietf-cdni-logging by General Area Review Team (Gen-ART) Assigned
Reviewed revision 15 (document currently at 27)
Result Almost ready
Completed 2015-02-12
review-ietf-cdni-logging-15-genart-lc-thomson-2015-02-12-00
I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-cdni-logging-15
Reviewer: Martin Thomson
Review Date: 2015-02-12
IETF LC End Date: 2015-02-18
IESG Telechat date: (if known)

Summary:

Major issues:

Minor issues:

S3.1 needs to define the basic atom that is used for the logging file.
It appears as though the atom is an octet.

S3.2 uses the wildcard ABNF rule (i.e., <text....>) too aggressively.
RECLINE (a potentially confusing name) could be more strictly defined.

    <CDNI Logging File> = 1*<DIRGROUP RECGROUP>

... is not valid and could be:

    CDNI-LOG-FILE = 1*(DIRGROUP / RECGROUP)

That implies that an empty file is not valid.  I'd have thought that *
rather than 1* would be better.

S3.3 again uses wildcard rules too aggressively.  DIRNAME (not the
unix command) should be defined more explicitly, even if it is just to
specify what characters are allowed.  Use text to further constrain
it.  As it stands, a generic processor cannot know that the name
doesn't permit the inclusion of (for instance) ":" or CRLF.  A common
method is to define something like this as:

    DIRNAME = Version-directive / UUID-directive / ... / extension-directive

More commonly, go up a level and define directive using the choice.
That way, each directive can define a name and value together using
ABNF.  e.g.

    Version-directive = "Version" ":" HTAB "CDNI" "/" 1*DIGIT "." 1*DIGIT

I wouldn't be so prescriptive about the version format.  Unless there
are specific semantics you want to extract from each digit.

What is a receiver expected to do if the version *isn't* CDNI/1.0 ?
Discard the entire file?  That doesn't seem like a good way to ensure
forward compatibility.

UUID: this a Universally Unique IDentifier (UUID)
         from the UUID Uniform Resource Name (URN) namespace specified
         in [RFC4122]) for the CDNI Logging File.

I don't know how to apply this.  Is it the UUID portion of the URN or a URN?

Please don't use MUST here: If the
         two values are equal, then the received CDNI Logging File MUST
         be considered non-corrupted.

MD5 collisions are easy to manufacture.  I'd also drop the MUST from
the following sentence.

S3.4: more wildcards

S3.4.1: I wouldn't worry about HTTP/2 being special here.  All that
speculation is painful, especially since HTTP/2 will likely be
published before this document.  Better to just cite it and avoid
getting caught up in the details.




Nits/editorial comments:

There are lots of uses of lowercase "may".

Page 6 has a few instances of missing hyphens in dCDN3/dCDN2 etc...
S6.2.1: chunck

S3.1 the DATE/TIME rules can/should reference RFC 3339

S3.3 using ":" HTAB as a separator makes it hard to construct these
files manually.

S3.4 use of "c:" in combination with the unordered list is confusing
when the prefix is actually "c-".  Maybe use the string "c-" quotes
and all to denote the prefix.

S3.4.1 citing RFC 7230 is sufficient for HTTP/1.1; and probably for
HTTP in general at this moment.