Subject: Re: [sfc] I-D Action: draft-ietf-sfc-nsh-tlv-02.txt

From: Greg Mirsky <[email-masked]>
Date: Wed, 1 Apr 2020 16:54:43 -0700

Hi Carlos and Med,
thank you for the discussion. Please find my notes in-lined below under
GIM>> tag.

Regards,
Greg

On Tue, Mar 31, 2020 at 7:22 AM Carlos Pignataro (cpignata) <cpignata=
[email-masked]> wrote:

> Hi, Med,
>
> It is somewhat hard for me to follow the inlined comments. If there’s
> something I miss, please let me know. I’ll preface with “[CMP]"
>
> 2020/03/31 午前10:07、[email-masked]:
>
> Hi Carlos,
>
> Please see inline.
>
> Cheers,
> Med
>
> *De :* Carlos Pignataro (cpignata) [mailto:[email-masked]
> <[email-masked]>]
> *Envoyé :* mardi 31 mars 2020 15:23
> *À :* BOUCADAIR Mohamed TGI/OLN
> *Cc :* [email-masked]; [email-masked];
> [email-masked]
> *Objet :* Re: [sfc] I-D Action: draft-ietf-sfc-nsh-tlv-02.txt
>
> Hi, Med,
>
> Really good comments!
>
> In general, I tend to agree with your second comment, which is a
> meta-comment, about adding more descriptive text. That said, I’d cautious
> that this document defines foundational data fields and not the usage and
> use cases of them.
> *[Med] Without being verbose, we need to explain why a TLV is useful/how
> it can be used. For example, a reader may ask: how the flow TLV is
> populated? Why it is needed if we can access to the inner packet? should
> the flow TLV by updated on-path? What if there are address rewriting SFs?*
>
>
>
> [CMP] I believe this document should answer the questions on syntax and
> definition/meaning, but not the use-case. Saying “Flow id is for marking
> packets of the same flow, and it is a 32-bit number” is as far as I think
> this should go.
>
> [CMP] What do others think?
>
GIM>> I agree with Med, the document should give at least one example of
how a TLV can be used. At the same time, future documents may define
another scenarios.

>
> *I may have answers to some of those questions but my point is that we
> need to have some of the details in the document (especially, when this is
> important for interoperability).  *
>
>
> Interoperability has many layers. Syntax and semantics need to be ensured
> in this doc.
>
GIM>> I think that the document should define actionable aspect of a TLV.
Just providing format seems insufficient.

>
>
> Based on that, see below.
>
>
> 2020/03/31 午前4:16、[email-masked]:
>
> Hi all,
>
> Some easy to fix comments:
>
> ·         You may consider adding more description to the actual usage of
> each TLVs.
>
> This document defines information elements rather than use cases for them.
> As such, I hesitate in adding too much usage which can be containing. That
> said, do you have specific thoughts on usage?
> *[Med] I do have for some (content type, for example), but not sure about
> the others. For example, who sets the tenant-ID? Is it by control or
> extracted from the packets? Idem for the policy-ID.  *
>
>
> [CMP] To me specific usages are outside scope. Many specifications in the
> IETF describe the fields while future companion documents define usage.
> This, to me, is one of those.
>
GIM>> Yes, and there are examples when forward-looking definitions where
taken from the document and moved into the document that explains how that
object is used.

>
>
> ·         The TLVs content should also be clarified. For example, “Akin,
> but not identical to the usage described in [RFC6437]” does not say what is
> the diff vs. 6437 nor why not reusing FL would be sufficient if an IPv6
> transport is used.
>
> Agreed. I would replace:
>
>    Flow ID provides a representation of the flow.  Akin, but not
>    identical to the usage described in [RFC6437].
>
>
> With at least
>    The Flow ID provides a field in the NSH MD Type 2 to label
>    packets belonging to the same flow.  Absence of this field, or
>    a value of zero denotes that packets have not been labeled.
>
>
> *[Med] That’s a good start. Thanks. The natural questions then are: what
> is a label? What to do with a labeled a packet? do we allow to change the
> label when progressing in an SFP? We need to think about this further. *
>
>
> [CMP] I did not mean to add a “label” as a noun, but rather label as in
> mark or tag.
>
> [CMP] Immutability is a great question! I tent to think that it could
> change, but what do you think?
>
>
> ·         Clarify whether conveying multiple instances of each TLV is
> allowed.
>
> That is not for this document to say.
> *[Med] RFC8300 says the following:*
>
>    If multiple instances of the same metadata are included in an NSH
>    packet, but the definition of that Context Header does not allow for
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    it, the SFC-aware SF MUST process the first instance and ignore
>    ^^
>    subsequent instances.  The SFC-aware SF MAY log or increase a counter
>    for this event.
>
> Some use cases can restrict to one interface for example, others to
> multiple.
>
>
> [CMP] Fair enough. We could add requirements on multiplicity.
>
GIM>> I agree that the question of multiple occurence of a TLV should be
specified for each TLV (or we can have a blanket statement with exceptions
though that might add to some confusion).

>
>
>
> ·         What is a “a network-centric forwarding context”?
>
> Here’s my take on text proposal on various fields (this is full text, not
> diff):
> *[Med] Thank you for drafting this. I hope I can provide text as well.*
>
>
> [CMP] Of course! That’s a back-of-napkin start only.
>
>
> 4.1.  Forwarding Context
>
>    The forwarding context used for segregation and forwarding scope can
>    Take several forms depending on the network environment. For example,
>    VXLAN/VXLAN-GPE VNID, VRF identification, and VLAN. This context
>    header carries this network-based forwarding context.
>
> 4.2.  Tenant Identifier
>
>    Tenant identification is often used for segregation within a multi-
>    tenant environment.  Orchestration system-generated tenant IDs are an
>    example of such data. This context header carries both the format and
>    value of the Tenant identifier.
>
>
> 4.3.  Content Type
> // I believe this needs further thinking on the namespace fo Content
> Types, could even be draft-penno-sfc-appid-05
>
> 4.4.  Ingress Network Information
>
>    This data identifies the ingress network node, and, if required,
>    ingress interface.
>
> // this should be split into Node and Ingress Interface.
>
> 4.4. Ingress Network Node Information
>
>
>    This context header identifies the ingress network node.
>
>
> 4.5. Ingress Network Interface Information
>
>
>    This context header identifies the ingress network interface.
>
>
>
> 4.6.  Flow ID
>    The Flow ID provides a field in the NSH MD Type 2 to label
>    packets belonging to the same flow.  Absence of this field, or
>    a value of zero denotes that packets have not been labeled.
>
>
> ...
>
>
> ·         Section 7: Please delete “IANA is requested to create a new
> "Network Service Header (NSH) TLV” as we do already have a registry at:
> https://www.iana.org/assignments/nsh/nsh.xhtml#optional-variable-length-metadata-types.
> You may use this NEW wording:
>
>    This document requests IANA to assign the following types from the
>    "NSH IETF- Assigned Optional Variable-Length Metadata Types" (0x0000
>    IETF Base NSH MD Class) registry available at:
>    https://www.iana.org/assignments/nsh/nsh.xhtml#optional-variable-
> <https://www.iana.org/assignments/nsh/nsh.xhtml#optional-variable-length-metadata-types>
>    length-metadata-types
> <https://www.iana.org/assignments/nsh/nsh.xhtml#optional-variable-length-metadata-types>
> .
>
> Indeed!!!
>
> And additionally should move the Types to TBAs.
>
> Thanks,
>
> Carlos.
>
>
>
> Thank you.
>
>
>
> [CMP] You are welcome!
>
> Carlos.
>
> Cheers,
> Med
>
> *De :* sfc [mailto:[email-masked] <[email-masked]>] *De la
> part de* [email-masked]
> *Envoyé :* lundi 30 mars 2020 09:29
> *À :* [email-masked]; [email-masked]
> *Cc :* [email-masked]; [email-masked]
> *Objet :* Re: [sfc] I-D Action: draft-ietf-sfc-nsh-tlv-02.txt
>
> Dear Carlos,
> Thank you for reminding this kind of modifications to the mailing list so
> that more people could notice and give comments.
> https://tools.ietf.org/rfcdiff?url2=draft-ietf-sfc-nsh-tlv-02.txt
> Your suggestion is reasonable.
>
> Does anyone aware of implementations?
> or How do you think of TBAs?
>
> I appreciate your comments.
>
> *Best Regards,*
> *魏月华 Corona Wei*
> M: +86 13851460269 E: [email-masked]
>
> 原始邮件
> *发件人：*CarlosPignataro(cpignata) <[email-masked]>
> *收件人：*魏月华00019655;
> *抄送人：[email-masked] <[email-masked]>;[email-masked] <
> [email-masked]>;
> *日* *期* *：*2020年03月28日 21:49
> *主* *题* *：**Re: [sfc] I-D Action: draft-ietf-sfc-nsh-tlv-02.txt*
> Dear Wei,
> Thanks for the quick responses and accepting the suggestions.
>
> Only one follow-up on this, which I bring to the top:
>
> Wei>> Only editorial improvement.
> The Types in previous version were:
> 0x1,    0x4,    0x6,   0x7,  0x8,     0x9，  0xA，  0xB
> The Types in current version are:
> 0x01,  0x02,  0x03, 0x04,0x05，0x06，0x07，0x08
>
>
> I do not believe that changing a protocol value is an editorial change (or
> editorial improvement) necessarily.
>
> Are there implementations following the previous allocations? Unused
> values in between might look weird but it is to necessarily unoptimized,
> unimproved, or wrong.
>
> If a goal is to have a gapless increasing list and there are no
> implementations, then TBAs can be used and assignment at the end (e.g., if
> we split the node/interface into two).
>
> On the other hand, I’d always prioritize implementations (the entropy of
> reality will take care of disorganize the list :-)
>
> I’d leave them as they were, since there are enough numbers in the space,
> or ask on the list for changing and wait.
>
> Thanks,
>
> Carlos.
>
>
>
> 2020/03/27 午後11:13、[email-masked]:
>
> Dear Carlos,
> I appreciated your comments.
> Please see my response in line with Wei>> tag
> *Best Regards,*
> *魏月华 Corona Wei*
> M: +86 13851460269 E: [email-masked]
>
>
>
> *发件人：*CarlosPignataro(cpignata) <cpignata=[email-masked]>
> *收件人：[email-masked] <[email-masked]>;
> *抄送人：[email-masked] <[email-masked]>;
> *日* *期* *：*2020年03月27日 02:36
> *主* *题* *：**Re: [sfc] I-D Action: draft-ietf-sfc-nsh-tlv-02.txt*
>
> _______________________________________________
> sfc mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sfc
> Hi, SFC,
>
> Apologies, a late response to
> https://mailarchive.ietf.org/arch/msg/sfc/pNT0kOnFHZeTBLXjuIH0hoKhJs4/
>
> Looking at the document I’ve got a few comments, prefaced with “CMP”:
>
>
> 2.1.  Terminology
>
>
> CMP: Instead of duplicating definition of terms, why not point to RFC 8300
> and RFC 7665 as needed, and only define new terms.
>
> Wei>> Will delete duplicated NSH, MD Type and SFC which have been defined
> in  RFC 8300 and RFC 7665
>
>        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
>
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>       |Ver|O|C|R|R|R|R|R|R|   Length  |    MD Type    | Next Protocol |
>
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>
>
>                          Figure 1: NSH Base Header
>
> CMP: This is an old version, missing TTLL from RFC 88300
> Wei>> Will modify this figure to align with RFC8300
>
>
>    where
>
>
>
>       Metadata Class (MD Class): Defines the scope of the Type field to
>
>       provide a hierarchical namespace.
>
>
>
>       Type - Indicates the explicit type of metadata being carried.  The
>
>       value is one from the Network Service Header (NSH) TLV Type
>
>       registry (Section 7).
>
>
>
>       Unassigned bit: One unassigned bit is available for future use.
>
>       This bit MUST NOT be set, and it MUST be ignored on receipt.
>
>
>
>       Length: Indicates the length of the variable-length metadata, in
>
>       bytes.  In case the metadata length is not an integer number of
>
>       4-byte words, the sender MUST add pad bytes immediately following
>
>       the last metadata byte to extend the metadata to an integer number
>
>       of 4-byte words.  The receiver MUST round the Length field up to
>
>       the nearest 4-byte-word boundary, to locate and process the next
>
>       field in the packet.  The receiver MUST access only those bytes in
>
>       the metadata indicated by the Length field (i.e., actual number of
>
>       bytes) and MUST ignore the remaining bytes up to the nearest 4-
>
>       byte-word boundary.  The length may be 0 or greater.
>
>
>
>       A value of 0 denotes a Context Header without a Variable-Length
>
>       Metadata field.
>
>
> CMP: Instead of copy/paste repeating this from RFC 8300, please add a
> pointer. Copy/past text gets desynchronized easily.
> Wei>> wil delete this part.
>
>
> 4.2.  Tenant Identifier
>
>
>
>    Tenant identification is often used for segregation within a multi-
>
>    tenant environment.  Orchestration system-generated tenant IDs are an
>
>    example of such data.
>
>
>
>        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 = 0x02  |U|  Length=12  |
>
>
> CMP: Seems like this Type, as well as others, were changed from the
> previous version. Why?
> Wei>> Only editorial improvement.
> The Types in previous version were:
> 0x1,    0x4,    0x6,   0x7,  0x8,     0x9，  0xA，  0xB
> The Types in current version are:
> 0x01,  0x02,  0x03, 0x04,0x05，0x06，0x07，0x08
>
>
> 7.  IANA Considerations
>
>
> CMP: This needs registration: Metadata Class = 0x0000
>
> CMP: Also, the Context Type (CT) needs registration.
>
> CMP: And these as well:
> * Tenant Type (TT)
> * Group Type (GT)
> * URI Type
> Wei>> Will follow your advice.
>
> Thanks!
>
> Carlos.
>
>
> _______________________________________________
> sfc mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sfc
>
>
>
> _______________________________________________
> sfc mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sfc
>