Re: [pim] [yang-doctors] Yangdoctors telechat review of draft-ietf-pim-igmp-mld-yang-10
Xufeng Liu <xufeng.liu.ietf@gmail.com> Tue, 12 February 2019 00:01 UTC
Return-Path: <xufeng.liu.ietf@gmail.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 6BB9F12941A; Mon, 11 Feb 2019 16:01:02 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.998
X-Spam-Level:
X-Spam-Status: No, score=-1.998 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.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 RCavAUYf5ILL; Mon, 11 Feb 2019 16:00:58 -0800 (PST)
Received: from mail-it1-x12b.google.com (mail-it1-x12b.google.com [IPv6:2607:f8b0:4864:20::12b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 83F3212426A; Mon, 11 Feb 2019 16:00:58 -0800 (PST)
Received: by mail-it1-x12b.google.com with SMTP id g85so2862952ita.3; Mon, 11 Feb 2019 16:00:58 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8ZtHPdlfnfqjSNL17gF8yM0PFLePbShZwtsbt9G7qH8=; b=V2BbDo+sQwxL5vN/rFX9dN+EMyCqAf7kFvw9fsN6m4A5JDNathdW2ODkkr41pyEJj3 zAPeRGVxMCdtU2g7sSjMPvaXUiEKZnO7On8KxlYm8YOjB+cA2WTGkZ+RlnYFQpzDAPNT AEOXR6jelvxstQpUdzg2Ar5QgxtIukSG2F0NHdrC5flLj1tDWz7CExnDLV/Jcv/icjg4 dHl7dnRfyZhZGQoH8WHb1DIAUT64QmbAz3a85XswG2BTlimXU3j642uBRyKkNinF+WwU tcnDCesHgYY7i/YeSv9Iq+dMLU0pjGtg2BIbfWefAgkjZ4sLXZ7tZoaVjinsmIOr4qu7 oi9w==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8ZtHPdlfnfqjSNL17gF8yM0PFLePbShZwtsbt9G7qH8=; b=Lbyrgjlz2Beit70iEcRdl9w8V5+m26OQlLui/o6X+C25y8CllQBccnh2vsi2E/uiCa h/8CZHKI1rOmgbK3Q8MLcuP5GUHvo8yPPQ5c14L1UZ8o7k9d6FZJtqkhtd6092L62NT0 LiePOfsEBOMHq32nisJBcks5h9JVMRWzYsaoVFa+O6jXUuZw4ks5hzoCkULOkhYiJWMs exqAzG3rRXfVOgRp2nEUDlAOqJcPWPBxKQ9sBxdNNRmFVTVHKaItituYBY6dP8Eg8udC o1serRe4N6suYENTiM6PoDO/frbdsm4Mfr+V+9OEtnaXL9fQGr0A7pCDznOYPOh6XSWe 0YXg==
X-Gm-Message-State: AHQUAuZR9PTXFodGhkO+FzxV1NMjNnBULqPARk3v1ccKxjx3ejJYtvH+ Y88huglKPoVXAos2zzcD3ugs87LAq069TATq4dE=
X-Google-Smtp-Source: AHgI3Ia+6vQaTdnb/MtiwgsXy3427yqriFxendU/MM5tTcURaI+b979yhl+3mMOFBTyiQ6ahdAMJFJQgdaMmmZCEwjc=
X-Received: by 2002:a24:9686:: with SMTP id z128mr463415itd.97.1549929657459; Mon, 11 Feb 2019 16:00:57 -0800 (PST)
MIME-Version: 1.0
References: <154955530928.5208.12848706122029839234@ietfa.amsl.com> <00d401d4bfc8$273c1380$4001a8c0@gateway.2wire.net> <01887BB2-429A-4230-A278-7577546F93D8@tail-f.com> <20190209075246.6gqnbmsrxvle5nx4@anna.jacobs.jacobs-university.de> <FECB8944-1B7E-4EB3-8BCE-16E413F31849@tail-f.com> <20190209085847.ovasmvidonjxdimy@anna.jacobs.jacobs-university.de> <F77DB351-48E7-46A5-9BFD-65AF41B431B4@tail-f.com>
In-Reply-To: <F77DB351-48E7-46A5-9BFD-65AF41B431B4@tail-f.com>
From: Xufeng Liu <xufeng.liu.ietf@gmail.com>
Date: Mon, 11 Feb 2019 19:00:45 -0500
Message-ID: <CAEz6PPRrA4sNm+Wg9gNzBxtTiLEEkkwE1V=2tGCUhXq2AMzKaA@mail.gmail.com>
To: Jan Lindblad <janl@tail-f.com>
Cc: Juergen Schoenwaelder <j.schoenwaelder@jacobs-university.de>, tom petch <daedulus@btconnect.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "pim@ietf.org" <pim@ietf.org>, "draft-ietf-pim-igmp-mld-yang.all@ietf.org" <draft-ietf-pim-igmp-mld-yang.all@ietf.org>
Content-Type: multipart/alternative; boundary="000000000000a8e7040581a71e7c"
Archived-At: <https://mailarchive.ietf.org/arch/msg/pim/GhiCy0RQOuArdahBk54fIEH1Wg0>
Subject: Re: [pim] [yang-doctors] 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: Tue, 12 Feb 2019 00:01:03 -0000
Hi Tom, The intended usage of the feature igmp, mld, and global-admin-enable is as what Jan explained. Various combinations of these features provide a certain level of flexibility. Hi Jan, The node "control-plane-protocol" where the augmentation is applied is a list entry, which can already achieve what a presence container can do. I don't think that we need to make igmp and mld presence containers in this case. The mandatory leaves are already possible because of the list statement. Right? Thanks, - Xufeng On Sat, Feb 9, 2019 at 6:13 AM Jan Lindblad <janl@tail-f.com> wrote: > > If there are individual enable leafs, then the combined enable leaf is > > (i) not needed and (ii) very easy to implement and clearly not > > justifying a feature. > > There is one global enable leaf for igmp, another for mld. Both are > conditioned on the same feature. Currently the containers igmp and mld are > not presence containers, so these global switches may have some use. > Presence containers would be better (as pointed out in my review), but that > does not preclude enable leafs. > > /jan > > > > On Sat, Feb 09, 2019 at 09:41:20AM +0100, Jan Lindblad wrote: > >> Jürgen, > >> > >>> They have a feature to control a global enable leaf??? > >> > >> That's right. It's called "global-admin-enable" and is used for nothing > else. But I guess features are cheap. > >> > >> /jan > >> > >> > >> > >>> On Sat, Feb 09, 2019 at 08:48:22AM +0100, Jan Lindblad wrote: > >>>> Tom, > >>>> > >>>>> 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 > >>>> > >>>> As you know I'm not the author of this YANG, but I can explain how it > works. > >>>> > >>>>> 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. > >>>> > >>>> No, this construct will not force an implementation to do both igmp > and mld. There are other features to control which protocols to implement > (feature-igmp, feature-mld). Leaf enable is modeled in a grouping (you can > think of it as a macro). This grouping is used once in igmp and once in > mld, so there are two separate enable leafs if both features are > implemented. > >>>> > >>>> The single global-admin-enable feature does force an implementation > that has both igmp and mld to support the global enable leaf either for > both protocols, or for none of them. It's not possible to support global > enable for one protocol but not the other, when both protocols are > implemented. > >>>> > >>>>> 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 > >>>> > >>>> I have no idea what SSM is, and the module does little to help on > that front. Good point. > >>>> > >>>> /jan > >>>> > >>>>> ----- 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. > >>>>>> > >>>>> > >>>> > >>>> _______________________________________________ > >>>> yang-doctors mailing list > >>>> yang-doctors@ietf.org > >>>> https://www.ietf.org/mailman/listinfo/yang-doctors > >>> > >>> -- > >>> Juergen Schoenwaelder Jacobs University Bremen gGmbH > >>> Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany > >>> Fax: +49 421 200 3103 <https://www.jacobs-university.de/> > >>> > >> > > > > -- > > Juergen Schoenwaelder Jacobs University Bremen gGmbH > > Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany > > Fax: +49 421 200 3103 <https://www.jacobs-university.de/> > > > >
- [pim] Yangdoctors telechat review of draft-ietf-p… Jan Lindblad
- Re: [pim] Yangdoctors telechat review of draft-ie… tom petch
- Re: [pim] Yangdoctors telechat review of draft-ie… Jan Lindblad
- Re: [pim] [yang-doctors] Yangdoctors telechat rev… Juergen Schoenwaelder
- Re: [pim] [yang-doctors] Yangdoctors telechat rev… Jan Lindblad
- Re: [pim] [yang-doctors] Yangdoctors telechat rev… Juergen Schoenwaelder
- Re: [pim] [yang-doctors] Yangdoctors telechat rev… Jan Lindblad
- Re: [pim] [yang-doctors] Yangdoctors telechat rev… Xufeng Liu
- Re: [pim] Yangdoctors telechat review of draft-ie… tom petch
- Re: [pim] Yangdoctors telechat review of draft-ie… Xufeng Liu
- Re: [pim] Yangdoctors telechat review of draft-ie… Xufeng Liu
- Re: [pim] Yangdoctors telechat review of draft-ie… Xufeng Liu