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.
_______________________________________________
rtgwg mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/rtgwg
