Subject: [Idr] Yangdoctors early review of draft-ietf-idr-bgp-model-09

From: Acee Lindem via Datatracker <[email-masked]>
Date: Sat, 15 Aug 2020 12:49:53 -0700

Reviewer: Acee Lindem
Review result: On the Right Track

Document: draft-ietf-idr-bgp-model-09.txt
Reviewer: Acee Lindem
Review Date: Aug 15, 2020
Review Type: Working Group Last Call
Intended Status: Standards Track
Summary: On the Right Track - Major Comments need to be addressed.

Modules: [email-masked]
         [email-masked]
	 [email-masked]
	 Plus a plethora of sub-modules 

Tech Summary: The document contains the base configuration and operational
              YANG model for the BGP protocol. The basic structure is good
	      but the major comments need to be addressed.

Major Comments:

     1. The draft claims to support a number of AFs in section 2.1. However,
        the only AFs that are complete are ipv4-unicast and ipv6-unicast. The
	draft shouldn't claim to fully support these AFs and they should be
	left to subsequent augmentations. There are already BESS models to
	cover many of thes AFI/SAFI combinations.
     2. The RIB sub-module is split into seven separate sub-module. This makes
        it virtually incomprehesible. At least some of the sub-modules have
	very little content. I would suggest these sub-modules be collasped
	into a single sub-module - ietf-bgp-rib.
     3. I know the discussion sub-modules in general is ongoing. However, the
        current state of the tools with respect to sub-modules makes the
	validation useless. Since YANG 1.0 and 1.1 both support sub-modules,
	I think the tools need to be fixed ASAP.
     4. The Security Considerations section is just the template with no
        discussion of the sutrees and data node vulnerabilities.
     5. Add complete tree diagrams in appendicies. 	

Minor Comments:

     1. General - Punctuation of descriptions is inconsistent. I'd consistently
        use punctuation for anything that isn't just replication of the YANG
	identifier (which isn't a good description anyway).
     2. Page 20, "container distance": I believe that "leaf local" has been
        ommitted.
     3. Page 25, "case ipsec": What is the reference for usage of transport
        mode IPsec with BGP? Please elaborate in description.
     4. Page 27, "supported-capabilities": The statement "BGP capabilities
        negotiated as supported by peer" mean? Shoudln't this just be
	"Negotiated BGP capabilities"? Shouldn't the leaf-list be named
	"negotiated-capabilities" rather than "supported-capabilities"?
     5. Page 29, "remote-helper": I'd support this to be renamed
        "graceful-restart-only" since "helper-only" is relative to the local
	peer.
     6. Page 30, "fsm-established-transitions": What is the difference with
        "established-transitions"? Is the description a cut-n-paste error?
     7. Page 33, "backward-transition": This implies any backward transition.
        I'd suggest renaming to "established-state-lost"
	or "established-backward-transition".
     8. Page 34, "clear-at": Is there a way to say to process the clear
        immediately?
     9. Page 41, "mtu-discovery": Please add a reference. Also, shouldn't the
        default be true?
    10. Page 42, "auth-password": Add a reference to RFC 2385.
    11. Page 62, "leaf enabled": What is more specific than per neighbor
        configuration?
    12. Page 65, "module description": I don't think usage is restricted to
        BGP policy. There could be other reasons to import ietf-bgp-types.
    13. Page 70, "no-peer": It would be good to include the standard
        community value in the description like the others.
    14. Page 71, "bgp-session-direction": Why uppercase for the values?
    15. Page 72, "bgp-ext-community-type": Many of these string patterns are
        defined in RFC 8294. Also, it would be good to add the syntax of the
	pattern you are representing in the description.
    16. Page 74, "peer-type", "description": RFC 4271 uses EBGP and IBGP rather
        than eBGP and iBGP.
    17. Page 74, "REMOVE_PRIVATE_AS_OPTION": Why is this all uppercase?
    18. Page 75, "percentage": This type is RFC 8294.
    19. Page 77, "module ietf-bgp-policy": This is missing the stardard
        RFC 8407 module template.
    20. Page 78, "bgp-set-med-type": Please describe the syntax of the pattern
        in the description.
    21. Page 79, "reference": Remove the "WG needs to decide...". We are making
        routing policy a standard in these models.
    22. Page 80, "member": Why is this a string? Is this a regular expression?
        Please describe the syntax in the description. If regular expressions, add
	a reference.
    23. Page 81, "set-community-action-common": This seems like a terribly
        complex way to model this. Why didn't you just use a YANG choice?
    24. Page 82, "next-hop-in": This is an inconsistent name. I'd suggest
        "next-hop-list-eq".
    25. Page 82, "afi-safi-in": Policies are normally applied at the AFI/SAFI
        level. By putting a match condition for these, you are adding a
	second way of doing this and adding considerable complexity to debugging
	policies.
    26. Page 82, "community-count": This container is empty and, even if it
        weren't, what possible use-case would there be to match on the number
	of communities as opposed to specific communities?
    27. Page 82, "as-path-length": This container is also empty. The use cases
        for matching on AS Path Length are well-known and should include ge/le
	matching.
    28. Page 84, "set-local-pref": The description indicates this is limited
        to route updates. This should apply to route attributes as well.
    29. Page 84, "set-med":  The description indicates this is limited
        to route updates. This should apply to route attributes as well.
    30. Pages 84-85, "set-community": This could be modeled with a YANG
        choice rather than method/when statements.
    31. Pages 85-86, "set-ext-community": This could be modeled with a YANG
        choice rather than method/when statements.
    32. Pages 94-95, "afi-saifs": As I stated in my major comment, only
        ipv4-unicast and ipv6-unicast are fully specified.
    33. Pages 95-96, "ietf-bgp-rib-ext": Since this model only has a single
        grouping, it doesn't seem like a very useful functional division.
    34. Page 107, Remove all the empty groups. They are useless.

Nits:

    1. Update all the copyrights to 2020.

*** draft-ietf-idr-bgp-model-09.txt.orig	2020-08-11 09:21:07.000000000 -0400
--- draft-ietf-idr-bgp-model-09.txt	2020-08-15 15:31:20.000000000 -0400
***************
*** 20,26 ****
  
     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.
  
  Status of This Memo
--- 20,26 ----
  
     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.
  
  Status of This Memo
***************
*** 119,125 ****
     This document describes a YANG [RFC7950] data model for the BGP-4
     [RFC4271] protocol, including various protocol extensions, policy
     configuration, as well as defining key operational state data,
!    including Routing Information Base (RIB).  The model is intended to
     be vendor-neutral, in order to allow operators to manage BGP
     configuration in heterogeneous environments with routers supplied by
     multiple vendors.  The model is also intended to be readily mapped to
--- 119,125 ----
     This document describes a YANG [RFC7950] data model for the BGP-4
     [RFC4271] protocol, including various protocol extensions, policy
     configuration, as well as defining key operational state data,
!    including a BGP Routing Information Base (RIB).  The model is intended to
     be vendor-neutral, in order to allow operators to manage BGP
     configuration in heterogeneous environments with routers supplied by
     multiple vendors.  The model is also intended to be readily mapped to
***************
*** 146,158 ****
     addresses policy configuration, by providing "hooks" for applying
     policies, and also defining BGP-specific policy features.  The BGP
     policy features are intended to be used with the general routing
!    policy model defined in A YANG Data Model for Routing Policy
!    Management [I-D.ietf-rtgwg-policy-model].
  
     The model conforms to the NMDA [RFC8342] architecture.  It has
     support for securing BGP sessions using TCP-AO [RFC5925] or TCP-MD5,
     and for configuring Bidirectional Forward Detection (BFD) [RFC5880]
!    for fast next hop liveliness check.
  
     For the base BGP features, the focus of the model described in this
     document is on providing configuration and operational state
--- 146,158 ----
     addresses policy configuration, by providing "hooks" for applying
     policies, and also defining BGP-specific policy features.  The BGP
     policy features are intended to be used with the general routing
!    policy model defined in "A YANG Data Model for Routing Policy
!    Management" [I-D.ietf-rtgwg-policy-model].
  
     The model conforms to the NMDA [RFC8342] architecture.  It has
     support for securing BGP sessions using TCP-AO [RFC5925] or TCP-MD5,
     and for configuring Bidirectional Forward Detection (BFD) [RFC5880]
!    for fast next-hop liveliness checking.
  
     For the base BGP features, the focus of the model described in this
     document is on providing configuration and operational state
***************
*** 177,183 ****
        that relate to a neighbor - controlling the import and export of
        NLRIs.
  
!    o  RIB contents.
  
     As mentioned earlier, any configuration items that are deemed to be
     widely available in existing major BGP implementations are included
--- 177,183 ----
        that relate to a neighbor - controlling the import and export of
        NLRIs.
  
!    o  BGP RIB contents.
  
     As mentioned earlier, any configuration items that are deemed to be
     widely available in existing major BGP implementations are included
***************
*** 210,220 ****
  
     2020-06-28 with the actual date of the publication of this document.
  
!    RFC ZZZZ, where ZZZZ is the number assigned to A YANG Data Model for
!    Routing Policy Management [I-D.ietf-rtgwg-policy-model].
  
!    RFC BBBB, where BBBB is the number assigned to YANG Data Model for
!    Bidirectional Forward Detection [I-D.ietf-bfd-yang].

--- 210,220 ----
  
     2020-06-28 with the actual date of the publication of this document.
  
!    RFC ZZZZ, where ZZZZ is the number assigned to "A YANG Data Model for
!    Routing Policy Management" [I-D.ietf-rtgwg-policy-model].
  
!    RFC BBBB, where BBBB is the number assigned to "YANG Data Model for
!    Bidirectional Forward Detection" [I-D.ietf-bfd-yang].

***************
*** 283,289 ****

     o  policy configuration -- hooks for application of the policies
!       defined in A YANG Data Model for Routing Policy Management
        [I-D.ietf-rtgwg-policy-model] that act on routes sent (received)
        to (from) peers or other routing protocols and BGP-specific policy
        features.
--- 283,289 ----

     o  policy configuration -- hooks for application of the policies
!       defined in "A YANG Data Model for Routing Policy Management"
        [I-D.ietf-rtgwg-policy-model] that act on routes sent (received)
        to (from) peers or other routing protocols and BGP-specific policy
        features.
***************
*** 293,299 ****
  
     These modules also make use of standard Internet types, such as IP
     addresses and prefixes, autonomous system numbers, etc., defined in
!    Common YANG Data Types [RFC6991].
  
  2.1.  BGP protocol configuration
  
--- 293,299 ----
  
     These modules also make use of standard Internet types, such as IP
     addresses and prefixes, autonomous system numbers, etc., defined in
!    "Common YANG Data Types" [RFC6991].
  
  2.1.  BGP protocol configuration
  
***************
*** 374,383 ****
     Users may specify configuration at a higher level and have it apply
     to all lower-level items, or provide overriding configuration at a
     lower level of the hierarchy.  Overriding configuration items are
!    optional, with neighbor specific configuration being the most
     specific or lowest level, followed by peer-group, and finally global.
     Global configuration options reflect a subset of the peer-group or
!    neighbor specific configuration options which are relevant to the
     entire BGP instance.
  
     The model makes the simplifying assumption that most of the
--- 374,383 ----
     Users may specify configuration at a higher level and have it apply
     to all lower-level items, or provide overriding configuration at a
     lower level of the hierarchy.  Overriding configuration items are
!    optional, with neighbor-specific configuration being the most
     specific or lowest level, followed by peer-group, and finally global.
     Global configuration options reflect a subset of the peer-group or
!    neighbor-specific configuration options which are relevant to the
     entire BGP instance.
  
     The model makes the simplifying assumption that most of the
***************
*** 406,412 ****
     Address-family configuration is made available in multiple points
     within the model - primarily within the global container, where
     instance-wide configuration can be set (for example, global protocol
!    parameters, the BGP best path route selection options, or global
     policies relating to the address-family); and on a per-neighbor or
     per-peer-group basis, where address-families can be enabled or
     disabled, and policy associated with the parent entity applied.
--- 406,412 ----
     Address-family configuration is made available in multiple points
     within the model - primarily within the global container, where
     instance-wide configuration can be set (for example, global protocol
!    parameters, the BGP best-path route selection options, or global
     policies relating to the address-family); and on a per-neighbor or
     per-peer-group basis, where address-families can be enabled or
     disabled, and policy associated with the parent entity applied.
***************
*** 480,487 ****
  2.2.  Policy configuration overview
  
     The BGP policy configuration model augments the generic YANG routing
!    policy model described in A YANG Data Model for Routing Policy
!    Management [I-D.ietf-rtgwg-policy-model], which represents a
     condition-action policy framework for routing.  This model adds BGP-
     specific conditions (e.g., matching on the community attribute), and
     actions (e.g., setting local preference) to the generic policy
--- 480,487 ----
  2.2.  Policy configuration overview
  
     The BGP policy configuration model augments the generic YANG routing
!    policy model described in "A YANG Data Model for Routing Policy
!    Management" [I-D.ietf-rtgwg-policy-model], which represents a
     condition-action policy framework for routing.  This model adds BGP-
     specific conditions (e.g., matching on the community attribute), and
     actions (e.g., setting local preference) to the generic policy
***************
*** 535,541 ****
     The RIB data model represents the BGP RIB contents.  The model
     supports five logical RIBs per address family.
  
!    A abridged version of the tree shows the RIB portion of the tree
     diagram.

--- 535,541 ----
     The RIB data model represents the BGP RIB contents.  The model
     supports five logical RIBs per address family.
  
!    An abridged version of the tree shows the RIB portion of the tree
     diagram.

***************
*** 652,667 ****
  
     The adj-rib-out-post table is a per-neighbor table containing routes
     eligible for sending (advertising) to the neighbor after output
!    policy rules have been applied
  
  3.  Relation to other YANG data models
  
!    The BGP model augments the Routing Management model A YANG Data Model
!    for Routing Management [RFC8349] which defines the notion of routing,
     routing protocols, and RIBs.  The notion of Virtual Routing and
!    Forwarding (VRF) is derived by using the YANG Schema Mount [RFC8528]
!    to mount the Routing Management module under the YANG Data Model for
!    Network Instances [RFC8529].

--- 652,667 ----
  
     The adj-rib-out-post table is a per-neighbor table containing routes
     eligible for sending (advertising) to the neighbor after output
!    policy rules have been applied.
  
  3.  Relation to other YANG data models
  
!    The BGP model augments the Routing Management model "A YANG Data Model
!    for Routing Management" [RFC8349] which defines the notion of routing,
     routing protocols, and RIBs.  The notion of Virtual Routing and
!    Forwarding (VRF) is derived by using the "YANG Schema Mount" [RFC8528]
!    to mount the ietf-routing module under the "YANG Data Model for
!    Network Instances" [RFC8529].

***************
*** 732,739 ****
  
  5.1.  URI Registration
  
!    in the IETF XML registry [RFC3688] [RFC3688].  Following the format
!    in RFC 3688, the following registration is requested to be made:
  
     URI: urn:ietf:params:xml:ns:yang:ietf-bgp
     URI: urn:ietf:params:xml:ns:yang:ietf-bgp-policy
--- 732,739 ----
  
  5.1.  URI Registration
  
!    Following the format in RFC 3688, the following registrations are requested
!    in the IETF XML registry [RFC3688]:
  
     URI: urn:ietf:params:xml:ns:yang:ietf-bgp
     URI: urn:ietf:params:xml:ns:yang:ietf-bgp-policy
***************
*** 744,750 ****
  
  5.2.  YANG Module Name Registration
  
!    This document registers three YANG module in the YANG Module Names
     registry YANG [RFC6020].
  
     name: ietf-bgp
--- 744,750 ----
  
  5.2.  YANG Module Name Registration
  
!    This document registers three YANG modules in the YANG Module Names
     registry YANG [RFC6020].
  
     name: ietf-bgp
***************
*** 816,822 ****
     The YANG model can be subdivided between the main module for base
     items, types, policy data, and the RIB module.  It references BGP
     Communities Attribute [RFC1997], Route Refresh Capability for BGP-4
!    [RFC2918], , NOPEER Community for BGP [RFC3765], BGP/MPLS IP Virtual
     Private Networks (VPNs) [RFC4364], BGP-MPLS IP Virtual Private
     Network (VPN) Extension for IPv6 VPN [RFC4659], Graceful Restart
     Mechanism for BGP [RFC4724], Multiprotocol Extenstions for BGP-4
--- 816,822 ----
     The YANG model can be subdivided between the main module for base
     items, types, policy data, and the RIB module.  It references BGP
     Communities Attribute [RFC1997], Route Refresh Capability for BGP-4
!    [RFC2918], NOPEER Community for BGP [RFC3765], BGP/MPLS IP Virtual
     Private Networks (VPNs) [RFC4364], BGP-MPLS IP Virtual Private
     Network (VPN) Extension for IPv6 VPN [RFC4659], Graceful Restart
     Mechanism for BGP [RFC4724], Multiprotocol Extenstions for BGP-4
***************
*** 1080,1086 ****
          }
          container distance {
            description
!             "Administrative distance (or preference) assigned to
               routes received from different sources
               (external, internal, and local).";
            leaf external {
--- 1080,1086 ----
          }
          container distance {
            description
!             "Administrative distances (or preferences) assigned to
               routes received from different sources
               (external, internal, and local).";
            leaf external {
***************
*** 1148,1154 ****
              key "afi-safi-name";
              description
                "AFI,SAFI configuration available for the
!                neighbour or group";
              uses mp-afi-safi-config;
              uses state;
              container graceful-restart {
--- 1148,1154 ----
              key "afi-safi-name";
              description
                "AFI,SAFI configuration available for the
!                neighbor or group";
              uses mp-afi-safi-config;
              uses state;
              container graceful-restart {
***************
*** 1237,1243 ****
              description
                "The BGP Identifier of this entry's BGP peer.
                 This entry MUST be 0.0.0.0 unless the
!                sessionstate is in the openconfirm or the
                 established state.";
              reference
                "RFC 4271, Section 4.2, 'BGP Identifier'.";
--- 1237,1243 ----
              description
                "The BGP Identifier of this entry's BGP peer.
                 This entry MUST be 0.0.0.0 unless the
!                session state is in the openconfirm or the
                 established state.";
              reference
                "RFC 4271, Section 4.2, 'BGP Identifier'.";
***************
*** 1560,1566 ****
                description
                  "This flag indicates whether the remote neighbor is
                   currently in the process of restarting, and hence
!                  received routes are currently stale";
              }

--- 1560,1566 ----
                description
                  "This flag indicates whether the remote neighbor is
                   currently in the process of restarting, and hence
!                  received routes are currently stale.";
              }

***************
*** 1575,1583 ****
                config false;
                description
                  "This flag indicates whether the local neighbor is
!                  currently restarting. The flag is unset after all NLRI
                   have been advertised to the peer, and the End-of-RIB
!                  (EOR) marker has been unset";
              }
              leaf mode {
                type enumeration {
--- 1575,1583 ----
                config false;
                description
                  "This flag indicates whether the local neighbor is
!                  currently restarting. The flag is cleared after all NLRI
                   have been advertised to the peer, and the End-of-RIB
!                  (EOR) marker has been cleared.";
              }
              leaf mode {
                type enumeration {
***************
*** 1780,1787 ****
                "The last error code and subcode seen by this
                 peer on this connection.  If no error has
                 occurred, this field is zero.  Otherwise, the
!                first byte of this two byte OCTET STRING
!                contains the error code, and the second byte
                 contains the subcode.";
              reference
                "RFC 4271, Section 4.5.";
--- 1780,1787 ----
                "The last error code and subcode seen by this
                 peer on this connection.  If no error has
                 occurred, this field is zero.  Otherwise, the
!                first octet of this two octet OCTET STRING
!                contains the error code, and the second octet
                 contains the subcode.";
              reference
                "RFC 4271, Section 4.5.";
***************
*** 1814,1821 ****
                path "../../neighbor/remote-address";
              }
              description
!               "IP address of the neighbor that went away from
!                established state.";
            }
            leaf last-error {
              type leafref {
--- 1814,1821 ----
                path "../../neighbor/remote-address";
              }
              description
!               "IP address of the neighbor that fell from
!                established state."
            }
            leaf last-error {
              type leafref {
***************
*** 1963,1969 ****

          multiple contexts within the BGP module. That is to say that
!         they may be application to a subset of global, peer-group or
          neighbor contexts.
  
          Copyright (c) 2019 IETF Trust and the persons identified as
--- 1963,1969 ----

          multiple contexts within the BGP module. That is to say that
!         they may be applicable to a subset of global, peer-group, or
          neighbor contexts.
  
          Copyright (c) 2019 IETF Trust and the persons identified as
***************
*** 2065,2071 ****
              this timer is 30 seconds.;
  
              The actual time interval for the KEEPALIVE messages is
!             indicated by operational value of keepalive. That value

--- 2065,2071 ----
              this timer is 30 seconds.;
  
              The actual time interval for the KEEPALIVE messages is
!             indicated by operational value of keepalive. The value

***************
*** 2202,2209 ****
             default "16.0";
             description
               "This is the upper limit of the instability metric. This
!                      value must be greater than the larger of 1 and
!                      suppress-above.";
           }
           leaf reach-decay {
             type yang:gauge32;
--- 2202,2209 ----
             default "16.0";
             description
               "This is the upper limit of the instability metric. This
!               value must be greater than the larger of 1 and
! 	      suppress-above.";
           }
           leaf reach-decay {
             type yang:gauge32;
***************
*** 2345,2351 ****
             "An upper-bound on the time that stale routes will be
              retained by a router after a session is restarted. If an
              End-of-RIB (EOR) marker is received prior to this timer
!             expiring stale-routes will be flushed upon its receipt - if

--- 2345,2351 ----
             "An upper-bound on the time that stale routes will be
              retained by a router after a session is restarted. If an
              End-of-RIB (EOR) marker is received prior to this timer
!             expiring, stale-routes will be flushed upon its receipt - if

***************
*** 2452,2458 ****
             description
               "Ignore the AS path length when selecting the best path.
                The default is to use the AS path length and prefer paths
!               with shorter length.";
           }
           leaf external-compare-router-id {
             type boolean;
--- 2452,2458 ----
             description
               "Ignore the AS path length when selecting the best path.
                The default is to use the AS path length and prefer paths
!               with a shorter length.";
           }
           leaf external-compare-router-id {
             type boolean;
***************
*** 2502,2508 ****
             default "false";
             description
               "Flag to enable sending/receiving of MED metric attribute
!                      in routing updates.";
           }
         }
       }
--- 2502,2508 ----
             default "false";
             description
               "Flag to enable sending/receiving of MED metric attribute
!               in routing updates.";
           }
         }
       }
***************
*** 2638,2644 ****
           default "false";
           description
             "This leaf indicates whether the IPv4 Unicast AFI,SAFI is
!             enabled for the neighbour or group";
         }
       }
  
--- 2638,2644 ----
           default "false";
           description
             "This leaf indicates whether the IPv4 Unicast AFI,SAFI is
!             enabled for the neighbor or group";
         }
       }
  
***************
*** 2811,2823 ****
             type uint32;
             description
               "Maximum number of prefixes that will be accepted from the
!               neighbour";
           }
           leaf shutdown-threshold-pct {
             type bt:percentage;
             description
               "Threshold on number of prefixes that can be received from
!               a neighbour before generation of warning messages or log
                entries. Expressed as a percentage of max-prefixes";
           }
           leaf restart-timer {
--- 2811,2823 ----
             type uint32;
             description
               "Maximum number of prefixes that will be accepted from the
!               neighbor";
           }
           leaf shutdown-threshold-pct {
             type bt:percentage;
             description
               "Threshold on number of prefixes that can be received from
!               a neighbor before generation of warning messages or log
                entries. Expressed as a percentage of max-prefixes";
           }
           leaf restart-timer {
***************
*** 2842,2848 ****
           type boolean;
           default "false";
           description
!            "If set to true, send the default-route to the neighbour(s)";
         }
       }
  
--- 2842,2848 ----
           type boolean;
           default "false";
           description
!            "If set to true, send the default-route to the neighbor(s)";
         }
       }
  
***************
*** 2987,2995 ****
             "Configure logging of peer state changes.  Default is to
              enable logging of peer state changes.
  
!             Note: Documenting out of ESTABLISHED state is desirable,
!                   but documenting all backward transitions is
!                   problematic, and should be avoided.";
         }
       }
     }
--- 2987,2995 ----
             "Configure logging of peer state changes.  Default is to
              enable logging of peer state changes.
  
!             Note: Documenting demotion from ESTABLISHED state is
! 	          desirable, but documenting all backward transitions
! 		  is problematic, and should be avoided.";
         }
       }
     }
***************
*** 3001,3012 ****
          groups";
       container ebgp-multihop {
         description
!          "eBGP multi-hop parameters for the BGPgroup";
         leaf enabled {
           type boolean;
           default "false";
           description
!            "When enabled the referenced group or neighbors are
              permitted to be indirectly connected - including cases
              where the TTL can be decremented between the BGP peers";
         }
--- 3001,3012 ----
          groups";
       container ebgp-multihop {
         description
!          "eBGP multi-hop parameters for the BGP peer-group";
         leaf enabled {
           type boolean;
           default "false";
           description
!            "When enabled, the referenced group or neighbors are
              permitted to be indirectly connected - including cases
              where the TTL can be decremented between the BGP peers";
         }
***************
*** 3035,3041 ****
          groups";
       container route-reflector {
         description
!          "Route reflector parameters for the BGPgroup";
         reference
           "RFC 4456: BGP Route Reflection.";
         leaf route-reflector-cluster-id {
--- 3035,3041 ----
          groups";
       container route-reflector {
         description
!          "Route reflector parameters for the BGP peer-group";
         reference
           "RFC 4456: BGP Route Reflection.";
         leaf route-reflector-cluster-id {
***************
*** 3256,3262 ****
          key "afi-safi-name";
          description
            "AFI, SAFI configuration available for the
!            neighbour or group";
          uses mp-afi-safi-config;
          container graceful-restart {
            if-feature "bt:graceful-restart";
--- 3256,3262 ----
          key "afi-safi-name";
          description
            "AFI, SAFI configuration available for the
!            neighbor or group";
          uses mp-afi-safi-config;
          container graceful-restart {
            if-feature "bt:graceful-restart";
***************
*** 3272,3278 ****
  
      grouping bgp-peer-group-base {
        description
!         "Parameters related to a BGP group.";
        leaf peer-group-name {
          type string;
          description
--- 3272,3278 ----
  
      grouping bgp-peer-group-base {
        description
!         "Parameters related to a BGP peer-group.";
        leaf peer-group-name {
          type string;
          description
***************
*** 3312,3318 ****
        container afi-safis {
          description
            "Per-address-family configuration parameters associated with
!            the group.";
          uses bgp-peer-group-afi-safi-list;
        }
      }
--- 3312,3318 ----
        container afi-safis {
          description
            "Per-address-family configuration parameters associated with
!            the BGP peer-group.";
          uses bgp-peer-group-afi-safi-list;
        }
      }
***************
*** 3512,3518 ****
          container prefixes {
            config false;
            description
!             "Prefix counters for the BGP session";
            leaf received {
              type uint32;
              description
--- 3512,3518 ----
          container prefixes {
            config false;
            description
!             "Prefix counters for for the AFI/SAFI in this BGP session";
            leaf received {
              type uint32;
              description
***************
*** 3600,3608 ****
                  Jeffrey Haas (jhaas at pfrc.org).";
      description
        "This module contains general data definitions for use in BGP
!        policy. It can be imported by modules that make use of BGP
!        attributes";
! 
      revision 2020-06-28 {
        description
          "Initial Version";
--- 3600,3608 ----
                  Jeffrey Haas (jhaas at pfrc.org).";
      description
        "This module contains general data definitions for use in BGP
!        other BGP modules and modules requiring BGP types. It can be
!        imported by modules that make use of BGP attributes";
!        
      revision 2020-06-28 {
        description
          "Initial Version";
***************
*** 3700,3706 ****
  
          "Multi-protocol extensions to BGP";
        reference
!         "RFC 4760: Multiprotocol Extenstions for BGP-4.";
      }
  
      identity route-refresh {
--- 3700,3706 ----
  
          "Multi-protocol extensions to BGP";
        reference
!         "RFC 4760: Multiprotocol Extensions for BGP-4.";
      }
  
      identity route-refresh {
***************
*** 3846,3852 ****
      identity bgp-well-known-std-community {
        description
          "Base identity for reserved communities within the standard
!          community space defined by RFC1997. These communities must
           fall within the range 0xFFFF0000 to 0xFFFFFFFF";
        reference
          "RFC 1997: BGP Communities Attribute.";
--- 3846,3852 ----
      identity bgp-well-known-std-community {
        description
          "Base identity for reserved communities within the standard
!          community space defined by RFC 1997. These communities must
           fall within the range 0xFFFF0000 to 0xFFFFFFFF";
        reference
          "RFC 1997: BGP Communities Attribute.";
***************
*** 3856,3863 ****
        base bgp-well-known-std-community;
        description
          "Do not export NLRI received carrying this community outside
!          the bounds of this autonomous system, or this confederation if
!          the local autonomous system is a confederation member AS. This

--- 3856,3863 ----
        base bgp-well-known-std-community;
        description
          "Do not export NLRI received carrying this community outside
!          the bounds of this autonomous system, or this confederation (if
!          the local autonomous system is a confederation member AS). This

***************
*** 3895,3901 ****
        base bgp-well-known-std-community;
        description
          "An autonomous system receiving NLRI tagged with this community
!          is advised not to re-advertise the NLRI to external bi-lateral
           peer autonomous systems. An AS may also filter received NLRI
           from bilateral peer sessions when they are tagged with this
           community value";
--- 3895,3901 ----
        base bgp-well-known-std-community;
        description
          "An autonomous system receiving NLRI tagged with this community
!          is advised not to re-advertise the NLRI to external bilateral
           peer autonomous systems. An AS may also filter received NLRI
           from bilateral peer sessions when they are tagged with this
           community value";
***************
*** 4128,4134 ****
        }
        description
          "Labels a peer or peer group as explicitly internal,
!          external or confederation.";
      }
  
      identity REMOVE_PRIVATE_AS_OPTION {
--- 4128,4134 ----
        }
        description
          "Labels a peer or peer group as explicitly internal,
!          external, or confederation.";
      }
  
      identity REMOVE_PRIVATE_AS_OPTION {
***************
*** 4546,4551 ****
--- 4546,4552 ----
              update";
         }
         leaf-list afi-safi-in {
+       I guess it will be for inheritance? Also, would name differently. 
           type identityref {
             base bt:afi-safi-type;
           }
***************
*** 4961,4969 ****
            type boolean;
            description
              "BGP attribute indicating that the prefix is an atomic
!              aggregate; i.e., the peer selected a less specific
               route without selecting a more specific route that is
!              included in it.";
            reference
              "RFC 4271: Section 5.1.6.";
          }
--- 4962,4970 ----
            type boolean;
            description
              "BGP attribute indicating that the prefix is an atomic
!              aggregate, i.e., the peer selected a less specific
               route without selecting a more specific route that is
!              subsumed by it.";
            reference
              "RFC 4271: Section 5.1.6.";
          }
***************
*** 4987,4994 ****

            description
!             "BGP multi-exit discriminator attribute used in BGP route
!              selection process";
            reference
              "RFC 4271: Section 5.1.4.";
          }
--- 4988,4995 ----

            description
!             "BGP multi-exit discriminator attribute used in the BGP
! 	     route selection process";
            reference
              "RFC 4271: Section 5.1.4.";
          }
***************
*** 5064,5070 ****
              type inet:ipv4-address;
              description
                "IP address of the router that performed the
!                  aggregation.";
            }
          }
          container as-path {
--- 5065,5071 ----
              type inet:ipv4-address;
              description
                "IP address of the router that performed the
!                aggregation.";
            }
          }
          container as-path {
***************
*** 5256,5262 ****
                }
                description
                  "Routing tables for IPv4 unicast -- active when the
!                        afi-safi name is ipv4-unicast";
                uses ipv4-loc-rib;

--- 5257,5263 ----
                }
                description
                  "Routing tables for IPv4 unicast -- active when the
!                  afi-safi name is ipv4-unicast";
                uses ipv4-loc-rib;

***************
*** 5437,5443 ****
       identity invalid-route-reason {
         description
           "Base identity for reason code for routes that are rejected as
!           invalid.  Some derived entities are based on BMP v3";
         reference
           "RFC 7854: BGP Monitoring Protocol.";
       }
--- 5438,5444 ----
       identity invalid-route-reason {
         description
           "Base identity for reason code for routes that are rejected as
!           invalid. Some derived entities are based on BMP v3.";
         reference
           "RFC 7854: BGP Monitoring Protocol.";
       }