[ippm] Robert Wilton's No Objection on draft-ietf-ippm-stamp-option-tlv-07: (with COMMENT)

[Robert Wilton via Datatracker <[e-mail]>]

Robert Wilton has entered the following ballot position for
draft-ietf-ippm-stamp-option-tlv-07: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ippm-stamp-option-tlv/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I support Ben's discuss regarding check the length of the STAMP packet to
determine whether it is a base or extended STAMP packet.

Comments:
Thank you for this document.  By and large I found this document fairly easy to
read and understand, but have comments on a few areas:

I found the document slightly confusing in that the extended stamp packets are
defined under the "STAMP Test Session Identifier" section.  I would have found
the document to be more coherent if the new extended packet structure was
pulled into its own section, perhaps with "STAMP Test Session Identifier" as a
sub-section.

Perhaps it might be helpful to explain why the MBZ sections are where they are
- I presume to still conform with the structure of the base STAMP packet
formats for session-reflectors that do not understand the new format.

In section 4:  TLV Extensions to STAMP

   A system that has received a STAMP test packet with extension TLVs
   MUST validate each TLV:

      If the U flag is set, the STAMP system MUST skip the processing of
      the TLV.  The implementation MUST try to process the next TLV if
      present in the extended STAMP packet.

      If the L flag is set, the STAMP system MUST stop processing the
      remainder of the extended STAMP packet.

      If the A flag is set, the STAMP system MUST discard all TLVs and
      MUST stop processing the remainder of the extended STAMP packet.

I was initially surprised that different behavior was specified for the
handling for the U, L and A flags given that the sender sets these to zero.  I
presume that the aim here is just to have commonality between the Session
Sender and Session Reflectors?  It might help clarify whether this section
applies to both sender and reflector.

In section 4.4:

   o  DSCP2 - The received value in the DSCP field at the Session-
      Reflector in the forward direction.

   o  ECN - The received value in the ECN field at the Session-Reflector
      in the forward direction.

These are the only two cases of forward direction.  Would it be better to
describe this as "ingress of the Session-Reflector", e.g. similar to the
description in the previous section?

4.5.  Direct Measurement TLV

   o  Session-Reflector Rx counter (R_RxC) is a four-octet-long field.
      MUST be zeroed by the Session-Sender on transmit and ignored by
      the Session-Reflector on receipt.  The Session-Reflector MUST fill
      it with the value of in-profile packets received.

"in-profile packets received" is unclear to me.

   o  Session-Reflector Tx counter (R_TxC) is a four-octet-long field.
      MUST be zeroed by the Session-Sender and ignored by the Session-
      Reflector on receipt.  The Session-Reflector MUST fill it with the
      value of the transmitted in-profile packets.

Presumably the Session-Reflector needs to be configured via an out of band
mechanism to specify how many in-profile packets are transmitted, and what
those packets are.  Further text in the beginning of section 4.5 may be helpful
here.

4.6.  Access Report TLV

This section seems to describe more than just a TLV given that it seems to put
additional constraints on the implementation (e.g. use of a retransmission
timer).  Possibly the document would have been more clear to split the
definition of the access report handling separately (e.g. as a separate section
earlier in the document) from the Access Report TLV definition.

Regards,
Rob