Subject: [Gen-art] Gen-art telechat review of draft-ietf-mpls-psc-updates-03.txt

From: Elwyn Davies <[email-masked]>
Date: Fri, 04 Apr 2014 19:11:48 +0100

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-mpls-psc-updates-03.txt
Reviewer: Elwyn Davies
Review Date: 4 April 2014
IETF LC End Date: 2014-04-09
IESG Telechat date: (if known) -

Summary:
Almost ready.  

Major Issues:

Minor Issues:
General: 
As discussed during 
Last Call/IETF review of draft-ietf-mpls-tp-psc-itu, RFC 6378 is missing
some details regarding 
- the on-the-wire format of messages
- detection of and behaviour in the event of reception of malformed
messages 
- behaviour in the event of receiving unknown TLV items
- behaviour in the event or receiving unexpected TLV items in a
particular mode.

Specifics:
On-the-wire format: 
- The encoding of the TLV Length field is not specified (unsigned binary
integer in network bit order).
- The encoding of the individual TLV length and Type fields in s2 should
also be specified (unsigned binary integer in network bit order). 
- The value of TLV Length should be more precisely specified.  Suggest:
  The TLV Length is the sum of the lengths of all TLVs in the message, 
  where the length of a TLV is the sum of the lengths of the three
  TLV fields, i.e., the the length of the value field + 4.

Malformed messages check:
- Check values of fields prior to TLV Length are consistent with s4.2 of
RFC 6378.
- Check overall length of message matches value in TLV Length + 12.
- Check Sum of lengths of TLVs matches value in TLV Length. 
- Check all TLV types received are recognized.

Behaviour in the event of receiving a malformed message:
- There has been discussion of appropriate behaviour on the MPLS mailing
list.

Behaviour in the event of receiving a well-formed but inappropriate TLV
in a message:
- This needs to be specified. (might be mode specific)

Nits/Editorial Comments:
General: s/i.e. /i.e., /g, s/e.g. /e.g., /g

Abstract/s1: Make the count of changes consistent (four/five currently).
Might be better just to say 'a number of changes' and leave the reader
to count.

s1: Since the number of items has grown to five and maybe six (depending
on how the above changes are inserted), it would helpful to link the
categorization to the actual section numbers.)

s2: It would be good to have the conventional picture here - just grab
the one from draft-mpls-tp-psc-itu.

s2, Value field: Better to say that this has the number of octets
specified in the length field rather than talking about multiples of 4
again.  The discussion of padding seems superfluous.

s4, next to last para: Should 'A remote No Request message SHALL be ignored' be
'A remote NR message SHALL be ignored' for consistency?

s5,para 8: s/In both cases the request which was driving/In both cases the request that was driving/

s6: It might be worth pointing out that extensions of PSC (like tp-psc-itu) may 
introcuce additional capabilities and state that it is up to these sepcifications to 
say how capability mismatch in this areas is/are handled.