Last Call Review of draft-ietf-pce-wson-rwa-ext-10
review-ietf-pce-wson-rwa-ext-10-genart-lc-davies-2018-12-28-00

Request Review of draft-ietf-pce-wson-rwa-ext
Requested rev. no specific revision (document currently at 17)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2018-12-27
Requested 2018-12-13
Draft last updated 2018-12-28
Completed reviews Rtgdir Last Call review of -08 by Ravi Singh (diff)
Genart Last Call review of -10 by Elwyn Davies (diff)
Secdir Last Call review of -10 by Watson Ladd (diff)
Genart Telechat review of -11 by Elwyn Davies (diff)
Assignment Reviewer Elwyn Davies
State Completed
Review review-ietf-pce-wson-rwa-ext-10-genart-lc-davies-2018-12-28
Reviewed rev. 10 (document currently at 17)
Review result Ready with Nits
Review completed: 2018-12-28

Review
review-ietf-pce-wson-rwa-ext-10-genart-lc-davies-2018-12-28

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-pce-wson-rwa-ext-10
Reviewer: Elwyn Davies
Review Date: 2018-12-28
IETF LC End Date: 2018-12-27
IESG Telechat date: Not scheduled for a telechat

Summary:

Ready but with a lot of nits.  The amount of cross referencing to other documents that supply objects or object formats, some of which are repurposed, make it difficult to be certain that all requisite definitions are present and exactly which external objects may be present.

Major issues:
None

Minor issues:

For avoidance of error during editing, the various TBD values included in the specifications and the requests in s8 should be linked by using TBDx values specific to each allocation.

s3, para 6: Given that the additional multiplexing techniques are being developed, is the resulting extensibility easy to cater for in the capabilities defined here?

s4.3: Various encoding errors are possible with this TLV (e.g., not exactly two link identifiers with the range case, unknown identifier types, no matching link for a given identifier).  Should some error behaviour be specified here (or is it covered generically - if so where?)

s6.2: Including instructions for future work to be done as per this section is inappropriate.  It will necessarily be overtaken by events one way or the other.  Unless there is already work in progress that can be referenced, the section should be removed.

Nits/editorial comments:

Tool issue: Note that the layout of 2nd and 3rd level headings does not correspond to IETF draft/RFC conventions (they are indented rather than at the left edge of the page).

General: The encoding of numeric fields should be specified (once for all cases is all that is needed).

General: s/TDB/TBD/ (7 places) (I assume)

s3, para 4: It would be helpful to insert a new second sentence that explains the term Lambda Switch Capable. Maybe
    The devices used in WSONs that are able to switch signals based on
    signal wavelength are known as Lambda Switch Capable (LSC).
The expansion of LSC can then be removed from para 5.
 
s3, para 4: It would be helpful to expand the 3R acronym for those less familiar with optical network jargon.

s3, para 4: Is it possible to say briefly in the document why all optical wavelength converters are out of scope?

s3, para 6: s/both signals/the two signals/

s3, para 9: s/generic property/generic properties/

s3, para 10: s/advanced modulation processing/advanced modulation processing capabiities/; s/Those modulation properties/These modulation capabilities/; s/by the means of/indicated by means of/

s4, para 1: s/that are going to be/that are/

s4, in the two paras before figure 3: The expansion of WA should be done on first occurrence (one para earlier).
s4, specification of WA Object:

- Reserved bits  ought to be explicitly zeroed (applies to various other places)

- Unused flag bits should be zeroed.

- s/The following new flags should be set/One flag bit is allocated as follows:/

- In specification of M: s/needs not be explicit/need not be explicit/

- In specification of M: s/Otherwise,/Otherwise (M bit set to 0),/; s/In such case,/If M is 0/

s4.3, above Fig 4:

>    The Wavelength Restriction Constraint TLV type is TBD, recommended
>    value is TBD.
Putting in ' recommended value is TBD' is pointless if a value is not given and it needs to be removed from the published version in any case.  If the authors have a preferred value for this then put in a note to IANA; otherwise leave it as TBDx for IANA to determine.

Same applies in  s5 above Fig 6.

s4.4, paras 1 and 2:
OLD:
   Path computation for WSON includes the check of signal processing
   capabilities, those capability MAY be provided by the IGP. Moreover,
   a PCC should be able to indicate additional restrictions for those
   signal compatibility, either on the endpoint or any given link.

   The supported signal processing capabilities are the one described
   in [RFC7446]:
NEW:
   Path computation for WSON includes checking of signal processing
   capabilities at each interface against requested capability; this requirement
   MAY be implemented by the IGP.  Moreover,
   a PCC should be able to indicate additional restrictions to
   signal processing compatibility, either on the endpoint or any given link.

   The supported signal processing capabilities are those described
   in [RFC7446]:

ENDS

s4.4: It is not clear to me where the XRO and IRO sub-objects would be used in this specification.

s4.4.1: Expand XRO (Exclude Route Object?) on first use. 

s4.4.1, last para: which sub-sub objects are permitted/expected here?  I couldn't be sure which part of RFC 7689 I should be looking at.

s4.4.2: Provide an expansion of IRO.

s4.4.2, last para: There is no sub-object called just 'processing' in RFC 7689.  Is WSON Processing Hop Attribute TLV (Section 4.2) what is meant?

s5, definitions of  Link Identifier and Allocated Wavelengths: What is the meaning of '(variable)' in these definitions?  I suspect they can be removed - am I right in thinnking they just flag up that there are various possibilities on each case?  If so, this is clear from the ss4.3.x definitions and 'variable' is just confusing.

s5, Allocated Wavelengths definition: s/to the link identifier/to be associated with the Link Identifier/

s5, last sentence:

>  The type
>    value of the Wavelength Restriction Constraint TLV is TBD by IANA.
THis duplicates the heading above Figure 6 and should be deleted.

s5.1: Should there be a a separate error values for flagging syntactical errors in the constraint specifications - see Minor Issue relating to s4.3 above.  Alternatively maybe it could be an additional value on PCEP-ERROR type 10 (Reception of Invalid Object).

s6.1, para 1 and para 3: s/allow configuring/allow configuration of/

s6.5: Will advertisement of WSON RWA capability need additional options in the [PCEP-LS] specification?  It isn't clear to me that the existing draft of PCEP-LS caters for the required adverts.  If it does, a little more detal here might help. If not then liaison with the authors of PCEP-LS is needed before both dcouments are approved.

s8.1: add an extra line to the assignment request with an entry for Object Type:

          2-15: Reserved

s10: RFC 2119 and RFC 8174 MUST be normative.

AFAICS almost all the 'Informative' references in s10.2 are actually normative because they define objects used in this specification.  Looking through the text, I think RFC 6163, RFC 7581 (just about) and the PCEP-LS draft (probably) are really informative;  it is unclear whether  RFC 3630 and RFC 5329 (associated with IPv4 and IPv6 addresses) are useful references at all.  All the others are normative.