Return-Path: X-Original-To: mpls@ietfa.amsl.com Delivered-To: mpls@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id AE5A8120059; Tue, 24 Dec 2019 13:56:57 -0800 (PST) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -1.997 X-Spam-Level: X-Spam-Status: No, score=-1.997 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GaYiSzJknumw; Tue, 24 Dec 2019 13:56:54 -0800 (PST) Received: from mail-io1-xd32.google.com (mail-io1-xd32.google.com [IPv6:2607:f8b0:4864:20::d32]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id AEC1C12001E; Tue, 24 Dec 2019 13:56:54 -0800 (PST) Received: by mail-io1-xd32.google.com with SMTP id c16so16466707ioo.8; Tue, 24 Dec 2019 13:56:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=m542OFajJCXu02pDyiKItRlCkdae0c6Dnov7Xtj0gzU=; b=XpT3ScU7B3fXONConL3teWPa5roMtcrAaxD0HS8FJqfLZY1dZ5m4DXtroEgMZRBZM+ x5pplnPXIvgzu3d+en69HGbDqZyc/wW2npPNJKQ9GBzaBj3ZqItXryO0IYyhMKrbqy/x t4K+IlxtJ1BrLMnBRsDIDmBBt3mhoDw96EX9OA2JEstpv9z46M487Xj8WDBRUAAZ4HMe ryfP3eHks6MK7/bdsrrXtGRNMq2Dpgufvfj7YHRM6lRYJzPVux7JMTFNTpDDGZU8r44O QNatvQiQyVpzUIv5kQOcWIluudmtrzgz5xam6jtLLiYlOsmvVlPsliV9PlPolCNJ5My/ jvFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=m542OFajJCXu02pDyiKItRlCkdae0c6Dnov7Xtj0gzU=; b=PLGckmlzfGDeEN89E2Y+0xVTLrCm3t7JBqfxKXkla/+56gMcKKu6dX1q4ZD8iQK5Ae dM3JVC1NFPil46SGZu/9DI4zx6U3ZmH1YfnxLsAYFjjzZDsRZ2BfQnD2rpMewF0gWzHS 5YZ0sqrtghb/mKT8ejZuykSNz3eX+05Z2QBk8oKAliWLkvMywdlmq+Ju1OQ3YcgP29pi CaGUr2+aIboKtWM9LSNK2kNi79G3pkkRt70h7V6IXNui3PutxLsOaGcynJ7dkfkyFc9l F7t9UdiAfPqpNIv1oD2q9Iwq03I/ZQKvY+RhWMsylkKKIVfu8Ym+xRtfR1Eeg/juyAU1 AKCg== X-Gm-Message-State: APjAAAVh1008gOxWl07/zYoJbq/Y2ESP9iik/q+B0w+ilGPabUFJDczA mN8jqkMxaWN/JOUlOzC1tVP3irEiowlXc2nJsQ== X-Google-Smtp-Source: APXvYqyU5JaRlJ4cqDvmm7JGc7uU59L++T7Td7ELzSUmvLJDpynn2RLrBAKBNkL7g7HDd3xylZ8LthMI0r4x1Z9+9aw= X-Received: by 2002:a5d:8a0f:: with SMTP id w15mr24373222iod.295.1577224613964; Tue, 24 Dec 2019 13:56:53 -0800 (PST) MIME-Version: 1.0 References: <157264352652.31784.14618175935712783529@ietfa.amsl.com> <888D8635-4482-4666-B257-1375E9C86481@cisco.com> In-Reply-To: <888D8635-4482-4666-B257-1375E9C86481@cisco.com> From: Yingzhen Qu Date: Tue, 24 Dec 2019 13:54:04 -0800 Message-ID: To: "Kamran Raza (skraza)" Cc: "rtg-dir@ietf.org" , "last-call@ietf.org" , "draft-ietf-mpls-ldp-yang.all@ietf.org" , "mpls@ietf.org" , "draft-ietf-mpls-ldp-yang@ietf.org" Content-Type: multipart/alternative; boundary="000000000000d8b052059a7a3871" Archived-At: Subject: Re: [mpls] Rtgdir last call review of draft-ietf-mpls-ldp-yang-06 X-BeenThere: mpls@ietf.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Multi-Protocol Label Switching WG List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Dec 2019 21:56:58 -0000 --000000000000d8b052059a7a3871 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Kamran, Thanks for the reply. Looking forward to the next rev. Happy Holidays! Thanks, Yingzhen On Mon, Dec 23, 2019 at 10:10 AM Kamran Raza (skraza) wrote: > Hi Yingzhen, > > Thanks for your detailed review. We are addressing your comments as well > as YD comments and plan to spin a new rev in new year. > Please see our response inline - tagged as [skraza] > > =EF=BB=BFOn 2019-11-01, 5:25 PM, "mpls on behalf of Yingzhen Qu via Datat= racker" < > mpls-bounces@ietf.org on behalf of noreply@ietf.org> wrote: > > Reviewer: Yingzhen Qu > Review result: Has Issues > > I have been selected as the Routing Directorate reviewer for this > draft. The > Routing Directorate seeks to review all routing or routing-related > drafts as > they pass through IETF last call and IESG review, and sometimes on > special > request. The purpose of the review is to provide assistance to the > Routing ADs. > For more information about the Routing Directorate, please see > http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir > > Although these comments are primarily for the use of the Routing ADs, > it would > be helpful if you could consider them along with any other IETF Last > Call > comments that you receive, and strive to resolve them through > discussion or by > updating the draft. > > Document: draft-ietf-mpls-ldp-yang > Reviewer: Yingzhen Qu > Review Date: Nov 1st, 2019 > Intended Status: Standards Track > > Summary: > > This document is near ready for publication. It has some issues that > should be > at least considered prior to publication. > > Comments: > > Thanks for working on this draft. As an active YANG contributor I > appreciate > the work here. > > Major issues: > > I=E2=80=99m not sure whether this should be considered as a major iss= ue, which > is how > the document is structured. The draft separates the configuration and > operation > states into two sections, and this seems to be a before NMDA product > and a bit > redundant to me. The tree structures are used in different ways > multiple times > in the document, and this significantly reduces the readability of th= e > modules. > > [skraza]: Ack. Will fix. > > Minor Issues: > > I feel the English in this draft could be improved, but I=E2=80=99m n= ot a > native > speaker. Maybe RFC editor will help with this? > > It seems that =E2=80=9Cmodel=E2=80=9D and =E2=80=9Cmodule=E2=80=9D ar= e used exchangeable in this > document, > please make them consistent. > > [skraza]: Ack. Will fix. > > =E2=80=9CYANG=E2=80=9D should be capital cased, and there are =E2=80= =9Cyang=E2=80=9D at multiple > places in this > document. Please fix them. > > [skraza]: Ack. Will fix. > > Please consider add names to figures. > > [skraza]: Ack. Will fix. > > I understand inheritance is an important feature of document. I=E2=80= =99d > suggest maybe > add a section/paragraph in =E2=80=9Coverview=E2=80=9D to introduce th= e concept and how > it works > instead of repeating the idea with examples in every major container > design. > > > [skraza]: Ack. Will see how to address this. > > Nits for your consideration: > [skraza]: Ack to all the below nits - will fix. > > Rgds- > -- > Kamran (on behalf of authors) > > Section 1.1 > Whereas, the "extended" category contains all other non-base features= . > Please consider remove =E2=80=9Call=E2=80=9D because not all other fe= atures are > covered. > > Section 3 > extended "ietf-mpls-ldp-extended" module that models the extended > LDP features and augments the base LDP > Please consider removed =E2=80=9Cextended=E2=80=9D at the beginning, = and add =E2=80=9Cmodule=E2=80=9D > at the > end, =E2=80=9Caugments the base LDP module=E2=80=9D. > > There are four main containers in our module(s): > I suppose you meant =E2=80=9Cfour types of containers=E2=80=9D? > > Section 4 > under LDP base and extended. > Please add a =E2=80=9Cmodule=E2=80=9D at the end. > > Section 5 > Following is the high-level configuration organization for base LDP: > Please add a =E2=80=9Cmodule=E2=80=9D at the end. > > Typo in figure 3 =E2=80=9Ctargeteted=E2=80=9D > > Typo in figure 4 =E2=80=9Ctargeteted=E2=80=9D > > Given the configuration hierarchy, the model allows inheritance such > that an item in a child tree is able to derive value from a simila= r > or related item in one of the parent. > for grammar, it should be =E2=80=9Cone of the parents=E2=80=9D, but t= his sentence is a > bit > confusing. I=E2=80=99d suggest add a bit more explanation how inherit= ance work. > > 5.1.1 > Missing period =E2=80=9C.=E2=80=9D at the end of the first sentence. > > The tree showing here is not a complete tree, just want to make sure > whether > this is intentional? > > 5.1.2 > Missing period =E2=80=9C.=E2=80=9D at the end of the first sentence. > > 5.2.1.1 > =E2=80=9CLSR id=E2=80=9D and =E2=80=9CLSR Id=E2=80=9D both are used h= ere, please keep them consistent. > > 5.2.1.5 > Missing =E2=80=9C.=E2=80=9D at the end of the first paragraph. > > =E2=80=9CA peer is uniquely identified using its > LSR Id and hence LSR Id is the key for peer list=E2=80=9D > Please consider simplify the sentence to =E2=80=9CA peer is uniquely > identified by its > LSR Id.=E2=80=9D > > Section 6 > =E2=80=9C Operational state of LDP can be queried and obtained from = read-only > state containers that fall under the same tree (/rt:routing/ > rt:control-plane-protocols/) as the configuration.=E2=80=9D > This sentence is a bit confusing. Please consider revise it. > > _______________________________________________ > mpls mailing list > mpls@ietf.org > https://www.ietf.org/mailman/listinfo/mpls > > > --000000000000d8b052059a7a3871 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Kamran,

Thanks for the reply. L= ooking forward to the next rev.

Happy Holidays!

Thanks,
Yingzhen

On Mon, Dec 23, 2019= at 10:10 AM Kamran Raza (skraza) <s= kraza@cisco.com> wrote:
Hi Yingzhen,

Thanks for your detailed review. We are addressing your comments as well as= YD comments and plan to spin a new rev in new year.
Please see our response inline - tagged as [skraza]

=EF=BB=BFOn 2019-11-01, 5:25 PM, "mpls on behalf of Yingzhen Qu via Da= tatracker" <mpls-bounces@ietf.org on behalf of noreply@ietf.org> wrote:

=C2=A0 =C2=A0 Reviewer: Yingzhen Qu
=C2=A0 =C2=A0 Review result: Has Issues

=C2=A0 =C2=A0 I have been selected as the Routing Directorate reviewer for = this draft. The
=C2=A0 =C2=A0 Routing Directorate seeks to review all routing or routing-re= lated drafts as
=C2=A0 =C2=A0 they pass through IETF last call and IESG review, and sometim= es on special
=C2=A0 =C2=A0 request. The purpose of the review is to provide assistance t= o the Routing ADs.
=C2=A0 =C2=A0 For more information about the Routing Directorate, please se= e
=C2=A0 =C2=A0 http://trac.tools.ietf.org/area/rt= g/trac/wiki/RtgDir

=C2=A0 =C2=A0 Although these comments are primarily for the use of the Rout= ing ADs, it would
=C2=A0 =C2=A0 be helpful if you could consider them along with any other IE= TF Last Call
=C2=A0 =C2=A0 comments that you receive, and strive to resolve them through= discussion or by
=C2=A0 =C2=A0 updating the draft.

=C2=A0 =C2=A0 Document: draft-ietf-mpls-ldp-yang
=C2=A0 =C2=A0 Reviewer: Yingzhen Qu
=C2=A0 =C2=A0 Review Date: Nov 1st, 2019
=C2=A0 =C2=A0 Intended Status: Standards Track

=C2=A0 =C2=A0 Summary:

=C2=A0 =C2=A0 This document is near ready for publication. It has some issu= es that should be
=C2=A0 =C2=A0 at least considered prior to publication.

=C2=A0 =C2=A0 Comments:

=C2=A0 =C2=A0 Thanks for working on this draft. As an active YANG contribut= or I appreciate
=C2=A0 =C2=A0 the work here.

=C2=A0 =C2=A0 Major issues:

=C2=A0 =C2=A0 I=E2=80=99m not sure whether this should be considered as a m= ajor issue, which is how
=C2=A0 =C2=A0 the document is structured. The draft separates the configura= tion and operation
=C2=A0 =C2=A0 states into two sections, and this seems to be a before NMDA = product and a bit
=C2=A0 =C2=A0 redundant to me. The tree structures are used in different wa= ys multiple times
=C2=A0 =C2=A0 in the document, and this significantly reduces the readabili= ty of the modules.

[skraza]: Ack. Will fix.

=C2=A0 =C2=A0 Minor Issues:

=C2=A0 =C2=A0 I feel the English in this draft could be improved, but I=E2= =80=99m not a native
=C2=A0 =C2=A0 speaker. Maybe RFC editor will help with this?

=C2=A0 =C2=A0 It seems that =E2=80=9Cmodel=E2=80=9D and =E2=80=9Cmodule=E2= =80=9D are used exchangeable in this document,
=C2=A0 =C2=A0 please make them consistent.

[skraza]: Ack. Will fix.

=C2=A0 =C2=A0 =E2=80=9CYANG=E2=80=9D should be capital cased, and there are= =E2=80=9Cyang=E2=80=9D at multiple places in this
=C2=A0 =C2=A0 document. Please fix them.

[skraza]: Ack. Will fix.

=C2=A0 =C2=A0 Please consider add names to figures.

[skraza]: Ack. Will fix.

=C2=A0 =C2=A0 I understand inheritance is an important feature of document.= I=E2=80=99d suggest maybe
=C2=A0 =C2=A0 add a section/paragraph in =E2=80=9Coverview=E2=80=9D to intr= oduce the concept and how it works
=C2=A0 =C2=A0 instead of repeating the idea with examples in every major co= ntainer design.


=C2=A0[skraza]: Ack. Will see how to address this.=C2=A0 =C2=A0

=C2=A0 =C2=A0 Nits for your consideration:
[skraza]: Ack to all the below nits - will fix.

Rgds-
--
Kamran (on behalf of authors)=C2=A0 =C2=A0 =C2=A0

=C2=A0 =C2=A0 Section 1.1
=C2=A0 =C2=A0 Whereas, the "extended" category contains all other= non-base features.
=C2=A0 =C2=A0 Please consider remove =E2=80=9Call=E2=80=9D because not all = other features are covered.

=C2=A0 =C2=A0 Section 3
=C2=A0 =C2=A0 extended "ietf-mpls-ldp-extended" module that model= s the extended
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 LDP features and augments the base LDP =C2=A0 =C2=A0 Please consider removed =E2=80=9Cextended=E2=80=9D at the beg= inning, and add =E2=80=9Cmodule=E2=80=9D at the
=C2=A0 =C2=A0 end, =E2=80=9Caugments the base LDP module=E2=80=9D.

=C2=A0 =C2=A0 There are four main containers in our module(s):
=C2=A0 =C2=A0 I suppose you meant =E2=80=9Cfour types of containers=E2=80= =9D?

=C2=A0 =C2=A0 Section 4
=C2=A0 =C2=A0 under LDP base and extended.
=C2=A0 =C2=A0 Please add a =E2=80=9Cmodule=E2=80=9D at the end.

=C2=A0 =C2=A0 Section 5
=C2=A0 =C2=A0 Following is the high-level configuration organization for ba= se LDP:
=C2=A0 =C2=A0 Please add a =E2=80=9Cmodule=E2=80=9D at the end.

=C2=A0 =C2=A0 Typo in figure 3 =E2=80=9Ctargeteted=E2=80=9D

=C2=A0 =C2=A0 Typo in figure 4 =E2=80=9Ctargeteted=E2=80=9D

=C2=A0 =C2=A0 Given the configuration hierarchy, the model allows inheritan= ce such
=C2=A0 =C2=A0 =C2=A0 =C2=A0that an item in a child tree is able to derive v= alue from a similar
=C2=A0 =C2=A0 =C2=A0 =C2=A0or related item in one of the parent.
=C2=A0 =C2=A0 for grammar, it should be =E2=80=9Cone of the parents=E2=80= =9D, but this sentence is a bit
=C2=A0 =C2=A0 confusing. I=E2=80=99d suggest add a bit more explanation how= inheritance work.

=C2=A0 =C2=A0 5.1.1
=C2=A0 =C2=A0 Missing period =E2=80=9C.=E2=80=9D at the end of the first se= ntence.

=C2=A0 =C2=A0 The tree showing here is not a complete tree, just want to ma= ke sure whether
=C2=A0 =C2=A0 this is intentional?

=C2=A0 =C2=A0 5.1.2
=C2=A0 =C2=A0 Missing period =E2=80=9C.=E2=80=9D at the end of the first se= ntence.

=C2=A0 =C2=A0 5.2.1.1
=C2=A0 =C2=A0 =E2=80=9CLSR id=E2=80=9D and =E2=80=9CLSR Id=E2=80=9D both ar= e used here, please keep them consistent.

=C2=A0 =C2=A0 5.2.1.5
=C2=A0 =C2=A0 Missing =E2=80=9C.=E2=80=9D at the end of the first paragraph= .

=C2=A0 =C2=A0 =E2=80=9CA peer is uniquely identified using its
=C2=A0 =C2=A0 =C2=A0LSR Id and hence LSR Id is the key for peer list=E2=80= =9D
=C2=A0 =C2=A0 Please consider simplify the sentence to =E2=80=9CA peer is u= niquely identified by its
=C2=A0 =C2=A0 LSR Id.=E2=80=9D

=C2=A0 =C2=A0 Section 6
=C2=A0 =C2=A0 =E2=80=9C=C2=A0 Operational state of LDP can be queried and o= btained from read-only
=C2=A0 =C2=A0 =C2=A0 =C2=A0state containers that fall under the same tree (= /rt:routing/
=C2=A0 =C2=A0 =C2=A0 =C2=A0rt:control-plane-protocols/) as the configuratio= n.=E2=80=9D
=C2=A0 =C2=A0 This sentence is a bit confusing. Please consider revise it.<= br>
=C2=A0 =C2=A0 _______________________________________________
=C2=A0 =C2=A0 mpls mailing list
=C2=A0 =C2=A0 mpls@ietf.= org
=C2=A0 =C2=A0 https://www.ietf.org/mailman/listinfo/mpls


--000000000000d8b052059a7a3871--