Subject: [Pals] Mirja Kühlewind's No Objection on draft-ietf-pals-mpls-tp-pw-over-bidir-lsp-08: (with COMMENT)

From: "Mirja Kuehlewind" <[email-masked]>
Date: Tue, 12 Jul 2016 00:47:41 -0700

Mirja Kühlewind has entered the following ballot position for
draft-ietf-pals-mpls-tp-pw-over-bidir-lsp-08: 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-pals-mpls-tp-pw-over-bidir-lsp/

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

A few questions/comments:

1) The shepherd writeup says: "An IPR discolusure has been filed and the
WG has not remarked on this."
    Does this mean the wg is not aware of it or it is aware and nobody
commented?

2) Acronym: I found a few acronyms that where not spelled out, e.g. FEC
as well as LDP and TE in the abstract. One can argue if that's needed but
usually I think it helps. Further there are quite a few acronyms that are
introduced somewhere and then never used again (e.g. Point-to-Point
(P2P); that makes it actually harder to read, so I would recommend to
remove them.

3) I think I don't understand the following MUST: "PSN Tunnel Binding TLV
is an optional TLV and MUST be carried in the Label Mapping message
[RFC5036] if PW to LSP binding is required." and again later "To perform
PSN binding, the Label Mapping messages MUST carry a PSN Tunnel Binding
TLV". Maybe just remove it?

4) Why is there a "MUST be zero" and a "Reserved" field? Shouldn't this
all just be a large "Reserved" field? And the "Reserve" field is not
discussed.

5) On the T (Tunnel Representation) bit: Why is that an extra bit?
Usually you can simply use the length field here and then either have the
LSP Number fields in the sub-TLV or not.

6) Why are there two bits for C and S if they are mutual exclusive and
one of them has to be set? In this case you should use one bit and say
e.g. 0 means S(trict) and 1 means C(o-routed path). This would also avoid
the error case when both are set! (Otherwise if it should be possible to
not set both flags, this case is not described  in the draft and must be
added.)

7) Sub-TLV: I'm not an LDP expert but I assume having sub-TLV is not a
general concept that is widely used this way but something introduced in
this document. If that's the case, I have a couple of questions/remarks:

   - Given that the PSN Tunnel Binding TLV already has a length field
that includes the sub-TLV, why do you need another length field in the
sub-TLV?
  - Why is there another 'Reserved' field in the sub_TLV (that is also
not discussed)?
  - If you already have different types of sub-TLVs here, why don't you
simply define some with or without the the LSP Number fields instead of
the T bit?
  - And finally do you really need these sub-TLVs  or would a flag be
enough to indicate IPv4 or IPv6 (or again just use the length field to
distinguish)?
 - And this should be normative "Each PSN Tunnel Binding TLV can only
have one such sub-TLV." -> "Each PSN Tunnel Binding TLV MUST have only
one such sub-TLV." and you must specify what happen if more then one is
presented (if you actually want to use sub-TLVs).

8) There are a couple of MUSTs basically twice in the document (once in
section 2 and another time in sections 4 and 5). I would recommend to
avoid these duplications and subsequent redundancy and really state it
only once to avoid any future confusions.