Re: [i2rs] Robert Wilton's No Objection on draft-ietf-i2rs-yang-l2-network-topology-18: (with COMMENT)

"Rob Wilton (rwilton)" <rwilton@cisco.com> Fri, 18 September 2020 13:20 UTC

Return-Path: <rwilton@cisco.com>
X-Original-To: i2rs@ietfa.amsl.com
Delivered-To: i2rs@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 009923A07A7; Fri, 18 Sep 2020 06:20:40 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -9.62
X-Spam-Level:
X-Spam-Status: No, score=-9.62 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, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com header.b=CuEnT63l; dkim=pass (1024-bit key) header.d=cisco.onmicrosoft.com header.b=imuTCzyA
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 bJHxpQvfSm2c; Fri, 18 Sep 2020 06:20:37 -0700 (PDT)
Received: from rcdn-iport-8.cisco.com (rcdn-iport-8.cisco.com [173.37.86.79]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 34E513A07A3; Fri, 18 Sep 2020 06:20:37 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=26722; q=dns/txt; s=iport; t=1600435237; x=1601644837; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=p51YFxf951AB/TL/GJXIT61FlZYTrrQ8DDixHFM6ZSA=; b=CuEnT63l4wJMCVtqNbq4K9KkNEEOoVexyWo2pfKAnCKGGyYEGrnejV8f m5HXvsB/uxHWa3JL3shgBlz7HrCO6RDvxdEYYJYbTY/fXsBXsVPNUbTmM MiF+2sgCyy2TOrNJG5vaDQRNiZ6HTZ0xHeg/e7bl2Mcnwwfn4vRWuAa9u E=;
IronPort-PHdr: =?us-ascii?q?9a23=3A4D/kYBXAfwPERt64/SRSPLToII/V8LGuZFwc94?= =?us-ascii?q?YnhrRSc6+q45XlOgnF6O5wiEPSBNyBufRImeqQuKflCiQM4peE5XYFdpEEFx?= =?us-ascii?q?oIkt4fkAFoBsmZQVb6I/jnY21ffoxCWVZp8mv9PR1TH8DzNFvesH305jkXSV?= =?us-ascii?q?3zMANvLbHzHYjfx828y+G1/cjVZANFzDqwaL9/NlO4twLU48IXmoBlbK02z0?= =?us-ascii?q?jE?=
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0C6BwCxsmRf/4sNJK1WCR4BAQsSDEC?= =?us-ascii?q?BRAuBUikoB3BZLywKhC+DRgONdoECl3KBLhSBEQNVCwEBAQ0BASUIAgQBAYR?= =?us-ascii?q?LAheCFAIkNQgOAgMBAQsBAQUBAQECAQYEbYVcDIVyAQEBBBIRBA0MAQEpDgE?= =?us-ascii?q?LBAIBCA4DBAEBAwIjAwICAjAUAQgIAgQBDQUIGoI5TIJLAy4BAwuqQwKBOYh?= =?us-ascii?q?hdn8zgwEBAQWBR0GDERiCEAMGgQ4qgnGDaYZSG4FBP4ERQ4IYNT6CXAIBAgG?= =?us-ascii?q?BJgEICgEHHBWDADOCLZAPgyKHI5xcCoJniHaMUoUlgwyJegWTd5J6gXeIapU?= =?us-ascii?q?bAgQCBAUCDgEBBYFWATdnWBEHcBU7gmlQFwINjh8MFxSDOoUUhUJ0AjUCBgo?= =?us-ascii?q?BAQMJfIxSAYEQAQE?=
X-IronPort-AV: E=Sophos;i="5.77,274,1596499200"; d="scan'208";a="827013228"
Received: from alln-core-6.cisco.com ([173.36.13.139]) by rcdn-iport-8.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA; 18 Sep 2020 13:20:35 +0000
Received: from XCH-ALN-002.cisco.com (xch-aln-002.cisco.com [173.36.7.12]) by alln-core-6.cisco.com (8.15.2/8.15.2) with ESMTPS id 08IDKYUu032385 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 18 Sep 2020 13:20:35 GMT
Received: from xhs-aln-003.cisco.com (173.37.135.120) by XCH-ALN-002.cisco.com (173.36.7.12) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 18 Sep 2020 08:20:34 -0500
Received: from xhs-rtp-001.cisco.com (64.101.210.228) by xhs-aln-003.cisco.com (173.37.135.120) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Fri, 18 Sep 2020 08:20:34 -0500
Received: from NAM11-DM6-obe.outbound.protection.outlook.com (64.101.32.56) by xhs-rtp-001.cisco.com (64.101.210.228) with Microsoft SMTP Server (TLS) id 15.0.1497.2 via Frontend Transport; Fri, 18 Sep 2020 09:20:33 -0400
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ntAbl9u9fn6LL6+LBeSw8npubFNZ+tkO+H7PZMeNLCNDiLOcFJBxmrzPnPuRd5oDn8coxOz9e6BGPI6lxtb6lCjlaMOgqOcZf0Gi5mQap8YBmNxesPvayfhRNLVb74deaOuxI5xFkaeDemMuOYfNDtZWUDCiUFTHBmNQP5pJgMwniaJV150Ve2EyZ3Ga1TqrFIw63d2FZOcxS6DoqoHpQEnoscfAx2/SNB3QYD3BYLBfOCXVfkz3KBEmi0sWJujLiJWG/726NfFCUeaT1vqBusZwhbTIfAjN5HJh1tUkjBLLxpvb0oLlsh6fxJ2CT719eQNI5DZoFOkkHrz7ZAvM1Q==
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-SenderADCheck; bh=p51YFxf951AB/TL/GJXIT61FlZYTrrQ8DDixHFM6ZSA=; b=lGi5mPQv4AA8FkvipuJF1/iHyjSqCpGdPraVzJiaNQGztwAfUoBXEd8r2JrFVZu2vigiGxVSEq1r+x8jVNvHYneg1xPfl7sfQQ7D/5mXUqQRtSrG581sF6PI5b/JNzy8shu96Shp97ERl4Q+9LCPCBkJkjJpkXeAJB1eVmj/oYjh7n+R1iCvA6XJGxysz2TcXcgp81riU4GPucolVughX0u6wEMyea5aasUI1Jqmqmwzo6frfS+VzpQIeQ7vmzVfDBy4SFLjonOKzK4ij+CGie6TsllXVNv8t/IYCNPA3R301pHBjUpf0hmyMFYyiDBJWKwfUNn+0QTmo9EpAmv0Dw==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=cisco.com; dmarc=pass action=none header.from=cisco.com; dkim=pass header.d=cisco.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cisco.onmicrosoft.com; s=selector2-cisco-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=p51YFxf951AB/TL/GJXIT61FlZYTrrQ8DDixHFM6ZSA=; b=imuTCzyAdNK3LtG7Q3XE7Z9Lp/O1yUTvD94lAi3UinY7OdBgqd3K4kk1/dwNLvn9ARlY+A8yNkDiiEMqTG9W1cmn/ewWUbbdF+Ju4I9L4txr73PI+PuJeoctFx8ynus9cIH3QeuQx0OnFeXpWUDlErBtYJQfUVgi7379h+sQCw0=
Received: from MN2PR11MB4366.namprd11.prod.outlook.com (2603:10b6:208:190::17) by MN2PR11MB4551.namprd11.prod.outlook.com (2603:10b6:208:269::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3391.14; Fri, 18 Sep 2020 13:20:32 +0000
Received: from MN2PR11MB4366.namprd11.prod.outlook.com ([fe80::4d3f:f3e:add7:dfc1]) by MN2PR11MB4366.namprd11.prod.outlook.com ([fe80::4d3f:f3e:add7:dfc1%3]) with mapi id 15.20.3370.019; Fri, 18 Sep 2020 13:20:32 +0000
From: "Rob Wilton (rwilton)" <rwilton@cisco.com>
To: Susan Hares <shares@ndzh.com>, "'The IESG'" <iesg@ietf.org>
CC: "draft-ietf-i2rs-yang-l2-network-topology@ietf.org" <draft-ietf-i2rs-yang-l2-network-topology@ietf.org>, "i2rs-chairs@ietf.org" <i2rs-chairs@ietf.org>, "i2rs@ietf.org" <i2rs@ietf.org>, Martin Vigoureux <martin.vigoureux@nokia.com>
Thread-Topic: Robert Wilton's No Objection on draft-ietf-i2rs-yang-l2-network-topology-18: (with COMMENT)
Thread-Index: AQHWjbIV2J4e1yCqYUuZDT0IWcTk+KluXHdg
Date: Fri, 18 Sep 2020 13:20:32 +0000
Message-ID: <MN2PR11MB4366515E624D55F0C0FDB609B53F0@MN2PR11MB4366.namprd11.prod.outlook.com>
References: <160036143082.20631.3406779727197755710@ietfa.amsl.com> <006201d68db2$0ca5b5d0$25f12170$@ndzh.com>
In-Reply-To: <006201d68db2$0ca5b5d0$25f12170$@ndzh.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: ndzh.com; dkim=none (message not signed) header.d=none;ndzh.com; dmarc=none action=none header.from=cisco.com;
x-originating-ip: [82.12.233.180]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 0c018250-ad37-4597-610b-08d85bd59f0d
x-ms-traffictypediagnostic: MN2PR11MB4551:
x-microsoft-antispam-prvs: <MN2PR11MB455150BDB5F7661B4464EDF9B53F0@MN2PR11MB4551.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: Ka3Tzk0KvNeMcTepunwarTGYI/idSvgPdmJ1si+O1DTIQWsBnGPRuY/GyjUyaFznKIyDlMI71+T0hwXblFbEX2paGR89/jHLjdZGiAZtjziZm/f0qeNScUXtE6o97XOq4ksPQ7J9rzN9Q746GFDWp/k1/QdNzQl0gRpBTjrXbFySpoHL2TiXLhu7w3Tw/1S911JnOi8dr6uXJjf5Oy+wvQlPyFx14wq6JotlnQWrcZP4vc6+Om05CDZQVwmfjQgAEMPbgbJ+IV/DF/B0SBF1T7FV9W28yOTvWBMGHVD1HMyRxystnFKy6Q2xIixXO5CUpn28afLth+43uovoDmq3GMD2bc5NaIcSAvN6mEqpYbPYjcxq2S6Ajrenfv8y2tBvfMYJ/h6pRRFIZ4phXtjizA==
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR11MB4366.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(39860400002)(376002)(346002)(136003)(366004)(396003)(52536014)(5660300002)(478600001)(8676002)(8936002)(33656002)(4326008)(66476007)(30864003)(76116006)(66446008)(6506007)(66946007)(86362001)(64756008)(66556008)(53546011)(66574015)(2906002)(110136005)(55016002)(54906003)(71200400001)(9686003)(7696005)(26005)(316002)(966005)(186003)(83380400001); DIR:OUT; SFP:1101;
x-ms-exchange-antispam-messagedata: goN/0eHmlNAwS+ushNO1f37eGHtIHAR0xPjLT+uv1o4IQ/NhNKNPvtbUoxevAmjKfiKKd12+/nx5bZc1R7uUzCF3K0buMG8OPrBvxbKCRTzOGKdMmHZ1WQ+iLe2SopWUIsycq7k/LtA7Xoy9wPjVQ9S5iem4vzX3uOeAPlqnOnpO3urEkPMQPGlWYyCoTIlstpMiiASaOKCdJth4pas1JbKF1VImM0jf8/kBcqpMNU5IeCwsBjL+ky8AohBfTzIJGQbVmhZNdfCwv6WH5pjG010XF2wXulKPlfr77KrojQC3kRNH5rvz/XWQiMe3ixxMRqH9tWVbY7KhMtomj0ay8c+RoqtwIrsgDJQHT5T0AzHux5Y7nwU+1mM7yLMnxc15/zyAVUIyvGWY9SHNcnTuB8p+Yc7jI/phDt26ZMyU1MiVRDwB6HYCFNVL+FpVIdkxJKlccmUzwxMydPCvHKryU0mBKaxAGNiLFhUeue0H6ukV/90VOVw6iDxJ/CnUaRWMPqwyOIbDihCYh/DHi1VPeJ4qFRBF9UjTNJRAoiAOtgwRx02Wo2+tW6iNlmN57xRgilqWYW0CGIgiPazBrnwkOCAdkMDVQQCzIknscN8mxuSR8M829uNP3dUMrhGbM136oNOV4uxVRAWrrdHlWSXQSA==
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MN2PR11MB4366.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0c018250-ad37-4597-610b-08d85bd59f0d
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Sep 2020 13:20:32.5362 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 5ae1af62-9505-4097-a69a-c1553ef7840e
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 2osZyramvdPspse4ONXtHNJtm6VtByMHF9Gfd3/cBLQkAo8HF+whc9x4LA0qfhVIV3zBLMh0q2FdpWDebn0XWw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4551
X-OriginatorOrg: cisco.com
X-Outbound-SMTP-Client: 173.36.7.12, xch-aln-002.cisco.com
X-Outbound-Node: alln-core-6.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2rs/Bs3FROoXPh0oG6ax_KaI63oFUDU>
Subject: Re: [i2rs] Robert Wilton's No Objection on draft-ietf-i2rs-yang-l2-network-topology-18: (with COMMENT)
X-BeenThere: i2rs@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Interface to The Internet Routing System \(IRS\)" <i2rs.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2rs>, <mailto:i2rs-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2rs/>
List-Post: <mailto:i2rs@ietf.org>
List-Help: <mailto:i2rs-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2rs>, <mailto:i2rs-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 18 Sep 2020 13:20:40 -0000

Hi Sue, IESG,

When I discussed this with Glenn Parsons (IEEE 802.1 WG chair) and Scott Mansfield (Chair for IEEE Yangsters) then the message was that if we ask the IEEE to provide an official position (i.e., via a liaison) then they can provide one.  However, my presumption is that this would take time and delay the publishing of this document, and hence I have not asked them to provide one.

I also don't think that a formal liaison would lead to a materially different outcome.  I believe that the individuals in IEEE who are interested in reviewing this draft have had the opportunity to do so, have provided comments back to the authors, and have confirmed in Tuesday's Yangsters meeting that they are comfortable with how the authors have addressed those comments in draft version -18.  As background, about 20-30 minutes were spent discussing this draft in this week's Yangsters meeting without any contention.

So, unless any members on the IESG feel otherwise, I believe that we are done and good to progress this document.

Regards,
Rob


> -----Original Message-----
> From: Susan Hares <shares@ndzh.com>
> Sent: 18 September 2020 12:52
> To: Rob Wilton (rwilton) <rwilton@cisco.com>om>; 'The IESG' <iesg@ietf.org>
> Cc: draft-ietf-i2rs-yang-l2-network-topology@ietf.org; i2rs-
> chairs@ietf.org; i2rs@ietf.org
> Subject: RE: Robert Wilton's No Objection on draft-ietf-i2rs-yang-l2-
> network-topology-18: (with COMMENT)
> 
> Rob and other IESG members:
> 
> Should we wait for an IEEE official position or can we continue with
> standardization?  I think the informal review will be the most
> substantive.
> 
> Sue Hares
> 
> -----Original Message-----
> From: Robert Wilton via Datatracker [mailto:noreply@ietf.org]
> Sent: Thursday, September 17, 2020 12:51 PM
> To: The IESG
> Cc: draft-ietf-i2rs-yang-l2-network-topology@ietf.org; i2rs-
> chairs@ietf.org; i2rs@ietf.org
> Subject: Robert Wilton's No Objection on draft-ietf-i2rs-yang-l2-network-
> topology-18: (with COMMENT)
> 
> Robert Wilton has entered the following ballot position for
> draft-ietf-i2rs-yang-l2-network-topology-18: No Objection
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-i2rs-yang-l2-network-topology/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Clearing discuss held for IEEE review:
> 
> The IEEE Yangsters (members of the IEEE YANG experts) who reviewed and
> contributed review comments to this draft are comfortable with version -
> 18.
> Note, that these represent informal reviews of individuals and do not
> represent a formal IEEE consensus position."
> 
> Regards,
> Rob
> 
> Mostly minor/editorial comments  (###) on the YANG model, but I do think
> that it would be helpful for these to be discussed and addressed as
> appropriate:
> 
> 4.  Layer 2 Topology YANG Module
> 
>      import ietf-inet-types {
>        prefix inet;
>        reference
>          "Section 4 of RFC 6991";
>      }
>      import ietf-yang-types {
>        prefix yang;
>        reference
>          "Section 3 of RFC 6991";
>      }
> 
> ###
> These references should probably both just be: "RFC 6991: Common YANG Data
> Types"
> 
>      revision 2020-06-29 {
>        description
>          "Initial revision";
>        reference
>          "RFC XXXX: A YANG Data Model for Layer 2 Network
>                     Topologies";
>      }
> 
> ###
> I would reorder these sections to be (which will solve the forward
> reference issue mentioned by Ben):
>    feature-stmt
>    identity-stmt
>    typedef-stmt
> 
> I'm surprised that pyang didn't flag this.
> 
>      /*
>       * Typedefs
>       */
> 
>      typedef vni {
>        type uint32 {
>          range "0..16777215";
>        }
>        description
>          "VXLAN Network Identifier or VXLAN Segment ID.
>           It allows up to 16 M VXLAN segments to coexist
>           within the same administrative domain.
> 
>           The use of value '0' is implementation-specific.";
>        reference
>          "RFC 7348: Virtual eXtensible Local Area  Network (VXLAN):
>                     A Framework for Overlaying Virtualized Layer 2
>                     Networks over Layer 3 Networks";
>      }
> 
>      typedef l2-flag-type {
>        type identityref {
>          base flag-identity;
>        }
>        description
>          "Base type for L2 flags. One example of L2 flag
>           type is trill which represents trill topology
>           type.";
>      }
> 
> ###
> This isn't really a base type.  Given where this flag is used, would this
> be better as an "network-flag-type", with a description more similar to
> the ones below?
> 
>      typedef node-flag-type {
>        type identityref {
>          base flag-identity;
>        }
>        description
>          "Node flag attributes. The physical node can be
>           one example of node flag attribute.";
>      }
> 
>      typedef link-flag-type {
>        type identityref {
>          base flag-identity;
>        }
>        description
>          "Link flag attributes. One example of link flag
>           attribute is the pseudowire.";
>      }
> 
> ###
> Should there be identities defined for l2-flag, node-flag and link-flag
> that derive from flag-identity?  That would them make these three typedefs
> reference different things rather than all referencing the same base
> flags.
> 
>      typedef l2-network-event-type {
>        type enumeration {
>          enum add {
>            value 0;
>            description
>              "A Layer 2 node or link or termination-point
>               has been added.";
>          }
>          enum remove {
>            value 1;
>            description
>              "A Layer 2 node or link or termination-point
>               has been removed.";
>          }
>          enum update {
>            value 2;
>            description
>              "A Layer 2 node or link or termination-point
>               has been updated.";
>          }
>        }
>        description
>          "Layer 2 network event type for notifications.";
>      }
> 
> ###
> Since these are events, I would suggest renaming these "add" ->
> "addition", "remove" -> "removal", "update"
> 
>      typedef neg-mode {
>        type enumeration {
>          enum full-duplex {
>            description
>              "Indicates full-duplex mode.";
>          }
>          enum auto-neg {
>            description
>              "Indicates auto-negotiation mode.";
>          }
>          enum half-duplex {
>            description
>              "Indicates half-duplex mode.";
>          }
>        }
>        description
>          "Indicates the type of the negotiation mode.";
>      }
> ###
> What negotiation do you mean?  Does this mean IEEE 802.3 auto-negotation?
> If so, it would be more correct to have a separate leaf for duplex vs
> auto-negotation.
> 
>      /*
>       * Features
>       */
> 
>      feature VLAN {
>        description
>          "Indicates that the system supports the
>           vlan functions (also known as an IEEE 802.1Q tag).";
>      }
> 
>      feature QinQ {
>        description
>          "Indicates that the system supports the
>           qinq functions (also known as IEEE 802.1ad double tag).";
>      }
> ###
> I think that the description should be more clear on what is supported
> here: 2 VLAN tags, as defined by IEEE 802.1ad.  The features should also
> include reference statements.
> 
> Possibly we need to find a better name for these features (IEEE will
> possibly comment on this).
> 
>      feature VXLAN {
>        description
>          "Indicates that the device supports VXLAN functions.";
>        reference
>          "RFC 7348: Virtual eXtensible Local Area  Network (VXLAN):
>                     A Framework for Overlaying Virtualized Layer 2
>                     Networks over Layer 3 Networks";
>      }
> 
>      /*
>       * Identities
>       */
> 
>      identity flag-identity {
>        description
>          "Base type for flags.";
>      }
> ###
> Probably the name should be just "flag" rather than "flag-identity"
> 
>      identity eth-encapsulation-type {
>        base ift:iana-interface-type;
>        description
>          "Base identity from which specific Ethernet
>           encapsulation types are derived.";
>        reference
>          "RFC 7224: IANA Interface Type YANG Module";
>      }
> 
> ###
> I would rename the base identity to probably "l2-encapsulation-type".  It
> also probably doesn't help to derive from ift:iana-interface-type.
> 
>      identity ethernet {
>      ...
>      identity vxlan {
>        base eth-encapsulation-type;
>        description
>          "VXLAN MAC in UDP encapsulation.";
>      }
> 
> ###
> Please add references for protocols defined in the identities.
> 
>      /*
>       * Groupings
>       */
> 
>      grouping l2-network-type {
> 
> Dong, et al.            Expires December 31, 2020              [Page 12]
> Internet-Draft     Layer 2 Network Topology Data Model         June 2020
> 
>        description
>          "Indicates the topology type to be L2.";
>        container l2-network {
>          presence "Indicates L2 Network";
>          description
>            "The presence of the container node indicates
>             L2 Topology.";
>        }
>      }
> 
>      grouping l2-network-attributes {
>        description
>          "L2 Topology scope attributes.";
>        container l2-network-attributes {
>          description
>            "Contains L2 network attributes.";
>          leaf name {
>            type string;
>            description
>              "Name of the L2 network.";
> ###
> Perhaps just "Name of the network".  Are there any restrictions here on
> how long the name can be, can it contain spaces, etc?
> 
>          }
>          leaf-list flag {
>            type l2-flag-type;
>            description
>              "L2 network flags.";
>          }
>        }
>      }
> 
> ###
> Perhaps expand the description to "Flags that can be associated with the
> network"
> 
> I would suggest changing the name of all "flag" to "flags" where they are
> leaf-lists.
> 
>      grouping l2-node-attributes {
>        description
>          "L2 node attributes";
>        container l2-node-attributes {
>          description
>            "Contains L2 node attributes.";
>          leaf name {
>            type string;
>            description
>              "Node name.";
>          }
>          leaf description {
>            type string;
>            description
>              "Node description.";
>          }
>          leaf-list management-address {
>            type inet:ip-address;
>            description
>              "System management address.";
>          }
>          leaf sys-mac-address {
>            type yang:mac-address;
>            description
>              "System MAC address.";
>          }
>          leaf management-vid {
>            if-feature "VLAN";
>            type dot1q-types:vlanid;
>            description
>              "System management VID.";
> 
> ###
> Please change "management-vid" to "management-vlan-id", and fix the
> description.
> 
> Probably could expand on all the descriptions of the "System" properties
> to explain what these system properties are and how they are used (e.g. to
> manage the device).
> 
>          }
>          leaf-list flag {
>            type node-flag-type;
>            description
>              "Node operational flags.";
>          }
>        }
>      }
> 
>      grouping l2-link-attributes {
>        description
>          "L2 link attributes";
>        container l2-link-attributes {
>          description
>            "Contains L2 link attributes.";
>          leaf name {
>            type string;
>            description
>              "Link name.";
>          }
>          leaf-list flag {
>            type link-flag-type;
>            description
>              "Link flags.";
>          }
>          leaf rate {
>            type decimal64 {
>              fraction-digits 2;
>            }
>            units "Mbps";
>            description
>              "Link rate.";
> 
> ###
> Please expand on this description.  Does this describe how the link is
> operating at the physical layer, or its bandwidth?
> 
>          }
>          leaf delay {
>            type uint32;
>            units "microseconds";
> 
>            description
>              "Link delay in microseconds.";
> 
> ###
> Is this uni-directional delay, or bi-directional?  Please clarify.
> 
>          leaf maximum-frame-size {
>            type uint32;
>            description
>              "Maximum L2 frame size. If L2 frame is an Ethernet
>               frame, the Ethernet header should be included;
>               if L2 frame is other type (e.g., PPP), the L2
>               header should be included.";
> 
> ###
> This needs to clarify whether the 4 byte CRC is included in the frame
> size.
> 
>              leaf eth-encapsulation {
>                type identityref {
>                  base eth-encapsulation-type;
>                }
> ###
> As per comments on the identities, I think that this should be
> "encapsulation".
> 
>                description
>                  "Encapsulation type of this
>                   termination point.";
>              }
>              leaf lag {
>                type boolean;
>                default "false";
>                description
>                  "Defines whether lag is supported or not."; ### Does this
> leaf indicate whether LAG is supported, or enabled on the link?
> 
>              }
>              leaf-list member-link-tp {
>                when "../lag = 'true'" {
>                  description
>                    "Relevant only when the lag interface is supported.";
>                }
>                type leafref {
>                  path "/nw:networks/nw:network/nw:node/"
>                     + "nt:termination-point/nt:tp-id";
>                }
>                description
>                  "Member link termination points.";
>              }
>              leaf mode {
>                type neg-mode;
>                default "auto-neg";
>                description
>                  "Exposes the negotiation mode.";
>              }
> ###
> Suggest changing the name to "ethernet-auto-neg-mode", and refining the
> description.
> 
> I would also suggest splitting duplex and auto-negotiation into 2 separate
> leaves to more accurately represent
> 
>              leaf port-vlan-id {
>                when "derived-from-or-self(../eth-encapsulation"
>                   + ", 'l2t:vlan')" {
>                  description
>                    "Only applies when the type of the Ethernet
>                     encapsulation is 'vlan'.";
>                }
>                if-feature "VLAN";
>                type dot1q-types:vlanid;
>                description
>                  "Port VLAN ID is the VLAN identifier that
>                   will be assigned to any untagged frames entering
>                   the switch on the specific port.";
>              }
> ###
> Probably naming this as the "default-untagged-vlan" may be more helpful.
> 
>              list vlan-id-name {
>                when "derived-from-or-self(../eth-encapsulation"
>                   + ", 'l2t:vlan')" {
>                  description
>                    "Only applies when the type of the Ethernet
>                     encapsulation is 'vlan'.";
>                }
> ###
> I think that I would name this list "vlans"
> 
>                leaf vlan-name {
> ###
> I would suggest renaming this leaf to just "name"
> 
>                  "Interface configured SVLANs and CVLANs.";
>                leaf svlan-id {
>                  type dot1q-types:vlanid;
>                  description
>                    "SVLAN ID.";
>                }
>                leaf cvlan-id {
>                  type dot1q-types:vlanid;
>                  description
>                    "CVLAN ID.";
>                }
>              }
> 
> ###
> Suggest "SVLAN" => "S-VLAN" and "CVLAN" => "C-VLAN"
> 
>              container vxlan {
>                when "derived-from-or-self(../eth-encapsulation"
>                   + ", 'l2t:vxlan')" {
>                  description
>                    "Only applies when the type of the Ethernet
>                     encapsulation is 'vxlan'.";
>                }
>                if-feature "VXLAN";
>                leaf vni-id {
>                  type vni;
>                  description
>                    "VXLAN Network Identifier (VNI).";
>                }
>                description
>                  "Vxlan encapsulation type.";
>              }
>            }
> 
>            //case ethernet
>            case legacy {
>              leaf layer-2-address {
>                type yang:phys-address;
>                description
>                  "Interface Layer 2 address.";
>              }
>              leaf encapsulation {
>                type identityref {
>                  base ift:iana-interface-type;
>                }
>                description
>                  "Other legacy encapsulation type of this
>                   termination point.";
>              }
>            }
>            //case legacy such as atm, ppp, hdlc, etc.
>          }
>          //choice termination-point-type
>          leaf tp-state {
>            type enumeration {
>              enum in-use {
>                value 1;
>                description
>                  "The termination point is in forwarding state.";
>              }
>              enum blocking {
>                value 2;
>                description
>                  "The termination point is in blocking state.";
>              }
>              enum down {
>                value 3;
>                description
>                  "The termination point is in down state.";
>              }
>              enum others {
> 
> ###
> Please rename others => other
>                value 4;
>                description
>                  "The termination point is in other state."; ### Please
> modify the description to something like "The termination point is in
> another state, not listed in the enumeration."
> 
>              }
>            }
>            config false;
>            description
>              "State of the termination point.";
>          }
>        }
>      }
> 
> 
>