Re: [yang-doctors] [Teas] Yangdoctors early review of draft-ietf-teas-yang-rsvp-10

Tarek Saad <tsaad.net@gmail.com> Thu, 04 July 2019 20:58 UTC

Return-Path: <tsaad.net@gmail.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id C21541201EF; Thu, 4 Jul 2019 13:58:01 -0700 (PDT)
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, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, 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 ucmNmg6LOcxk; Thu, 4 Jul 2019 13:57:59 -0700 (PDT)
Received: from mail-qt1-x833.google.com (mail-qt1-x833.google.com [IPv6:2607:f8b0:4864:20::833]) (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 84F98120165; Thu, 4 Jul 2019 13:57:55 -0700 (PDT)
Received: by mail-qt1-x833.google.com with SMTP id z4so5749228qtc.3; Thu, 04 Jul 2019 13:57:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:thread-topic:thread-index:date:message-id :references:in-reply-to:accept-language:content-language :content-transfer-encoding:mime-version; bh=PsUCDz9OJDXEv8a0yOKSnbEcls4k8LAaoDVSs4LV6Mc=; b=LMMQQm6Bxst7MMn8NbIrHIeKWVuPeJrqUlQbXROWclMgWH3RsqFSleyMOh5LQtPlsb ibYRiHYzBo1/UvNHNZUDW0Sql0EABg12gVHiOp3CKvH8rui6NGsRrSTzT2p9WoBqWHNg zX/gUEfRF+R5JqWF0TlO5XhNjBbICwfnZNztKK9zHckxYGXtm96cczCgeTltK+ktcyCM xiZJa2JlSxdHqujvj3zbUn7pPnGrXlSABNzmDo+C8VKtwj58kHxXtD0CiKI9k7s92qYx YpLiJz83wPmEQ0utGG29zc1KvdXkEjz6tKbVWEKweeUl58J0ePA2Kqmxkwf0LciQL4Cx 4azw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:thread-topic:thread-index :date:message-id:references:in-reply-to:accept-language :content-language:content-transfer-encoding:mime-version; bh=PsUCDz9OJDXEv8a0yOKSnbEcls4k8LAaoDVSs4LV6Mc=; b=ZLfION+S67LZadwxyEU5GCMH9u91ykLDodEX/kXA+Egagw7I5WkRsbo20qHbTGabjT 8kcGklRH7E+5e9ylrix8DNNOBE/1dr/9iOSrG7nT26Lj0ktOrqCFU05/Fzprkx+tKNkV UA8Jq0/KQ1lXDBOqT4D+Hv4ZtbO/3fEG9NRbHbltZV5ogACtxLD7+noBMJDrrZhQLzAi Yat6U/DoPYHMJb/PZlgaeLQNUF4ppl2SPZj8OF29agNqeNR3pMdqny7OSNccFbGqrxWV cffTF39jWaNgHinAK20bjPXJMiZ96lKIB05xHhwn6tB75OmXU9DNPBJlQUrPHGOzR82d oY5A==
X-Gm-Message-State: APjAAAWjcB9TSQmbppRlAPtDXmMGY+VVSeFA4k1ojqeM9zlAVaeb00yk H5lSB+ajlCjpRlB0If/9ArZoJ25PmXs=
X-Google-Smtp-Source: APXvYqyaq8ySlXiKOyrH0ZT/8Qy+p3lhC38AZGxTqbQWKPrF9Zh1ThVbkrNzLcBUhwFbQBfBBU8T1A==
X-Received: by 2002:aed:39e2:: with SMTP id m89mr37854129qte.349.1562273874586; Thu, 04 Jul 2019 13:57:54 -0700 (PDT)
Received: from BN7PR06MB3844.namprd06.prod.outlook.com ([40.97.228.109]) by smtp.gmail.com with ESMTPSA id n10sm2915824qke.72.2019.07.04.13.57.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Jul 2019 13:57:54 -0700 (PDT)
From: Tarek Saad <tsaad.net@gmail.com>
To: Ebben Aries <exa@arrcus.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "ietf@ietf.org" <ietf@ietf.org>, "teas@ietf.org" <teas@ietf.org>, "draft-ietf-teas-yang-rsvp.all@ietf.org" <draft-ietf-teas-yang-rsvp.all@ietf.org>
Thread-Topic: [Teas] Yangdoctors early review of draft-ietf-teas-yang-rsvp-10
Thread-Index: ATYwNTczeRFm9XP/JpKoKpP9j48Bi74E6909
X-MS-Exchange-MessageSentRepresentingType: 1
Date: Thu, 4 Jul 2019 20:57:53 +0000
Message-ID: <BN7PR06MB384418BA97CF178459C200C3FCFA0@BN7PR06MB3844.namprd06.prod.outlook.com>
References: <155692863223.7173.7717533907709205656@ietfa.amsl.com>
In-Reply-To: <155692863223.7173.7717533907709205656@ietfa.amsl.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-Exchange-Organization-SCL: -1
X-MS-TNEF-Correlator:
X-MS-Exchange-Organization-RecordReviewCfmType: 0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/sOj-z3wPsTOstMnaaeKXi3C6MXI>
Subject: Re: [yang-doctors] [Teas] Yangdoctors early review of draft-ietf-teas-yang-rsvp-10
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 04 Jul 2019 20:58:02 -0000

Hi Ebben,

Thanks much for your review and comments.
We have uploaded a new revision ver-11 that addresses your comments.
The diff is at https://tools.ietf.org/rfcdiff?url2=draft-ietf-teas-yang-rsvp-11.txt
Please look inline for on resolution details and do let us know if you have further comments.


´╗┐On 5/3/19, 8:11 PM, "Teas on behalf of Ebben Aries via Datatracker" <teas-bounces@ietf.org on behalf of noreply@ietf.org>; wrote:

    Reviewer: Ebben Aries
    Review result: On the Right Track
    
    2 modules in this draft:
    - ietf-rsvp@2019-02-18.yang
    - ietf-rsvp-extended@2019-02-18.yang
    
    No YANG compiler errors or warnings (pyang 2.0, yanglint 1.1.16, confdc 6.6.1)
    
    Module ietf-rsvp@2019-02-18.yang:
    - Remove WG Chairs from contact information per
      https://tools.ietf.org/html/rfc8407#appendix-B
[TS]: done.

    - Use of 'state' containers.  It is stated in Section 2.3 that 'Derived state
      data is contained under a "state" container...'.  My only comments here are:
      a) Should use caution when using 'state' containers in NMDA compliant
      modules.  Rob put together a nice doc here that I won't reiterate:
      https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ
      Using such nomenclature locks you into r/o nodes only.
[TS]: removed reference to "state" container for consistency.

      b) In some cases, the hierarchy is a bit redundant (statistics/state).
      Other NMDA compliant modules will not introduce another 'state' hierarchy
      for instance (see ietf-interfaces)
[TS]: removed 'state' container.

    - leaf 'packet-len' is abbreviated while most other leafs are not
[TS]: fixed.

    - authentication-key is of type string.  Is this leaf mean to be clear-text?
      There is nothing associated with this type that would imply special
      treatment in handling.
[TS]: Yes, this is entered in clear text. This is analogous with the 'keystring' defined in rfc8177. We discussed this amongst authors, and we think there may be a gap in YANG/NMDA on retrieving applied secret string configuration (e.g. passwords). I'm not sure if this was discussed before for other protocols or if there are any extensions in NMDA or YANG to allow retrieving/displaying it e.g. "****" instead of the clear text that was entered. 

    - crypto-algorithm: Are all identities from ietf-key-chain supported?
[TS]: We are not adding any restriction on the support of any of the algorithms defined in ietf-key-chain standard model. However, a vendor model can apply a strict check using a deviation.

    - Pluralization on counters:  e.g. 'in-error' vs. 'in-errors'. Maintain
      consistency with other modules (see ietf-interfaces)
[TS]: done.

    - Normative references are missing for some of the modules imported.  These
      must be added per https://tools.ietf.org/html/rfc8407#section-3.9
[TS]: done.
    
    Module ietf-rsvp-extended@2019-02-18.yang:
    - Remove WG Chairs from contact information per
      https://tools.ietf.org/html/rfc8407#appendix-B
[TS]: done.

    - Use of 'state' containers.  It is stated in Section 2.3 that 'Derived state
      data is contained under a "state" container...'.  My only comments here are:
      a) Should use caution when using 'state' containers in NMDA compliant
      modules.  Rob put together a nice doc here that I won't reiterate:
      https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ
      Using such nomenclature locks you into r/o nodes only.
      b) In some cases, the hierarchy is a bit redundant (statistics/state).
      Other NMDA compliant modules will not introduce another 'state' hierarchy
      for instance (see ietf-interfaces)
    - Pluralization on counters:  e.g. 'in-error' vs. 'in-errors'. Maintain
      consistency with other modules (see ietf-interfaces)
    - 9 features are defined with no 'if-feature' statements.  Where are these
      feature conditions meant to be used?
[TS]: The defined features are for extended RSVP feature support that other modules (outside the scope of this document) may choose to use to control access to specific data nodes.

    - Normative references are missing for some of the modules imported.  These
      must be added per https://tools.ietf.org/html/rfc8407#section-3.9
[TS]: Added.
    
    General comments on the draft/modules:
    - The state container and list key designs appear very familiar to that of
      OpenConfig modules however not consistent with IETF module design.
      Consistency is key from each producing entity (e.g. IETF in this case)
[TS]: addressed and removed instances of 'state' container.

    - The draft mentions RPCs and Notifications however none are defined within
      the modules
[TS]: We added two new RPCs to the draft:
- one to clear rsvp session (single or all)
- one to clear rsvp neighbor (single or all)

    - Section 2.3: Has examples of RPCs and Notifications that are non-existant in
      the modules
[TS]: We added a new section for notifications and suggested leveraging I-D.ietf-netconf-subscribed-notifications] and I-D.ietf-netconf-yang-push in general to register on specific data nodes.

    - Abstract: s/RVSP/RSVP/
    - Abstract: s/remote procedural/remote procedure/
    - Section 2: s/extensiion/extension/
    - Section 4: Update the security considerations section to adhere to
      https://tools.ietf.org/html/rfc8407#section-3.7 and
      https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines
[TS]: Fixed above.

    - Various (missing) wording/pluralization cleanup throughout that I'll defer
      to the RFC Editor.  A once over proofread should solve this.
[TS]: in progress.

Regards,
Tarek    
    _______________________________________________
    Teas mailing list
    Teas@ietf.org
    https://www.ietf.org/mailman/listinfo/teas