[Teas] binary wasRe: Yangdoctors early review of draft-ietf-teas-yang-te-types-03 (was -01)

tom petch <ietfa@btconnect.com> Thu, 24 January 2019 16:17 UTC

Return-Path: <ietfa@btconnect.com>
X-Original-To: teas@ietfa.amsl.com
Delivered-To: teas@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 67102130EEB; Thu, 24 Jan 2019 08:17:35 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.106
X-Spam-Level:
X-Spam-Status: No, score=0.106 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.142, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RATWARE_MS_HASH=2.148, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=btconnect.onmicrosoft.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 GgcKmb5dfyFk; Thu, 24 Jan 2019 08:17:32 -0800 (PST)
Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70138.outbound.protection.outlook.com [40.107.7.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 70817130EC2; Thu, 24 Jan 2019 08:17:31 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=btconnect.onmicrosoft.com; s=selector1-btconnect-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=8aRHHG8ewkkHzGF0Pvy8Bhc2vIpssAH4g8UPoj+H6VU=; b=QPL51c8qw07u1dqV3QaZOKM4Ay3F/93FR8tei4nu40lKv0DSB3J8jd03hU6Ceu7L6kT3IpCRMpiaxFc1AQXGGFI0OZckF+zW4e17Tki2uL94Ij9dFnQz3soImj8jGNGIRbBGC94S+tIYwIfJYSS5HA8HK0+KDgn6iYWQ1yb7IDw=
Received: from DB7PR07MB5210.eurprd07.prod.outlook.com (20.178.43.92) by DB7PR07MB4571.eurprd07.prod.outlook.com (52.135.141.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1558.11; Thu, 24 Jan 2019 16:17:28 +0000
Received: from DB7PR07MB5210.eurprd07.prod.outlook.com ([fe80::c9a4:134f:d375:61ce]) by DB7PR07MB5210.eurprd07.prod.outlook.com ([fe80::c9a4:134f:d375:61ce%2]) with mapi id 15.20.1558.016; Thu, 24 Jan 2019 16:17:28 +0000
From: tom petch <ietfa@btconnect.com>
To: Jan Lindblad <janl@tail-f.com>, "Tarek Saad (tsaad)" <tsaad@cisco.com>
CC: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "draft-ietf-teas-yang-te-types.all@ietf.org" <draft-ietf-teas-yang-te-types.all@ietf.org>, "teas@ietf.org" <teas@ietf.org>
Thread-Topic: binary wasRe: [Teas] Yangdoctors early review of draft-ietf-teas-yang-te-types-03 (was -01)
Thread-Index: AQHUtABMeIciEDVhd0m0wRKA4uUqmg==
Date: Thu, 24 Jan 2019 16:17:28 +0000
Message-ID: <016101d4b400$2a781720$4001a8c0@gateway.2wire.net>
References: <154090780735.15255.3911131220920609603@ietfa.amsl.com> <973699DE-882E-4531-A7D5-32AFEF4359E7@cisco.com> <6CC3CA10-0768-4C99-9237-30A78E1EC3DA@tail-f.com> <BB36593B-0A4E-4F88-A088-3C35BBCAB902@cisco.com> <39E705F8-EE93-4F16-AD3A-39B2E6FCC37E@tail-f.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-clientproxiedby: LO2P265CA0306.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a5::30) To DB7PR07MB5210.eurprd07.prod.outlook.com (2603:10a6:10:6d::28)
authentication-results: spf=none (sender IP is ) smtp.mailfrom=ietfa@btconnect.com;
x-ms-exchange-messagesentrepresentingtype: 1
x-mailer: Microsoft Outlook Express 6.00.2800.1106
x-originating-ip: [86.139.215.184]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; DB7PR07MB4571; 6:B87wzZw9ISuXnA/NGhX3ynv8B/bBRjmbC4lh9Wso+1VVsYWAQbqnE9bJ1ldNQb0Mtsy9FDo4fcaOghNmVMXcCuadc/a3sP8N50Fi+E2ZQvnC0xl2LZBKcnpqKyfAB4fUC08r6qwvICXJ1ZKnQSXvfe0zl+WwH5KZJOwRXONbHy/8ViYPQN7TXrrcE+2tcpMpT1fmeixkaJrdxbeF/q02dYyTT3fsCGR4V1wZ19G0FymkLZGB56le0eZP9pisq9YCJLt6YcD+qY0Pkcv70Q205gjTiWARvEdwpiqF515Qu0as8PIQzg+GClAMG3XSnV09otX2RTglrWV4Luutlcnjsb2Bp1lV018lIOMRuqsKU2cGSmKt+lM/ekaMQaIIWELLp8ZaSJrepNVcrgPxUq1+S/2zcEU1VNbhZ5xqt1IhPg0iPSumhn3JUVL/fw6bCp/7maRrTY0PZLS7rjerDTm5ag==; 5:RY3ePFWa2try6YDJ0k4u8e5LxvkVflIlRCr8V9feEEAeczBnTsSrtLHeIa8l+akPo6p/bT+GbI2K6xBo9d5qsu2c2iIKcuKUlHZimfi8KN9xDto469A0IY2WFXYIZ4Zz2AKAnhUjScuge6gmcT02fydcLCcF422lf1S7SvPlANFB97O/RV3gLOy//7fZt0ozRZzX8eAXbJWzscCISl5bbw==; 7:QL0wRD4VOK5ZywjgKa4sVESZAVTiIOMVpnS1MyOtiQ7+HleJlQPdtc5i/hGiDT+y9vPX/iBB4VssZEqqFLVtPKI+sfRobfdsg5n+v6GIna0llmUqZa5jOa9vAd+VhQF0V8+jHA74+vNmOCrC2+VnqQ==
x-ms-office365-filtering-correlation-id: 0cba8e7a-4b4e-4cf1-d9ee-08d682176f09
x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600110)(711020)(4605077)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020); SRVR:DB7PR07MB4571;
x-ms-traffictypediagnostic: DB7PR07MB4571:
x-microsoft-antispam-prvs: <DB7PR07MB45715928A3BF7A5D82BAC319A29A0@DB7PR07MB4571.eurprd07.prod.outlook.com>
x-forefront-prvs: 0927AA37C7
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(136003)(396003)(346002)(366004)(39860400002)(376002)(199004)(189003)(13464003)(51914003)(4744004)(81156014)(6436002)(71200400001)(44736005)(6486002)(54906003)(4326008)(110136005)(8936002)(966005)(8676002)(99286004)(97736004)(81166006)(25786009)(256004)(93886005)(2906002)(4720700003)(1556002)(71190400001)(14444005)(86362001)(84392002)(86152003)(446003)(486006)(316002)(44716002)(62236002)(26005)(102836004)(6306002)(186003)(30864003)(76176011)(106356001)(61296003)(14496001)(53546011)(6506007)(52116002)(476003)(68736007)(3846002)(478600001)(6116002)(6512007)(9686003)(66066001)(7736002)(33896004)(305945005)(386003)(53936002)(14454004)(50226002)(81686011)(53946003)(81816011)(105586002)(74416001)(7726001); DIR:OUT; SFP:1102; SCL:1; SRVR:DB7PR07MB4571; H:DB7PR07MB5210.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:0; MX:1;
received-spf: None (protection.outlook.com: btconnect.com does not designate permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: s3hoQR1GMldZ338YymS892BM72+dYz0jkb5x04Z2prKkB2LLKKrwbMSBV44c3uRx+WJ4NpTm1FckLxSK4ndwZF1dO/WBytRyOyJOE1CaADyxoLbL1ZrN/0A374dNd17FraKaw4P9O9X3ffZ+s2BknSJ/1NF2TWgygzK7F6/KXu6Qp3I8zz4XXHJV+Q1mk/+9FxOJM2OK0u3XgBacA/4rLCPPZZRX9hHHmdAa5iLtREXhEC2M9VZXPEE59rnq33PGGCw4aQuohnKRoK5kn9On/DYYip//ybML8Sl/9CtwBJNWyQsT+mGrPUW4Hiv+Q6ZHPkg4ueCJnL0K46Oo+HWHh4K6qv5oYx1c6I5vVJqqsRNClLiZpEs/qt/fSwW3raLLb3RWCOvLrJVpG+QwzQ6Gv43idy/4PVOaUSwPWwWt6A0=
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <D2F36C2DFF4024468145779B4C1485E0@eurprd07.prod.outlook.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-OriginatorOrg: btconnect.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0cba8e7a-4b4e-4cf1-d9ee-08d682176f09
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jan 2019 16:17:27.4226 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: cf8853ed-96e5-465b-9185-806bfe185e30
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR07MB4571
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/Qyqo9UCg6Zylfn4zFW3IaTzOXw4>
Subject: [Teas] binary wasRe: Yangdoctors early review of draft-ietf-teas-yang-te-types-03 (was -01)
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Traffic Engineering Architecture and Signaling working group discussion list <teas.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/teas>, <mailto:teas-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/teas/>
List-Post: <mailto:teas@ietf.org>
List-Help: <mailto:teas-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/teas>, <mailto:teas-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 24 Jan 2019 16:17:35 -0000

Tarek

I echo Jan's comments about the representation of binary.  You cite
RFC7951 which gives the representation in JSON which I do not see as the
normal representation of YANG; rather I expect to see XML, as defined by
RFC7950, so this sort of suggests that you expect to use JSON and not
XML - which I find odd.

And you quote RFC7951 as saying 'big-endian' which is not text I can see
in either RFC7950 or RFC7951, nor does it make sense to me for a binary
string, only for multi-octet objects such as int, longint etc.

Is this for use with such as RFC7579 s2.6.3?  If so, a reference would
be useful under grouping label-restriction-info; you already have one
such for grouping label-set-info

Tom Petch

----- Original Message -----
From: "Jan Lindblad" <janl@tail-f.com>;
To: "Tarek Saad (tsaad)" <tsaad@cisco.com>;
Cc: <yang-doctors@ietf.org>;;
<draft-ietf-teas-yang-te-types.all@ietf.org>;; <ietf@ietf.org>;;
<teas@ietf.org>;
Sent: Monday, January 21, 2019 10:10 AM
Subject: Re: [Teas] Yangdoctors early review of
draft-ietf-teas-yang-te-types-03 (was -01)


Tarek, team,

> Hi Jan
>
> Thanks again for your thorough review and comments. We have uploaded
version -03 of the draft that addresses your comments. Please see Inline
[TS] for the resolution action taken.
> Please let us know if you have any further comments.

Very good to see progress. There are still a few things I think we
should discuss. Look for [janl2] inline below.


> From: Jan Lindblad <janl@tail-f.com <mailto:janl@tail-f.com>>
> Date: Monday, November 5, 2018 at 4:20 AM
> To: Tarek Saad <tsaad@cisco.com <mailto:tsaad@cisco.com>>
> Cc: "yang-doctors@ietf.org <mailto:yang-doctors@ietf.org>"
<yang-doctors@ietf.org <mailto:yang-doctors@ietf.org>>,
"draft-ietf-teas-yang-te-types.all@ietf.org
<mailto:draft-ietf-teas-yang-te-types.all@ietf.org>"
<draft-ietf-teas-yang-te-types.all@ietf.org
<mailto:draft-ietf-teas-yang-te-types.all@ietf.org>>, "ietf@ietf.org
<mailto:ietf@ietf.org>" <ietf@ietf.org <mailto:ietf@ietf.org>>,
"teas@ietf.org <mailto:teas@ietf.org>" <teas@ietf.org
<mailto:teas@ietf.org>>
> Subject: Re: Yangdoctors early review of
draft-ietf-teas-yang-te-types-01
> Resent-From: <alias-bounces@ietf.org <mailto:alias-bounces@ietf.org>>
> Resent-To: Tarek Saad <tsaad@cisco.com <mailto:tsaad@cisco.com>>,
<rgandhi@cisco.com <mailto:rgandhi@cisco.com>>,
<xufeng.liu.ietf@gmail.com <mailto:xufeng.liu.ietf@gmail.com>>,
<vbeeram@juniper.net <mailto:vbeeram@juniper.net>>,
<igor.bryskin@huawei.com <mailto:igor.bryskin@huawei.com>>,
<lberger@labn.net <mailto:lberger@labn.net>>, <mhartley.ietf@gmail.com
<mailto:mhartley.ietf@gmail.com>>, <martin.vigoureux@nokia.com
<mailto:martin.vigoureux@nokia.com>>, <db3546@att.com
<mailto:db3546@att.com>>, <aretana.ietf@gmail.com
<mailto:aretana.ietf@gmail.com>>, Lou Berger <lberger@labn.net
<mailto:lberger@labn.net>>
> Resent-Date: Monday, November 5, 2018 at 4:19 AM
>
> Tarek,
>
> See my comments below [janl].
>
>>     General questions:
>>
>>     #1: There are many locations in the YANG talking about an "ERO
subobject index"
>>     (and once RRO index, record route subobject). What is this, and
how is it
>>     supposed to be used? The document is silent on this matter, and I
have seen
>>     modules with problems around numeric index leafs much like this
earlier. Are
>>     these numbers stable, i.e. remains the same forever?
>>
>> [TS]: The list of explicit route objects defines a TE path (e.g.
hops), where the index is used as a key to identify a specific entry in
the list. A lower index implies the TE path traverses this entry
earlier. We will clarify the description and remove mention of
“subobject” in the next update to the document.
>
> [janl] Very good re description changes.
>
> I'm a little worried that numeric indices will end up in the same
quagmire+impasse where ACLs have been the last decades. Isn't there a
risk that people will want to insert entries between two consecutive
indices? Is "renumber" an operation that someone might ask for on these
indices?
> [TS]: The list is ordered by the key (index).. We’ve added a note in
the description that entries with lower index are visited first by the
path. It is understandable that the list may have to be reconfigured if
an entry can not be fit in between two existing ones. Note, a change in
the configured ERO list is generally handled as non-destructive event to
an existing/provisioned tunnel (e.g. handled as make-before-break).
>
>       leaf index {
>         type uint32;
>         description
>           "Route object entry index. A lower index indicates
>            path traverses the hop earlier than the higher index
>            hop(s)";

[janl2] Thanks for the clarification. My strong advice in the situation
you describe would be to change the lists with a uint32 index key to an
ordered-by user list with a string key.

Several decades' experience attempting to automate management of ACLs
keyed by integers speak quite strongly against that solution. The IETF
standard YANG modules for ACLs have now abandoned this approach and
adopted ordered-by user lists with string keys.

Let's say a manager has pushed a path to a set of devices. Now it needs
to configure a re-route around a network problem. This means replacing
one of the configured legs with five legs. Now, if they are keyed by
integers, this may make it necessary to renumber rules to make this
re-route fit. Of course an "experienced" manager may leave some room in
between rules so that this renumbering doesn't happen so often. The
problem is that if it could ever happen, whoever programs the manager
still needs to write (and test) the code for this. Then there are
references to these rules (exclude, include) that also need to be
updated if this happens. More code, more test. And if a human goes in
and makes this change manually on a device, the manager no longer has
any clue as to what happened. All it sees is a bunch of new rules with
new rule names; and a bunch of rules with the same names, many of which
have different content; it has no way of learning the intent. What has
been added, changed and removed? When nothing is stable, reconciliation
of a change becomes pretty much impossible. ACL management is
notoriously inefficient today, largely due to this seemingly small (and
completely unnecessary) problem with the management approach.

The fix is simple. Make sure rule identities do not also double as
sequencing information. If identifiers are also used to convey important
configuration information, so that existing identifiers must change
whenever the intent is adjusted, that leads to an expensive solution. In
YANG, there is a mechanism that is made for this situation. That's
order-by user lists. In such lists, the order of entries is controlled
by the user (operator, programmer) without relying on the key value(s)
to determine that order. The user can insert any number of rules before
or after any other, or first or last in the list. The user can move
entries around without changing their names.

This makes the device interface significantly easier to use for both
operators and programmers. While integer rule names are still possible,
the string type is better as some folks might falsely believe the an
integer key would be used as a sorting metric.

To fix this in the model, the lists currently keyed by leaf index would
look something like this instead:

  list example-route-state {
    ordered-by user;
    key name;
    uses record-route_state;
    ...
  }

Where the record-route_state looks like this:

  grouping record-route_state {
    description
      "The record route grouping";
    leaf name {
      type string;
      description
        "Record route hop name. The name is used to
         identify an entry in the list. Records listed
         earlier in the list means the path traverses it earlier";
    }

I'm not sure I understand the author's intent perfectly, but my
impression is that some of the lists keyed by index are meant to refer
to route-objects in other lists. Like list route-object-include-object
entries would only make sense when they point to (use the same index as)
an existing route-object. Is that so? Any such reference should be
modeled using the leafref type, and a path pointer to the list that
contains relevant entries. E.g. like this:

    list route-object-include-object {
      key name;
      description
        "List of explicit route objects to be included
         in path computation";
      leaf name {
        type leafref {
          path "/path/to/the/rule/name"; // fill in which list we're
pointing to here
        }
        description
          "Route object entry name. Points out a route object
           in list xxxx that will be included in the path.";
      }

Let me know if this makes sense. Happy to discuss and/or explain
further.

>>     #2: There are few leafs (5) with default values given, and none
with mandatory.
>>     Probably needs to increase before we get to last call.
>> [TS]: In general, the team tried to avoid setting defaults unless it
is strictly dictated by another RFC/standard. The team will review again
to see if any was missed and will take action.
>
> [janl] That's all fine. Just remember to describe what happens when a
leaf has no value every time there is no default or mandatory. Easy to
forget, experience shows. Missing this causes non-interoperability in
the standard.
> [TS]: the team has opted to assign defaults to all optional leafs in
the latest revision of the draft.

[janl2] Very good.

>>     #3: There are several choices in the module that are meant to be
augmented with
>>     additional cases. In many instances, this is explicitly spelled
out, very good.
>>     If this is meant to happen in all choices, it would be nice to
point this out
>>     in every instance. Also, if there are any specific assumptions or
>>     considerations to keep in mind when augmenting in a new
technology, please note
>>     that in the description as well.
>> [TS]: yes, we will update as necessary.
>
> [janl] Great!
>
>>     Issues and nits:
>>
>>     #4: Unclear data type
>>     419:
>>       typedef admin-group {
>>         type binary {
>>           length 4;
>>
>>     What is the format of this binary? If this is always a 4-byte
binary, wouldn't
>>     a numeric type be more user friendly, e.g. uint32?
>> [TS]: the standard defines admin group as 4-octet binary (9 -
Administrative group (4 octets) ), and for extended-admin-group as
series of those. We are also defining a union to encompass both as
admin-groups below. Do you still have concerns on this?
>>
>>   typedef admin-groups {
>>     type union {
>>       type admin-group;
>>       type extended-admin-group;
>>     }
>>     description "TE administrative group derived type";
>>   }
>
> [janl] I'm have two concerns. The first is that I don't know what the
format of the type is. How do I know how to construct one of these
values, or interpret an existing one? Is this a randomly chosen value?
The other concern is how an operator would enter this value. The binary
type is not the most user friendly of types. A numeric representation in
the management interface might make better sense.
> [TS]: RFC3630 and RFC5305 defines admin-groups (aka affinity colors)
as binary flags that are user specified (e.g. bit-position X is for RED,
and bit-position Y is for GREEN). So, binary type would be closest to
this representation. On other hand, RFC7591 states that a binary value
is represented as a JSON string -- base64 encoding
(https://tools.ietf.org/html/rfc7951#section-6.6
<https://tools.ietf.org/html/rfc7951#section-6.6>). Let us know if there
are still concerns on this.

[janl2] The YANG binary type is mostly used for large binary chunks that
an operator would never type, e.g. a 2048-bit RSA key or a new software
image. As I understand, the admin-group type is used for something an
operator might configure or read. If so, I would suggest changing this
type. If this is a 4-octet value, an uint32 would work nicely, would it
not? "2149847551" Or a dotted-quad? "128.36.17.255" Or maybe you could
invent a more human friendly interface, e.g. a string of zeros and ones
with dashes at strategic positions?
"10000000-00100100-00010001-11111111"

If you leave this as type binary, at least don't expect operators to be
able to enter these values with reasonable effort. The binary type is
not meant for this.

I noticed you also modeled the leaf as-number as type binary. This is
probably not a good idea. Suggest using uint16 or uint32.

>>     #5: identifier in container with optional leafs
>>     1496:
>>       grouping te-topology-identifier {
>>
>>     The name suggests this is used as an identifier, but all the
leafs are
>>     optional. This is not typical. They are also in a container,
precluding them
>>     from being used as list keys. Is that as intended?
>> [TS]: this container is not meant to be reused for a list, rather
contains ID(s) that together can be used to lookup a specific topology
that the TE tunnel is using. We can clarify this in the description
>
> [janl] Hmm, ok. Still not sure what happens if you omit some value(s),
but I guess I should wait to see your clarification.
> [TS]: we have addressed this by specifying defaults for the optional
leafs.

[janl2] Nice.

>>     #6: Optional -id leafs again
>>     1700:
>>               leaf node-id {
>>               leaf link-tp-id {
>>     1768:
>>             leaf address {
>>     1783:
>>             leaf node-id {
>>             leaf link-tp-id {
>>
>>     Leafs that appear to be used as identifiers are optional
>> [TS]: such IDs identify a specific node, link, or address in the
network but in our applications we do not to use them as keys in such
lists. For example list explicit-route-objects defines a separate index
leaf to be used as key -- since, a path may contain the same node-id or
address repeated multiple times (looping path)..
>
> [janl] So what happens if you omit specifying some of them?
> [TS]: This was reworked and added a mandatory to leafs that are must,
and added defaults to optional leafs.

[janl2] ok.

>>     #7: binary length in bits?
>>     1731:
>>                leaf as-number {
>>                 type binary {
>>                   length 16;
>>     1773:
>>             leaf ip-flags {
>>               type binary {
>>                 length 8;
>>     1805:
>>               leaf label-flags {
>>                 type binary {
>>                   length 8;
>>
>>     It appears to me the modeler might have thought the length is
given in bits.
>>     The value of length is in bytes, however.
>> [TS]: good catch, thanks. We'll correct it to indicate number of
octets as RFC6020 defines.
>>
>>     #8: Must expression copy paste
>>     1852:
>>         container label-end {
>>           must "not(../label-end/te-label/direction) or "
>>             + "not(te-label/direction) "
>>             + "or ../label-end/te-label/direction =
te-label/direction" {
>>
>>     This must expression appears to have been copied from
label-start. In any case,
>>     it always evaluates to true and has no effect.
>> [TS]: thanks, we can see the bug in the expression as is, and this
will be fixed/updated.
>>
>>         container label-end {
>>           must "not(../label-start/te-label/direction) or "
>>             + "not(te-label/direction) "
>>             + "or ../label-start/te-label/direction =
te-label/direction" {
>
> [janl] This revised must statement is superfluous. The must statement
on container label-start already covers this case.
> [TS]: addressed

[janl2] This may not work as intended anymore. The must expression is
fine in itself, but now the direction leaf has a default (forward),
which means it will always exist with one value or another. The case
that the   not(.... /direction)   expressions test for will therefore
never happen. Only the last part (direction = direction) will be
relevant, and I'm not sure that's what you wanted.

>>     #9: Unclear bit field
>>     1885:
>>         leaf range-bitmap {
>>           type binary;
>>           description
>>             "When there are gaps between label-start and label-end,
>>              this attribute is used to specify the positions
>>              of the used labels.";
>>         }
>>
>>     Need more information on how to interpret this leaf. Which bits
map to what,
>>     and what does the bit field values 0 and 1 indicate?
>> [TS]: ]: Each bit-position in the bitmap will map to an offset from
the label-start. For example, if label-start=16000 and bitmap=0x11, then
labels={16000, 16004} are relevant. We will clarify this in the next
update.
>
> [janl] Very good. Since you are using type binary and refer to the
contents using integers, you should also specify how you map those
integer values to bit positions in the binary. E.g. big-endian?
> [TS]: We have clarified this in the description of the leaf that it is
big-endian representation

[janl2] Good that you added a description, but it doesn't make me much
wiser, I'm afraid. Even if there is a reference to RFC7951 (which is
only applicable to RESTCONF, btw), the new description doesn't describe
what the values mean (even when encoded according to 7951s6.6).

Try to describe the algorithm I, as operator or network automation
programmer, should use to figure out what values to use here. The
encoding is also important, but if this is something operators should
enter, the binary type isn't what you want. If you pick a different
type, the encoding will probably fall out automatically.

Best Regards,
/jan


>
> Regards,
> Tarek
>>     #10: Canonical representation
>>     67:
>>       typedef te-bandwidth {
>>
>>     The type is based on a string with a pattern allowing hex
characters and an
>>     upper or lowercase P. Since the pattern allows multiple
representations of the
>>     same underlaying value (0x1p10 presumably means the same as
0x1p0xa and
>>     0x1P0XA) the question comes up if there is a canonical
representation of this
>>     value, e.g. using all lowercase and all hex, or if the string
must be
>>     remembered exactly as given by the client. The description could
answer this
>>     question.
>> [TS]: We will update the description to indicate more strict
canonical form (all upper case).
>>
>>     #11: Mix of upper and lowercase
>>     The module specifies many enumeration and identity values. Some
are all
>>     lowercase. Some are all uppercase. The principle of least
astonishment suggests
>>     to pick one and stick with it. YANG recommendations suggest to
use all
>>     lowercase when in doubt.
>>
>>       typedef te-link-direction {
>>       typedef te-label-direction {
>>       typedef te-hop-type {
>>       identity LSP_METRIC_TYPE {
>>       identity LSP_METRIC_RELATIVE {
>>       identity LSP_METRIC_ABSOLUTE {
>>       identity LSP_METRIC_INHERITED {
>> [TS]: we will update to all lower-case to align with YANG
recommendations.
>
> [janl] Great!
>
> /jan




------------------------------------------------------------------------
--------


> _______________________________________________
> Teas mailing list
> Teas@ietf.org
> https://www.ietf.org/mailman/listinfo/teas
>