Last Call Review of draft-ietf-sfc-nsh-19
review-ietf-sfc-nsh-19-genart-lc-romascanu-2017-08-22-00

Request Review of draft-ietf-sfc-nsh
Requested rev. no specific revision (document currently at 28)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2017-08-25
Requested 2017-08-11
Authors Paul Quinn, Uri Elzur, Carlos Pignataro
Draft last updated 2017-08-22
Completed reviews Rtgdir Early review of -10 by Acee Lindem (diff)
Secdir Last Call review of -18 by Christian Huitema (diff)
Opsdir Last Call review of -18 by Jürgen Schönwälder (diff)
Rtgdir Last Call review of -18 by Acee Lindem (diff)
Opsdir Last Call review of -19 by Jürgen Schönwälder (diff)
Tsvart Last Call review of -19 by Wesley Eddy (diff)
Genart Last Call review of -19 by Dan Romascanu (diff)
Secdir Telechat review of -25 by Christian Huitema (diff)
Assignment Reviewer Dan Romascanu
State Completed
Review review-ietf-sfc-nsh-19-genart-lc-romascanu-2017-08-22
Reviewed rev. 19 (document currently at 28)
Review result Almost Ready
Review completed: 2017-08-22

Review
review-ietf-sfc-nsh-19-genart-lc-romascanu-2017-08-22

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-sfc-nsh-19
Reviewer: Dan Romascanu
Review Date: 2017-08-22
IETF LC End Date: 2017-08-25
IESG Telechat date: 2017-09-14

Summary:

This document describes the header (called Network Service Header - NSH) to be inserted in packets and frames in order to create packets and frames that realize service functions described by other SFC documents. It needs to be read in conjunction with RFC 7665 and other documents as the SFC control plane I-D in order to make sense of the required functionality. This part of the SFC solution seems almost ready, but a few issues mentioned below need in my opinion clarification before the document is approved.  

Major issues:

1. Section 11.1 claims: 'An IEEE EtherType, 0x894F, has been allocated for NSH'. I could not find this value in the tables displayed by the IEEE Registration Authority (RAC) - for example at http://standards-oui.ieee.org/ethertype/eth.txt. Neither does IANA at https://www.iana.org/assignments/ieee-802-numbers/ieee-802-numbers.xhtml (which would not be in any case the authoritative source). Can you please indicate the source that this is indeed a confirmed IEEE EtherType registration. 

2. Section 5 refers to ietf-rtgwg-dt-encap which is expired. If I understand correctly the status of this work, there is a design team in place in the Routing Area created to look at common issues for the different data plane encapsulations being discussed in various WGs including SFC. This leaves the issue unsettled at this stage and this may be a problem for a standards track document. I suggest that first the reference to the expired document is removed, second that the issue is marked as 'open' and subject for future work. 

Minor issues:

1.  Two values 'Experiment 1' and 'Experiment 2' are defined in section 2.2 and 11.2.5 for the Next Protocol. This seems a little odd. Why 2? Why are they defined at the end of the range? Maybe there is an explanation for those who know the history but for an un-initiated reader or a future implementer this seems odd. 

2. In Section 2.2 I see: 

> All other flag fields, marked U, are unassigned and available for
   future use, see Section 11.2.1.  Unassigned bits MUST be set to zero
   upon origination, and MUST be ignored and preserved unmodified by
   other NSH supporting elements.  Elements which do not understand the
   meaning of any of these bits MUST NOT modify their actions based on
   those unknown bits.

The way the last sentence is written it seems to open the path for elements that claim to 'understand' the meaning of some Unassigned bit to modify its behavior based upon it. This does not seem good, as at transmission all Unassigned bits MUST be set to zero. I would suggest to make the statement simpler and just say that at reception all elements MUST NOT modify their actions based on these bits. 

3. In Section 2.2 I see: 

> 0xF - This value is reserved for experimentation and testing, as per
   [RFC3692].  Implementations not explicitly configured to be part of
   an experiment SHOULD silently discard packets with MD Type 0xF.

Why is this a SHOULD and not a MUST?

4. I believe that  [I-D.ietf-sfc-control-plane] needs to be a Normative Reference. There are several places in the document (e.g. in Section 2.3) where this document is referred to describe behavior or actions related to the fields of the header. 

5. The version number has only two bits assigned. Moreover, this document reserves already two values without any explanation why (only 00 is mandatory to use, as far as I understand). This needs to be explained (maybe 'historical' reasons - but unclear for future readers and users) and may be a limitation as the protocol develops. 


Nits/editorial comments: 

1. Please make sure that acronyms are expanded at first occurrence (e.g. ECMP) and if necessary appropriate references are being provided.