Last Call Review of draft-ietf-pcp-description-option-03
review-ietf-pcp-description-option-03-opsdir-lc-korhonen-2014-02-04-00

I have reviewed the document "PCP Description Option"
(draft-ietf-pcp-description-option-03) as part of the Operational directorate's
ongoing effort to review all IETF documents being processed by the IESG.  These
comments were written primarily for the benefit of the operational area
directors.  Document editors and WG chairs should treat these comments just
like any other last call comments.

IETF State:        Submitted to IESG for Publication
IESG State:        In Last Call (ends 2014-02-04)
IANA Review State: IANA - Review Needed
IANA Action State: None

The draft is: Almost ready for publication.

Summary:

The document defines a new DESCRIPTION option that can be used to
add a human readable text/note to PCP-instatiated mappings. The
option and its semantics are rather straight forward. Below are
few comments that are not really pointing out errors just trying
to improve the document.

Comments:

** Section 2

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | DESCRIPTION   |  Reserved     |           Length              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Small nit but reflecting the RFC6887 and the text in other parts of the
document that reference Figure 1 I would change the figure to

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |Option Code=TBA|  Reserved     |           Length              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Also

         Number: <to be assigned in the optional-to-process range>
to
         Number: <TBA>

** Section 3

   If the PCP client does not receive a DESCRIPTION option in a response
   to a request enclosing a DESCRIPTION option, this means the PCP
   server does not support that Option.  If the DESCRIPTION option is

This is not entirely align with earlier text. If the response does not
contain the DESCRIPTION option it may also mean the server was configured
to ignore it.. while the server still understands the option. I would
clarify this here, since there is subtle difference between server not
understanding or purposely discarding the option.

s/Option/option

   configured maximum length MUST NOT exceed 1016 octets.  The suggested
   maximum length is 128 octets.  If a PCP client includes a DESCRIPTION

Why not saying the "suggested maximum length" is the "default maximum length"?
Would sound a bit more formal.

Why there is no similar recommendation for the PCP client?

Although I kind of can figure out where the maximum length of 1016 octets
come from, it would be nice to have a pointer/reference why 1016 and not
1096?

   If the description text carried in the DESCRIPTION option is null
   terminated, the behavior of the PCP server is implementation-
   specific.  The PCP server may decide to discard the option or store

Hmm.. earlier it was said that the text MUST NOT be null terminated. Why
it needs then the above comment? If the text is null terminated, it is an
error in implementation, right? Would be better to send an error or just
discard the description in this case instead of trying to add a possibility
to do something implementation specific. I recommend removing any text
hinting an alternative handling for null terminated text since it is "MUST
NOT". Rather use the same treatment as in invalid UTF-8 case.

   and return the exact description text, including Null characters.

s/Null/null

** Section 5

In order to make IANA considerations more clear I would suggest changing

      DESCRIPTION
to
      DESCRIPTION set to TBA

since the body text uses "TBA" in places.

IDnits: none to worry about.

- Jouni