A YANG Data Model for Automatic Multicast Tunneling (AMT)
draft-ietf-mboned-amt-yang-09

Thanks to the reviewers, Mike O., Behcet S., Robert W. (first time I read a YANG reviewer's review, actually,) for their insight.

Update:  Thank you for addressing my discuss.  I'm leaving my original comments below for historical reasons (they have been addressed too).

Thank you to Mike Ounesworth for their secdir review.

Section 6:  Please add a sentence or two to describe the vulnerability created by using a secret key beyond its validity period.  I can probably supply a sentence, if one doesn't come easily to mind.

Section 6, para 2:  I recommend using draft-ietf-tls-8446bis vice RFC 8446.  The draft is in Auth 48.

Thanks for the work done in this document, I have not reviewed the mcast aspects as it is the produce of the MBONED WG.

Please find some comments below, a reply by the authors will be welcome even if only for my own education.

Section 3: is `secret key timeout` a valid term ? I would have expected 'lifetime' or 'validity' rather than 'timeout'. Same applies for some leaves nodes.

Leaf nodes anycast-prefix and local address: why not a MUST in `If 'family' is IPv4, it SHOULD be an IPv4 prefix;` ?

Is there a reason why the `leaf local-address` node specification in the relay tree is different than in the gateway tree ?

Regards,

-éric

Thanks for the work done in this document, I enjoyed reading this. I have reviwed previous AMT documents, but do not see specific transport topics in this document. There were, however,  a few places where terminolog is defined, but could be more consistently applied.

NiTs: 
/a AMT Relay/ perhaps ought to be /an AMT Relay/
/AMT relay/AMT Relay/ - in various places, since "Relay" is already capitalised without AMT?
/AMT gateway/AMT Gateway/ - for consistency in text and the comments?
/Support of relay or gateway is indicated/Support of an AMT Relay or an AMT Gateway is indicated/
/relay discovery message/Relay Discovery message/ - as per RFC7450.
/relay discovery address/Relay Discovery Address/ - as per RFC7450.
/relay address/Relay Address/ - as per RFC7450.

Regards,

Gorry
- Thanks again, I note the document was updated to address these comments.

Thank you for addressing my comments.

-------------------------------------------------------------------------------
COMMENT
-------------------------------------------------------------------------------

Section 4.2.2, paragraph 13

Forgive my lack of YANG expertise, but doesn't YANG support constraints
to further restrain what values are acceptable? Would a "must" not have worked
here rather than creating a more constrained type?

-------------------------------------------------------------------------------
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.2.1, paragraph 2
-    The AMT YANG module augments the core routing YANG module "ietf-
-    routing" specified in [RFC8349].  Specifically, the AMT YANG module
-   ---------
+    The AMT YANG module augments the core routing YANG module "ietf-routing"
+                                                                    ++++++++

Section 4.2.2, paragraph 6
-       This data node includes 'family', 'anycast-prefix', and 'local-
-       address'.  The 'family' indicates the address family (IPv4 or
-     -----------
+       This data node includes 'family', 'anycast-prefix', and 'local-address'.=
+                                                                      ++++++++++

Section 4.2.2, paragraph 11
-       'local- port'), the tunnel status ('state'), the multicast flow
-              -

Section 4.2.2, paragraph 11
-       ('multicast-group- num'), the message counter carried by the
-                         -

Section 4.2.2, paragraph 11
-       tunnel ('request-message- count', 'membership-query-message-
-                                -
-       count', and 'membership-update- message-count'), and the time on
-      --------                        -
+       tunnel ('request-message-count', 'membership-query-message-count',
+                                                                  +++++++

Section 4.2.2, paragraph 12
-       address ('source-address') and multicast group address ('group-
-       address').
+       address ('source-address') and multicast group address ('group-address').
+                                                                      ++++++++++

Section 4.2.2, paragraph 13
-       Design note: The four data nodes ('gateway-address', 'gateway-
-       port', 'local-address', and 'local-port) do not reuse the standard
-      -------
+       Design note: The four data nodes ('gateway-address', 'gateway-port',
+                                                                     ++++++

Section 4.2.3, paragraph 7
-       number of AMT membership query messages received ('membership-
-       query').
+       number of AMT membership query messages received ('membership-query').
+                                                                     ++++++++

Section 4.2.3, paragraph 8
-       messages sent ('relay- discovery'), the number of AMT membership
-                             -

Section 5, paragraph 6
-       mismatch (e.g., IPv4 'anycast-prefix' paired with IPv6 'local-
-       address' under the same IPv4 'family' entry) indicates a
-      ---------
+       mismatch (e.g., IPv4 'anycast-prefix' paired with IPv6 'local-address'
+                                                                     ++++++++

Section 6, paragraph 5
-      Under /rt:routing/rt:control-plane-protocols/rt:control-plane-
-      protocol/: Unauthorized access to any data nodes in these subtrees
-     -----------
+      Under /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol/:
+                                                                    ++++++++++

Section 6, paragraph 11
-      Under /rt:routing/rt:control-plane-protocols/rt:control-plane-
-      protocol/: amt/relay and amt/gateway. Unauthorized access to any
-     -----------
+      Under /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol/:
+                                                                    ++++++++++

Section 6, paragraph 5

The additional indent level here makes it render as a figure in the HTML
version. I assume that's not intentional; if you can't fix it yourself, please
make a note to work with the RFC Editor to get this rendered the way you intend.
Same for the next-to-last paragraph of this section.

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". [EN_A_VS_AN]

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

Alternately, consider simply "one or more"

Section 4.2.2, paragraph 7
>  'gateway-port', 'local-address', and 'local-port) do not reuse the standard
>                                       ^
Unpaired symbol: "'" seems to be missing. [EN_UNPAIRED_QUOTES]

Section 4.2.2, paragraph 17
>  the most recent occasion at which any one or more of the AMT Gateway message
>                                    ^^^^^^^
Did you mean "anyone"? [ANY_BODY]

Alternately, consider simply "one or more"

Section 4.2.3, paragraph 2
> hangwang.04414@h3c.com> Editor: Zheng(Sandy) Zhang <mailto:zhang.zheng@zte.co
>                                      ^
It appears that a white space is missing. [SPACE_BEFORE_PARENTHESIS]

Section 4.3, paragraph 34
>  the most recent occasion at which any one or more of this AMT tunnel's count
>                                    ^^^^^^^
Did you mean "anyone"? [ANY_BODY]

Alternately, consider simply "one or more"

Section 4.3, paragraph 40
>  the most recent occasion at which any one or more of this AMT tunnel's messa
>                                    ^^^^^^^
Did you mean "anyone"? [ANY_BODY]

Alternately, consider simply "one or more"

Section 6, paragraph 4
> ail: linchangwang.04414@h3c.com Zheng(Sandy) Zhang ZTE Corporation China Ema
>                                      ^
It appears that a white space is missing. [SPACE_BEFORE_PARENTHESIS]

Thank you to Behcet Sarikaya for the GENART review.

I support the DISCUSS position of Chris Inacio.