Early Review of draft-ietf-rtgwg-yang-rip-02
review-ietf-rtgwg-yang-rip-02-rtgdir-early-meuric-2017-01-13-00

Request Review of draft-ietf-rtgwg-yang-rip
Requested rev. no specific revision (document currently at 08)
Type Early Review
Team Routing Area Directorate (rtgdir)
Deadline 2017-01-12
Requested 2016-12-09
Requested by Jonathan Hardwick
Other Reviews Yangdoctors Early review of -02 by Ladislav Lhotka (diff)
Genart Last Call review of -06 by Roni Even (diff)
Genart Telechat review of -07 by Roni Even (diff)
Review State Completed
Reviewer Julien Meuric
Review review-ietf-rtgwg-yang-rip-02-rtgdir-early-meuric-2017-01-13
Posted at https://mailarchive.ietf.org/arch/msg/rtg-dir/xgVPFy96qM5GAY0tP8g6QY7lI_0
Reviewed rev. 02 (document currently at 08)
Review result Has Issues
Draft last updated 2017-01-13
Review completed: 2017-01-13

Review
review-ietf-rtgwg-yang-rip-02-rtgdir-early-meuric-2017-01-13

Hello,

I have been selected as the Routing Directorate QA reviewer for this
draft. For more information about the Routing Directorate, please see
€‹http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

At this stage, the intend of the following is not to discuss the WG's
decision about the I-D, but rather to help improving its content.

Please not that I am not deep Yang expert, but RFC 6087 has provided me
with valuable guidelines.

_Summary_
The Yang module specified in the I-D may be almost complete to move
forward. The carrying document however deserves an update before going
to the next step. I do not repeat every comment raised by Yang doctors
in last December, but those need to be addressed as well.

_Comments_
- Add "import ietf-isis" and "import ietf-bgp" (page 9)
- According to RFC 6087, section 3.1, "the module description statement
MUST contain a reference to the latest approved IETF Trust Copyright
statement" (p 10).
- Both "prefix-set-ref" and "route-policy-ref" are defined as new types
(p 11): is there a reason not to consider them as generic types
specified elsewhere (e.g., among routing types).
- Yangvalidator raises errors on the 6 "must" expressions (cf. Yang
doctors' review).
- The security section does not say anything about the read/write fields
nor the "clear route" RPC: it really requires some work, please see the
template in RFC 6087, section 6.1.
- Normative references needs to be updated, at least with the following:
  * RFC 6991
  * RFC 7223
  * RFC 7277
  * draft-ietf-rtgwg-routing-types
  * draft-ietf-rtgwg-yang-key-chain
  * draft-ietf-ospf-yang
  * draft-ietf-isis-yang-isis-cfg
  * draft-ietf-idr-bgp-model
  * draft-ietf-bfd-yang
- Reading RFC 1724 (RIPv2 MIB) is clearly unnecessary to understand the
document, the reference must thus be moved to the informative list.

_Nits_
- In the "bfd-protocol-parms" string (page 10), the abbreviation for
"parameters" is unusual; was "params" intended?
- In "originate-default-route-container" (p 12), to be consistent: s/RIP
or RIPng instance/RIP routing instance/
- In "redistribute-container" (p 12): s/BFP autonomous system/BGP
autonomous system/
- In "list isis" (p 12-13): s/ISIS/IS-IS/  [5 times]
- In "list ospfv2" (p 14-15): s/OSPF routing instance into the RIP
routing instance/OSPFv2 routing instance into the RIPv2 routing
instance/  [twice]
- In "route-type" of "list ospfv2" (p 15): s/OSPF routes matching the
specified route type into the RIP routing instance/OSPFv2 routes
matching the specified route type into the RIPv2 routing instance/
- In "list ospfv3" (p 15): s/OSPF routing instance into the RIP routing
instance/OSPFv3 routing instance into the RIPng routing instance/  [twice]
- In "route-type" of "list ospfv3" (p 16): s/OSPF routes matching the
specified route type into the RIP routing instance/OSPFv3 routes
matching the specified route type into the RIPng routing instance/
- In "ripv2" (p 16): s/RIP routing instance into the current RIP routing
instance/RIPv2 routing instance into the current RIPv2 routing
instance/  [twice]
- In "leaf listen" of "list interface" (p 29): s/RIP or RIPng/RIPv2 or
RIPng/
- In "container ipv4" (p 31): s/A RIPv2 RIP neighbor/A RIPv2 neighbor/
- In "container ipv6" (p 33): s/A RIPv2 RIP neighbor/A RIPng neighbor/
- In "leaf ipv6-prefix" of "container routes" (p 34): s/in
RFC5952)and/in RFC5952) and/


Regards,

Julien