Opened 5 years ago

Last modified 5 years ago

#24 new defect

-07 review (M. Boucadair)

Reported by: mohamed.boucadair@… Owned by: draft-ietf-sfc-nsh@…
Priority: major Milestone:
Component: nsh Version:
Severity: - Keywords:
Cc:

Description

I'm adding this review to the tracker as I'm not sure all the points were addressed.

==
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:

  1. 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

Change History (5)

comment:1 follow-up: Changed 5 years ago by 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:

  1. 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.

comment:2 in reply to: ↑ 1 Changed 5 years ago by jguichar@…

Please review the email responses sent by Paul Quinn on 9/10/2016 that seem to address many of the below comments.

Replying to 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:

  1. 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.

comment:3 Changed 5 years ago by jguichar@…

Several items are now closed for this ticket as indicated in the previous comments. Updating this ticket to reflect only the open items.
#########

(1) Section 2:

Comment: 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.

*or* to reword it as follows:

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

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

(2) Section 2.3, bullet point 5:

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

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

(3) Section 3.2:

NSH implementations MUST support MD-Type = 0x1, and SHOULD support MD- Type = 0x2.
Comment: 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.

(4) 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.

Comments:
4.1. What is the behavior of the implementation if C-bit is set but not critical metadata is found in the payload?
4.2. What is the behavior of the implementation if C-bit is unset but a critical metadata is found in the payload?
4.3. What is the behavior of the implementation with regards to this bit if a critical metadata is added in-path?
4.4. What is the behavior of the implementation with regards to this bit if a critical metadata is stripped in-path?
4.5. Should the specification recommend an order of metadata so that critical metadata are always positioned first?

(5) Section 3.4:

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

(6) Section 3.5.1:

Comment:
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.

(7) 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 | + | + | | + | + | |
+----------------+--------+--------+-------+--------+-------+---------+

comment:4 Changed 5 years ago by jguichar@…

Updating this ticket based upon WG chair review.

(1) Section 2: CLOSED

Comment: 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.

*or* to reword it as follows:

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

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

NSH document editor comments: The “easy to implement” is a reference to the nature of type-1 and how, as noted by hardware implementers, this vastly simplifies implementation. Further, the language is widely used in RFCs (e.g.7321, 3160, etc.).

WG chairs comments: having reviewed the email thread we agree with the SFC document editors comment below as the language used is consistent with other cited RFCs and given no other comments on this point the chairs feel there is no need for a textual change. This item should be marked as CLOSED.

(2) Section 2.3, bullet point 5: CLOSED

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

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

NSH document editors comments: Given that “legacy” nodes are supported via proxy, this is indeed true.

WG chairs comments: the WG chairs agree that legacy nodes are supported via proxy and this is clear from the SFC architecture and therefore the chairs feel there is no need for a textual change. This item should be marked as CLOSED.

(3) Section 3.2: CLOSED

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

Comment: 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.

WG chairs comments: this point has been discussed several times on the SFC mailing list with no consensus for making MD#2 a MUST. A compromise was added however to indicate that an SFF MUST have the capability of skipping the MD#2. The chairs do not feel there is a need for a textual change. This item should be marked as CLOSED.

(4) Section 3.2: OPEN

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.

Comments:

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

WG chairs comments: Suggest to reword this section as follows but gain consensus from the WG on the text:

C bit: Indicates that a critical metadata TLV is present. When this bit is not set, an SFF is free to examine or not examine metadata according to local policy and implementation. This bit may only be set for NSH messages of MD type 0x2. When the bit is set on such messages, the SFF MUST examine the type fields of the metadata in the NSH header for information which affects its behavior, as the bit indicates that the writer understood some piece of metadata to be critical for processing. For NSH messages with an MD type of 0x1, the C bit MUST be set to 0x0.

(5) Section 3.4: CLOSED

Comments:

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

WG chairs comments: having reviewed this item the chairs do not see any consensus from the WG for textual changes and therefore this item should be marked CLOSED.

(6) Section 3.5.1: CLOSED

Comment:

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:

WG chairs comments: having reviewed this item the chairs do not see any consensus from the WG for textual changes. This item should be marked as CLOSED.

comment:5 Changed 5 years ago by jguichar@…

Updating this ticket to reflect outstanding OPEN items (please see previous comments for CLOSED items):

(4) Section 3.2: OPEN

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.

Comments:

4.1. What is the behavior of the implementation if C-bit is set but not critical metadata is found in the payload?

4.2. What is the behavior of the implementation if C-bit is unset but a critical metadata is found in the payload?
4.3. What is the behavior of the implementation with regards to this bit if a critical metadata is added in-path?
4.4. What is the behavior of the implementation with regards to this bit if a critical metadata is stripped in-path?
4.5. Should the specification recommend an order of metadata so that critical metadata are always positioned first?

WG chairs comments: Suggest to reword this section as follows but gain consensus from the WG on the text:

C bit: Indicates that a critical metadata TLV is present. When this bit is not set, an SFF is free to examine or not examine metadata according to local policy and implementation. This bit may only be set for NSH messages of MD type 0x2. When the bit is set on such messages, the SFF MUST examine the type fields of the metadata in the NSH header for information which affects its behavior, as the bit indicates that the writer understood some piece of metadata to be critical for processing. For NSH messages with an MD type of 0x1, the C bit MUST be set to 0x0.

Note: See TracTickets for help on using tickets.