Re: [pim] Yangdoctors telechat review of draft-ietf-pim-igmp-mld-yang-10

tom petch <daedulus@btconnect.com> Fri, 08 February 2019 16:07 UTC

Return-Path: <daedulus@btconnect.com>
X-Original-To: pim@ietfa.amsl.com
Delivered-To: pim@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 28420127598; Fri, 8 Feb 2019 08:07:22 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.246
X-Spam-Level:
X-Spam-Status: No, score=0.246 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RATWARE_MS_HASH=2.148, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-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 sgThW5YkizYF; Fri, 8 Feb 2019 08:07:19 -0800 (PST)
Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20111.outbound.protection.outlook.com [40.107.2.111]) (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 C4C7E126C15; Fri, 8 Feb 2019 08:07:15 -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=spplsx1rnSH+VHC+YZAbZjv6IETodSUf/woYOaQswIE=; b=Yyz/T06/xjLFIRDhVkpzG39uVf/0TuHxXQW8OqdRVIUqGRjAJhAomgoljZTMyenBYUIig2PmYdqLfyrcGymqEtuHay+zeLtbHIdxw6S2eWv+GGViwCkH89a3srxsx1v9pDGqpjdaOHEwL4w/3KSpb6UFGrDLaPAXoYd6xEXfa3M=
Received: from DB5PR07MB0952.eurprd07.prod.outlook.com (10.161.200.147) by DB5PR07MB1030.eurprd07.prod.outlook.com (10.161.200.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1601.17; Fri, 8 Feb 2019 16:07:12 +0000
Received: from DB5PR07MB0952.eurprd07.prod.outlook.com ([fe80::1dbd:b660:a8fd:673c]) by DB5PR07MB0952.eurprd07.prod.outlook.com ([fe80::1dbd:b660:a8fd:673c%3]) with mapi id 15.20.1622.010; Fri, 8 Feb 2019 16:07:12 +0000
From: tom petch <daedulus@btconnect.com>
To: Jan Lindblad <janl@tail-f.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "pim@ietf.org" <pim@ietf.org>, "draft-ietf-pim-igmp-mld-yang.all@ietf.org" <draft-ietf-pim-igmp-mld-yang.all@ietf.org>
Thread-Topic: Yangdoctors telechat review of draft-ietf-pim-igmp-mld-yang-10
Thread-Index: AQHUv8hZblbpzYkupkqonoDx7ke5vg==
Date: Fri, 8 Feb 2019 16:07:12 +0000
Message-ID: <00d401d4bfc8$273c1380$4001a8c0@gateway.2wire.net>
References: <154955530928.5208.12848706122029839234@ietfa.amsl.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-clientproxiedby: LO2P265CA0302.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a5::26) To DB5PR07MB0952.eurprd07.prod.outlook.com (2a01:111:e400:510e::19)
authentication-results: spf=none (sender IP is ) smtp.mailfrom=daedulus@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; DB5PR07MB1030; 6:JzIUw+p51C20zWUIU9RsB8C8YWL30mkykMRBi/xrjUIMwSIvRYk2kpX0OkXyWkwcIAqiDaE8Ea08SIPnyR2x5gGPvKB7g/32tLOU+dE3+Im+1le27ZI9g+UPgYUtj3YJOuqC01Zm9Y6bK0DB/xm+v+mvGVFv+bIDNmv9/CahnBfw4wypfS5z5LBhZa3Y9sPjBY3DF/HCt+tM+W3yFmkywXSFS5xHx15+QM6XSJat6/MHBvbRLVVoBdUhNiXZGxTRIEe1krIt0DfQtUvoD6Xyn2P5T8cyHEXU68zf6CPth5wAPDQ7xO6a2aMj07H/tbiic0RspogvNlNW86xRzc74s9MI01aqAQJ4Q7Lgkp5VuAZIhns2mJDz0r9HrYRbHsU1qE0OLovh650zinBGGe9VKBv5jP0MNDWQ6tVs35m5TyyrnHFU0tDmU0jlIFt4ZOrJEmobnc7Lo2jU/h/kQQgETQ==; 5:LG0PNLX71lCVQ4zmZ8U9/nKsisGftOJdqkAW6JScZRVljXWq/pT3wIdFj374cmV++HnPJ436DJHAf5y4JJLpv2GAc4MsIrBVnMmoF8OayR2o8DTZc+Q5PF423T9iw8M4gq1LUsyC1HdxkRPEkx0hMo33scSyTsVffAm3gPsjV9ZRWuMWkad/wOUfufTsPlwM6beWsqTt8IVfeAcAwpNBuA==; 7:urXOOc4E9C12d22onFR6p4cEoT54ogkxvza5rYIt1+Cu36EsN2GxVfhWRyEcwTRA0rtShlrdCiMSRwsr9hgQaWA5sZEl4bIOxJHmyNzbXuwojUVIg3L9+fgZvX/4pviNQjYll/9bmZM0AytW1/SnkA==
x-ms-office365-filtering-correlation-id: f781f6fc-ea3d-41eb-e3d3-08d68ddf7c09
x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600110)(711020)(4605077)(2017052603328)(7193020); SRVR:DB5PR07MB1030;
x-ms-traffictypediagnostic: DB5PR07MB1030:
x-microsoft-antispam-prvs: <DB5PR07MB103081ED7F588C53CB35B8F9C6690@DB5PR07MB1030.eurprd07.prod.outlook.com>
x-forefront-prvs: 094213BFEA
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(396003)(366004)(39860400002)(136003)(376002)(346002)(50084003)(199004)(189003)(13464003)(68736007)(6246003)(478600001)(7736002)(86152003)(2501003)(14454004)(14444005)(71200400001)(66066001)(99286004)(229853002)(256004)(71190400001)(86362001)(44736005)(44716002)(62236002)(1556002)(52116002)(84392002)(50226002)(97736004)(476003)(81686011)(6436002)(61296003)(76176011)(2906002)(6512007)(105586002)(3846002)(81816011)(9686003)(6116002)(106356001)(8936002)(386003)(6506007)(81156014)(81166006)(4720700003)(446003)(102836004)(110136005)(54906003)(33896004)(14496001)(8676002)(305945005)(4326008)(26005)(486006)(186003)(316002)(25786009)(53936002)(6486002)(74416001)(7726001); DIR:OUT; SFP:1102; SCL:1; SRVR:DB5PR07MB1030; H:DB5PR07MB0952.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: trn5ByzU68BHo34ugjynSc/i5ZCVs66plFIIe31RV5A/1DiKY3AkaOYQLRVbqlhKiSqiArfeS5F5E4zguwU+4RhhSrBPkgmapDEJ6pbkF+1oUQsExi08lqQ4Si3nfU249DJpygsQL/BErq7F02fpImOPQz6FAicBbhe94ZR+9Tanwx4uD/jYgTZz9nTCA4PSM302Lx8lvwyP8HyScKkohn/adXpK8mO3obisyPEwlWxanKunLhpK2DxnVH+dqmn7u8brBbQUI0p1lgGHngX8Oki6PQlOAJmfjYBRf2v8aA9/tsqB9SBPlYae6dd5lPZvVj563UTgRRYC519sTgNoKI5ANljT2gUnZY8DeNQf5tHkAx51C+luCcOtfFhAsuVuKB7qEplWxoUQ34JD/dhbi4oROFY/ccgFeOAlPWnH3ZE=
Content-Type: text/plain; charset="utf-8"
Content-ID: <08AAC5815C29644FB57FB092F8F7FD2A@eurprd07.prod.outlook.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-OriginatorOrg: btconnect.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f781f6fc-ea3d-41eb-e3d3-08d68ddf7c09
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Feb 2019 16:07:11.4267 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-id: cf8853ed-96e5-465b-9185-806bfe185e30
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR07MB1030
Archived-At: <https://mailarchive.ietf.org/arch/msg/pim/ROyVNPKJ36-zj9WrPPssNZHaWow>
Subject: Re: [pim] Yangdoctors telechat review of draft-ietf-pim-igmp-mld-yang-10
X-BeenThere: pim@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Protocol Independent Multicast <pim.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/pim>, <mailto:pim-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/pim/>
List-Post: <mailto:pim@ietf.org>
List-Help: <mailto:pim-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/pim>, <mailto:pim-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 08 Feb 2019 16:07:23 -0000

Jan

Agree with your comments but I had a more fundamental problem trying to
understand theYANG module, about the repeated use of IGMP and/or MLD as
in
      feature global-admin-enable {
Is that one or or the other or both? Or
        leaf enable {
          type boolean;           default false;
          description "true to enable IGMP or MLD in the routing
instance;
Is that one or the other or both?
And so on. I am uncertain how the phrases are meant to be interpreted.

The only way I could make sense of this was to assume that an
implementation must implement and support both IGMP and MLD; which is of
course blown out of the water by including separate features for IGMP
and MLD.

In  a similar vein, SSM is repeatedly mentioned, never explained, never
referenced and in a sense, this is another protocol to add to IGMP and
MLD

Tom Petch

----- Original Message -----
From: "Jan Lindblad" <janl@tail-f.com>;
Sent: Thursday, February 07, 2019 4:01 PM


> Reviewer: Jan Lindblad
> Review result: Ready with Issues
>
> This is my YANG-doctor review of draft-ietf-pim-igmp-mld-yang-10. In
the
> spring, I did an early review of the -02, and in the early fall a last
call
> review of the -07 version. The module has progressed significantly
since then.
> In particular the two main issues with -07 have been fixed, which is
great!
> There are still a few things that should be looked at, however.
>
> Since some of the current issues are the same as addressed in earlier
comments,
> I will reuse the numbering from the -07 review, and add a few points
at the end:
>
> #3 Top level structures not optional
>
> If-feature statements have been introduced so that an implementor can
choose
> whether to implement igmp and/or mld separately. This is very good.
RFC 8349,
> which the current module augments, recommend that the top level
containers be
> presence containers. This is currently not the case, and I see no
reason why
> they aren't.
>
>    It is also RECOMMENDED that protocol-specific data nodes be
>    encapsulated in an appropriately named container with presence.
Such
>    a container may contain mandatory data nodes that are otherwise
>    forbidden at the top level of an augment.
>
> At this time, there are no mandatory leafs under the top level
containers,
> which makes this recommendation less of a concern, but if a mandatory
leaf is
> introduced (e.g. see #9 below), making the top level containers
presence
> containers will be essential.
>
> #4 Unclear meaning of optional leaves
>
> While a lot of default values have been introduced (great!), there are
still
> many optional configuration leafs with no default value, not
mandatory, not
> self-evident and no description of what absence signifies. A few words
in the
> description might be all it takes to fix this.
>
> This happens with many leafs, but a couple of examples:
>
>      leaf max-entries {
>        if-feature global-max-entries;
>        type uint32;
>        description
>          "The maximum number of entries in IGMP or MLD.";
>      }
>
> If no max-entries is configured, what is the prescribed behavior? Will
all
> implementations agree that there is no max then? If so, maybe mention
that in
> the description?
>
>      leaf source-policy {
>        if-feature intf-source-policy;
>        type leafref {
>          path "/acl:acls/acl:acl/acl:name";
>        }
>        description
>          "Name of the access policy used to filter sources.
>           A device can restrict the length
>           and value of this name, possibly space and special
>           characters are not allowed.";
>      }
>
> If no source policy is specified, does that mean there is no policy?
>
> There are many more.
>
> #8 What policy name?
>
> In leaf ssm-map-group-policy, the description begins with "Name of the
policy
> ....". Does this mean that the policy name can be chosen freely, or
does the
> policy name entered here have to match some policy name specified
somewhere
> else?
>
> If this name can be chosen freely without considering some other list,
it's all
> fine.
>
>        leaf ssm-map-group-policy {
>          type string;
>          description
>            "Name of the policy used to define ssm-map rules.
>             A device can restrict the length
>             and value of this name, possibly space and special
>             characters are not allowed. ";
>
> #9 Default for version
>
> Leaf version is given a default of 2; this sounds excellent. This
clarifies
> what protocol version to use even if the operator doesn't specify.
Note that a
> default can never be changed, however, so only do this if 2 will feel
like a
> good number even 10 years from now.
>
> If no suitable default value that withstands time can be selected,
another
> option is to make it mandatory for the client to configure. Note that
in such a
> case #3 above becomes very important to resolve.
>
> The authors should
>      leaf version {
>        type uint8 {
>          range "1..3";
>        }
>        default 2;
>
> #10 Inaccurate description or expression
>
> In the interface-name leaf, there is a must statement and description
which
> contradict each other. The must statement requires that the ipv4
container is
> configured, while the description says ipv4 should be enabled. It is
possible
> to configure the ipv4 container but have ipv4/enabled = 'false', so
the must
> expression only cares whether the ipv4 presence container is
configured, not if
> it is enabled. Which one reflects the desired behavior?
>
>            leaf interface-name {
>              type if:interface-ref;
>              must "/if:interfaces/if:interface[if:name = current()]/"
>                 + "ip:ipv4" {
>                description
>                  "The interface must have IPv4 enabled.";
>
> If ipv4 needs to be enabled, the must expression should be
>
>              must "/if:interfaces/if:interface[if:name = current()]/"
>                 + "ip:ipv4/ip:enabled = 'true'" {
>
> If ipv4 needs to be configured, the description should say "The
interface must
> have IPv4 configured (but not necessarily enabled)."
>
> The same logic applies to MLD and ipv6 as well.
>