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

[tom petch <[e-mail]>]

_______________________________________
From: Tarek Saad <[e-mail]>
Sent: 26 February 2020 19:34

Hi Tom,

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

<tp>

Basically yes either OK or <tp2> in response 

Tom Petch

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.

OK
            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).
"

OK

    [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.";
      }

OK

            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.

OK

            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.

<tp2> yes please add it is a better description although I cannot find a use of loadshare.  I will dig around to see if I can find the term used anywhere else but meantime go ahead with this one.  To date I can only find it in SS7:-)


              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.

OK

                    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?

<tp2>
 yes that would be a good change

</tp>

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