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

["Francois Le Faucheur (flefauch)" <[e-mail]>]

> On 9 Mar 2015, at 23:36, Martin Thomson <[e-mail]> wrote:
> 
>> 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).
> 
> I think that you could express that better than you have.
> 
> LoggingFile = 1*(1*DIRGROUP *RECGROUP)
> 
> Might work.

I have updated to:

"
CDNI-LOG-FILE = 1*(DIRGROUP RECGROUP)
“

This defines the base syntax, and what defines whether/how-many/where actuel directives can be present is the “occurences” specification below (semantic dependent).



> 
>>> 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”.
>> “
> 
> Creating carve-outs in registries like that is inadvisable.  Someone
> might attach semantics to them and then you get into trouble.  See the
> RFC on x-.

I checked RFC 6648 and see your point.
So I have removed the text in IANA sections recommendation a naming structure.


> 
> 
> 
>>> 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.
> 
> If you aren't fixing it now, fix it later and don't worry about saying
> that you will.  Because you might not.

But we are not saying that we will. We are saying that it is not addressed in this doc. It may be useful, and it may be addressed in the future. Unless I am missing something, I believe this is correct and useful.

>> 
>> 
>>> 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.
> 
> We get this question all the time.  You don't need to expand, because
> RFC 7230 does that for you…

This is not a important point at all and I apologize to waste time on it. But as RFC7230 is worded, it makes it clear that it defines only one part of HTTP/1.1 and that HTTP/1.1 is defined by teh set of RFCs. 
In other words RFC7230 refers to the other RFC but it does not include them. To me HTTP/1.1 is really defined by the set of RFCs and not simply RFC7230. So I’m more comfortable referncing the set.
If you guys feel strongly to remove it, I’ll just obey.

Thanks again

> 
>> Finally got to the bottom !
>> 
>> Thanks much for the review.
>> 
>> Francois