Re: [mpls] MPLS wglc draft-ietf-mpls-base-yang

[Tarek Saad <[e-mail]>]

Hi Tom,

Thanks again. Please see inline for more responses. We will upload a new rev once you ack ok for below.

On 2/21/20, 11:30 AM, "tom petch" <[e-mail]> wrote:

    Tarek
    
    progressing ... <tp> inline
    
    ________________________________________
    From: Tarek Saad <[e-mail]>
    Sent: 19 February 2020 06:00
    
    Thanks much for your review and useful comments. We have posted a new revision -12 at https://urldefense.com/v3/__https://tools.ietf.org/html/draft-ietf-mpls-base-yang-12__;!!NEt6yMaO-gk!RFlhNE079NN1bKO-FktmwgK2FeDnE2z--K8tf-tk57WePssEoox0hX_EJhhQwA$  that addresses those. Let us know if you have further comments.
    Also, see inline for responses on closure.
    
    On 1/21/20, 7:38 AM, "tom petch" <[e-mail]> wrote:
    
    Some more technical thoughts on this I-D
    
            Abstract
    
            LSP is not in the RFC Editor list of 'so well known ...'
    [TS]: it is listed at https://urldefense.com/v3/__https://www.rfc-editor.org/materials/abbrev.expansion.txt__;!!NEt6yMaO-gk!RFlhNE079NN1bKO-FktmwgK2FeDnE2z--K8tf-tk57WePssEoox0hX9YKDyWLw$ 
    
    <tp> as Acee points out, it lacks the asterisk that says that no expansion is needed :-(
[TS]: yes, admittedly I was ignorant about that.
    
            s.2.1
               The other MPLS route(s) that are non-IP prefix routes are modelled by
               introducing a new "mpls" address-family RIB as per recommendation .
    
            Where is that?
    
    <tp> I was looking for a reference for the recommendation - is this the routing RFC, RFC8349 s.5.2?
[TS]: Yes, that would be RFC8349 section 3:
"
   o  The data model should be suitable for the common address families
      -- in particular, IPv4 and IPv6 -- and for unicast and multicast
      routing, as well as Multiprotocol Label Switching (MPLS).
"
    
    
    [TS]: Please refer to identity below defined in the model which has rt:address-family as base.
      identity mpls {
            base rt:address-family;
            description
              "This identity represents the MPLS address family.";
      }
            s.2.2
               interfaces-mpls:
                      A YANG grouping that describes the list of MPLS enabled interfaces
                      on a device.
    
               label-blocks:
                      A YANG grouping that describes the list of MPLS enabled interfaces
                      on a device.
    
            um
    [TS]: we corrected the description. See below:
    NEW:
    A YANG grouping that describes the list of assigned MPLS label blocks and its properties.
    
    
            YANG Module
                     Copyright (c) 2018 IETF Trust
    
                    leaf loadshare {
            I  would  value a reference for this.
    [TS]: rfc3031 section 3.11 and  3.12.
    
    <TP>  well sort of; those sections talk about sharing across equal cost paths which seems somewhat simpler - I understand the YANG DESCRIPTION but would expect it to be described in an RFC somewhere.
[TS]: I've across yet another reference that tackles optimizing distribution of traffic on ECMP/LAG -- rfc7424 (e.g. sec 5.4). Let me know if you are OK with this and we can refer to it.
    
    
              grouping interface-mpls-properties {
            ...
                      type boolean;
                      description
                       "'true' if mpls encapsulation is enabled on the interface.
                       'false' if mpls encapsulation is enabled  on the interface.";
    
    <tp> so true means enabled and false means ..  enabled ; clear but logical?
[TS]: thanks for spotting this. We will correct this typo.

    
                    leaf mtu {  type uint32; description
                            "MPLS Maximum Transmission Unit (MTU) in bytes";
    
            um what is included in MTU here  c.f.draft-ietf-netmod-intf-ext-yang-07
            which  includes L2 header but not FCS in Layer 2 MTU;
            a reference might help
    [TS]: includes L2 MTU + maximum MPLS label stack size. rfc3032 section 3.2.
    
    <tp> well no; MTU does not appear in RFC3032 s.3.1; other terminology does and I want to map the terminology of this I-D - MTU- to the terminology of s.3.1
[TS]: the section we referenced (RFC3032 section 3.2) has:

"
3.2. Maximum Initially Labeled IP Datagram Size

   Every LSR which is capable of

      a) receiving an unlabeled IP datagram,
      b) adding a label stack to the datagram, and
      c) forwarding the resulting labeled packet,

   SHOULD support a configuration parameter known as the "Maximum
   Initially Labeled IP Datagram Size", which can be set to a non-
   negative value.
"

Would renaming the leaf to maximum-labeled-packet-size help in any way?

Regards,
Tarek


    That is all
    
    Tom Petch
    </tp>
    
              grouping interfaces-mpls {    description "List of MPLS interfaces";
                    list interface {   key "name";  description "List of MPLS
            interfaces";
                      leaf name {   type if:interface-ref;
                            description   "The name of a configured MPLS interface";
    
            no conditional so every RIP or dial up X.25 e.g.is going to get this:-( probably ok
    
              augment "/rt:routing" {
            ditto, although probably less significant
    [TS]: added feature mpls, and added if-feature check under the augmentations.
    
              augment "/rt:routing/rt:ribs/rt:rib/rt:routes/rt:route/"
                            + "rt:next-hop/rt:next-hop-options/rt:simple-next-hop" {
                    description
                      "Augment 'simple-next-hop' case in IP unicast routes.";
    
            other I-D augment control plane/protocols/static-routes
    [TS]: the MPLS Static model is a separate model that is described in draft-ietf-mpls-static-yang.
    
    Regards,
    Tarek
    
            Tom Petch