Last Call Review of draft-ietf-sfc-nsh-18
review-ietf-sfc-nsh-18-opsdir-lc-schoenwaelder-2017-08-04-00

I am the OPS-DIR reviewer and in general the document seems to be in
good shape. I understand that the WG will work on management
specifications later and so I will not ask for them now. ;-)

The only section that worries me a bit is the fragmentation
considerations section, which points to a not helpful section of an
expired I-D. I understand that MSH assumes that the underlay will
provide a large enough MTU. Still, I think it would help if the spec
would spell out what happens if on a mis-configured network the MTU
does not fit and I think the document should require that such events
(not all of them but sufficiently many) are logged so that such
problems can be identified. (Hunting MTU black-holes is no fun.)

The rest is mostly editorial, consider the comments as you see fit.

- Introduction

  I was not sure what a 'service plane protocol' is and I think this
  term is not needed here either. Here is a possible rewrite:

  OLD

    Network Service Header (NSH) defines a new service plane protocol
    specifically for the creation of dynamic service chains and is
    composed of the following elements:

  NEW

    Network Service Header (NSH) defines a new protocol for the
    creation of dynamic service chains and is composed of the
    following elements:

  That said, it is somewhat odd that a header defines a
  protocol. Perhaps more accurate:

  NEWER

    Network Service Header (NSH) is used by a new protocol for the
    creation of dynamic service chains and is composed of the
    following elements:

  I figured out that 'service plane' is defined later informally but
  as a first time reader I think it is preferable to avoid this term
  in the introduction.

- Definition of Terms

    Network Node/Element:  Device that forwards packets or frames based
       on an outer header (i.e., transport) information.

  I got confused by '(i.e., transport)' likely because transport can
  have multiple meanings. Perhaps simply remove '(i.e., transport)'?

    NSH-aware  SFC-encapsulation-aware, or SFC-aware [RFC7665].

  Does this mean all three terms mean the same thing and they are
  interchangeable? I guess not. I assume NSH-aware means SFC-aware
  when using an NSH, i.e., SFC-aware is the general concept and
  NFC-aware is the concrete case when NSH is used. It would be nice
  to   not have to guess...

- NSH-based Service Chaining

  Back to the term 'service plane' - should this be a defined term?
  It is defined informally here but it seems to be used in several
  places, perhaps it deserves a definition in the terminology section.

  What exactly is a 'network transport protocol'?

- NSH Base Header

  I tried to search for the explanation of U in Fig. 2 and did not
  find it. Perhaps change

    All other flag fields are unassigned [...]

  to

    All fields marked U are unassigned [...]

- Optional Variable Length Metadata

  s/order their appear/order they appear/

  s/must process first instance/must process the first instance/

- NSH Actions

  s/un-encapsulated packet/un-encapsulated packet./

  s/selection a different/selection of a different/

- Fragmentation Consideration

  The document refers to draft-ietf-rtgwg-dt-encap-02.txt, more
  specifically section 6. However, this I-D has expired. Section 6 of
  this document is a terminology section. Perhaps the pointer should
  have been to section 9? But even then, does the referenced text
  really help? Would it make sense to inline the text that is supposed
  to help here?

  I think this is a real management and operations concern. I assume
  (it is not state explicitly) that packets are dropped if
  fragmentation is needed but there is no way to fragment. Is there
  also an error reporting mechanism? Or a logging mechanism? Or are
  these then silently black-holed packets?