Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-yang-05 capitals
Daniele Ceccarelli <daniele.ceccarelli@ericsson.com> Tue, 12 June 2018 15:54 UTC
Return-Path: <daniele.ceccarelli@ericsson.com>
X-Original-To: ccamp@ietfa.amsl.com
Delivered-To: ccamp@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id EFD38130E80 for <ccamp@ietfa.amsl.com>; Tue, 12 Jun 2018 08:54:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.31
X-Spam-Level:
X-Spam-Status: No, score=-4.31 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_DKIMWL_WL_HIGH=-0.01, URIBL_BLOCKED=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=ericsson.com header.b=fAq68af/; dkim=pass (1024-bit key) header.d=ericsson.com header.b=OxfoPyWK
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 LbngHqqJGqQU for <ccamp@ietfa.amsl.com>; Tue, 12 Jun 2018 08:54:50 -0700 (PDT)
Received: from sessmg22.ericsson.net (sessmg22.ericsson.net [193.180.251.58]) (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 8D29D130E62 for <ccamp@ietf.org>; Tue, 12 Jun 2018 08:54:42 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1528818879; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=an7dWhDAJC2ajvjWs7rO7WPRcKFuaIiad2becX82fCE=; b=fAq68af/ChI2P1+zNuyw9S+I3yPhJHg44X8uG2GZTJO8K/lVAJepWX3/wuXkfRRY k5OFVttS6mkO8TGlNYlmf3NkO31ivgCEzK6gn3N8YIp+4GwEErXELkiW7KTSEGGm 8StHP3E8icrWApV/P9dVaxE9tp1Yad64JWyMmsOzJf4=;
X-AuditID: c1b4fb3a-323229c000005fee-85-5b1fecbfa06b
Received: from ESESSHC002.ericsson.se (Unknown_Domain [153.88.183.24]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id 40.6D.24558.FBCEF1B5; Tue, 12 Jun 2018 17:54:39 +0200 (CEST)
Received: from ESESBMR506.ericsson.se (153.88.183.202) by ESESSHC002.ericsson.se (153.88.183.24) with Microsoft SMTP Server (TLS) id 14.3.382.0; Tue, 12 Jun 2018 17:54:30 +0200
Received: from ESESBMB505.ericsson.se (153.88.183.172) by ESESBMR506.ericsson.se (153.88.183.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Tue, 12 Jun 2018 17:54:30 +0200
Received: from EUR01-VE1-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB505.ericsson.se (153.88.183.172) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Tue, 12 Jun 2018 17:54:30 +0200
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=an7dWhDAJC2ajvjWs7rO7WPRcKFuaIiad2becX82fCE=; b=OxfoPyWK9z4Os02A6tShS19AXyB4B3c8AfjH0h2PZ1P/K1/VWfGdHG1hzao4Fid3d733k3avXBTjDrI848Ke3lgBzvGOeCcvUXJTK8nH06XSvngPrutloEERpzwUz+HBtRaNeAxSWE7NbjUP0YX0EJBqli2OmVt3w/beQBVi7yU=
Received: from HE1PR07MB1675.eurprd07.prod.outlook.com (10.166.124.141) by HE1PR07MB4410.eurprd07.prod.outlook.com (20.176.167.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.863.6; Tue, 12 Jun 2018 15:54:25 +0000
Received: from HE1PR07MB1675.eurprd07.prod.outlook.com ([fe80::6041:9382:35f0:ab5d]) by HE1PR07MB1675.eurprd07.prod.outlook.com ([fe80::6041:9382:35f0:ab5d%3]) with mapi id 15.20.0863.010; Tue, 12 Jun 2018 15:54:24 +0000
From: Daniele Ceccarelli <daniele.ceccarelli@ericsson.com>
To: "t.petch" <ietfc@btconnect.com>, Jonas Ahlberg <jonas.ahlberg@ericsson.com>, Jan Lindblad <janl@tail-f.com>
CC: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "ccamp@ietf.org" <ccamp@ietf.org>, "Yemin (Amy)" <amy.yemin@huawei.com>, "draft-ietf-ccamp-mw-yang.all@ietf.org" <draft-ietf-ccamp-mw-yang.all@ietf.org>, ietf <ietf@ietf.org>
Thread-Topic: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-yang-05 capitals
Thread-Index: AQHUAkTsbtt/9+gds0WlGEtcHPDtJqRcxopw
Date: Tue, 12 Jun 2018 15:54:24 +0000
Message-ID: <HE1PR07MB1675CD879967FBEF488E279AF07F0@HE1PR07MB1675.eurprd07.prod.outlook.com>
References: <152836670897.30871.16818219844116536782@ietfa.amsl.com> <9C5FD3EFA72E1740A3D41BADDE0B461FCF9DE258@dggemm508-mbx.china.huawei.com> <HE1PR0701MB2332F59B6B96EDFC974C823189780@HE1PR0701MB2332.eurprd07.prod.outlook.com> <B986ADA5-4131-43FA-A21B-0C789382EF97@tail-f.com> <HE1PR0701MB2332B8DECF4116EFACA55D9F897F0@HE1PR0701MB2332.eurprd07.prod.outlook.com> <040701d40244$92fa5ce0$4001a8c0@gateway.2wire.net>
In-Reply-To: <040701d40244$92fa5ce0$4001a8c0@gateway.2wire.net>
Accept-Language: it-IT, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is ) smtp.mailfrom=daniele.ceccarelli@ericsson.com;
x-originating-ip: [198.24.6.136]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; HE1PR07MB4410; 7:eZUQ40zrVsPAiqB4RsAJpJTHgEhhitynA1YSoCWaeqyMV7PpcGWXWOHacp50HOkkRcJ4B3Tx8j0A0Q+y0nHHqIXBXX4RMoOcFAr78ct1+xA/RUwFMpY3Ei4v6GvjM2J/hXpG2HvjDd08Lrnd+JcQa12Y+UqNA6HxFAj0VP0+opFWbqfBq/bFbziy1XMKCaRKjv/8vq7KJ7fXDBSaSD6dlhPLb4dsuLtPkepc1imrJM5PltLHhIE26u/YCQoOCfNq
x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR;
x-forefront-antispam-report: SFV:SKI; SCL:-1; SFV:NSPM; SFS:(10009020)(346002)(39380400002)(39860400002)(366004)(396003)(376002)(13464003)(76094002)(40764003)(199004)(189003)(51444003)(99286004)(53546011)(102836004)(44832011)(53946003)(14454004)(55016002)(345774005)(2906002)(76176011)(26005)(59450400001)(8666007)(186003)(3660700001)(97736004)(6116002)(6436002)(6506007)(9686003)(3280700002)(86362001)(3846002)(105586002)(4326008)(7736002)(81166006)(305945005)(74316002)(478600001)(81156014)(8676002)(5250100002)(33656002)(66066001)(5660300001)(53936002)(7696005)(446003)(2900100001)(11346002)(296002)(316002)(93886005)(6246003)(229853002)(106356001)(25786009)(486006)(68736007)(476003)(561944003)(110136005)(551934003)(8936002)(54906003)(579004)(559001); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR07MB4410; H:HE1PR07MB1675.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1;
x-microsoft-antispam: UriScan:(219612443155931); BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:HE1PR07MB4410;
x-ms-traffictypediagnostic: HE1PR07MB4410:
x-microsoft-antispam-prvs: <HE1PR07MB441007772C769C5EE0BB2151F07F0@HE1PR07MB4410.eurprd07.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(37575265505322)(178726229863574)(278428928389397)(50582790962513)(219612443155931)(788757137089)(17755550239193);
x-ms-exchange-senderadcheck: 1
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3231254)(944501410)(52105095)(10201501046)(93006095)(93001095)(3002001)(149027)(150027)(6041310)(20161123562045)(20161123564045)(201703131423095)(201703031522075)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123560045)(6072148)(201708071742011)(7699016); SRVR:HE1PR07MB4410; BCL:0; PCL:0; RULEID:; SRVR:HE1PR07MB4410;
x-forefront-prvs: 07013D7479
received-spf: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts)
x-microsoft-antispam-message-info: oYX+u85IP2KEUVuJg659EYV0wMHuA4MGrtGduMx34qdafN/0Ro1ebTZAmyF07OxLz96nf2oTr8W8WGd9wFCmWjuKWyr/fWdUr/psqwS4zHBzGXdRk7uwsL3spPB0w+H0TIYzA1VmFQUBmT/0GjghCN3k8t9JSt4aAd2EzyLdmwfqn3v0Dpb/CPvH8IkcimGs
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-MS-Office365-Filtering-Correlation-Id: 442c1614-cb5e-4b09-0936-08d5d07cc55b
X-MS-Exchange-CrossTenant-Network-Message-Id: 442c1614-cb5e-4b09-0936-08d5d07cc55b
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jun 2018 15:54:24.7081 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 92e84ceb-fbfd-47ab-be52-080c6b87953f
X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR07MB4410
X-OriginatorOrg: ericsson.com
X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTYRjG+3a+bcfl8rQUXwxRFwkZaYp/HCRMk8AixC6QzChXHi+p03ZM 0iBspuYFEa+58kJbeIk0dZKZZN4QhSzS1NSwdLbUVEyQnLfmzoL++z3f87zf+74fH0lI6vkO ZLQikVEq5LFSgQiXhbyCYx2/nEKPV309Sjc/fCmg9U/GMD041MyjfzRWYnpkegrTO+o8ROe1 vUN+wsC23j/CwAc9i/xArXadF9ho1OBgLBOdCGdio5MYpYdvmCjqy8csnDBSx7tTvNGOU1Gq hpeNrEigvGFrtlaYjUSkhOpB0FuzjDihQ1CxMmxx1hAYMyYsjpYHj/SrZoGpfAImdgox5xTy YCZTY6mZRpBZrjMJkhRQPqDvOrfb0ZaKh6XNRv4uE9QcghX95V0+QIXC41GdgMtcgQVdGcGx F+xUZ6FdxtRhGCuaNk8uNmXe980IuF7VBKh+q8zFVtRJSB+cMDOiHCH/zVPENbOHcX2lZW0K tO0fCI7tYG5mm8/lr0NDeqsl4wyG2RnMsSN8qswxrwyUjgfLGXoBZ3hCX20HwRlrAhjNGMCc 6EOwWDxkucoNVCOTfI5joGXrpyX0GMF2SbcgH3mp/xtRbXoygjoCDW0e3LELFOV8F6rNa++H /jI9rkK4DtmxDMvGRXp5uTPK6BssG69wVzCJTcj0izp1Gz6tqNPg34UoEkmtxWGfnUIlfHkS mxzXhYAkpLbi8y2mI3G4PDmFUcZfU96OZdgudJDEUnvxqQhaJqEi5YlMDMMkMMp/Lo+0ckhF VofSLjm75M4HhBr2uNTU958xivbeH4hggjwKNAmbNONsqBhfsnlxWhoU9m1fcEh8aUrVakfI WMmwon6+rGBWFTB5r2K9vH/0WURUU/pNm2j1Wadyma2Lf7ZnzYK13duLMaWyWw6+r12ft1zw roCrGpWfsW1Kkgt3XdMya7tZVylmo+SeboSSlf8FHRKaz0EDAAA=
Archived-At: <https://mailarchive.ietf.org/arch/msg/ccamp/Qw-z0i8jTlMnDa4FXdYns3d9Qpk>
Subject: Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw-yang-05 capitals
X-BeenThere: ccamp@ietf.org
X-Mailman-Version: 2.1.26
Precedence: list
List-Id: Discussion list for the CCAMP working group <ccamp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ccamp>, <mailto:ccamp-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ccamp/>
List-Post: <mailto:ccamp@ietf.org>
List-Help: <mailto:ccamp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ccamp>, <mailto:ccamp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 12 Jun 2018 15:54:53 -0000
Agree with Tom here, if it is not a big issue I'd prefer to keep them in capital letter as they show up in capital letters in all the draft/RFC and IANA registries. BR Daniele > -----Original Message----- > From: t.petch <ietfc@btconnect.com> > Sent: martedì 12 giugno 2018 06:56 > To: Jonas Ahlberg <jonas.ahlberg@ericsson.com>; Jan Lindblad <janl@tail- > f.com> > Cc: yang-doctors@ietf.org; ccamp@ietf.org; Yemin (Amy) > <amy.yemin@huawei.com>; draft-ietf-ccamp-mw-yang.all@ietf.org; ietf > <ietf@ietf.org> > Subject: Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-mw- > yang-05 capitals > > Top posting and tweaking the subject line slightly since I am picking up one > minor point only > > > There are a couple of identities with capitals. Consider changing to > all lowercase; that is the YANG convention. > > > > identity E1 { > > identity STM-1 { > > [Amy] will fix it. > > [Jonas] OK. > > Disagree! If I see e1 I will probably think exponent - if I see E1, I know exactly > what is intended, in a wide range of contexts. YANG guidelines allows > "Upper-case characters, the period character, > and the underscore character MAY be used if the identifier represents > a well-known value that uses these characters. " > and I think that E1 and STM-1 fall within that MAY. > > I see > identity client-signal-OC3_STM1 > in > draft-ietf-ccamp-otn-tunnel-model-01 > and > identity STM-1 { > in > draft-fioccola-ccamp-l1csm-yang-01 > > so I think that CCAMP needs to decide between STM-1 and STM1 - the > former works for me - and then tell the YANG doctors that that is how it > is:-) YMMV > > Tom Petch > > ----- Original Message ----- > From: "Jonas Ahlberg" <jonas.ahlberg@ericsson.com> > To: "Jan Lindblad" <janl@tail-f.com> > > > > Hi Jan & Amy, > > See comments & questions in green [Jonas2]. > > Regards > > JonasA > > > > From: Jan Lindblad <janl@tail-f.com> > > Sent: den 11 juni 2018 15:29 > > > > > Jonas, > > > > Thank you for your review and valuable comments. > > I’ve not responded to the latest mail since I have added comments to > some of the items removed in that mail. > > I’ve copied and pasted your comments into this mail instead, in order > to make the thread of comments complete. > > > > I’ve added my comments in green [Jonas]. > > > > [janl2] Very good. Sorry for deleting text prematurely ;-) > > > > I would like to ask CCAMP for support with the question about if the > protection group is likely to be reused elsewhere or if it can be merged with > the microwave module again. > > > > Regards > > JonasA > > > > From: Yemin (Amy) > <amy.yemin@huawei.com<mailto:amy.yemin@huawei.com>> > > Sent: den 8 juni 2018 11:37 > > > > Hi Jan, > > > > Thanks very much for the comments. I found them are quiet useful to > improve the models. > > Please see my reply below in blue. > > The draft co-authors may add more reply to your comments. > > > > BR, > > Amy > > -----Original Message----- > > From: CCAMP [mailto:ccamp-bounces@ietf.org] On Behalf Of Jan Lindblad > > Sent: Thursday, June 07, 2018 6:18 PM > > > > Reviewer: Jan Lindblad > > Review result: Ready with Issues > > > > YD review of draft-ietf-ccamp-mw-yang > > > > I have now reviewed the YANG modules and corresponding examples of > the -05 version of this draft. I have not read much of the RFC text, so I can't > vouch for how well the text aligns with the model. I find the proposed > modules in good shape. Most of my comments below are alternate ways of > modeling something that the wg may consider, or a few things where I > propose a better option for something that would be acceptable even as it is. > There is a single issue that must be fixed IMO, see #8 below. > > > > Let's start with module ietf-microwave-types. > > > > #1) Consider adding structure to related identities > > > > This module consists mostly of a long list of identities based off of > coding-modulation. If it makes sense that in the future someone might be > interested in doing something with all qam-4096 based identities or all - > strong identities, say, it may make sense to model the identities as based on > each other in a tree style. E.g. > > > > identity qam-4096 { > > base coding-modulation; > > description > > "4096 QAM coding and modulation scheme."; > > } > > > > identity qam-4096-strong { > > base qam-4096; > > description > > "4096 QAM strong coding and modulation scheme."; > > } > > > > identity qam-4096-light { > > base qam-4096; > > description > > "4096 QAM light coding and modulation scheme."; > > } > > > > Or even go to "multiple inheritance" with multiple bases for > identities, e.g. > > for qam-4096 and strong. This would allow future applications to > filter the identities on such criteria. Just a thought. > > [Amy] change to base qam-4096. Multiple bases seem not applicable > here. > > [Jonas] I agree. > > > > #2) Convention to use all lowercase in YANG symbols > > > > There are a couple of identities with capitals. Consider changing to > all lowercase; that is the YANG convention. > > > > identity E1 { > > identity STM-1 { > > [Amy] will fix it. > > [Jonas] OK. > > > > Next, let's look at module ietf-interface-protection. > > > > I can't say I understand exactly why this is a separate module. It > publishes a single grouping, which is required by ietf-microwave-radio-link, > and as far as I understand would probably never be used anywhere else. > When the grouping is used a single time in ietf-microwave-radio-link, it is > immediately refined. > > Would probably reduce the clutter by merging the two modules and > resolving the refine. > > [Amy] They were in one model. During the WG discussion, comment was > raised that the interface protection function could be generic and be used by > other technologies in future, so we split the models. > > I’m open to discuss about this. > > [janl] Ok, I didn't quite see how the grouping would be used anywhere > else, but if it indeed is usable elsewhere, having it in a separate module isn't > a bad idea. > > [Jonas] I’m in favor of reverting back to one single module gain, > since it will simplify the model as you mention above. The argument for > breaking it out into a separate module is that it can be used elsewhere. > I’m doubtful if it ever will be, but here I think we need input from the rest of > the community (at least from the CCAMP WG). > > > > > > #3) Config true leaf name status > > > > I find it counter-intuitive that a leaf called status (or state) is a > configuration item. I had to re-read the model several times to get my head > around the fact that this is indeed meant to be config true. > Perhaps consider a name change? > > > > leaf status { > > [Amy] status should be config false. > > [Jonas] Correct. Our mistake. > > > > #4) Action external-commands > > > > There is a single action called external-commands (even in plural). It > takes a single argument, which is the name of the operation to execute. > No output. To me, a more natural modeling would be to make each of the > external commands an action, over time possibly with different input and > output. > > [Amy] add output to describe the action result (success, fail, > inprogress). But prefer to use one action. > > Change the name to external-command. > > [janl] Hmm, I don't quite understand your preference for a single > action. I see how this choice closes doors, but no real benefit. This is no big > deal, though, what you have works. Just feels less evolvable for no reason. > > [Jonas] I’m not quite sure I’ve understood what you propose. Do you > mean that we should replace the single action statement and the list of > identifiers with several action statements: > > action manual-switch-working; > > action manual-switch-protection; > > … > > action clear; > > The reason for using identities was that we wanted to make it easy to > extend the module with additional, potentially vendor specific, commands by > augmentation. > > But maybe there are other ways to handle that in case of using the > alternative approach you suggest. > > > > [janl2] Yes, exactly. Separate actions like this has the advantage > that they can take (different) input and output parameters, and that they > can be access controlled separately using NACM. This way different > operators could have/not have access to different operations. Future > standard or vendor modules could still augment whatever additional actions > they like to the same location in the tree. > > [Jonas2] OK I understand and I see the advantage of this approach. > Amy, should we consider change to this or are there other benefits with the > original approach? > > > > Finally, we have module ietf-microwave-radio-link. > > > > #5) Use derived-from when comparing identities > > > > It's more future-proof and more likely to be interoperable if you use > proper XPATH functions to determine kinship than using plain equality > > > > augment "/if:interfaces/if:interface" { > > when "if:type = 'mw-types:radio-link-terminal'"; > > > > is better written as > > > > augment "/if:interfaces/if:interface" { > > when "derived-from-or-self(if:type, > 'mw-types:radio-link-terminal')"; > > > > This allows future sub-typing (sub-classing) of radio-link-terminal, > i.e. new identities that are based on radio-link-terminal to reflect some > special kind of RLT. It also improves chances of interoperability. > > [Amy] will adopt this. > > [Jonas] Agree. > > > > #6) Blank id reasonable? > > > > Leafs that function as an id usually do not have defaults. Does a > blank id make sense here? If it does, maybe it would make more sense to > leave it without a default and explain what happens if not set in the > description instead? Or mark it as mandatory if it has to be set. > > > > leaf id { > > type string; > > default ""; > > [Amy] remove default. Add mandatory. > > [Jonas] I think that the name of the attribute could be misleading. It > ’s not a formal id of the terminal, it is a text string that is used by an optional > system feature in a far-end RLT (on the opposite end of the microwave link) > to verify that the correct two RLTs (far-end and > near-end) are connected. > > I don’t think we should make it mandatory. I would suggest to leave it > without default and clarify the consequences in the description. > > > > #7) Use derived-from when comparing identities (again) > > > > leaf-list carrier-terminations { > > type if:interface-ref; > > must "/if:interfaces/if:interface[if:name = current()]" > > + "/if:type = 'mw-types:carrier-termination'" { > > > > is better written as > > > > must "derived-from-or-self(/if:interfaces/if:interface[if:name = > > current()]" > > + "/if:type, 'mw-types:carrier-termination')" { > > > > It is possible to write this in a more compact way, but there's > nothing wrong with the above. > > > > must "derived-from-or-self(deref(current())/.." > > + "/if:type, 'mw-types:carrier-termination')" { [Amy] will > > adopt this. > > [Jonas] Agree. > > > > #8) Badly broken frequency duplex config > > > > If you read the descriptions in these related leafs: > > > > leaf tx-frequency { > > type uint32; > > units "kHz"; > > mandatory true; > > description > > "Selected transmitter frequency."; > > } > > leaf rx-frequency { > > type uint32; > > units "kHz"; > > description > > "Selected receiver frequency. > > Overrides existing value in duplex-distance. > > Calculated from tx-frequency and duplex-distance if > > only duplex-distance is configured. > > Must match duplex-distance if both leaves are > > configured in a single operation."; > > } > > > > leaf duplex-distance { > > type uint32; > > units "kHz"; > > description > > "Distance between Tx & Rx frequencies. > > Used to calculate rx-frequency when > > rx-frequency is not specifically configured. > > Overrides existing value in rx-frequency. > > Calculated from tx-frequency and rx-frequency if only > > rx-frequency is configured. > > Must match rx-frequency if both leaves are configured > > in a single operation."; > > } > > > > It appears that the author intends the system to fill in the value for > one of these leaves based on the value set for the other. This is a big no-no. > A system should never alter its own configuration, or automation flows > (which is the whole purpose with YANG and NETCONF, remember) will > break. Also, the validity of the configuration should not depend on how > many operations are used to inject it. > > > > I find this a serious flaw that must be fixed before the module can be > released. > > > > I propose fixing it like this: > > > > leaf tx-frequency { > > type uint32; > > units "kHz"; > > mandatory true; > > description > > "Selected transmitter frequency."; > > } > > choice freq-or-distance { > > leaf rx-frequency { > > type uint32; > > units "kHz"; > > description > > "Selected receiver frequency." > > } > > leaf duplex-distance { > > type uint32; > > units "kHz"; > > description > > "Distance between Tx & Rx frequencies." > > } > > } > > > > If you would like to have read-only computed values accessible in the > model, maybe you could add: > > > > leaf actual-rx-frequency { > > type uint32; > > units "kHz"; > > description > > "Computed receiver frequency." > > config false; > > } > > leaf actual-duplex-distance { > > type uint32; > > units "kHz"; > > description > > "Computed distance between Tx & Rx frequencies." > > config false; > > } > > > > Many other ways of doing this properly are also possible. Let me know > if you'd like to discuss options. > > [Amy] I think choice is a good way to model those leafs. I suggest to > use it. > > That’s the real value of YANG doctors! Thanks. > > [Jonas] Thanks for your help. We have discussed this a lot without > finding a good solution. I agree that we should use it. > > The only question I have is the name of the attributes. Should the > attribute used for configuration or the attribute showing the resulting be > called rx-frequency? > > config-rx-frequency & rx-frequency v.s. rx-frequency & > actual-rx-frequency > > > > [janl2] I used actual-rx-frequency because you had some other leafs > called actual-... > > > > Yet another possibility could be to model it like this (pseudo YANG): > > > > leaf tx { ... } > > choice freq-or-dist { > > container by-frequency { > > leaf rx { ... } > > leaf dist { config false; } > > } > > container by-distance { > > leaf dist { ... } > > leaf rx { config false; } > > } > > } > > > > I.e. three leaves with the same names, just different paths (since the > container names differ) and different configness. I think my initial proposal is > a little cleaner, tho, which is why I went with that one earlier. Or you could > move the actual-... leaves into the choice, so that only the one that's > meaningful exists. > > > > [Jonas2] I think that your first proposal has a benefit of always > providing access to the actual configuration through the same leafs (and > path) (tx-frequency, actual-rx-frequency, actual-duplex-distance, > independent of how they have been configured. > > One use-case we have discussed is that an operator uses > duplex-distance for the initial configuration of the rx-frequency and then in a > second step fine tuning it by setting the rx-frequency directly. Would that > work with a choice statement, i.e. that you first choose duplex-distance and > then in a subsequent operation overrides that and choose rx-frequency? > > > > #9) Check that lower threshold is less than upper threshold > > > > Would it make sense to add a must statement checking that the lower > threshold is less than (or equal?) to the upper threshold? > > > > leaf atpc-lower-threshold { > > when "../power-mode = 'atpc'"; > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The lower threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > leaf atpc-upper-threshold { > > [Amy] can add the must statement. > > [Jonas] Good! > > > > #10) Choice more convenient > > > > There are a few leafs that act as discriminators for when clauses in > other leafs. Such constructs might be a little smoother when modeled as a > choice instead. I'll take one and show as an example. This power-mode > construct: > > > > leaf power-mode { > > type enumeration { > > enum rtpc { > > description > > "Remote Transmit Power Control (RTPC)."; > > reference "ETSI EN 302 217-1"; > > } > > > > enum atpc { > > description > > "Automatic Transmit Power Control (ATPC)."; > > reference "ETSI EN 302 217-1"; > > } > > } > > mandatory true; > > description > > "A choice of Remote Transmit Power Control (RTPC) > > or Automatic Transmit Power Control (ATPC)."; > > } > > > > leaf maximum-nominal-power { > > type power { > > range "-99..40"; > > } > > units "dBm"; > > mandatory true; > > description > > "Selected output power in RTPC mode and selected > > maximum output power in ATPC mode. Minimum output > > power in ATPC mode is the same as the system > > capability, available-min-output-power."; > > reference "ETSI EN 302 217-1"; > > } > > > > leaf atpc-lower-threshold { > > when "../power-mode = 'atpc'"; > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The lower threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > leaf atpc-upper-threshold { > > when "../power-mode = 'atpc'"; > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The upper threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > > > could be modeled as: > > > > choice power-mode { > > container rtpc { > > description > > "Remote Transmit Power Control (RTPC)."; > > reference "ETSI EN 302 217-1"; > > leaf maximum-nominal-power { > > type power { > > range "-99..40"; > > } > > units "dBm"; > > mandatory true; > > description > > "Selected output power."; > > reference "ETSI EN 302 217-1"; > > } > > } > > container atpc { > > description > > "Automatic Transmit Power Control (ATPC)."; > > reference "ETSI EN 302 217-1"; > > > > leaf maximum-nominal-power { > > type power { > > range "-99..40"; > > } > > units "dBm"; > > mandatory true; > > description > > "Selected maximum output power. Minimum output > > power is the same as the system > > capability, available-min-output-power."; > > reference "ETSI EN 302 217-1"; > > } > > > > leaf atpc-lower-threshold { > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The lower threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > leaf atpc-upper-threshold { > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The upper threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > } > > mandatory true; > > description > > "A choice of Remote Transmit Power Control (RTPC) > > or Automatic Transmit Power Control (ATPC)."; } > > > > [Amy] Choice is a better way. Is it possible to further refine your > proposal? > > Since maximum-nominal-power will be used by both RTPC and ATPC, how > about to move it out of the choice, then use maximum-nominal-power in the > choice? Like this: > > leaf maximum-nominal-power { > > type power { > > range "-99..40"; > > } > > units "dBm"; > > mandatory true; > > description > > "Selected maximum output power. Minimum output > > power is the same as the system > > capability, available-min-output-power."; > > reference "ETSI EN 302 217-1"; > > } > > > > choice power-mode { > > container rtpc { > > description > > "Remote Transmit Power Control (RTPC)."; > > reference "ETSI EN 302 217-1"; > > > > use maximum-nominal-power; > > } > > container atpc { > > description > > "Automatic Transmit Power Control (ATPC)."; > > reference "ETSI EN 302 217-1"; > > > > use maximum-nominal-power; > > > > leaf atpc-lower-threshold { > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The lower threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > leaf atpc-upper-threshold { > > type power { > > range "-99..-30"; > > } > > units "dBm"; > > mandatory true; > > description > > "The upper threshold for the input power at far-end > > used in the ATPC mode."; > > reference "ETSI EN 302 217-1"; > > } > > } > > mandatory true; > > description > > "A choice of Remote Transmit Power Control (RTPC) > > or Automatic Transmit Power Control (ATPC)."; > > } > > > > [janl] Yes, this is certainly very reasonable. I placed the > maximum-nominal-power inside the choice because the description string > was giving the leaf different interpretations in each case. In that case I felt it > better to have separate leaves with clear descriptions. If you feel the leaf can > be shared between the options with a clear definition of its meaning, this is > reasonable. > > [Jonas] One purpose of the mode attribute was to show if the > carrier-termination was configured for RTPC or ATPC, but I guess the same > can be achieved by looking for which container, rtpc or atpc, has been > instantiated. > > > > [janl2] Yes, that's the idea. > > [Jonas2]. Good! > > > > I suggest that we go for Jan’s proposal since the attribute has > different meaning in the two cases. One purpose of the > > > > #11) Choice more convenient (again) > > > > Same thing again for > > > > leaf coding-modulation-mode { > > [Amy] will update it. > > [Jonas] OK, see above. > > > > #12) Unusual exponential notation > > > > Do you really mean 10e-9 (=10*10^-9 =10^-8), or do you mean the more > traditional notation 1e-9? > > > > leaf ber-alarm-threshold { > > type enumeration { > > enum "10e-9" { > > [Amy] should be 1e-9. Need to confirm with the co-authors. > > [Jonas] Yes, we mean 10^-9, 10^-8, …, 10^-1. > > > > [janl2] A perpahs simpler way to model this would be to only keep the > part that contains information, i.e. as > > > > leaf ber-alarm-threshold { > > type int8 { > > range -9..-1; > > } > > default -6; > > description "Specification of at which BER as a power of 10 an alarm > should be raised. 10^-6 is the default."; > > reference "ETSI EN 302 217-1"; > > } > > > > #13) Separate module with grouping, used a single time with refine > > > > Module ietf-interface-protection defines a grouping > (protection-groups), which is used a single time, yet is refined when it is used > below. As noted before comment #3, I find this way of laying out the YANG > unnecessarily hard to read and understand, for no clear benefit. > > > > container radio-link-protection-groups { .... > > uses ifprot:protection-groups { > > > > refine protection-group/members { > > must "/if:interfaces/if:interface[if:name = current()]" > > + "/if:type = 'mw-types:carrier-termination'" { > > > > Also, as noted in comment #7, the must statement is better written > using derived-from-or-self. This applies regardless of the current refine > statement is kept, or if the must statement moves to the actual leaf-list it > applies to. > > [Amy] Will update the must statement. > > [Jonas] If I remember correctly, I think it was done with the ambition > that it should be made generic so that other interface technologies could use > the same protection module. In case, we revert back to one single module I > suggest that we also simply the structure as Jan suggests. > > > > Appendix A.1 & A.2 > > > > Besides the actual YANG modules, there are also a couple of examples > in Appendix A.1 and A.2. I tried to use them and uncovered a couple of > issues. > > > > #14) Config false leaf in config example > > > > Both examples list > > > > "tx-oper-status": on > > > > This is a config false item, which could never be part of a > configuration message (and it also lacks comma at the end). > > [Amy] will remove it. > > [Jonas] OK > > > > #15) Wrong type in example > > > > Both examples list > > > > "coding-modulation-mode": 0, > > > > 0 not a legal value, should be "single" to match the rest of the > example data. > > [Amy] will fix it. > > [Jonas] OK > > > > #16) Missing mandatory parameter > > > > Both examples lacks leaf maximum-nominal-power, which is mandatory > according to the YANG, so the transaction fails to validate. > > [Amy] will add it. > > [Jonas] OK > > > > #17) Examples perhaps a tad basic > > > > The examples are demonstrating only a small part of the module > functionality. A bigger example, e.g. including xpic with interface pointers > might be useful. > > > > Feel free to reach out to me to discuss any of this. Thank you. > > [Amy] will add additional example. > > [Jonas] Good suggestion. Amy, let’s what configuration to show. > > > > > > Best Regards, > > /jan > > > >
- Re: [CCAMP] Yangdoctors last call review of draft… Jonas Ahlberg
- Re: [CCAMP] Yangdoctors last call review of draft… Daniele Ceccarelli
- Re: [CCAMP] Yangdoctors last call review of draft… Daniele Ceccarelli
- Re: [CCAMP] Yangdoctors last call review of draft… t.petch
- Re: [CCAMP] Yangdoctors last call review of draft… Jan Lindblad
- Re: [CCAMP] Yangdoctors last call review of draft… Jonas Ahlberg
- Re: [CCAMP] Yangdoctors last call review of draft… Yemin (Amy)
- [CCAMP] Yangdoctors last call review of draft-iet… Jan Lindblad
- Re: [CCAMP] Yangdoctors last call review of draft… Yemin (Amy)
- Re: [CCAMP] Yangdoctors last call review of draft… Jan Lindblad
- Re: [CCAMP] Yangdoctors last call review of draft… Jan Lindblad
- Re: [CCAMP] Yangdoctors last call review of draft… Jonas Ahlberg