Subject: [sfc] WG review for draft-ietf-sfc-nsh-07.txt

From: "Jim Guichard (jguichar)" <[email-masked]>
Date: Tue, 23 Aug 2016 16:48:17 +0000

Dear WG:

Please review this new version of the SFC encapsulation to make sure that your comments from the previous WGLC have been addressed. 

Any comments and/or concerns please post to the mailing list within the next two weeks so that this work can be progressed.

As a heads up, if there are still substantive comments we plan to hold an interim meeting on September 15th to address any outstanding issues

Thanks!

Jim, Martin, & Thomas

On 8/23/16, 11:47 AM, "sfc on behalf of [email-masked]" <[email-masked] on behalf of [email-masked]> wrote:

>
>A New Internet-Draft is available from the on-line Internet-Drafts directories.
>This draft is a work item of the Service Function Chaining of the IETF.
>
>        Title           : Network Service Header
>        Authors         : Paul Quinn
>                          Uri Elzur
>	Filename        : draft-ietf-sfc-nsh-07.txt
>	Pages           : 37
>	Date            : 2016-08-23
>
>Abstract:
>   This document describes a Network Service Header (NSH) inserted onto
>   packets or frames to realize service function paths.  NSH also
>   provides a mechanism for metadata exchange along the instantiated
>   service path.  NSH is the SFC encapsulation required to support the
>   Service Function Chaining (SFC) Architecture (defined in RFC7665).
>
>
>The IETF datatracker status page for this draft is:
>https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/
>
>There's also a htmlized version available at:
>https://tools.ietf.org/html/draft-ietf-sfc-nsh-07
>
>A diff from the previous version is available at:
>https://www.ietf.org/rfcdiff?url2=draft-ietf-sfc-nsh-07
>
>
>Please note that it may take a couple of minutes from the time of submission
>until the htmlized version and diff are available at tools.ietf.org.
>
>Internet-Drafts are also available by anonymous FTP at:
>ftp://ftp.ietf.org/internet-drafts/
>
>_______________________________________________
>sfc mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/sfc

---

From: "Andrew G. Malis" <[email-masked]>
Date: Tue, 23 Aug 2016 13:10:35 -0400

Jim,

I noticed an editing error, at the bottom of p. 12 it still discusses three
reserved bits in fig. 6, but now there’s only one bit.

Cheers,
Andy

On Tue, Aug 23, 2016 at 12:48 PM, Jim Guichard (jguichar) <
[email-masked]> wrote:

> Dear WG:
>
> Please review this new version of the SFC encapsulation to make sure that
> your comments from the previous WGLC have been addressed.
>
> Any comments and/or concerns please post to the mailing list within the
> next two weeks so that this work can be progressed.
>
> As a heads up, if there are still substantive comments we plan to hold an
> interim meeting on September 15th to address any outstanding issues
>
>
> Thanks!
>
> Jim, Martin, & Thomas
>
>
>
>
> On 8/23/16, 11:47 AM, "sfc on behalf of [email-masked]" <
> [email-masked] on behalf of [email-masked]> wrote:
>
> >

---

From: <[email-masked]>
Date: Fri, 26 Aug 2016 09:31:23 +0000

Hi Jim, all,

Likewise, I would like to see this document progress as well. 

In addition to the comment raised in a separate message about the C-bit of TLVs, I have the following comments:

(1) Section 2: I suggest to delete the following sentence: 

   NSH is designed to be easy to implement across a range of devices,
   both physical and virtual, including hardware platforms.

Reason: the use of "easy to implement" should be justified if that text is maintained. Otherwise, this is subjective. 

or to reword it as follows: 

   "NSH can be implemented in both physical and virtual platforms."

(2) Section 2.3: I suggest to change this OLD text:

The semantics of
       the shared metadata is communicated via a control plane, which is
       outside the scope of this document, to participating nodes.

NEW:

The semantics of 
the shared metadata is communicated via a control plane (Section 3.3 of [I-D.ietf-sfc-control-plane]).

(3) Section 2.3- Not sure I would maintain this text: 

   5.  NSH offers a common and standards-based header for service
       chaining to all network and service nodes.

"to all" is not accurate as "legacy" nodes are still there.  

(4) Section 2.3:

OLD:
Transport Agnostic: NSH is transport independent and is often
       carried via a network transport protocol,

NEW:
Transport Agnostic: NSH is transport independent. Any network transport protocol can be used to transport NSH-encapsulated traffic.

(5) Section 3.2:

   NSH implementations MUST support MD-Type = 0x1, and SHOULD support
   MD- Type = 0x2.

Because:
* of potential interoperability issues.
* MD#2 is more compact when no metadata is to be supplied
* MD#2 allows to convey more information compared to MD#1

I suggest we revisit that sentence to have MD#2 be mandatory to be supported (my favorite option), or at least require that both MDs defined in this spec MUST be supported. 

(6) Section 3.2:

   C bit: Indicates that a critical metadata TLV is present.  This bit
   acts as an indication for hardware implementers to decide how to
   handle the presence of a critical TLV without necessarily needing to
   parse all TLVs present.  For an MD Type of 0x1 (i.e. no variable
   length metadata is present), the C bit MUST be set to 0x0.

6.1. What is the behavior of the implementation if C-bit is set but not critical metadata is found in the payload?
6.2. What is the behavior of the implementation if C-bit is unset but a critical metadata is found in the payload?
6.3. What is the behavior of the implementation with regards to this bit if a critical metadata is added in-path?
6.4. What is the behavior of the implementation with regards to this bit if a critical metadata is stripped in-path?   
6.5. Should the specification recommend an order of metadata so that critical metadata are always positioned first?

(7) Section 3.3: The following text 

" ... however the
   control plane MAY configure the initial value of SI as appropriate
   (i.e. taking into account the length of the service function path). "

is not aligned with the outcome of the discussion we had during the WG LC of draft-ietf-sfc-control-plane (https://www.ietf.org/mail-archive/web/sfc/current/msg04594.html):

   The control plane must instruct the classifier about the initial
   values of the Service Index (SI).

7.1. I suggest to modify NSH draft accordingly. 
7.2. Please consider adding a reference to draft-ietf-sfc-control-plane

(8) Section 3.4: I still do think this section is underspecified. E.g., 

8.1. The use of "Mandatory Context Header" conflicts with this text in Section 3:

   A Network Service Header (NSH) contains service path information and
   optionally metadata that are added to a packet or frame and used to
   ^^^^^^^^^^^^^^^^^^^
   create a service plane.

8.2. The specification does not forbid that all context headers are set to zero. Otherwise, MD#2 should be used instead.
8.3. The specification does not specify how these context headers are to be validated. 
8.4. I still don't understand why four headers are chosen.  

(9) Section 3.5.1:

The length of "Metadata Class" is over-dimensioned. I suggest to reduce the length of this field and increase the length of "Type".
 
For example, let's consider the proposal in https://tools.ietf.org/html/draft-napper-sfc-nsh-broadband-allocation-00#section-4.2: 

    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
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |     TLV Class = 3GPP          |C|    Type     |R|R|R|   Len   |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |    Data ...
   +-+-+-+-+-+-+-+-+

                         Figure 5: TLV Allocation

   The intended use of the header is for TLVs associated with 3GPP Radio
   Access Networks as described in [TS.29.230].  This TLV can be used by
   3GPP to extend the metadata as per use cases.  Having this TLV helps
   to carry more information that does not fit within the MD Type 0x01.

The list of codes in Table 7.1 of TS.29.230 cannot be conveyed with a "Type" field of 7 bits.

(10) Section 4: 

OLD:

+---------------+------------------+-------+----------------+---------+
 |                |  Insert         |Select |   Update       |Service  |
 |                |  or remove NSH  |Service|    NSH         |policy   |
 |                |                 |Function|               |selection|
 | Component      +--------+--------+Path   +----------------+         |
 |                |        |        |       | Dec.   |Update |         |
 |                | Insert | Remove |       |Service |Context|         |
 |                |        |        |       | Index  |Header |         |
 +----------------+--------+--------+-------+--------+-------+---------+
 |                |   +    |   +    |       |        |   +   |         |
 |Classifier      |        |        |       |        |       |         |
 +--------------- +--------+--------+-------+--------+-------+---------+
 |Service Function|        |   +    |  +    |        |       |         |
 |Forwarder(SFF)  |        |        |       |        |       |         |
 +--------------- +--------+--------+-------+--------+-------+---------+
 |Service         |        |        |       |   +    |   +   |   +     |
 |Function  (SF)  |        |        |       |        |       |         |
 +--------------- +--------+--------+-------+--------+-------+---------+
 |SFC Proxy       |   +    |   +    |       |   +    |       |         |
 +----------------+--------+--------+-------+--------+-------+---------+

NEW:

+---------------+------------------+-------+----------------+---------+
 |                |  Insert         |Select |   Update       |Service  |
 |                |  or remove NSH  |Service|    NSH         |policy   |
 |                |                 |Function|               |selection|
 | Component      +--------+--------+Path   +----------------+         |
 |                |        |        |       | Dec.   |Update |         |
 |                | Insert | Remove |       |Service |Context|         |
 |                |        |        |       | Index  |Header |         |
 +----------------+--------+--------+-------+--------+-------+---------+
 |                |   +    |   +    |       |        |   +   |         |
 |Classifier      |        |        |       |        |       |         |
 +--------------- +--------+--------+-------+--------+-------+---------+
 |Service Function|        |   +    |  +    |        |       |         |
 |Forwarder(SFF)  |        |        |       |        |       |         |
 +--------------- +--------+--------+-------+--------+-------+---------+
 |Service         |        |        |       |   +    |   +   |   +     |
 |Function  (SF)  |        |        |       |        |       |         |
 +--------------- +--------+--------+-------+--------+-------+---------+
 |SFC Proxy       |   +    |   +    |       |   +    |   +   |         |
 +----------------+--------+--------+-------+--------+-------+---------+

(11) Section 7.2: Please consider adding an IPv6 example.

Thank you.

Cheers,
Med

> -----Message d'origine-----
> De : sfc [mailto:[email-masked]] De la part de Jim Guichard
> (jguichar)
> Envoyé : mardi 23 août 2016 18:48
> À : [email-masked]
> Objet : [sfc] WG review for draft-ietf-sfc-nsh-07.txt
> 
> Dear WG:
> 
> On 8/23/16, 11:47 AM, "sfc on behalf of [email-masked]" <sfc-
> [email-masked] on behalf of [email-masked]> wrote:
> 
> >