Subject: [RTG-DIR] Rtgdir early review of draft-ietf-idr-bgp-model-07

From: Yingzhen Qu via Datatracker <[email-masked]>
Date: Tue, 10 Dec 2019 14:17:37 -0800

Reviewer: Yingzhen Qu
Review result: Has Issues

Hello,

I have been selected to do a routing directorate “early” review of this draft.
https://datatracker.ietf.org/doc/draft-ietf-idr-bgp-model/

The routing directorate will, on request from the working group chair, perform
an “early” review of a draft before it is submitted for publication to the
IESG. The early review can be performed at any time during the draft’s lifetime
as a working group document. The purpose of the early review depends on the
stage that the document has reached. As this document is in working group last
call, my focus for the review was to determine whether the document is ready to
be published. Please consider my comments along with the other working group
last call comments. For more information about the Routing Directorate, please
see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir Document:
draft-ietf-idr-bgp-model-07 Reviewer: Yingzhen Qu Review Date: 10 December 2019
Intended Status: Standards Track

Summary
Thank you for writing this document. It is a critical piece for IETF YANG
support in general. This document is very well written. I think that it is
almost ready to be published, but there are a few points below that I would
like to discuss for clarification. I also spotted a few nits that should be
fixed at some point before publication.

Comments and Questions
1) about RIB
This model defines BGP RIB, which augments RFC 8349. There are a few places it
says just RIB instead of BGP RIB, and I’d suggest making it clear to be BGP
RIB. For example: In Abstract:
   This document defines a YANG data model for configuring and managing
   BGP, including protocol, policy, and operational aspects, such as
   RIB, based on data center, carrier and content provider operational
   requirements.
Section 1.1:
o  RIB contents.
2) About VRF support
This question has been asked during WG session when the draft was presented.
VRF support is through Network Instance model (RFC8529), however there is issue
when trying to map a VRF to one of the several BGP instances. Example
configuration is shown below:

RP/0/RSP0/CPU0:ASR9001_RT21(config-bgp-vrf-af)#do show running-config  router
bgp Tue Nov 19 10:02:04.130 UTC router bgp 100 instance a1 bgp router-id
12.12.12.12 address-family vpnv4 unicast ! neighbor 121.1.1.1
  remote-as 100
  address-family vpnv4 unicast
  !
!
vrf vrf-a1                                 <— it needs some sort of binding
information to relate this VRF to BGP instance a1. In this case,
                                                        the key point is there
                                                        are one than  one BGP
                                                        default instances.
  rd auto
  address-family ipv4 unicast
   redistribute static
  !
!
!
router bgp 200 instance a2
bgp router-id 13.1.1.1
address-family vpnv4 unicast
!
address-family ipv6 unicast
!
address-family l2vpn evpn
!
neighbor 122.1.1.1
  remote-as 100
  address-family vpnv4 unicast
  !
  address-family l2vpn evpn
  !
!
vrf a3
  rd 13.1.1.1:2
  address-family ipv4 unicast
  !
!
vrf vrf-a2
  rd 13.1.1.1:1
  address-family ipv4 unicast
   redistribute connected
  !
!
!
router bgp 100
bgp router-id 1.1.11.1
address-family ipv4 unicast
!
address-family vpnv4 unicast
!
address-family l2vpn vpls-vpws
!

Nits
Overall the document is very well written and read well. Only a couple of nits:
Section 3
   The BGP model augments the Routing Management model A YANG Data Model
   for Routing Management [RFC8349] which defines the notion of routing,
   routing protocols,
It seems that this long sentence could use a couple “,”.

Section 4:
   These are the subtrees and data nodes
   and their sensitivity/vulnerability:
There are three paragraphs ending with this sentence, meant for writeable, read
only and RPC operations data nodes, but the detailed nodes are missing.