MIME-Version: 1.0
References: <57400053-7869-46ab-a3b4-cfac94d78b68@alum.mit.edu>
In-Reply-To: <57400053-7869-46ab-a3b4-cfac94d78b68@alum.mit.edu>
From: Yingzhen Qu <yingzhen.ietf@gmail.com>
Date: Tue, 28 May 2024 16:05:21 -0700
Message-ID: 
 <CABY-gOMePwa8FcNdVeACHzebHaxAMCWmVSq1wXbTQByFd2Usxw@mail.gmail.com>
To: Paul Kyzivat <pkyzivat@alum.mit.edu>
Content-Type: multipart/alternative; boundary="000000000000c47f9b06198baba0"
Message-ID-Hash: DAD7LBXZVJ6L4PCR6QABHWT6UD5BYHCG
CC: draft-ietf-mpls-msd-yang.all@ietf.org,
 General Area Review Team <gen-art@ietf.org>, last-call@ietf.org,
 mpls@ietf.org
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?=
Archived-At: 
 <https://mailarchive.ietf.org/arch/msg/mpls/pg03C5XA2QkCoxIXzAfXD6_jBLg>

--000000000000c47f9b06198baba0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Paul,

Thanks for the review and comments. Version -07 has been uploaded to
address your comments. Detailed answers below.

Thanks,
Yingzhen

On Mon, May 27, 2024 at 10:51=E2=80=AFAM Paul Kyzivat <pkyzivat@alum.mit.ed=
u> 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>.
>
> 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.

--000000000000c47f9b06198baba0
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr">Hi Paul,<div><br></div><=
div>Thanks for the review and comments. Version -07 has been uploaded to ad=
dress your comments. Detailed answers below.</div><div><br></div><div>Thank=
s,</div><div>Yingzhen</div></div><br><div class=3D"gmail_quote"><div dir=3D=
"ltr" class=3D"gmail_attr">On Mon, May 27, 2024 at 10:51=E2=80=AFAM Paul Ky=
zivat &lt;<a href=3D"mailto:pkyzivat@alum.mit.edu">pkyzivat@alum.mit.edu</a=
>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I a=
m the assigned Gen-ART reviewer for this draft. The General Area<br>
Review Team (Gen-ART) reviews all IETF documents being processed<br>
by the IESG for the IETF Chair.=C2=A0 Please treat these comments just<br>
like any other last call comments.<br>
<br>
For more information, please see the FAQ at<br>
<br>
&lt;<a href=3D"https://trac.ietf.org/trac/gen/wiki/GenArtfaq" rel=3D"norefe=
rrer" target=3D"_blank">https://trac.ietf.org/trac/gen/wiki/GenArtfaq</a>&g=
t;.<br>
<br>
Document: draft-ietf-mpls-msd-yang-05<br>
Reviewer: Paul Kyzivat<br>
Review Date: 2024-05-27<br>
IETF LC End Date: 2024-06-04<br>
IESG Telechat date: ?<br>
<br>
Summary:<br>
<br>
This draft is on the right track but has open issues, described in the <br>
review.<br>
<br>
Disclaimer:<br>
<br>
This reviewer is &quot;YANG challenged&quot;. Hence I have not verified the=
 syntax <br>
or semantics of the YANG content. I trust there has been or will be a <br>
separate review by a YANG expert.<br>
<br>
ISSUES: 3<br>
NITS: 1<br>
<br>
1) ISSUE - General<br>
<br>
I find the language in this document to be extremely difficult to <br>
follow. (For instance, see the confusing language cited in the next <br>
issue.) The main problem is that it is formally referencing and defining <b=
r>
terms/entities named using sequences of words and acronyms including <br>
spaces. (This includes YANG entities, IANA registry table names, and the <b=
r>
names of entries in IANA registries.)<br>
<br>
These terms are used in descriptions with nothing to distinguish them <br>
from the surrounding text. Hence it becomes a struggle to parse the <br>
text. I strongly recommend revising the document to syntactically set <br>
off the formal names of the entities being defined and referenced from <br>
the surrounding text. For instance: &#39;IGP MSD-Types&#39;<br>
<br></blockquote><div>[Yingzhen]: Please see the answer to issue #2.</div><=
div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0=
px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
2) ISSUE - Confusing language in Section 1<br>
<br>
I can&#39;t parse: &quot;... defines the identities for Maximum SID Depth (=
MSD) <br>
Types as the IANA the IGP MSD-Types registry.&quot;<br>
<br>
I still can&#39;t follow it even if I make the change I requested above:<br=
>
<br>
&quot;... defines the identities for Maximum SID Depth (MSD) Types as the <=
br>
IANA the &#39;IGP MSD-Types&#39; registry.&quot;<br>
<br>
. I *think* you might mean:<br>
<br>
&quot;... defines the identities for &#39;Maximum SID Depth (MSD)&#39; Type=
s in the <br>
IANA &#39;IGP MSD-Types&#39; registry.&quot; But I am far from certain of t=
his.<br>
<br></blockquote><div>[Yingzhen]: The first paragraph of section 2.1 has be=
en written and added information about the IANA registry. Hope it&#39;s cle=
ar now. If more clarification is needed, please kindly let us know.</div><d=
iv>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0p=
x 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
3) ISSUE - Possible missing IANA action<br>
<br>
Section 4 defines a YANG module named &#39;iana-msd-types&#39;. I *think* t=
here <br>
is an intent to add this as a new entry in the IANA registry &#39;IGP <br>
MSD-Types&#39;. But I don&#39;t find anything that does this. (I would expe=
ct <br>
something more in IANA Considerations.)<br>
<br></blockquote><div>[Yingzhen]: No, the YANG Model types represent the ex=
isting IANA registry without modifying it.=C2=A0</div><div><br></div><block=
quote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1=
px solid rgb(204,204,204);padding-left:1ex">
4) NIT<br>
<br>
The document heading shows &quot;Workgroup: Internet&quot; but shouldn&#39;=
t it be the <br>
MPLS WG?<br>
<br></blockquote><div>[Yingzhen]: fixed.=C2=A0</div></div></div></div>

--000000000000c47f9b06198baba0--