Re: [sfc] NSH document updates

Kyle Larose <klarose@sandvine.com> Wed, 10 May 2017 18:13 UTC

From: Kyle Larose <klarose@sandvine.com>
To: James N Guichard <james.n.guichard@huawei.com>, "sfc@ietf.org" <sfc@ietf.org>
Thread-Topic: NSH document updates
Thread-Index: AdLFrALNtMYuKwwIS0uxdED9bjYxiQECMbSw
Date: Wed, 10 May 2017 18:13:18 +0000
Message-ID: <D76BBBCF97F57144BB5FCF08007244A7705D37D7@wtl-exchp-1.sandvine.com>
References: <BF1BE6D99B52F84AB9B48B7CF6F17DA3DD863E@SJCEML701-CHM.china.huawei.com>
In-Reply-To: <BF1BE6D99B52F84AB9B48B7CF6F17DA3DD863E@SJCEML701-CHM.china.huawei.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: multipart/alternative; boundary="_000_D76BBBCF97F57144BB5FCF08007244A7705D37D7wtlexchp1sandvi_"
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/sfc/nnXbZEvb_q_63hMs2htrhrzjkdE>
Subject: Re: [sfc] NSH document updates
Precedence: list

I support these changes, aside from a few nits below.

-------------------------------------------------------------------------

* In item 5:

In the Base Header, we have a field called "MD Type". In the suggested
replacement text for 12.2.4, there is talk about "MD Types," which
confused me, since it seems to be talking not about the field in the
base header, but instead about the field in the MD-Type 2 context header.

In particular:

"Designated Experts evaluating new allocation requests
   from the "Expert Review" range should principally
   consider whether a new MD class is needed compared to
   adding >>>>MD types<<<< to an existing class."

That phrase only makes sense to me if "MD types" means values for the type
field in the context header. However, that is called the "Type" field, not
the "MD Type" field in section 3.5.1. We should use consistent terminology
here to avoid confusion.


* Regarding item 6, I have a few concerns about the naming.

"MD TLV Type Registry" seems pretty general. Is this truly a registry for
all types of metadata using TLVs? Perhaps we should qualify it:
"NSH MD-Type 2 TLV Type Register". I know it's a bit of a mouthful, but
it pays to be precise.


* In item 9, there is a typo.

Line "unexpected outcomes forothers, thus it is recommended to analyze the"
should be replaced with
     "unexpected outcomes for others, thus it is recommended to analyze the


* In item 11, a few things.

First, the alignment seems off in the base header format.
    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|R|    TTL    |   Length  |R|R|R|R|MD Type| Next Protocol |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

The bit offsets are supposed to be between the "+" symbols, right?

Second, why does the replacement text for 12.2.1 keep the same structure as the
old text? Previously it spoke about the five reserved bits within the first 10
bits. There is no longer a contiguous set of bits in the example. Should we just
list the location of the reserved bits?

I suggest:

   Bits 0-1 - Version
   Bit 2 - OAM (O bit)
   Bit 3 - Reserved
   Bits 16-19 - Reserved

Change to
   Bit 3, and bits 16-19 are reserved.

Thanks,

Kyle



From: sfc [mailto:sfc-bounces@ietf.org] On Behalf Of James N Guichard
Sent: Friday, May 05, 2017 11:17 AM
To: sfc@ietf.org
Subject: [sfc] NSH document updates

Dear WG:

Please review the attached .txt which contains all of the changes captured by the chairs for the NSH document.

Please provide comments and support before 5/12 so that the NSH document editors may make the required changes and publish a new version of the document.

Thanks!

Jim & Joel

[sfc] NSH document updates James N Guichard
Re: [sfc] NSH document updates Kyle Larose