Received-SPF: Pass (protection.outlook.com: domain of alum.mit.edu designates
 18.7.68.33 as permitted sender) receiver=protection.outlook.com;
 client-ip=18.7.68.33; helo=outgoing-alum.mit.edu; pr=C
Message-ID: <166d173e-e343-4e49-bfc9-1b0676e81faf@alum.mit.edu>
Date: Wed, 29 May 2024 16:50:20 -0400
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Yingzhen Qu <yingzhen.ietf@gmail.com>
References: <57400053-7869-46ab-a3b4-cfac94d78b68@alum.mit.edu>
 <CABY-gOMePwa8FcNdVeACHzebHaxAMCWmVSq1wXbTQByFd2Usxw@mail.gmail.com>
From: Paul Kyzivat <pkyzivat@alum.mit.edu>
In-Reply-To: 
 <CABY-gOMePwa8FcNdVeACHzebHaxAMCWmVSq1wXbTQByFd2Usxw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2024 20:50:22.9103
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 14385d5d-3ec2-4230-0cb7-08dc8020f5dc
X-MS-Exchange-CrossTenant-Id: 3326b102-c043-408b-a990-b89e477d582f
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: 
 TenantId=3326b102-c043-408b-a990-b89e477d582f;Ip=[18.7.68.33];Helo=[outgoing-alum.mit.edu]
X-MS-Exchange-CrossTenant-AuthSource: 
	CH2PEPF00000143.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB5741
Message-ID-Hash: LIDJK2JP2JPCKYNN4A56ZPMRV3VKAPBA
X-Message-ID-Hash: LIDJK2JP2JPCKYNN4A56ZPMRV3VKAPBA
X-MailFrom: pkyzivat@alum.mit.edu
X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency;
 loop; banned-address; member-moderation; header-match-mpls.ietf.org-0;
 nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size;
 news-moderation; no-subject; digests; suspicious-header
CC: draft-ietf-mpls-msd-yang.all@ietf.org,
 General Area Review Team <gen-art@ietf.org>, last-call@ietf.org,
 mpls@ietf.org
X-Mailman-Version: 3.3.9rc4
Precedence: list
Subject: =?utf-8?q?=5Bmpls=5D_Re=3A_Gen-ART_Last_Call_review_of_draft-ietf-mpls-msd-y?=
	=?utf-8?q?ang-05?=
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
Archived-At: 
 <https://mailarchive.ietf.org/arch/msg/mpls/LJuzkszrn2xCW2TwjZoQ5GWesDU>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Owner: <mailto:mpls-owner@ietf.org>
List-Post: <mailto:mpls@ietf.org>
List-Subscribe: <mailto:mpls-join@ietf.org>
List-Unsubscribe: <mailto:mpls-leave@ietf.org>

Yingzhen

On 5/28/24 7:05 PM, Yingzhen Qu wrote:
> Hi Paul,
> 
> Thanks for the review and comments. Version -07 has been uploaded to 
> address your comments. Detailed answers below.

Version -07 has resolved my concerns.

	Thanks,
	Paul

> Thanks,
> Yingzhen
> 
> On Mon, May 27, 2024 at 10:51 AM Paul Kyzivat <pkyzivat@alum.mit.edu 
> <mailto:pkyzivat@alum.mit.edu>> wrote:
> 
>     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
>     <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>>.
> 
>     Document: draft-ietf-mpls-msd-yang-05
>     Reviewer: Paul Kyzivat
>     Review Date: 2024-05-27
>     IETF LC End Date: 2024-06-04
>     IESG Telechat date: ?
> 
>     Summary:
> 
>     This draft is on the right track but has open issues, described in the
>     review.
> 
>     Disclaimer:
> 
>     This reviewer is "YANG challenged". Hence I have not verified the
>     syntax
>     or semantics of the YANG content. I trust there has been or will be a
>     separate review by a YANG expert.
> 
>     ISSUES: 3
>     NITS: 1
> 
>     1) ISSUE - General
> 
>     I find the language in this document to be extremely difficult to
>     follow. (For instance, see the confusing language cited in the next
>     issue.) The main problem is that it is formally referencing and
>     defining
>     terms/entities named using sequences of words and acronyms including
>     spaces. (This includes YANG entities, IANA registry table names, and
>     the
>     names of entries in IANA registries.)
> 
>     These terms are used in descriptions with nothing to distinguish them
>     from the surrounding text. Hence it becomes a struggle to parse the
>     text. I strongly recommend revising the document to syntactically set
>     off the formal names of the entities being defined and referenced from
>     the surrounding text. For instance: 'IGP MSD-Types'
> 
> [Yingzhen]: Please see the answer to issue #2.
> 
>     2) ISSUE - Confusing language in Section 1
> 
>     I can't parse: "... defines the identities for Maximum SID Depth (MSD)
>     Types as the IANA the IGP MSD-Types registry."
> 
>     I still can't follow it even if I make the change I requested above:
> 
>     "... defines the identities for Maximum SID Depth (MSD) Types as the
>     IANA the 'IGP MSD-Types' registry."
> 
>     . I *think* you might mean:
> 
>     "... defines the identities for 'Maximum SID Depth (MSD)' Types in the
>     IANA 'IGP MSD-Types' registry." But I am far from certain of this.
> 
> [Yingzhen]: The first paragraph of section 2.1 has been written and 
> added information about the IANA registry. Hope it's clear now. If more 
> clarification is needed, please kindly let us know.
> 
>     3) ISSUE - Possible missing IANA action
> 
>     Section 4 defines a YANG module named 'iana-msd-types'. I *think* there
>     is an intent to add this as a new entry in the IANA registry 'IGP
>     MSD-Types'. But I don't find anything that does this. (I would expect
>     something more in IANA Considerations.)
> 
> [Yingzhen]: No, the YANG Model types represent the existing IANA 
> registry without modifying it.
> 
>     4) NIT
> 
>     The document heading shows "Workgroup: Internet" but shouldn't it be
>     the
>     MPLS WG?
> 
> [Yingzhen]: fixed.