Re: [sfc] Shepherd review of draft-ietf-sfc-nsh-tlv

[[e-mail]]

Dear Corona,
thank you for your kind consideration of my comments. 
I think that the current text in Section 4.5 allows leaving the format of the Flow ID TLV as-is without introducing any new field:
The Flow ID is right justified
(appears in the least significant bits of the Flow ID word) and is
padded on the left with bits which MUST be sent as zero and ignored
on receipt.
If the Authors decide to add the MBZ field as you've proposed, the text quoted above will need to be changed accordingly.
I'll accept any decision the Authors decide to take.

Regards,
Greg Mirsky
Sr. Standardization Expert
预研标准部/有线研究院/有线产品经营部  Standard Preresearch Dept./Wireline Product R&D Institute/Wireline Product Operation Division
E: [e-mail]
www.zte.com.cn
------------------Original Mail------------------
Sender: wei yuehua00019655
To: gregory mirsky10211915;
CC: [e-mail];[e-mail];[e-mail];
Date: 2021/06/16 00:15
Subject: Re: [sfc] Shepherd review of draft-ietf-sfc-nsh-tlv
_______________________________________________
sfc mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/sfc

Dear Greg,
All the comments are reasonable and helpful.  will reflected to the new version.
I have a further question about the last commet:  "I think that the length of the Flow ID field (Section 4.5) is four octets. Unless there are plans to use other formats, the Flow ID TLV may be defined as the fixed-length and the value of the Length field defined as 4. (Also, in the figure that displays the format s/~/|/ two times.)"
I will replace var with 4. then, shall I add MBZ or reserved to 8 bits of the word?
replace:
0                   1                   2                   3
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
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|    Metadata Class = 0x0000    |  Type = TBA5  |U| Length = 4  |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|                     Flow ID                                   |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
with:
0                   1                   2                   3
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
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|    Metadata Class = 0x0000    |  Type = TBA5  |U| Length = 4  |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|     MBZ       |                   Flow ID                     |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Best Regards,
Yuehua Wei
M: [phone] E: [e-mail]


Sender: Gregory10211915
To: [e-mail];
CC: [e-mail];[e-mail];
Date: 2021年06月13日 04:27
Subject: [sfc] Shepherd review of draft-ietf-sfc-nsh-tlv
Dear Authors,
I've reviewed the document and would like to share my notes.
The document is well-written and is easy to read, thank you. I have several questions that, I believe, would be easy to resolve:
In the Introduction, SFC NSH is characterized as an "encapsulation required to support the SFC architecture". It seems that "required" is a statement a bit too bold since there are other solutions, e.g., RFC 8595, that support SFC architecture as defined in RFC 7665. Perhaps a slight re-wording as below be acceptable:
OLD TEXT:
The Network Service Header (NSH) [RFC8300] is the Service Function
Chaining (SFC) encapsulation required to support the SFC architecture
[RFC7665].
NEW TEXT:
The Network Service Header (NSH) [RFC8300] is the Service Function
Chaining (SFC) encapsulation that supports the SFC architecture
[RFC7665].
Section 3 opens with the following:
An NSH is composed of a 4-octet Base Header, a 4-octet Service Path
Header and Context Headers.
Since the use of a context header in the SFC NSH is optional (as noted further in the text), it might be appropriate to reflect that in the sentence:
An NSH is composed of a 4-octet Base Header, a 4-octet Service Path
Header and optional Context Headers.
The interpretation of the Context Type field in Section 4.1 refers to the Tenant ID field:
Context Type (CT) is four bits-long field that defines the length
and the interpretation of the Tenant ID field.
I think that instead of the Tenant ID it is meant to refer to the Forwarding Context ID field.
I've noticed that in all figures that display the format of TLVs in the document, with the exception of Figure 7, the value of the Length field is shown. Perhaps, for consistency, in Figure 7 can update s/Length/Length = var/.
I think that the length of the Flow ID field (Section 4.5) is four octets. Unless there are plans to use other formats, the Flow ID TLV may be defined as the fixed-length and the value of the Length field defined as 4. (Also, in the figure that displays the format s/~/|/ two times.)
Regards,
Greg Mirsky
Sr. Standardization Expert
预研标准部/有线研究院/有线产品经营部  Standard Preresearch Dept./Wireline Product R&D Institute/Wireline Product Operation Division
E: [e-mail]
www.zte.com.cn
_______________________________________________
sfc mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/sfc