Subject: Re: [IPFIX] WG Last Call for draft-ietf-ipfix-protocol-rfc5101bis-03

From: Andrew Feren <[email-masked]>
Date: Tue, 04 Dec 2012 11:56:20 -0500

Hi Benoit, All,

Attached is a marked up version of the draft.  Notes all tagged with ACF.

-Andrew

On 11/23/2012 03:54 AM, Benoit Claise wrote:
> Dear all, Nevil,
>
> Trying to avoid any potential confusing, correcting the subject to the 
> correct draft name: draft-ietf-ipfix-protocol-rfc5101bis-03
>
> Regards, Benoit
>>
>> Hi all:
>>
>> Following up on Brian's email dated 20 November ...
>>
>> The WG Last Call for this I-D starts now, and will run until Monday,
>> 10 December.
>>
>> Do please read the draft, and post your comments to the list. It's
>> important that we can show it has been well-considered/reviewed within
>> the WG, so short comments like "yes, this does clearly describe the
>> IPFIX Information Model as we want it to be" are important and useful!
>>
>> Cheers, Nevil
>>
>
> _______________________________________________
> IPFIX mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ipfix

---

From: Brian Trammell <[email-masked]>
Date: Sat, 8 Dec 2012 15:59:48 +0100

Hi, Andrew, all,

(copying the list; as this conversation probably belongs there...)

replies inline...

On Dec 8, 2012, at 12:39 AM, Andrew Johnson wrote:
> On 6 Dec 2012, at 21:03, Brian Trammell <[email-masked]> wrote:
>> On Dec 6, 2012, at 6:02 PM, Andrew Johnson wrote:
>> 
>>> Hi Brian
>>> 
>>> I'm working my way through the changes to RFC5101.  For the most part I like the clarifications and tweaks that have been made, but I have a few niggles that hopefully you can straighten out for me.  Sorry for suddenly get involved at WGLC, but I'm pretty sure my comments are minor, I'm just missing some of the background to why these things changed.
>> 
>> The more review, and the more comments, the better, and "during WGLC" is still on time, so thanks!
>> 
>>> 1. Will new collectors fail to recognise RFC5101 compliant Metering Process
>>> reports because of the new scope field?  Should the new field be a SHOULD
>>> so that collecting processes have to treat it as optional?  (Alternatively,
>>> "For exporting processes that are only dealing with a single Metering
>>>  Process, the meteringProcessId scope fields MAY be omitted"?)
>>> 
>>> If this sort of thing is allowed because it's clear that the old style
>>> is RFC5101 and the new style is RFCXXXX, then I'm fine with the change.
>> 
>> Hm. I'd submit that any collector that didn't allow for additional fields in a "recognized template" was probably not very well designed, but we don't say that anywhere in the documents. It's clear to me because the specific reporting requirements are exporter-only, and the collectors should be designed in such a way to handle anything an exporter might reasonably throw at them. But maybe we could add some text in these sections to say that these are the minimal IEs; exporters may add other appropriate IEs to these templates as desired. Pre 5101-bis collectors that required exactly those fields (in exactly that order), though, would be out of luck. Personally, I don't find this problematic, but that's just a matter of opinion on my part.
> 
> I don't have a problem with it either, as long is acceptable when updating 5101.  I also agree that collectors should be prepared to accept additional elements in the reports, although I'd always thought it would be additional non-scope extensions providing different stats, rather than additional scope elements changing the specificity of where the stats apply to.
> 

I'll suggest (this is a note to self, more than anything) adding a sentence to each of these templates on this point, and default assumptions to be made at collectors when new scope elements are added.

>>> 2. The new text for the timestamps seems to prohibit interpreting a rolled
>>> over timestamp accurately.  i.e. in 2036 dateTimeMicroseconds and
>>> dateTimeNanoseconds become useless.  NTP seems to be able to cope just
>>> fine with the rollover, and it seems pretty short sighted of us to
>>> design a protocol that won't last another 25 years.
>> 
>> Hm. This depends on the assumptions you make.
> 
> The definition says "It can represent dates beginning between 1 January 1900 and 8 February 2036".  It doesn't say anything about worrying about the era.

True; it's precently implicitly an Era 0 date.

> 
>> In NTPv4, there's an "era" concept, which explicitly handles 64-bit NTP wraparound. We should probably change these definitions to incorporate this concept, stating that the era is implicit, and that near era boundaries, the collector should make a presumption that eras tend to be the current era, where current is defined by the current time at the collector.
> 
> Agreed, the era idea needs to be included in the definition.  In my code I make the assumption that the times are in the past (or very near future in case of clock skews).  Usually it'll be pretty obvious but we need to point out that it's possible.

Adding implicit eras to the NTP types makes them work, and indeed, we could add implicit eras to the other timestamp types, as well, in order to allow more flexible interpretation thereof (maybe a specific subsection on wraparound?). 

>> This breaks horribly for files; if you want data archiving, you need a way to store era number for these timestamps in the file.
> 
> Don't you have any timestamps in message parts that can guide the interpretations.  Maybe you can recommend opening a new file every hundred years or so.

Ah, wasn't concerned with lifetimes (and indeed a clever collector should be able to handle a century-long file by keeping wrap counters on a per-record basis); was more concerned with unambiguous interpretation of files that have been stored for years, decades, or centuries. 

> 
>> This only fixes the issues with NTP timestamps. dateTimeSeconds and Export Time in the header are still only good for another 95 years or so (which is why I would _not_ base the era determination on the Export Time). Sometime between now and February 8, 2106, we should probably define new timestamp types which have representation independent from epoch, with a 64-bit value somewhere for defining the epoch.
> 
> I've pencilled it into my diary for 1st April, 2099.  Are you free?

We've kicked the can down the road for another century, which I think is good enough for me. Can I get back to you in late '98?

>>> 3. In section 8, when sending the withdraw all templates message, should
>>> flowset ID 2 be used to contain the withdrawal of template ID 2?
>>> Similarly, when withdrawing all options templates, does the message
>>> have to be sent in flowset 3?
>> 
>> That's my interpretation of 5101.
> 
> It didn't seem to be explicitly stated, although it's how I'd have chosen to do it.  If everyone would just do it they way I would, we wouldn't even need a standards body :).

So we'll make this explicit in 5101bis as well.

>>> 4. In section 8.2: "Since there is no guarantee of the ordering of
>>> exported IPFIX, Messages across SCTP Streams".  I thought PR-SCTP
>>> allowed explicit ordering using an optional Stream-Sequence field.
>>> Doesn't that mean the SCTP order can be assured for templates?
>>> 
>>> What happened to the requirement to use reliable, in-order delivery?
>> 
>> There's ordering within the stream, and ordering of sending across streams, but you can't guarantee the ordering of processing, especially if the collector is designed to handle streams independently.
> 
> Ah!  I'd missed the across streams bit.  There's an RFC somewhere that recommends against that :)
> 
> 
>>> 5. "Collecting Processes MAY implement a buffer to handle out-of-order
>>> Template management events"
>>> 
>>> It's a bit more complicated than a buffer.  If a template is
>>> redefined (withdrawn and re-used), data with an old timestamp will
>>> need interpreted using the old template.  I'm not suggesting you
>>> add all the details to the document, just pointing out that
>>> implementing a "buffer" would have some challenges :)
>> 
>> Absolutely. Would you suggest a better word than "buffer" here? (Reading it again, I can see that there might be confusion between an "event buffer" and a uint8_t*…
> 
> "Collecting Processes MAY store older versions of the template for a time in order to handle out-of-order Template management events"?

Hm. it's not just the templates they'll need to store; maybe they should buffer sets, too; again, we don't necessarily want to constrain collector implementors to do things a certain way, we just want to make sure they might get stuff from exporters that it will be very easy to interpret incorrectly...

Cheers,

Brian

---

From: Gerhard Muenz <[email-masked]>
Date: Mon, 10 Dec 2012 22:57:32 +0100

Dear all,

I had a look at the differences between RFC5101 and the RFC5101bis draft 
(not at the unchanged parts). The changes in RFC5101bis make things much 
clearer.

As we see in Paul's review, there are many more smaller issues that need 
to be improved and those parts of the RFC which have not yet been 
touched. All in all, I agree with Paul's comments and with the comments 
made by the other reviewers.

Here are some additional comments:

There are occurrences of "template ID" and "template", but I think it 
should be "Template ID" and "Template" all over the place.
I suggest that you check the writing of all the other IPFIX terms as well.

In 4.1 and 4.2, it is unclear whether meteringProcessId is a mandatory 
or optional scope field. If it is mandatory (I don't think that it 
should be!), the "Note..." paragraphs below are superfluous. If it is 
optional as in RFC5101, please denote it in the description of the 
Template fields.

In 8.1, look for "allow receipt and processing of and Data Records" and 
correct it (and -> any).

Still in 8.1, I do not think that this paragraph applies to UDP where we 
now also allow TWM (as far as I understand):
"If a Collecting Process receives a new Template Record or Options 
Template Record for an already-allocated Template ID, without having 
received a withdrawal, it MUST ignore the new Template Record and 
discard the old Template Record for the allocated ID; it SHOULD log the 
error."

Last paragraph in 8.4: "...the Collecting Process SHOULD maintain the 
following for all the current Template Records and Options Template 
Records: <IPFIX Device, Exporter source UDP port, Observation Domain ID, 
Template ID, Template Definition, Last Received>."
The UDP Transport Session is defined by a four-tuple. Collector IP 
address and port are missing in the list. Nevertheless, a Collecting 
Process can easily listen on an interface which is assigned multiple IP 
addresses. So, at least Collector IP address must be included.

In 10.3.2, "Exporting Processes exporting IPFIX Messages via UDP MUST 
include a valid UDP checksum." does not make sense. The IPFIX Messages 
does not have a UDP checksum field. It's the underlying UDP datagram. I 
also suggest to add a reference to the RFC where the UDP checksum usage 
is explained.

In 9.2, I would prefer MAY over SHOULD in "If the Template Records (or 
other definitions such as Common Properties) have not been received at 
the time Data Records are received, the Collecting Process SHOULD store 
the Data Records for a short period of time and decode them after the 
Template Records (or other definitions) are received."
BTW, SHOULD contradicts the MAY used earlier in 8.
Also, it must be mentioned that at late arrival of the Template, the 
Export Time must be compared to the one of the earlier received Data 
Records.

Thanks,
Gerhard