From nobody Tue Mar 16 03:07:06 2021 Return-Path: X-Original-To: rift@ietfa.amsl.com Delivered-To: rift@ietfa.amsl.com Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 468143A20D2 for ; Tue, 16 Mar 2021 03:07:05 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -2.097 X-Spam-Level: X-Spam-Status: No, score=-2.097 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Pu9bIHr4fiJl for ; Tue, 16 Mar 2021 03:07:03 -0700 (PDT) Received: from mail-il1-x12a.google.com (mail-il1-x12a.google.com [IPv6:2607:f8b0:4864:20::12a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id F164E3A20D5 for ; Tue, 16 Mar 2021 03:07:01 -0700 (PDT) Received: by mail-il1-x12a.google.com with SMTP id h18so12123430ils.2 for ; Tue, 16 Mar 2021 03:07:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=DIrAw4bhZKYjd16xVxem4Fcw0Hmi+xTfdasa1tNVQVE=; b=ik81C4okoy15PomVsxivpXQlZk6ZqZkVCF5mMiH9m5411Li+kyJHWDQCrbwILyZaeT HpgNGSDUXAsiCdwl584BurS07hl07dQ0P5auTIgkFeTopVjXGuYo9YQeYTn+tMQ2n4hK 6Oj24meNBcw0UKb0ZBXV6rfX2bTskUEAjeR3MWxnYal/JP1o2QYJk7rjlfGQQKCjdqjU bGxOIeXtvb0hzkVu7JAOOF6mkf4zK7n4MhkWspX7GhUyUJX1ItgNWC+npG/Izxn11VQ7 fpUlDZrn+yCkBCEJmMRpJJoPjF+5lwGYpJygHWkY3hJqHN8aVRzrwtm+V09+IhhfTWO+ 7gQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=DIrAw4bhZKYjd16xVxem4Fcw0Hmi+xTfdasa1tNVQVE=; b=N857/4a0my9iuvurSI1A5N7YgYZdlwi6W0CkRS4L+qvGXqc1d1tfMx+CljVzulO07r 8oyF7BUalVCHrJzOyCWSJG8kTe5pHcrtPFVL1hmvo/6LFjR4jYv8sChXWhwcw9g/1fFK TI7PuYn7nzimK6tb8jYCmvTSAS3BaGS/2nk2G7PsUxV4vk2v+Ql9odD0n7BimWDlOpTc SdgC/Oea1Rz1OoLmspY9XNXkVHl20gWuZnItz7HbOL8NtyDG7nl0e3Mqg1xogJ5Pg33I BlYLk+Jn8g2lyFLwQ3UE334hJNsdDRyxvCt+dzProHDltl9tRVVog2Wq9STqUZE78big Y6mg== X-Gm-Message-State: AOAM5308RIst+l16a1BVAqlt5xoePYQBCBS2MhFyh2iWRMMgxKAVCkma 95flbRejKGrwo7su2a8ymuIFIZScR2BDlZb+lbE= X-Google-Smtp-Source: ABdhPJyGSRTsFZgeQrrZ2flLYVrU92jY/qTlpLvR0yPlHk72JzPG+8M6b/ZiAVyazjreJLXF1jRJV6znHvwp3lNCCks= X-Received: by 2002:a92:7d0d:: with SMTP id y13mr3174250ilc.269.1615889220491; Tue, 16 Mar 2021 03:07:00 -0700 (PDT) MIME-Version: 1.0 References: <202103161029002897305@zte.com.cn> In-Reply-To: <202103161029002897305@zte.com.cn> From: Tony Przygienda Date: Tue, 16 Mar 2021 11:06:24 +0100 Message-ID: To: "zhang.zheng" Cc: Bruno Rijsman , rift@ietf.org Content-Type: multipart/alternative; boundary="000000000000fbdcdd05bda486b0" Archived-At: Subject: Re: [Rift] RIFT YANG datamodel implementation X-BeenThere: rift@ietf.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion of Routing in Fat Trees List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Mar 2021 10:07:05 -0000 --000000000000fbdcdd05bda486b0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Mar 16, 2021 at 3:29 AM wrote: > Hi tony, > > Sorry for the late response. > > Thank you very much for your detailed review! > > About you comments, could you please find my comments/question below with > Sandy>? > > Much appreciate for it. > > Best regards, > > Sandy > =E5=8E=9F=E5=A7=8B=E9=82=AE=E4=BB=B6 > *=E5=8F=91=E4=BB=B6=E4=BA=BA=EF=BC=9A*TonyPrzygienda > *=E6=94=B6=E4=BB=B6=E4=BA=BA=EF=BC=9A*Bruno Rijsman; > *=E6=8A=84=E9=80=81=E4=BA=BA=EF=BC=9A*rift@ietf.org; > *=E6=97=A5 =E6=9C=9F =EF=BC=9A*2021=E5=B9=B403=E6=9C=8810=E6=97=A5 19:29 > *=E4=B8=BB =E9=A2=98 =EF=BC=9A**Re: [Rift] RIFT YANG datamodel implementa= tion* > _______________________________________________ > RIFT mailing list > RIFT@ietf.org > https://www.ietf.org/mailman/listinfo/rift > > Just reviewed stuff carefully as well (also based on _real_ implemenation > ;-) but not Yang implementation). comment on -02 > > > +--ro hierarchy-indications? enumeration > > > that's bit more complex, we're lacking > > leaf_only_and_leaf_2_leaf_procedures =3D 1, > > which should be configurable > > Sandy> The type of "hierarchy-indications" is enumeration, and " > leaf_only_and_leaf_2_leaf_procedures" is included in it. Now the leaf is > not configurable, do you mean this leaf should be changed to configurable= ? > yes, you want to be able to configure that > > also, observe that level =3D=3D 24 that can be configued is NOT equivalen= t to > top-of-fabric which is a special flag > > so > > leaf configured-level { type level; description = "The configured level value of this node."; } > > should be something much more complex around a union of > > tof > > leaf-only > > leaf-with-leaf-2-leaf > > numeric > > Sandy> I'd like to add more description: "The value '0' indicates leaf-on= ly, the value "1" indicates leaf-with-leaf-2-leaf, and the value "2' indica= tes tof." Do you think it's OK? > > > well, you can do that but it's really an enum where the value should not matter > then > > overload > > in configuration sense should be > > overload { > > status: true/false > > timeout: optional timeout on config element > > } > > so I'd split between configured-overload and in-overload or just overload > Sandy> What's the meaning of "timeout"? Now in the model, the local "over= load" is configurable, the "overload" in node TIE isn't configurable. > > timeout is used very often. Basically there should be 2 timeouts on-startup this means node goes into overload until this tiemr expires when starting up immediate it means, set overload (but not on startup, right when configure) and remove after timeout expired > > if we allow to configure maximum nonce delta we should probably also allo= w to configure lifetime delta (both are pretty dangerous since they can bre= ak convergence real fast) > > +--rw holdtime? > > should be global-holdtime > Sandy> OK. > > ack > since that can be ultimately per link > > same for tide generation > > +--rw tie-security-key-id? uint32 > > that should refer to standard yang security chain as e.g. OSPF uses > > that should be also originating-key-id or something since we need (missin= g right now) the keys we are willing to accept as RW > Sandy> May I change this to the following, it's borrowed from OSPF YANG m= odel. The crypto-algorithm has many types, does RIFT support all of them, o= r just some of them? > | | | | +--rw tie-security > | | | | +--:(auth-key-chain)? > | | | | | +--rw key-chain? > | | | | | key-chain:key-chain-ref > | | | | +--:(auth-key-explicit) > | | | | +--rw key-id? uint32 > | | | | +--rw key? string > | | | | +--rw crypto-algorithm? > | | | | identityref > > on acceptance we need following variants > > SecurityChecking { > NoChecking, > Permissive, > Loose, > Strict, > } > Sandy> OK. > > ack > same @ link level for the security keys > Sandy> OK. Once you confirm the security key format, it will be syned to = interface. > > ack, basically keep to whatever OSPF does,. I thought ACee abstracted the whole keychain IGP stuff and we should use that > look @ YAML model we use in open source RIFT to see further details > > own pod missing > Sandy> The "pod" is the fourth leaf in node, please check it agin, do I m= ake some mistakes? > > maybe I missed > > @ interface level > > what is > > +--ro hal? level > > it seems backwards, HAL is of level type, no? > Sandy> The "HAL" is "The highest defined level value seen from all valid = level offers received." Does the description need to be improved? > > well, name the node HAL and the type is level > also list of HAL systemid offers is helpful here > Sandy> OK. I can add the systemid which offer the level. > > yepp, there maybe multiple > also on link add instance-name > Sandy> Do you mean the RIFT instance name? The whole model is following a= n instance, so the interface is also belong the same instance with the node= . Different instances are distinguished by the node. > > DATABASE > > > | +--ro (type)? | | +--:(prefix) = | | +--:(positive-disaggregation) | | +--:(n= egative-disaggregation) | | +--:(external) = | | +--:(positive-external-disaggregation) | | +--:(p= gp) > > oder strictly per thrift model since order has meaning here > Sandy> OK. I'll modify the order according to section 8.2.28.1. > > ack > | | +--ro link-id-pair* [remote-id] | = | | +--ro local-id? uint32 | | | +--ro remote-id = uint32 | | | +--ro if-index? uint32 Rijsman, et al.= Expires August 26, 2021 [Page 7] Internet-Draft = RIFT YANG Model February 2021 | = | | +--ro if-name? if:interface-ref | | +--ro cost= ? uint32 | | +--ro bandwidth? = uint32 | | +--ro flood-reduction? = boolean | | +--ro received-link-capabilities = | | +--ro bfd? boolean |= | +--ro v4-forwarding-capable? boolean > > missing > > 14: optional set > (python.immutable =3D "") address_families; > Sandy> OK. I'll add it. > > ack --000000000000fbdcdd05bda486b0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, Mar 16, 2021 at 3:29 AM <<= a href=3D"mailto:zhang.zheng@zte.com.cn">zhang.zheng@zte.com.cn> wro= te:

Hi tony,=C2=A0

Sorry for the late response.=C2=A0

Thank you very much for your detailed review!=C2=A0

=

About you comments, could you= please find my comments/question below with Sandy>?

Much appreciate for it.

Best regards,

Sandy

=E5=8E=9F=E5=A7=8B=E9=82=AE=E4=BB=B6<= /div>
=E5=8F= =91=E4=BB=B6=E4=BA=BA=EF=BC=9ATonyPrzygienda=
=E6=94=B6=E4=BB=B6=E4=BA=BA=EF=BC=9ABruno Rijsman;
=E6=8A=84=E9=80= =81=E4=BA=BA=EF=BC=9Arift@ietf.org;
=E6=97=A5 =E6=9C=9F =EF=BC=9A2021=E5=B9=B403=E6=9C=8810= =E6=97=A5 19:29
=E4=B8=BB =E9=A2=98 =EF=BC=9A= Re: [Rift] RIFT YANG datamodel implementation
_______________________________________________
RIFT=C2=A0mailing= =C2=A0list
RIFT@ietf.= org
https://www.ietf.org/mailman/listinfo/rift

Just reviewed stuff carefully as well (also based on _real_ imple= menation ;-) but not Yang implementation). comment on -02=C2=A0
<= br>
          +--ro hierarchy-indications?       enumeration

that's bit more complex, we're lacking <= br>

=C2=A0 =C2=A0 leaf_only_and_leaf_2_leaf_procedures =3D 1,=

which should be configurable

Sandy> The type of "= ;hierarchy-indications" is enumeration, and "leaf_only_and_leaf_2_leaf_procedures" is included = in it. Now the leaf is not configurable, do you mean this leaf should be ch= anged to configurable?=C2=A0

<= /div>

yes, you want to be able to configure= that
=C2=A0


<= /p>

also, observe that level =3D=3D 24 that can be configued is NOT equ= ivalent to top-of-fabric which is a special flag

so 

       leaf configured-level {          type level;         =
 description            "The configured level value of this node."=
;;        }

should be something much more complex around =
a union of 

tof
leaf-only
lea=
f-with-leaf-2-leaf 

numeric

Sandy> I'd like=
 to add more description: "The value '0' indicates leaf-only, =
the value "1" indicates leaf-with-leaf-2-leaf, and the value &quo=
t;2' indicates tof." Do you think it's OK?

<= br>
well, you can do that but it's really an enum where the v= alue should not matter
=C2=A0
then 

overload 

in c=
onfiguration sense should be 

overload { 
s=
tatus: true/false
timeout: optional timeout on config element=
 

} 

so I'd split between configured-overload =
and in-overload or just overload 
Sandy> What's the meaning of &q=
uot;timeout"? Now in the model, the local "overload" is conf=
igurable, the "overload" in node TIE isn't configurable.
<=
/pre>

timeout is used very often. Basically there should be 2 ti= meouts

on-startup

this means node goes into overload until this tiemr expires when startin= g up

immediate

= it means, set overload (but not on startup, right when configure) and remov= e after timeout expired
=C2=A0

if we allow to configure maximum nonce d=
elta we should probably also allow to configure lifetime delta (both are pr=
etty dangerous since they can break convergence real fast) 

+--rw ho=
ldtime?

should be global-holdtime 
Sandy> OK.

ack

= 

since that can be ultimately per link 

same for tide generation 

          +--rw tie-security-key-id=
?         uint32

that should refer to standard yang secur=
ity chain as e.g. OSPF uses 

that should be also originat=
ing-key-id or something since we need (missing right now) the keys we are w=
illing to accept as RW 
Sandy> May I change this to the following, it=
's borrowed from OSPF YANG model. The crypto-algorithm has many types, =
does RIFT support all of them, or just some of them?
          |     |  =
   |     |  +--rw tie-security
          |     |     |     |     +--:(auth-key-chain)?
          |     |     |     |     |  +--rw key-chain?
          |     |     |     |     |         key-chain:key-chain-ref
          |     |     |     |     +--:(auth-key-explicit)
          |     |     |     |        +--rw key-id?     uint32
          |     |     |     |        +--rw key?        string
          |     |     |     |        +--rw crypto-algorithm?
          |     |     |     |                identityref
on a=
cceptance we need following variants

SecurityChecking {
   NoChec=
king,
   Permissive,
   Loose,
   Strict,
}
Sandy> OK.
=
ack 
sa=
me @ link level for the security keys
Sandy> OK. Once you confirm the=
 security key format, it will be syned to interface.

ack,= basically keep to whatever OSPF does,. I thought ACee abstracted the whole= keychain IGP stuff and we should use that 


look @ YAML model we u=
se in open source RIFT to see further details 

own pod mi=
ssing 
Sandy> The "pod" is the fourth leaf in node, please =
check it agin, do I make some mistakes?
=

maybe I missed 



@ interface level

what is 

 =
   +--ro hal?    level 

it seems backwards, HAL is of lev=
el type, no? 
Sandy> The "HAL" is "The highest defined=
 level value seen from all valid level offers received." Does the desc=
ription need to be improved?

well, name the node HAL and t= he type is level
=C2=A0
also list of HAL systemid offers is helpful here 
Sandy> =
OK. I can add the systemid which offer the level.

yepp, th= ere maybe multiple
=C2=A0
also on link add instance-name 
Sandy> Do you mean the=
 RIFT instance name? The whole model is following an instance, so the inter=
face is also belong the same instance with the node. Different instances ar=
e distinguished by the node. 
DATABASE

    =
            |  +--ro (type)?                 |  |  +--:(prefix)            =
     |  |  +--:(positive-disaggregation)                 |  |  +--:(negativ=
e-disaggregation)                 |  |  +--:(external)                 |  |=
  +--:(positive-external-disaggregation)                 |  |  +--:(pgp)
oder strictly per thrift model since order has meaning here Sandy> OK. I'll modify the order according to section 8.2.28.1.
=

ack


                |  |  +--ro link-id-pair* [remote-id]      =
           |  |  |  +--ro local-id?    uint32                 |  |  |  +--r=
o remote-id    uint32                 |  |  |  +--ro if-index?    uint32  <=
span>Rijsman, et al.          Expires August 26, 2021                [Page =
7] Internet-Draft               RIFT YANG Model               =
February 2021                  |  |  |  +--ro if-name?     if:interf=
ace-ref                 |  |  +--ro cost?                         uint32   =
              |  |  +--ro bandwidth?                    uint32             =
    |  |  +--ro flood-reduction?              boolean                 |  | =
 +--ro received-link-capabilities                 |  |     +--ro bfd?      =
               boolean                 |  |     +--ro v4-forwarding-capable=
?   boolean

missing 

=C2=A0 =C2=A014: optional set=
<common.AddressFamilyType> 
=C2=A0 =C2=A0 =C2=A0 =C2=A0(python.imm=
utable =3D "") =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0address_families;
Sandy> OK. I'll add it.

<= div>ack


--000000000000fbdcdd05bda486b0--