Re: [Last-Call] Yangdoctors last call review of draft-ietf-rtgwg-qos-model-06

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Juergen,

I would agree that the document is not ready for a LC, or for a YANG doctors review at this time. If a YANG doctors review was requested, it was in error.

The authors are working to address the YANG doctors review comments you provided from your last review. The next version (-07) that is going to be published would have only partially addressed those comments. To that list we will add the comments you provide below. Once we believe we have addressed all the comments, we will ask for a review.

Cheers.

On Mar 6, 2022, at 9:26 AM, Jürgen Schönwälder via Datatracker <noreply@xxxxxxxx> wrote:

Reviewer: Jürgen Schönwälder
Review result: Not Ready

Document: draft-ietf-rtgwg-qos-model-06
Date: 2022-03-06

Summary: The document is not ready, essential parts are missing, other
parts are far from being done. Please do not ask me again to review
this document, YANG doctors are not here to step authors and WGs
through their YANG model writing and reviewing process. The basic
quality checks are to be done by the WGs and not YANG doctors. On the
technical side, I am concerned about way too many features and way too
many YANG modules. And it all feels like being designed to support
vendor specific diffserv models more than trying to converge on a
(straight-forward to implement) standard configuration model for
DiffServ.

* 1. Introduction

 s/is a preferred approach/is an approach/

 I asked before why you reference RFC 6020 (YANG 1). I think you
 should be using YANG 1.1 (RFC 7950) and not reference RFC 6020 at
 all.

 There are still some language quirks. My comments concerning
 "sectionitis" (one sentence sections) remain.

* 2. QoS Model Design

 The text defining terms like classifier has been revised but I am
 still not happy about it. I do not see why you not simply import
 terms from other documents. An example:

   A classifier consists of packets which may be grouped when a logical
   set of rules are applied on different packet header fields.

 A classifier consists of packets? Why not import and use the RFC
 2475 definition? Another one:

   A meter qualifies if the traffic arrival rate is based on agreed
   upon rate and variability.

 A meter qualifies? I do not think this is what RFC 2475 says a meter
 is - something that does metering, e.g., counting.

* 3. DiffServ Model Design

 "DiffServ architecture [RFC3289] and [RFC2475] describe" seems to be
 wrong, I think the architecture is in [RFC2475], why do you cite
 [RFC3289] here?

4. Modules Tree Structure

 s/ietf-qos-classifier consists/The ietf-qos-classifier module consists/

 Why do I have both ietf-qos-classifier:/classifiers/classifier and
 ietf-qos-policy:/policies/classifier? An explanation or justification
 seems to be missing here.

 s/ietf-qos-action module contains/The ietf-qos-action module defines/

 s/ietf-qos-target module/The ietf-qos-target module/

 "Many of the YANG types defined in [RFC6991] are represented as
 leafs in the classifier module." I am not sure what this tells me,
 perhaps simply remore this statement or replace it with something
 more useful.

 AQM pops up here our of the blue. At least expand the acronym on
 first use.

* 5. Modules

 Descriptions should be proper sentences.

* 5.1 ietf-qos-classifier

 I wonder whether 'logical-not' would not better be called 'negate'
 and whether this should have a default of false. My assumption here
 is that you evaluate the filter and then you negate the returned
 boolean value. The phrase "filter looks for absence of a pattern
 defined by the filter" seems to complicate things.

* 5.2 ietf-qos-policy

 Why is this YANG 1 and not YANG 1.1?

 The descriptions are rather poor and incomplete. Why was this sent
 to a yang doctor for review? At least one of the co-authors should
 know the YANG guidelines document...

* 5.3 ietf-qos-action

 There are several identities with the exact same description. Does
 this mean all these identities do the same thing? The quality of all
 description statements needs to improve. Also put the statements
 into canonical order (see grouping queue which has descriptions at
 the end instead of the top).

* 5.4 ietf-qos-target

 Why is this YANG 1 and not YANG 1.1?

 Where did you get the WG chairs from?? Please follow the template
 given in RFC 8407 appendix B. We did cut the template down...

* 5.5 ietf-diffserv

 Where did you get the WG chairs from?? Please follow the template
 given in RFC 8407 appendix B. We did cut the template down...

 Do you really want a list source-ipv4-prefix or do you want a
 leaf-list source-ipv4-prefix?

 Do you really want a list destination-ipv4-prefix or do you want a
 leaf-list destination-ipv4-prefix?

 Do you really want a list source-ipv6-prefix or do you want a
 leaf-list source-ipv6-prefix?

 Do you really want a list destination-ipv6-prefix or do you want a
 leaf-list destination-ipv6-prefix?

 For the protocol-min and protocol-max leafs you could use
 inet:protocol-number if you are OK to depend on RFC6991bis.

* 5.6 ietf-queue-policy

 Where did you get the WG chairs from?? Please follow the template
 given in RFC 8407 appendix B. We did cut the template down...

* 5.7 ietf-scheduler-policy

 Where did you get the WG chairs from?? Please follow the template
 given in RFC 8407 appendix B. We did cut the template down...

* 6. IANA Considerations

 Why is this still empty?

* 7. Security Considerations

 Why is this still empty?

* 8. Acknowledgement

  "MITRE has approved this document for Public Release, Distribution
   Unlimited, with Public Release Case Number 19-3027."

 What? Who cares about what MITRE approves? Do people understand the
 IETF rules and note well?

* Appendix

 I do not review this. Frankly, the purpose of standards is to try to
 find agreement on common solutions and to showcase how easy it is to
 write N different vendor specific Diffserv Models.





Mahesh Jethanandani






-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux