Re: [Gen-art] genART review of draft-ietf-cdni-logging-15

"Francois Le Faucheur (flefauch)" <flefauch@cisco.com> Mon, 09 March 2015 17:13 UTC

Return-Path: <flefauch@cisco.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E919D1A909B for <gen-art@ietfa.amsl.com>; Mon, 9 Mar 2015 10:13:04 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -13.911
X-Spam-Level:
X-Spam-Status: No, score=-13.911 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, J_CHICKENPOX_31=0.6, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gbjLLOnmre5r for <gen-art@ietfa.amsl.com>; Mon, 9 Mar 2015 10:13:03 -0700 (PDT)
Received: from rcdn-iport-4.cisco.com (rcdn-iport-4.cisco.com [173.37.86.75]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C53C21A9054 for <gen-art@ietf.org>; Mon, 9 Mar 2015 10:13:02 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=9410; q=dns/txt; s=iport; t=1425921182; x=1427130782; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=fOwgw8bmCWc+gXK3jfitRI00K+SfFiCpIN1ChKltqwY=; b=M5Fz6uYh/r+zqICJLwv+n01kezZ7JezQRjWI2Yy5bEdy/4+A1PSGFp4s eC2MBpS2AHxPlHLM2TvPBJQs0tlX8y/ZwqhWm/ZCydGYPe6tEc7GGAUj2 nX3eoEk96YOzbhjGVBgKaAwzS6YFI8M2K9HdU9T3tuHkCn0s5mcON1zZa w=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0AHDQC30/1U/5tdJa1cgwZSXoMGvSuIJgIcgQ5NAQEBAQEBfIQQAQEDASMRRQULAgEIGgImAgICMBUQAgQOiCwIpU2bMwEBAQEBAQEBAQEBAQEBAQEBAQEBAReBIYl2hDszBwSCZC+BFgWODYICiVOBGoMojzIjggIBG4FQgjN/AQEB
X-IronPort-AV: E=Sophos;i="5.11,368,1422921600"; d="scan'208";a="402163877"
Received: from rcdn-core-4.cisco.com ([173.37.93.155]) by rcdn-iport-4.cisco.com with ESMTP; 09 Mar 2015 17:13:02 +0000
Received: from xhc-rcd-x01.cisco.com (xhc-rcd-x01.cisco.com [173.37.183.75]) by rcdn-core-4.cisco.com (8.14.5/8.14.5) with ESMTP id t29HD1hI004568 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Mon, 9 Mar 2015 17:13:02 GMT
Received: from xmb-rcd-x10.cisco.com ([169.254.15.156]) by xhc-rcd-x01.cisco.com ([173.37.183.75]) with mapi id 14.03.0195.001; Mon, 9 Mar 2015 12:13:01 -0500
From: "Francois Le Faucheur (flefauch)" <flefauch@cisco.com>
To: Martin Thomson <martin.thomson@gmail.com>
Thread-Topic: genART review of draft-ietf-cdni-logging-15
Thread-Index: AQHQWoxMsF8c8rajIkOza8YGALyW4A==
Date: Mon, 09 Mar 2015 17:13:01 +0000
Message-ID: <76ADC0A7-9093-4722-9BE3-E27CFEA88F5A@cisco.com>
References: <CABkgnnU95w0SzS+PVZL0Lh0bCjfKr7DwYp0xajJ8-WBKnmao6w@mail.gmail.com>
In-Reply-To: <CABkgnnU95w0SzS+PVZL0Lh0bCjfKr7DwYp0xajJ8-WBKnmao6w@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.55.161.202]
Content-Type: text/plain; charset="utf-8"
Content-ID: <A052EDD4944E914A972D954130580335@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/iFbCQry-N8yqJ_zJz9jlfX5jqbE>
Cc: "Francois Le Faucheur (flefauch)" <flefauch@cisco.com>, "gen-art@ietf.org" <gen-art@ietf.org>, "draft-ietf-cdni-logging.all@tools.ietf.org" <draft-ietf-cdni-logging.all@tools.ietf.org>
Subject: Re: [Gen-art] genART review of draft-ietf-cdni-logging-15
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 09 Mar 2015 17:13:05 -0000

Hi Martin,

Getting onto you first batch of comments:

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

We have the following statement:
" A CDNI Logging File MUST contain a sequence of lines containing US-
   ASCII characters [CHAR_SET] terminated by CRLF.”
I see that as defining the atom as an ASCII character. 

> 
> S3.2 uses the wildcard ABNF rule (i.e., <text....>) too aggressively.

Can you give me the specifics (unless this refers to what you've expanded below already)

> RECLINE (a potentially confusing name) could be more strictly defined.

potentially confusing, but hopefully not really given the consistent structure across DIRLINE/DIRGROUP/RECLINE/RECGROUP.

> 
>    <CDNI Logging File> = 1*<DIRGROUP RECGROUP>
> 
> ... is not valid and could be:
> 
>    CDNI-LOG-FILE = 1*(DIRGROUP / RECGROUP)

I have changed it to :
"
CDNI Logging File = 1*<DIRGROUP RECGROUP>
"

This is to avoid defining an extra name (CDNI-LOG-FILE) and have to explain that CDNI-LOG-FILE is actually the CDNI Logging File.
Let me know if that is not valid.

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

A logging file with no Logging Records is considered valid and allowed, but a logging file without any directive is not considered valid.
This is consistent with the text that I added based on our previous discussion stating that “occurrences” for all directives need to be respected (in other words, some directives are mandatory).

> 
> 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’ll park these ABNF comments since I got many others and will handle them together.


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

Fair enough. I have relaxed it and simply made the format NHTABSTRING.

In the corresponding IANA section for the Version registry, I have added:
“
Within the registry, Version values are to be allocated by IANA
   according to the "Specification Required" policy specified in
   [RFC5226].  Version values are to be allocated by IANA with a format
   of NAMEFORMAT (see Section 3.1).  Version values corresponding to
   specifications produced by the IETF CDNI Working Group are to be
   allocated a name starting with "CDNI".  All other Version values are
   to be allocated a name that does not start with "CDNI”.
“

I have kept CDNI/1.0 as the value allocated by this document.


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

This has been discussed and fixed already.


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

Good point.

Text has been modified to:
“
this a Uniform Resource Name (URN) from the
         Universally Unique IDentifier (UUID) URN namespace specified in
         [RFC4122]).  The UUID contained in the URN uniquely identifies
         the CDNI Logging File.
"


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

Done. Replaced MUST by “is to be”.

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

Done. Replaced MUST by “is to be”.

> 
> S3.4: more wildcards

Sorry. I didn’t get that.

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

The thing is that there are additional things in HTTP2 that could be logged and the WG has not yet discussed whether there is real value in doing that, and how. So we do not have specific extensions for those. So we need to be clear that we “only” log the same thing for HTTP/2 as for HTTP/1.1, even if we know that one may potentially want to log more. 
And I’d rather we don;t hold publication of this document until teh WG has worked out exactly waht to do for the few additional things specific to HTTP/2.
So I think the current text is quite appropriate.


> 
> 
> Nits/editorial comments:
> 
> There are lots of uses of lowercase "may”.

Right. But we’ve been quite careful at using “MAY" as per RFC2119 and “may" where appropriate.
Do you see specific occurences of “may” that are not appropriate?

> 
> Page 6 has a few instances of missing hyphens in dCDN3/dCDN2 etc…

Fixed.

> S6.2.1: chunck
> 

Fixed

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

Good point.

NEW:
“
Dates are encoded as "full-date" specified in [RFC3339].
“
and 

NEW:
“
Times are encoded as "partial-time" specified in [RFC3339].
“

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

This is a recommendation from ELF and commonly used today. Since we want to be as consistent as possible with ELF, I’d rather we stick to HTAB.

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

Done. For example it says:
“
"c-" refers to the User Agent that issues the request (corresponds to the "client" of W3C Extended Log Format)
"

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

RFC3270 itself says that HTTP1.1 is specified collectively by RFC3270, RFC3271,… RFC3275. So we may as well be consistent with that.


Finally got to the bottom !  

Thanks much for the review.

Francois