[MBONED]Mahesh Jethanandani's No Objection on draft-ietf-mboned-amt-yang-08: (with COMMENT)

[Mahesh Jethanandani via Datatracker <[e-mail]>]

Mahesh Jethanandani has entered the following ballot position for
draft-ietf-mboned-amt-yang-08: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mboned-amt-yang/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 4.2.2, paragraph 2
>           |  |     +--rw anycast-prefix     inet:ip-prefix
>           |  |     +--rw local-address      inet:ip-address

The tree diagram here and in the Appendix does not match the
model. The tree diagram here seems to indicate that these
two nodes are mandatory (lack of ? after the node name)
where the model lacks any 'mandatory true' statement. Please
sync the tree here and in the Appendix with the module.

Section 4.2.3, paragraph 2
>              |  +--rw interface* [interface]
>              |     +--rw name                      if:interface-ref

The tree diagram here and the Appendix seem to indicate that the key
for this list is the 'interface'. However, the module seems to be making
the 'name' the key. What gives?

Section 4.3, paragraph 33
>                  description
>                    "A unicast IP address of the AMT Relay Address
>                     which is obtained as a result of the discovery
>                     process.

The description does not match the definition. The description seems
to imply that the value is learnt as part of the discovery process,
while the parameter is rw, i.e., operator configured.

Section 4.3, paragraph 34
>            leaf tunnel-limit {
>              type uint32;
>              description
>                "The total number of endpoints.";
>            }

This leaf description is sparse and imprecise.
Section 4.2.2 provides a much better description: "the maximum
number of endpoint Gateways that a Relay can serve simultaneously."
That description should be reflected here.

Section 4.3, paragraph 38
>                     When the 'relay-type' value is 1 or 2, this
>                     data node is used to indicate the AMT Relay of
>                     the AMT Relay RR.";

Where is this condition being enforced?

The 'relay-type' leaf determines whether 'discovery-address' or
'domain-name' is populated, i.e.,
- if relay-type = ipv4-address (1) or ipv6-address (2) → discovery-address
should be populated
- if relay-type = domain-name (3) → domain-name should be populated

There are no when or must constraints enforcing this relationship.
A configuration could have relay-type = ipv4-address with only
domain-name set (and no discovery-address) with no validation error.
At a minimum, when conditions should gate which leaf is relevant,
or must constraints should ensure the appropriate leaf is populated
for a given relay-type.

Section 4.3, paragraph 42
>                leaf gateways-timed-out {
>                  type yang:gauge64;
>                  description
>                    "Number of Gateways that timed out because
>                     of inactivity.";
>                }

The type seems to be wrong. gauge64 should be zero-based-counter64
as this is a cumulative count, based on the description, not a gauge
whose values can go up or down.

Section 4.3, paragraph 42
>                leaf relay-discovery-message-count {
>                  type yang:zero-based-counter64;
>                  config false;
>                  description
>                    "Number of AMT Relay Discovery messages sent
>                     on the interface.";
>                }
>                leaf relay-advertisement-message-count {
>                  type yang:zero-based-counter64;
>                  config false;
>                  description
>                    "Number of AMT Relay advertisement messages received
>                     on the interface.";
>                }
>                leaf request-message-count {
>                  type yang:zero-based-counter64;
>                  config false;
>                  description
>                    "Number of AMT membership request messages sent
>                     on the interface.";
>                }
>                leaf membership-query-message-count {
>                  type yang:zero-based-counter64;
>                  config false;
>                  description
>                    "Number of AMT membership query messages received
>                     on the interface.";
>                }
>                leaf membership-update-message-count {
>                  type yang:zero-based-counter64;
>                  config false;
>                  description
>                    "Number of AMT membership update messages sent
>                     on the interface.";
>                }

The relay tunnels/tunnel container correctly includes discontinuity-time.
The gateway-message-statistics container also correctly includes one.
But the per-interface counters in pseudo-interfaces/interface have no
associated discontinuity-time. These should either be added individually
per interface, or the gateway-message-statistics discontinuity-time should
be documented as covering per-interface counters too.

"Appendix B.", paragraph 0
>    This section presents a simple and illustrative example of how to
>    configure AMT.

Thank you to the authors for providing instance data examples. No
examples however, have been provided for the ATM Gateway pseudo-interface
configuration, which is the other major component defined in this draft.
Given that the gateway configuration (discovery-method, relay-discovery-address,
relay-address, timeouts) is less self-explanatory than the relay configuration,
a complementary example be appreciated by implementors.

The IANA review of this document seems to not have concluded yet.

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term "maler"; alternatives might be "individual", "people", "person"
 * Terms "natively" and "native"; alternatives might be "built-in",
   "fundamental", "ingrained", "intrinsic", "original"

-------------------------------------------------------------------------------
NIT
-------------------------------------------------------------------------------

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool) so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 4.3, paragraph 24
>      identity discoverying {
>        base tunnel-state-base;
>        description
>          "The Relay Discovery message has been sent
>           and is waiting for the Advertisement message.";
>      }

s/discoverying/discovering/

Section 2.2, paragraph 11
> r the private secret (or key) used by a AMT Relay to compute Response Message
>                                       ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Section 4.2.2, paragraph 7
>  the most recent occasion at which any one or more of the tunnel's counters s
>                                    ^^^^^^^
Did you mean "anyone"?