Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-dwdm-if-param-yang-09

tom petch <ietfc@btconnect.com> Fri, 02 June 2023 10:19 UTC

Return-Path: <ietfc@btconnect.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 2AC6EC152F00; Fri, 2 Jun 2023 03:19:43 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -6.898
X-Spam-Level:
X-Spam-Status: No, score=-6.898 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_PASS=-0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham 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 ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id k_nucl0S6ca9; Fri, 2 Jun 2023 03:19:41 -0700 (PDT)
Received: from EUR03-AM7-obe.outbound.protection.outlook.com (mail-am7eur03on20714.outbound.protection.outlook.com [IPv6:2a01:111:f400:7eaf::714]) (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 1A910C15108D; Fri, 2 Jun 2023 03:19:40 -0700 (PDT)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fD7uomWQG8qUaIRijLdXB9RdpNztfwg0HQmzgEZwQk13Sz7PCmtr8DRxOfujvkgMoVsBr8SDdXy9OtM94UD7lmoqlXGqrLfXy9TAwanNngczROK8fNwOY+23XokezGcOvHKGf3VvvQ7IETyPukXtSDiPwCPjPEH8o2ZDKVF+FprVmYqbcaAs02xkEYaRm9t6VFkNDIcORsLiuafQGjB9aJmrfsRjF9G65x3x+OxBVKK4E9vLX/byrV53UbKZ+RIjkxMSqGdSSZ1YgozuSSaEWcywCd5mwjqra0eFs7n4nKjBH03uqi3ATquuegbyVhm2MrQRmULRLlIOV8DFw3KB0w==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=nYsjx4zfvhNnuYydzPcMzgB4b9Rt+aFNA/uzlRUOZxA=; b=B3HZCnFWJ79oy/SZ7ixzZPjVpsjwUXJpVLJ9vSCttdLPegUSIkJqZOe+kUCnW76F2+Sz+Es+oRdpK3D4DZtPcSQJcTVCh1ohaWj+uuVW/qbdKpAEQzl4oP0a32hId5JGNpXep2Rhayr3QHx3QyQzfA37oHFzM3NslpQZabFVVw3yPiYxN0Qz517BdRgn/VanCSbwlofK49qTX3RlIUGFr13aRsTQQ/r7ieQWG9qXNq4mNkAfJ0IvMdrvqYPwxRkcR1uTPFPGowKdt5bady74WDwrB+yVpFKxcNb6ym5cVRHK0fn9HPjOtYaRLPCq4VD9JO4s8M6FVRLIa5ossWjxow==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=btconnect.com; dmarc=pass action=none header.from=btconnect.com; dkim=pass header.d=btconnect.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=btconnect.onmicrosoft.com; s=selector2-btconnect-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nYsjx4zfvhNnuYydzPcMzgB4b9Rt+aFNA/uzlRUOZxA=; b=jKsF31nsny3aBs58NwvskXki1n3WAtx/c9lpJj3YEF6o7dO5lY8YHziAoqm1JAjTc+KeLnugn+awqGFYvanALFFtw5ZqtuXZOq+tiUiYf/yttdwOYvFryhswreRLCvx9Xfvc7LPkfNvHyZYnk0oG048klQUzJgiER+sMZJjbrR8=
Received: from AM7PR07MB6248.eurprd07.prod.outlook.com (2603:10a6:20b:134::11) by DUZPR07MB9815.eurprd07.prod.outlook.com (2603:10a6:10:4d8::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6433.26; Fri, 2 Jun 2023 10:19:36 +0000
Received: from AM7PR07MB6248.eurprd07.prod.outlook.com ([fe80::a928:74cd:caef:f589]) by AM7PR07MB6248.eurprd07.prod.outlook.com ([fe80::a928:74cd:caef:f589%7]) with mapi id 15.20.6455.024; Fri, 2 Jun 2023 10:19:35 +0000
From: tom petch <ietfc@btconnect.com>
To: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, Jan Lindblad <janl@tail-f.com>
CC: "ccamp@ietf.org" <ccamp@ietf.org>, "draft-ietf-ccamp-dwdm-if-param-yang.all@ietf.org" <draft-ietf-ccamp-dwdm-if-param-yang.all@ietf.org>, "last-call@ietf.org" <last-call@ietf.org>
Thread-Topic: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-dwdm-if-param-yang-09
Thread-Index: AQHZj+Q/9mbSx/Ef3EGW2BE4SrB5Ha910b0LgAGE+dU=
Date: Fri, 02 Jun 2023 10:19:35 +0000
Message-ID: <AM7PR07MB62480D13FD5D014914207DB0A04EA@AM7PR07MB6248.eurprd07.prod.outlook.com>
References: <168511383378.23139.10276744261548181432@ietfa.amsl.com> <AM7PR07MB6248513A51D6A351D6552471A049A@AM7PR07MB6248.eurprd07.prod.outlook.com>
In-Reply-To: <AM7PR07MB6248513A51D6A351D6552471A049A@AM7PR07MB6248.eurprd07.prod.outlook.com>
Accept-Language: en-GB, en-US
Content-Language: en-GB
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
msip_labels:
authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=btconnect.com;
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: AM7PR07MB6248:EE_|DUZPR07MB9815:EE_
x-ms-office365-filtering-correlation-id: 51a2a797-89eb-4f9a-3e08-08db6352dd98
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: rDJlho5zxNqqBHrjk9n6QoLHUE0NFhynV6pmWflfNwhzkBqcPHnMHzSndDhv1wdc3upcfQEUmdjmQSEuSW77TFU8KzKcJajlzN3oi+CVPq0du9DswuhiJTl2EubYrrh7DXTZ1ToKQ/KRKyY8WZx6vjOML7ZWVlmWDh0gKqwMPMortXLci4WwRGb9EgGMEy8Vnp8+enOoQfXpMjN+odscyEGPCLmzoV0uLhMRMXBucWfvlVZRF2Uq7KIoqc1M9h9biAZNrD8J1xeiKOCFGYeVrzF6RSmwpoYg1DpiBgsE2O0k4/X1j4h/1/gx5FC54RexNmDMlcXPxce9CHBkI0vPf/qteywX8+CQV07clzQ5At9yoI011cUqlUIbKAyrVUt4qQtbzcbVw/+3o/YTpr2x2ZE53Hgi1dUIy0UPtBhuKpGhuHEutvvBlMa6fLa0qladxaBWn6aWrY6O27PB3RMxjoeP64MlTqXL6Ye7DpME2uRPXhh8NiB9YaUjq3TsvQbVScGUiuPrVgkbpKb+qv8FWXJr7kpJa6+1P22d0tnjFemYqWtvkypwt9Ky9Ku/6c2T1onxUkFWtHAyHYg+uZy06xPvHZ15RMiToZDi01KKnBm/AF9qqmY887+SvbhhQj6t5vx7BJ460GMNVRET/QsjUw==
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM7PR07MB6248.eurprd07.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(396003)(39860400002)(376002)(346002)(136003)(366004)(451199021)(66946007)(55016003)(83380400001)(82960400001)(38070700005)(4326008)(122000001)(41300700001)(38100700002)(316002)(7696005)(6506007)(9686003)(26005)(966005)(71200400001)(66476007)(64756008)(66446008)(33656002)(66556008)(91956017)(76116006)(52536014)(2906002)(8676002)(8936002)(5660300002)(54906003)(110136005)(478600001)(86362001)(186003); DIR:OUT; SFP:1102;
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: aCFSuKy6A2m8v2s0QXY4mTJWSr4BwDIFSbOPpBP7R2JtJeYf6mOIf0f+eW/XZxJlxr6g9yreX4NoiSl+5Ww+N3H9sv87qcmY3Q6XQBeVSMY70JFNhEIC6YzGvrfT/DI4stma/g8J/MJ+kQqAEnBbWMW4Lf6uZvJntxAAMiG3q2U8msBR2YP5CaURB6TpnMOKIvFYSUMG2b+wNt8eKIoOHq9fHQMFEKmFVvw6NDQA3zd8RcJbSxfTusy3FWY6X3X8McosWCZs7VMmDUSC2lFA1KoHjelcr4S7Lb8P1dVrkzLB2yUGL5qxWgPyLQAHnON/NznNYEuSsWM3TjJZPKHQiSD7ao6013RveWTsylCY10iKHf0J2FjvQbwUya7SlOnChKH4P8IriHDlOwLCiDVRw9+8B/VzYdvTcYenQft9E2qTzycJjKirTtYSC5y70aTFRUeinb4wwpIZGB6fa0vPMXx13h5RO8p7/P1M3/w+5NNOpculd7c6bFexG8fxd4Ujqn0TxKsDtswXVrKzAOtPAbKnz8renv/w082I79ujZeyKR7ehsHkm5IZkXlGCGZ9cVQVOhqa3C1Xoe03AGLnW+HbUKktdfsThhXbQ5kL5TIjSKdFlxF9prmbd61CL22O7OkOPVXlJQ+PyRHUG4MaVCv9mojIC40rOLA6CgNDBpmsCCA1TjS8JdPFIwKM1MAwkIMNzxV1njGjdNkpbmtSgIiHu0wTj1iSB7N6svo+BUFkC9LbG3GsQt1FXrxYeVphQyKwnrrypEFfF68+PqspKeHgwkKMZL+QEqKyLtyca7T5rfa8Tw1dHqgr5ihpSrZFPgN6SSfvC+iaE2TEFDZKZT+iNWw38b8eQ3ZxEZX3lhlWJpsK/eO9MJyB+YlZzc/YkDz4hstZMT4mRy3pojMZ7c8CqIIq7CT2/IP2AuVTVe9L2Tl6yyfd50TYkYjZpi88QEFGtTCcJbAC/sKIk8O9KHe+YT+zM5Oi0xOMdQ58Rw+I75th5sg/Jjqw7Q4SAQ3dKsAZmIpwEl9FEIomVC1ZsEvrJnx/2Xi8PME/Hvv+axtwQGnW1OMubJBRoQAbmGOXHpW0gP7VmcfhxDYauyxHnRmxA2aTCDmyh+oPbIOtVquIVKCR8oZ0YGWnVrHrmBDGGt8eEWQdD2R2ENadIj4PwWwZanCrUbu76QLrkEBKoVvfuc9f+rmj05FNwuUhs6zODHANsrSzJx6HEZumU8dRgRJ69DieJEQPBJ0rNgHZpGeneri7rVkHAEnLRm+fRisyiLvQf94iEyWDhStm4K6GbY+NbXC3oYfW0wjJ9+J2x+LdvihFjRijA9mO4eNBRnnwD2UdTpCYC/R9Ru/78fxcDy/lb7PwuHkvFsCMsZl7kr40rceQqRYGAXG7S/4KonJcL2zF8GxGLIejRKfbpqWA30qBxnlbzFaNlMs1uek/6M4OlTdrL+RSHJky78yA1jejtUiagc7cw9rr2R547BJJXvk8OERF+diwVhyw2XWyszOrEfjOE3r8FUoR86mzRnc2Zv+sU3x3NsCAGEv19pCVj2xnkjkx1rm8PcrXvG4MTAPg=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: btconnect.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM7PR07MB6248.eurprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 51a2a797-89eb-4f9a-3e08-08db6352dd98
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Jun 2023 10:19:35.8756 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: cf8853ed-96e5-465b-9185-806bfe185e30
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ReAcCEKBLdjOeOzF2dFmjqcfdhQ7++g/m+I3biwXbUvzCkZ7M+5qCbKr17VU9YIKp+xU4wZsixEKP1KDr9DTXQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DUZPR07MB9815
Archived-At: <https://mailarchive.ietf.org/arch/msg/ccamp/lSpJLDuHPbDuQ45sTgLcdpc2DkE>
Subject: Re: [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-dwdm-if-param-yang-09
X-BeenThere: ccamp@ietf.org
X-Mailman-Version: 2.1.39
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: Fri, 02 Jun 2023 10:19:43 -0000

a follow up in line
________________________________________
From: CCAMP <ccamp-bounces@ietf.org> on behalf of tom petch <ietfc@btconnect.com>
Sent: 01 June 2023 12:22

From: CCAMP <ccamp-bounces@ietf.org> on behalf of Jan Lindblad via Datatracker <noreply@ietf.org>
Sent: 26 May 2023 16:10

Reviewer: Jan Lindblad
Review result: Not Ready

This is my YANG-Doctor Last-Call review of
draft-ietf-ccamp-dwdm-if-param-yang-09. I am afraid this draft is not ready for
last call.

There are severe YANG-issues on every level here. It is quite obvious that the
authoring team has not been in the habit of compiling their YANG modules any
time recently. At first I tried to fix the issues I encountered in order to
provide helpful feedback, but after an hour I stopped. It's both too many
issues and too hard for me to guess what the authors' intent may have been.

<tp>

It is not just the YANG that requires serious work as Jan says, the rest of it needs serious work as well
e.g. lots of unused I-D references
missing I-D references to at least two I-D
missing reference for YANG import
abbreviations need expanding on first use
module not model
s.3 out of date
Security Considerations out of date
contact URL out of date
incorrect format for registering a module

while on the YANG I see the choice of prefix as inappropriate - too long -
and I wonder about the lack of constraint on the augment ie at present every interface, of every kind, will carry all these additions.

<tp2>
on the subject of I-D references, the Normative ones look plausible, it is just that they are not used.  At the same time, the YANG module is very short of references so I think that the sort of references currently in the I-D need adding to the YANG module.

For an example of a module well clad in references, look at
draft-ietf-ccamp-rfc9093-bis

Tom Petch

Tom Petch


Issue #1: Code cutoff

On page 12 shortly after   revision "2016-03-17"   you will find:
   <CODE ENDS>
   <CODE BEGINS>
This breaks the tools that extract YANG modules from RFC documents. Please
remove.

Issue #2: Yang tree

In section 4.3 of the draft, there is a YANG tree pasted. The tree
representation is often useful when a small piece of it is shown next to some
text that explains the usage and meaning of this part of the model. Here the
entire module is just pasted, which is not customary or particularly useful. I
recommend removing it, or add descriptive text for each section.

Issue #3: SNMP/MIB references

There are plenty of references to SNMP related documents, even though this
document is not related to SNMP in any way. The entire chapter 2, with the
reference to RFC 3410, for example, should probably be removed.

Issue #4: Appendix C

Appendix C contains an example, but I'm not quite sure what the example is
trying to show. I'd recommend to either explain/exemplify some more, or remove.

Issue #5: YANG compilation errors

- There are plenty of unbalanced/misplaced curly braces

         list cd-penalties {}

- Indentation is generally off (which makes it harder to find the misplaced
braces)

- There are plenty of lines with missing ; at the end of the line

           description "list of CD penalties"
         }

- There are plenty of prefixes that do not match any imported prefix

         leaf roll-off {
           type layer0-types:roll-off

- A few leafs are declared to be of the non-existent type "strings" (->
"string"?)

- There are a couple of lists that names a key that does not exist

      list cd-penalties {
        key "penalties";
        description "list of CD penalties"
      }

- There is a list with no key statement at all. This may be legal, but I
suspect it's an omission in this case

- There are a number of leafs with parenthesis in the name

    leaf (G.698.2) {
      type strings;

- There are a few uses-statements placed inside leafs

Issue #6: Dependency on non-RFC modules

This module imports ietf-layer0-types-ext, which I would guess refers to the
work in progress defined in
https://www.ietf.org/archive/id/draft-ietf-ccamp-layer0-types-ext-01.txt If
these two documents are meant to go through the IETF processes together, this
may not be a problem.

All in all, I would think serious work remains in order to ready this draft for
last-call. The YANG module in particular seems to require a lot of work. I did
not really look closely into the YANG types, ranges, enums vs. identitites,
defaults vs. mandatory etc. that I would normally do in a LC-review. Attention
to such details will have to be paid when the module is more mature.

By the way, YANG is supposed to be written in all caps.

Best Regards,
/Jan



_______________________________________________
CCAMP mailing list
CCAMP@ietf.org
https://www.ietf.org/mailman/listinfo/ccamp

_______________________________________________
CCAMP mailing list
CCAMP@ietf.org
https://www.ietf.org/mailman/listinfo/ccamp