[Pals] Re：[RTG-DIR] Routing Directorate review of draft-ietf-pals-rfc4447bis

[Lizhong Jin <[e-mail]>]

Hi,

I read the old version of 4447bis draft before, but did not find the merging of 6723. In current version, it is more of merging 4447 and 6723 as one draft, which I believe is a better way. RFC6723 solves a technical bug of RFC4447. I give some inline comments below for Adrian's review regarding 6723.

Regards
Lizhong

在2016年02月21日 23:16，Adrian Farrel 写道:

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them as the draft advances, and strive to resolve them through discussion or by updating the draft.

Thanks,
Adrian

===

Document: draft-ietf-pals-rfc4447bis-03.txt
Reviewer: Adrian Farrel
Review Date: 21 February 2016
IETF LC End Date: Not yet started
Intended Status: Internet Standard

= Summary =

I have some minor concerns about this document that I think should be resolved before publication.

= Comments =

I'm OK to see 4447 being advanced to full standard, although I'm also a little surprised that there was energy to do it.

The work is largely mechanical. My comments are a mix of minor process concerns (that your AD should be able to resolve), minor text clarifications, and nits.

It's been a while since I was concerned with LDP, but I have no reason to doubt the integrity of this document.

= Major Issues =

No major issues found.

= Minor Issues =

I believe that this document should be marked as obsoleting 4447 and
6723. It will result in a new RFC with a new number, so 4447 will no
longer be relevant. And, as Section 1 says: a new section (6.3) on
control-word renegotiation by label request message has been added,
obsoleting RFC 6723.

By the way, it is section 7.3 (not 6.3) that you have added!

[NB: Q16 of the Shepherd write-up seems a bit garbled on this front.
Especially "Will publication of this document change the status of any
existing RFCs?" seems to need a much clearer answer!]

---

I'm not up to speed on the new process for advancing a specification to
Internet-Standard when it has normative references that are not
similarly advanced/advancing. I do know that the process changed
recently and I am sure that your AD can look it up.

---

It might be a reasonable question to ask why some of the
(informatively) referenced related PW RFCs (such as encapsulations)
are not being advanced at the same time, especially those that are
described as "companion documents".

---

It seems unnecessary petty-fogging process, but the inclusion of
section 7.3 seems to me to defeat the stability requirement for
advancement to Internet Standard unless your claim here is that you
are advancing 4447 and 6723 together in this single document. That
might be fine. 

[Lizhong] similar feeling from me. It seems to combine 4447 and 6723 into one draft. If that is the intention, I suggest to list the 6723 authors in this draft. 

---

The Errata that have been deliberately disregarded need to be marked as
Rejected. I think the ones that have been actioned could usefully be
marked as Verified or have a note added saying "fixed in RFCabcd".

As far as I can tell:

3554 was not actioned
938 was done
86 was done
3555 was done
3115 no longer applies
3114 was done
3112 was done
1530 was done

---

Because you (incorrectly) said that section 6.3 was new text, I gave it
careful review. I think the text is in rather poor state:

For example, in 6.3.1...

  Using the procedures
  outlined in this section, a simple label withdraw method MAY also be
  supported as a legacy means of signaling PW status and AC status.

...Since this is *the* specification now, what does "legacy" mean?

Indeed, you end 6.3.1 with...
  The PW status
  signaling procedures described in this section MUST be fully
  implemented.
...which kind of implies ... hmm.

I think you could rework 6.3.1 to handle this more gracefully and avoid
the "clunk" of the 2nd para...

  Once the PW status negotiation procedures are completed and if they

...where we are surprised to hear about status negotiation.

For example:

OLD
  The PEs MUST send Label Mapping Messages to their peers as soon as
  the PW is configured and administratively enabled, regardless of the
  attachment circuit state.  The PW label should not be withdrawn
  unless the operator administratively configures the pseudowire down
  (or the PW configuration is deleted entirely).  Using the procedures
  outlined in this section, a simple label withdraw method MAY also be
  supported as a legacy means of signaling PW status and AC status.  In
  any case, if the label-to-PW binding is not available the PW MUST be
  considered in the down state.

  Once the PW status negotiation procedures are completed and if they
  result in the use of the label withdraw method for PW status
  communication, and this method is not supported by one of the PEs,
  then that PE must send a Label Release Message to its peer with the
  following error:

  "Label Withdraw PW Status Method Not Supported"

  If the label withdraw method for PW status communication is selected
  for the PW, it will result in the Label Mapping Message being
  advertised only if the attachment circuit is active.  The PW status
  signaling procedures described in this section MUST be fully
  implemented.
NEW
  The PEs MUST send Label Mapping Messages to their peers as soon as
  the PW is configured and administratively enabled, regardless of the
  attachment circuit state.  The PW label should not be withdrawn
  unless the operator administratively configures the pseudowire down
  (or the PW configuration is deleted entirely).

  Section 6.3.2 describes a mechanism for signaling the status of the
  PW and AC.  Additionally, a simple label withdraw method MAY be used
  to signal the PW status and AC status.  In any case, if the label-to-
  PW binding is not available the PW MUST be considered to be in the
  down state.

  The PEs negotiate which PW and AC status signaling mechanism to use
  by following the procedures in Section 6.3.3.  If the PW negotiation
  procedures are completed and if they result in the use of the simple
  label withdraw method for PW status communication, and if this method
  is not supported by one of the PEs then that PE must send a Label
  Release Message to its peer with the following error:

  "Label Withdraw PW Status Method Not Supported"

  If the label withdraw method for PW status communication is selected
  for the PW, it will result in the Label Mapping Message being
  advertised only if the AC is active.
  
  The PW status signaling procedures described in Section 6.3.2 MUST be
  fully implemented.
END

---

Section 6.3.2 achieves ambiguity by stating...
  The general format of the Notification Message is:
...and then showing the specific case of "PW status".

You can fix this by re-ordering and tweaking slightly, as follows.

Note that the old text says:
  Since this notification does not
  refer to any particular message, the Message Id and Message Type
  fields are set to 0.
...but I think the Message Type is set to 0x0001 :-)


OLD
  The Status TLV is transported to the remote PW peer via the LDP
  Notification message.  The general format of the Notification Message
  is:

   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
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |0|   Notification (0x0001)     |      Message Length           |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                       Message ID                              |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                       Status (TLV)                            |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                      PW Status TLV                            |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |           PWId FEC TLV or Generalized ID FEC TLV              |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |               PW Grouping ID TLV (Optional)                   |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+



  The Status TLV status code is set to 0x00000028, "PW status", to
  indicate that PW status follows.  Since this notification does not
  refer to any particular message, the Message Id and Message Type
  fields are set to 0.
NEW
  The Status TLV is transported to the remote PW peer via the LDP
  Notification message as described in [RFC5036].
  
  The Status TLV status code is set to 0x00000028, "PW status", to
  indicate that PW status follows.  The format of the Notification
  Message for carrying the PW Status is as follows:
  

   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
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |0|   Notification (0x0001)     |      Message Length           |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                       Message ID                              |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                       Status (TLV)                            |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                      PW Status TLV                            |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |           PWId FEC TLV or Generalized ID FEC TLV              |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |               PW Grouping ID TLV (Optional)                   |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


  Since this notification does not refer to any particular message,
  the Message Id field is set to 0.
END

---

Section 8 could be tidied a little.

The use of "in general" gives the IANA no way to know whether they
should or should not update a reference.

Are you sure that you want to remove the nice, concise listing of code
points that are present in RFC 4447?

8.1, 8.2, and 8.3 all say "new". I suggest deleting that word.

And lastly, "IANA needs to..." is not what we say :-) "IANA is requested
to..." is nicer.

[NB: The Shepherd write-up seems to report all this wrongly at Q17]

---

Shouldn't the whole of Section 10 be moved to an interop report
somewhere? It's presence within the document seems somewhat temporally
unstable. I understand that you want to write it down and record it
somewhere, also that you probably developed it in the I-D as a useful
place for working text. For advice about how and where to do this you
can look at RFC 5657.

You seem to have answered the four points from 6410, but may be missing
a little corroboration.

>  The pseudowire technology was first deployed in 2001 and has been
>  widely deployed by many carriers. [RFC7079] documents the results of
>  a survey of PW implementations, with specific emphasis on Control
>  Word usage.  [EANTC] documents a public multi-vendor interoperability
>  test of MPLS and Carrier Ethernet equipment, which included testing
>  of Ethernet, ATM and TDM pseudowires.

7079 is a slightly related piece of survey work, but the only mention of
LDP (which is the subject of *this* document) is to point out an issue
with non-implementation.

The EANTC reference really needs something that helps us find the report
of the interop event. Possibly this is
http://www.eantc.de/fileadmin/eantc/downloads/events/2007-2010/MPLSEWC09/EANTC-MPLSEWC2009-WhitePaper-v1.0.pdf
Looking at that paper, it definitely supports the claim of multiple
interoperating implementations of 4447.

We lack, however, any statements about "widespread deployment and
successful operational experience."

>  The errata against [RFC4447] are all editorial in nature and have
>  been addressed in this document.

Good (except for those not addressed as above, and except that some of
those errata are incorrectly marked as "Technical").

>  All features in this specification have been implemented by multiple
>  vendors.

How do we know this?
Further, are all the features used? One point of this step in the
process is to prune out features that are unnecessary (and so complicate
implementation and operation, and cost money).

>  There is no IPR filed against this document or its predecessor.

This may be overly vague (you probably mean "No IPR disclosures have
been made to the IETF related to this document, to RFC 4447 or RFC 6723,
or to the Internet-Drafts that resulted in RFC 4447 and RFC 6723.").

---

= Nits =

In the Abstract, probably...

OLD
  This document has been written to address errata in a previous
  version of this standard.
NEW
  This document has been written to address errata in a previous
  version of this specification.
END

But, actually, I think you are missing the main purpose which is to
advance the spec to full standard.

On reflection, I think that this paragraph probably served well in the
draft as it was being processed, but can be removed now. The equivalent
text obviously needs to be in the shepherd write-up.

[ditto the paragraph in the introduction]

---

Thank you for including Section 1. It's important and helpful.

Three tiny points:

Second para should probably s/However/Additionally/

The RFC Editor requires that section 1 is the Introduction. A simple
solution is to swap sections 1 and 2, or to move the current section 2
to be the new section 1 and the current section 1 to be section 1.1.

The text says "A note was added to clarify that the C-bit is part of
the FEC." I think you should s/was/has been/ and you could also usefully
give a little hint as to where the note was added.
                                                                                 
---

Shouldn't you mention the inclusion of text referencing 7358? This is
clearly a change to the original text. I wonder, however, whether what
really want to do is take that part of 7358 that updated 4447 and simply
write it here so that this spec has no need of that reference.

Either way, you also need to take care about the stability requirement
for advancement.

---

In 6.2.2.1
please note that IANA has "PW Interface Parameters TLV" not "Interface
Parameters TLV".

The is a good chance to make this consistent.

---

In 6.2.2.2 and the rest of the document, please note that the IANA has
"PW Group ID TLV" not "PW Grouping ID TLV".

The is a good chance to make this consistent.

---

6.3.2 has a figure which claims to include a "PWId FEC TLV" or a
"Generalized ID FEC TLV". However, 6.1 and 6.2 describe these as
"elements" not TLVs and they show up in the Forwarding Equivalence Class
(FEC) Type Name Space.

I think you can clarify 6.3.2 by explaining that what is included here
is "a FEC TLV containing either a PWId FEC element or a Generalized FEC
element.

Then, just after the figure you have "The PW FEC TLV" but it is just a
"FEC TLV"

---

In 7.3 you have "re-provisioned". I wonder whether you mean
"re-configured". "Provisioning" has a context initial set-up, so re-
provisioning suggests (to me?) re-initialization.
[Lizhong] re-configured is fine here.
---

In the first para of 7.3 you have "PW C-bit negotiation procedure" and
"Control-Word negotiation procedures".

Could you decide on a single name, and on whether it is singular or
plural.
[Lizhong] Could use control-word negotiation procedures in both places.
---

7.3
OLD
       -i. If local PE has previously sent a Label Mapping message, it
           MUST send a Label Withdraw message to remote PE and wait
NEW
       -i. If the local PE has previously sent a Label Mapping message, it
           MUST send a Label Withdraw message to the remote PE and wait
END

---

7.3
OLD
      -ii. the local PE MUST send a label release message to the remote
NEW
      -ii. The local PE MUST send a Label Release message to the remote
END

---

7.3
s/negotaiation/negotiation/
s/renegotation/renegotiation/

---

7.3

  The above C-bit renegotiation process SHOULD NOT be interrupted until
  it is completed, or unpredictable results might occur.

This is a bit odd. Normally the "or" that follows a "SHOULD" or "SHOULD
NOT" explains the conditions under which you might want to vary the
"SHOULD".

But I'm struggling: interrupted by whom and by doing what?
[Lizhong] there is not this sentence in 6723. The implementation should treat above procedure as atomic.

---

Some of the formatting in the references seems to have been hit with a
global change of /. /.  / You might want to polish.

---

The latest version of G.707 is dated January 2007. Is there any reason
to not update the reference?

---

Have a look at Section 8 of RFC 5036 for an example of a nice way to
handle "legacy authors" and credits for past work.

In any case, the RFC Editor will require that Section 15 is renamed
"Contributors".

---

There are two trivial nits from idnits.