Last Call Review of draft-ietf-sfc-nsh-18
review-ietf-sfc-nsh-18-opsdir-lc-schoenwaelder-2017-08-04-00
| Request | Review of | draft-ietf-sfc-nsh |
|---|---|---|
| Requested revision | No specific revision (document currently at 28) | |
| Type | Last Call Review | |
| Team | Ops Directorate (opsdir) | |
| Deadline | 2017-08-11 | |
| Requested | 2017-07-26 | |
| Requested by | Alia Atlas | |
| Authors | Paul Quinn , Uri Elzur , Carlos Pignataro | |
| Draft last updated | 2017-08-04 | |
| 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) |
|
| Comments |
Requesting in parallel with doing my AD review. Earlier reviews appreciated! |
|
| Assignment | Reviewer | Jürgen Schönwälder |
| State | Completed | |
| Review |
review-ietf-sfc-nsh-18-opsdir-lc-schoenwaelder-2017-08-04
|
|
| Reviewed revision | 18 (document currently at 28) | |
| Result | Has Issues | |
| Completed | 2017-08-04 |
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?