Re: [Netconf] comments on draft-ietf-netconf-subscribed-notifications-12

Martin Bjorklund <mbj@tail-f.com> Fri, 08 June 2018 09:02 UTC

Return-Path: <mbj@tail-f.com>
X-Original-To: netconf@ietfa.amsl.com
Delivered-To: netconf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E40E8130E35 for <netconf@ietfa.amsl.com>; Fri, 8 Jun 2018 02:02:13 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.901
X-Spam-Level:
X-Spam-Status: No, score=-1.901 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
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 ubkUp-5v58Xd for <netconf@ietfa.amsl.com>; Fri, 8 Jun 2018 02:02:07 -0700 (PDT)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id 63ACE130E4F for <netconf@ietf.org>; Fri, 8 Jun 2018 02:02:07 -0700 (PDT)
Received: from localhost (unknown [173.38.220.61]) by mail.tail-f.com (Postfix) with ESMTPSA id 835A81AE027A; Fri, 8 Jun 2018 11:02:06 +0200 (CEST)
Date: Fri, 08 Jun 2018 11:02:05 +0200 (CEST)
Message-Id: <20180608.110205.217184993423575402.mbj@tail-f.com>
To: evoit@cisco.com
Cc: netconf@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <381e3937e0054984812ea69de97c7659@XCH-RTP-013.cisco.com>
References: <20180607.173213.944977899308364449.mbj@tail-f.com> <381e3937e0054984812ea69de97c7659@XCH-RTP-013.cisco.com>
X-Mailer: Mew version 6.7 on Emacs 24.5 / Mule 6.0 (HANACHIRUSATO)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/NTvIjEY5cjYvqVJb_JcpiKH86ys>
Subject: Re: [Netconf] comments on draft-ietf-netconf-subscribed-notifications-12
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.26
Precedence: list
List-Id: Network Configuration WG mailing list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 08 Jun 2018 09:02:15 -0000

Hi,


"Eric Voit (evoit)" <evoit@cisco.com> wrote:
> Hi Martin,
> 
> Updated file at:
> https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-13.txt 
> 
> > From: Martin Bjorklund, June 7, 2018 11:32 AM

[...]

> > o  Section 2.3
> > 
> >   s/"QoS" feature/"qos" feature/  (twice)

You forgot this one.  The feature in the YANG model is "qos", not
"QoS".


> > o  Section 4
> > 
> >   You have:
> > 
> >           leaf pushed-notifications {
> >             type yang:counter64;
> >             config false;
> >             description
> >               "Operational data which provides the number of update
> >                notification messages pushed to a receiver.";
> >           }
> > 
> >   Is this a left-over from previous versions?  This document doesn't
> >   define YANG push, and it doesn't define the term "update
> >   notification message".
> > 
> >   Should it simply be notifications-sent, with description "A count of
> >   the number of notifications sent to the receiver."?
> 
> Text wasn't left over.   But to make it as clean as possible, I changed to:
> 
>           leaf count-sent {
>             type yang:counter64;

You may want to use yang:zero-based-counter64.

>             config false;
>             description
>               "The number of event records sent to the receiver.  The 
>               count is initialized when a dynamic subscription is 
>               established, or when a configured subscription 
>               transitions to the valid state.";
>           } 
>           leaf count-excluded {
>             type yang:counter64;

>From the description below, you definately want
yang:zero-based-counter64.

>             config false;
>             description
>               "The number of event records explicitly removed either 
>               via an event stream filter or an access control filter so 
>               that they are not passed to a receiver.  This count is 
>               set to zero each time 'count-sent' is initialized.";
>           }


Ok with this, but I would prefer more descriptive leaf names.
event-records-sent and event-records-excluded perhaps.


> > o  Section 4
> > 
> >     leaf encoding  should have "if-feature configured;"
> > 
> >     (it is dependent on ../transport, which has the if-feature)
> 
> The encoding can be set by RPC, so the issue is with the constraint.
> > (E.g., when HTTP is used, JSON and CBOR might be encodings.)
> 
> Maybe the way to address this (as can be seen in the draft) is to
> > enhance the constraint to: 
> 
> When  ' not(boolean(../transport))  or derived-from(../transport, "sn:configurable-encoding")'

You don't need the boolean(...):

  when 'not(../transport) or derived-from(...)';



/martin



> And add an identity error to cover when an attempt is made establish
> a subscription with an unsupported encoding:
> 
>   identity encoding-not-supported {
>     base establish-subscription-error;
>     description
>       "Unable to encode notification messages in the desired format.";
>   }
> 
> 
> >     Also, the description for this leaf is in -12:
> > 
> >         "The type of encoding for the subscribed data. If not
> >         included as part of the RPC, the encoding MUST be set by the
> >         publisher to be the encoding used by this RPC.";
> > 
> >    and in the not-yet-published -13 (from github):
> > 
> >         "The type of encoding for the subscribed data.   If not
> >         included, the encoding used will be the default for one
> >         encoding expected with a transport.";
> > 
> >    I can't parse from "the encoding used ...".
> 
> Made it:
> 
> "The type of encoding for notification messages.   For a dynamic subscription, if not included as part of an establish-subscription RPC, the encoding will be populated with the encoding used by that RPC.  For a configured subscription, if not explicitly configured the encoding with be the default encoding for an underlying transport.";
> 
> Thanks,
> Eric
> 
> > /martin
> > 
> > _______________________________________________
> > Netconf mailing list
> > Netconf@ietf.org
> > https://www.ietf.org/mailman/listinfo/netconf
>