Subject: [PSAMP] [Fwd: Preview: Review comments for draft-ietf-psamp-info-07]

From: Andrew Johnson <[email-masked]>
Date: Tue, 04 Dec 2007 19:26:54 +0000

Dear Authors

I have read PSAMP-INFO and you can find my comments below.

Overall the document is fine and my comments are mostly minor editorial
ones.  I do have a concern over the dateTime types that is probably best
cleared up in IPFIX-INFO, but if it's too late for that, then something
should be added to the description of the new dateTime IEs here.

regards

Andrew

Section 1
=========

paragraph 3:
  "from a XML document" should be "from an XML document".

paragraph 4:
  "served as source" should be "served as the source".

Section 2
=========
  "This Document: "Information Model for Packet Sampling Exports" (this
   document), defines an information and data model for PSAMP."

There is no need to say "this document" twice.  Remove the second
mention of this.

Section 3.1
===========
Only the used terminology definitions have been copied from the IPFIX
INFO document, however "exporters" are mentioned in the Appendix, but
not defined.

Section 3.2
===========
  "An Observed Packet Stream denotes the packets that flow past the
   Observation Point."

Considering "Flow" is formally defined, consider rewording to avoid any
confusion.  Perhaps to:
  "An Observed Packet Stream denotes the packets that are seen at the
   Observation Point."

The same comment applies to the definition of "Packet Stream".

Section 8.2
===========
The table would have less ugly wrapping, and be 3 lines shorter, drawn
as follows:

   +-----+-------------------------+-----+-----------------------------+
   |  ID | Name                    |  ID | Name                        |
   +-----+-------------------------+-----+-----------------------------+
   | 301 | selectionSequenceId     | 318 | SelectorIdTotalPacketsObser |
   |     |                         |     | ved                         |
   | 302 | selectorId              | 319 | SelectorIdTotalPacketsSelec |
   |     |                         |     | ted                         |
   | 303 | informationElementId    | 320 | fixedError                  |
   | 304 | selectorAlgorithm       | 321 | relativeError               |
   | 305 | samplingPacketInterval  | 322 | observationTimeSeconds      |
   |     |                         |     |                             |
   | 306 | samplingPacketSpace     | 323 | observationTimeMilliseconds |
   | 307 | samplingTimeInterval    | 324 | observationTimeMicroseconds |
   | 308 | samplingTimeSpace       | 325 | observationTimeNanoseconds  |
   | 309 | samplingSize            | 326 | digestHashValue             |
   | 310 | samplingPopulation      | 327 | hashIPPayloadOffset         |
   | 311 | samplingProbability     | 328 | hashIPPayloadSize           |
   | 312 | dataLinkFrameSize       | 329 | hashOutputRangeMin          |
   | 313 | ipHeaderPacketSection   | 330 | hashOutputRangeMax          |
   |     |                         |     |                             |
   | 314 | ipPayloadPacketSection  | 331 | hashSelectedRangeMin        |
   |     |                         |     |                             |
   | 315 | dataLinkFrameSection    | 332 | hashSelectedRangeMax        |
   | 316 | mplsLabelStackSection   | 333 | hashDigestOutput            |
   |     |                         |     |                             |
   | 317 | mplsPayloadPacketSectio | 334 | hashInitialiserValue        |
   |     | n                       |     |                             |
   +-----+-------------------------+-----+-----------------------------+

Why the blank lines?  They don't seem to be used to group elements, can
they be removed?

Section 8.2.1
=============
"a subset of packets" should be "a subset of the packets", or just "a
subset".

Section 8.2.4
=============
  "Some parameters for these algorithms are not covered by this
   information model since they very much depend on the underlying
   hardware."

Are the parameters really dependant on hardware, or just the particular
PSAMP implementation?

Section 8.2.13 - 17
===================
  "If insufficient octets are available for the length specified in
   the Template, the Information Element MUST NOT be padded."

This isn't very clear.  I believe the intention is that a new Template
must be created with a shorter length for the field.  The IE itself
wouldn't be padded, the data would.  Perhaps rewrite as:

  "The data for this field MUST NOT be padded.  If insufficient octets
   are available for the length specified in the Template, then a new
   Template MUST be used."

Section 8.2.22 - 25
===================
  "Description:
      This Information Element specifies the absolute time in seconds of
      an observation.

   Abstract Data Type:  dateTimeSeconds"

I'm not sure what that description means exactly, and time data types in
IPFIX don't seem to be well defined either:

  "The type "dateTimeSeconds" represents a time value in units of
   seconds normalized to the GMT time zone."

So what does that mean?  I still don't know exactly what I should put as
the value for this field.

>From IPFIX INFO, section 5.9:
  "Time stamps ... are absolute and have a well defined fixed time
   base, such as, for example, the number of seconds since 0000 UTC
   Jan 1st 1970."

This is known as "epoch time".  Unix time or POSIX time has an epoch of
0000 UTC Jan 1st 1970.  NTP uses 0000 UTC Jan 1st 1900.  The epoch for
the IPFIX timestamps is not provided in the Information Elements or the
data types.

I think the intention is that the 0000 UTC Jan 1st 1970 wasn't supposed
to be just an example, but was going to be the actual epoch defined in
the data types.  Alternatively, each Information Element definition has
to define the epoch.

If the exact encoding is provided elsewhere then a reference should be
provided as it is for the float types.

Section 8.2.33
==============
Again, this is more an issue with the definition of the data type in
IPFIX INFO:
  "The type "boolean" represents a binary value.  The only allowed
   values are "true" and "false"."

It isn't clear what to actually send to represent true and false.  I
would guess all zero's for false, and probably anything else for true,
but it should be specified.

Again, if the exact encoding is provided elsewhere then a reference
should be provided as it is for the float types.

_______________________________________________
PSAMP mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/psamp