Hi Carlos, Thank you for addressing my comments. Your suggested edits seem to address the issues.
Regards, Dan On Fri, Sep 1, 2017 at 3:55 AM, Carlos Pignataro (cpignata) < cpign...@cisco.com> wrote: > Hi, Dan, > > Many thanks for your detailed and valuable review, and apologies for the > delay in Ack-ing and responding. > > Please see inline. > > — > Carlos Pignataro, car...@cisco.com > > “Sometimes I use big words that I do not fully understand, to make myself > sound more photosynthesis." > > > On Aug 22, 2017, at 11:34 AM, Dan Romascanu <droma...@gmail.com> wrote: > > > > Reviewer: Dan Romascanu > > Review result: Almost Ready > > > > I am the assigned Gen-ART reviewer for this draft. The General Area > > Review Team (Gen-ART) reviews all IETF documents being processed > > by the IESG for the IETF Chair. Please treat these comments just > > like any other last call comments. > > > > For more information, please see the FAQ at > > > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > > > Document: draft-ietf-sfc-nsh-19 > > Reviewer: Dan Romascanu > > Review Date: 2017-08-22 > > IETF LC End Date: 2017-08-25 > > IESG Telechat date: 2017-09-14 > > > > Summary: > > > > This document describes the header (called Network Service Header - NSH) > to be > > inserted in packets and frames in order to create packets and frames that > > realize service functions described by other SFC documents. It needs to > be read > > in conjunction with RFC 7665 and other documents as the SFC control > plane I-D > > in order to make sense of the required functionality. This part of the > SFC > > solution seems almost ready, but a few issues mentioned below need in my > > opinion clarification before the document is approved. > > Thank you for this summary — you raise good points. > > > > > Major issues: > > > > 1. Section 11.1 claims: 'An IEEE EtherType, 0x894F, has been allocated > for > > NSH'. I could not find this value in the tables displayed by the IEEE > > Registration Authority (RAC) - for example at > > http://standards-oui.ieee.org/ethertype/eth.txt. Neither does IANA at > > https://www.iana.org/assignments/ieee-802-numbers/ieee-802-numbers.xhtml > (which > > would not be in any case the authoritative source). Can you please > indicate the > > source that this is indeed a confirmed IEEE EtherType registration. > > I am not sure how often that page is updated, but the same link you > include, http://standards-oui.ieee.org/ethertype/eth.txt, shows: > > 0x894F > 894F: NSH (Network Service Header). Reference: draft-ietf-sfc-nsh-18 > > Granted, that registration will be updated with the RFC number instead of > the I-D, when it publishes. > > > > > 2. Section 5 refers to ietf-rtgwg-dt-encap which is expired. If I > understand > > correctly the status of this work, there is a design team in place in the > > Routing Area created to look at common issues for the different data > plane > > encapsulations being discussed in various WGs including SFC. This leaves > the > > issue unsettled at this stage and this may be a problem for a standards > track > > document. I suggest that first the reference to the expired document is > > removed, second that the issue is marked as 'open' and subject for > future work. > > > > Good suggestion. Instead of removing the reference all together, we can > mark it as exemplifying of one approach. > > > Minor issues: > > > > 1. Two values 'Experiment 1' and 'Experiment 2' are defined in section > 2.2 and > > 11.2.5 for the Next Protocol. This seems a little odd. Why 2? Why are > they > > defined at the end of the range? Maybe there is an explanation for those > who > > know the history but for an un-initiated reader or a future implementer > this > > seems odd. > > There is no strong rational behind the number of values (two) or the > location (towards the end of the range). There are two values as it seems > appropriate given the same of the number space (2 out of 256). > > Perhaps, it’s somewhat following common practice started at > https://tools.ietf.org/html/rfc3692#section-2.1 (2 values for and 8-bit > field, closer to the end). > > > > > 2. In Section 2.2 I see: > > > >> All other flag fields, marked U, are unassigned and available for > > future use, see Section 11.2.1. Unassigned bits MUST be set to zero > > upon origination, and MUST be ignored and preserved unmodified by > > other NSH supporting elements. Elements which do not understand the > > meaning of any of these bits MUST NOT modify their actions based on > > those unknown bits. > > > > The way the last sentence is written it seems to open the path for > elements > > that claim to 'understand' the meaning of some Unassigned bit to modify > its > > behavior based upon it. This does not seem good, as at transmission all > > Unassigned bits MUST be set to zero. I would suggest to make the > statement > > simpler and just say that at reception all elements MUST NOT modify their > > actions based on these bits. > > > > Very good point, and good improvement. Applied. > > > 3. In Section 2.2 I see: > > > >> 0xF - This value is reserved for experimentation and testing, as per > > [RFC3692]. Implementations not explicitly configured to be part of > > an experiment SHOULD silently discard packets with MD Type 0xF. > > > > Why is this a SHOULD and not a MUST? > > > > To use MUSTs very sparingly and leave a little bit of leeway for > experimentation, following appropriate justification. > > > 4. I believe that [I-D.ietf-sfc-control-plane] needs to be a Normative > > Reference. There are several places in the document (e.g. in Section > 2.3) where > > this document is referred to describe behavior or actions related to the > fields > > of the header. > > > > The control plane I-D is, by WG decision, intended to not be normative as > NSH is the data plane independent of the CP spec. > > I see that [I-D.ietf-sfc-control-plane] is cited in two places: > > 1. > [I-D.ietf-sfc-control-plane] provides an example of such in > > 2. > process. These considerations are deployment-specific. However, the > control plane is entitled to instruct SFC-aware SFs with the data > structure of context header together with its scoping (see > Section 3.3.3 of [I-D.ietf-sfc-control-plane]). > > > We already caught the issue with #2 and changed it to, in our working copy > working with the chairs, to: > > process. These considerations are deployment-specific. However, the > control plane is entitled to instruct SFC-aware SFs with the data > structure of context header together with its scoping (see e.g., > Section 3.3.3 of [I-D.ietf-sfc-control-plane]). > > As that’s one possible, and non-normative way, out of many ways. > > > 5. The version number has only two bits assigned. Moreover, this document > > reserves already two values without any explanation why (only 00 is > mandatory > > to use, as far as I understand). This needs to be explained (maybe > 'historical' > > reasons - but unclear for future readers and users) and may be a > limitation as > > the protocol develops. > > The version field is indeed a two-bit field. One reserved and will not be > used. One used by this spec, and two unassigned ones. > > We really expect to not be bumping up version number ofter — or ever… > > > > > Nits/editorial comments: > > > > 1. Please make sure that acronyms are expanded at first occurrence (e.g. > ECMP) > > and if necessary appropriate references are being provided. > > > > Ack — done. > > Thanks again! > > Carlos. > > > > >
_______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
