Re: [sfc] #24 (nsh): -07 review (M. Boucadair)

[<[e-mail]>]

Hi Jim, 

I have already considered that answer form Paul. 

I consider them as open because of my answer here: https://www.ietf.org/mail-archive/web/sfc/current/msg05040.html.

Cheers,
Med

> -----Message d'origine-----
> De : sfc issue tracker [mailto:[e-mail]]
> Envoyé : vendredi 28 octobre 2016 00:26
> À : [e-mail]; BOUCADAIR Mohamed IMT/OLN;
> [e-mail]
> Cc : [e-mail]
> Objet : Re: [sfc] #24 (nsh): -07 review (M. Boucadair)
> 
> #24: -07 review (M. Boucadair)
> 
> 
> Comment (by [e-mail]):
> 
>  Please review the email responses sent by Paul Quinn on 9/10/2016 that
>  seem to address many of the below comments.
> 
> 
> 
>  Replying to [comment:1 mohamed.boucadair@…]:
>  > Updating the tracker to record the pending points:
>  >
>  > ==
>  > (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: CLOSED
>  >
>  >  (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: CLOSED
>  >
>  >  (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: CLOSED
>  >
>  >  (8) Section 3.4:
>  >
>  >  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: CLOSED.
>  >
>  >
> 
> --
> -------------------------------------+------------------------------------
> -
>  Reporter:                           |       Owner:  draft-ietf-sfc-
>   [e-mail]       |  [e-mail]
>      Type:  defect                   |      Status:  new
>  Priority:  major                    |   Milestone:
> Component:  nsh                      |     Version:
>  Severity:  -                        |  Resolution:
>  Keywords:                           |
> -------------------------------------+------------------------------------
> -
> 
> Ticket URL: <https://trac.tools.ietf.org/wg/sfc/trac/ticket/24#comment:2>
> sfc <https://tools.ietf.org/sfc/>