Re: [Netconf] mbj's WGLC comments on netconf-event-notifications-08
Martin Bjorklund <mbj@tail-f.com> Mon, 11 June 2018 07:12 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 60990130E0F for <netconf@ietfa.amsl.com>; Mon, 11 Jun 2018 00:12:55 -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 VGgd0HkW_3EN for <netconf@ietfa.amsl.com>; Mon, 11 Jun 2018 00:12:52 -0700 (PDT)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id C2125130DFE for <netconf@ietf.org>; Mon, 11 Jun 2018 00:12:52 -0700 (PDT)
Received: from localhost (unknown [173.38.220.61]) by mail.tail-f.com (Postfix) with ESMTPSA id 88EEB1AE01AA; Mon, 11 Jun 2018 09:12:49 +0200 (CEST)
Date: Mon, 11 Jun 2018 09:12:48 +0200
Message-Id: <20180611.091248.42505202577647987.mbj@tail-f.com>
To: evoit@cisco.com
Cc: netconf@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <0e6e711ab209437e881335756c268e07@XCH-RTP-013.cisco.com>
References: <20180316.145936.984795473579499350.mbj@tail-f.com> <20180608.163924.639364006777002795.mbj@tail-f.com> <0e6e711ab209437e881335756c268e07@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/kz9RJj4dfqCe5Vw6hAKnJ_mrync>
Subject: Re: [Netconf] mbj's WGLC comments on netconf-event-notifications-08
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: Mon, 11 Jun 2018 07:12:55 -0000
Hi, Thanks for addressing my comments. Some follow-ups inline. "Eric Voit (evoit)" <evoit@cisco.com> wrote: > > From: Martin Bjorklund, June 8, 2018 10:39 AM > > > > Hi, > > > > I haven't seen any reply to this WGLC review. > > Hi Martin, > > Below. And with changes reflected in: > https://github.com/netconf-wg/notif-netconf/blob/master/draft-ietf-netconf-netconf-event-notifications-10.txt Thanks. I have some additional comments on this version, but I don't know if I should wait until you have published a new version or not. > > Martin Bjorklund <mbj@tail-f.com> wrote: > > > Hi, > > > > > > Here are my WGLC comments on > > > draft-ietf-netconf-netconf-event-notifications-08 > > > > > > > > > o On p. 3, theres a missing " (which messes up the colors in the > > > emacs mode I use) > > > > > > OLD: > > > > > > SHOULD NOT" > > > > > > NEW: > > > > > > "SHOULD NOT" > > Your text is reflected in git version > > > > There's another one on p. 5: > > > > > > OLD: > > > > > > responses to an establish-subscription request) or "modify- > > > subscription-error-datastore (for error responses to a modify- > > > > > > NEW: > > > > > > responses to an establish-subscription request) or "modify- > > > subscription-error-datastore" (for error responses to a modify- > > Based on comments with Kent, this has been turned into a more > descriptive table format. > > > > o Section 3 > > > > > > As I have noted before, you mustn't require the :interleave > > > capability to be supported. That capability is for 5277 only. This > > > new mechanism *requires* that rpc's can be sent while there are > > > active subscriptions, so there is no need for a capability. > > > > > > Remove this section. > > Done. > > > > o Section 4 > > > > > > What is the reason for not allowing 5277 subscriptions on the same > > > session as these new subscriptions? > > The biggest reason is that existing 5277 implementations can safely > assume that only one subscription can be established on a NETCONF > session. And therefore all returned <notification> elements will > belong to that subscription. Even if the subscriber doesn't support > subscribed-notifications or subsequently send an > <establish-subscription>, without this constraint a configured > subscription to the same receiver could inject notifications on the > RFC-5277's NETCONF transport session. (E.g., a <subscription-started> > state change notification.) > > > > AFAICT this should just work fine, and no special rule is needed. > > It likely could on a well behaved RFC-5277 implementation. But > caution is needed as RFC5277 implementations hadn't previously needed > to worry about such co-existence. I am ok with this. > > > o Section 5 > > > > > > You write: > > > > > > A NETCONF publisher MUST support XML encoding of RPCs and > > > Notifications. > > > > > > This is already specificed in RFC 6241. You are not changing this, > > > so this sentence should be removed. > > Previously changed this text to indicate the mandatory support of the > "encode-xml" feature. IMO this shows that the decoupling of transport and encoding is not ideal. It seems to imply that if I implement this draft I have to support XML also for RESTCONF? If the answer is "no" then the encoding should not be decoupled from the transport. > > > o Section 5 > > > > > > You write: > > > > > > A NETCONF publisher supporting > > > [I-D.draft-ietf-netconf-subscribed-notifications] MUST support the > > > "NETCONF" event stream identified in that draft. > > > > > > This is already specificed in that draft. You are not changing this, > > > so this sentence should be removed. > > In subscribed-notifications, the "NETCONF" stream is defined, but it > is not mandatory support. Instead, just that the stream name is > reserved. There will be IoT clients out there which don't need the > NETCONF stream. Ok. > > > o Section 6.2 > > > > > > (editorial, and clarified) > > > > > > OLD: > > > > > > For a configured subscription, there is no guarantee a transport > > > session is currently in place with each associated receiver. In > > > cases where a configured subscription has a receiver in the > > > connecting state and the protocol configured as NETCONF, but no > > > NETCONF transport session exists to that receiver, the publisher MUST > > > initiate a transport session via NETCONF call home [RFC8071], section > > > 4.1 to that receiver. Until NETCONF connectivity is established and > > > a subscription-started state change notification is successfully > > > sent, that receiver MUST remain in a status of either "connecting" or > > > "timeout". > > > > > > NEW: > > > > > > For a configured subscription, there is no guarantee a transport > > > session is currently in place with each associated receiver. In > > > cases where a configured subscription has a receiver in the > > > "connecting" state (see section 2.5.1 of [RFCXXXX] and the protocol > > > is configured as NETCONF, but no > > > NETCONF transport session exists to that receiver, the publisher MUST > > > initiate a transport session via NETCONF call home [RFC8071], section > > > 4.1 to that receiver. Until NETCONF connectivity is established and > > > a "subscription-started" state change notification is successfully > > > sent, that receiver MUST remain in either the "connecting" or the > > > "timeout" state. > > The git version is now... > > For a configured subscription, there is no guarantee a transport > session is currently in place with each associated receiver. In cases > where a configured subscription has a receiver in the "connecting" > state as described in > [I-D.draft-ietf-netconf-subscribed-notifications], section 2.5.1, and > the "transport" for that subscription is "NETCONF", but no NETCONF ^^^^^^^^^ This should be "nsn:netconf" (an identity defined in this draft). > transport session exists to that receiver (or all existing NETCONF > transport sessions are currently supporting [RFC5277] subscriptions), > then the publisher MUST initiate a transport session via NETCONF call > home [RFC8071], section 4.1 to that receiver. Until NETCONF > connectivity is established and a "subscription-started" state change > notification is successfully sent, that receiver MUST remain in either > the "connecting" or the "timeout" state. > > > > > OLD: > > > > > > If the call home fails because the publisher receives receiver > > > credentials which are subsequently declined per [RFC8071], > > > Section 4.1, step S5 authentication, then that receiver MUST be > > > assigned a "timeout" status. > > > > > > NEW: > > > > > > If the call home fails because the publisher receives receiver > > > credentials which are subsequently declined per [RFC8071], > > > Section 4.1, step S5 authentication, then that receiver MUST be > > > placed in the "timeout" state. > > Your text is reflected in git version > > > > OLD: > > > > > > If the call home fails to establish for any other reason, the > > > publisher MUST NOT progress the receiver to the "active" state. > > > Additionally, the publisher SHOULD place the receiver into a > > > "timeout" status after a predetermined number of either failed call > > > home attempts or NETCONF sessions remotely terminated by the > > > receiver. > > > > > > NEW: > > > > > > If the call home fails to establish for any other reason, the > > > publisher MUST NOT progress the receiver to the "active" state. > > > Additionally, the publisher SHOULD place the receiver into the > > > "timeout" state after a predetermined number of either failed call > > > home attempts or NETCONF sessions remotely terminated by the > > > receiver. > > Your text is reflected in git version > > > > OLD: > > > > > > NETCONF Transport session connectivity SHOULD be verified via > > > Section 4.1, step S7. > > > > > > NEW: > > > > > > NETCONF Transport session connectivity SHOULD be verified as > > > described in [RFC8071], Section 4.1, step S7. > > Update made > > > > o Section 7 > > > > > > You write: > > > > > > Notification messages transported over NETCONF will be identical in > > > format and content to those encoded using one-way operations defined > > > within [RFC5277], section 4. > > > > > > "identical in content"? What does this section tell me? > > Tweaked the words to: > > Notification messages transported over the NETCONF protocol will use > the one-way operations defined within [RFC5277], section 4. I would prefer to be more explicit: Notification messages transported over the NETCONF protocol will use the "notification" message defined in [RFC5277], section 4. > > > o Section 8 > > > > > > o "error-app-tag" with the value being a string that corresponds to > > > an identity associated with the error, as defined in > > > [I-D.draft-ietf-netconf-subscribed-notifications] section 2.4.6 > > > > > > This needs to explained better. See also my WGLC comments on the > > > other drafts. > > Current git version includes your requested updates, such as which > base identity to use for each RPC, and the JSON encoding format for > the identities. Hmm, I think I probably have to see updated versions of all three drafts to see that it is consistently explained. But wasn't the idea to have a "reason" leaf in each error-info structure that contains this identity? If so, shouldn't we remove this "error-app-tag" handling? > > > Also, I don't think the 5th bullet is complete; it doesn't mention > > > "establish-subscription-error-stream" for example. > > There is a whole table on this is the current git version. > > > > What is this section trying to tell me that isn't already said, e.g. > > > in section 3.8 of the push draft. Maybe the other drafts should be > > > less specific and all such text moved here. As it is now it is not > > > quite clear. > > Alex is removing the text from the yang-push draft. > > > > o Section 8 > > > > > > You write: > > > > > > Note that "error-path" does not need to be included with the "rpc- > > > error" element, as subscription errors are generally not associated > > > with nodes in the datastore but with the choice of RPC input > > > parameters. > > > > > > This is a misconception how error-path works. Please remove this > > > sentence. For info, check RFC 6241. > > Removed > > > > o Appendix A.2.1 > > > > > > I think it is useful to show an example of something that is easily > > > missed; that notifications can be sent at any time: > > > > > > I suggest: > > > > > > OLD: > > > > > > | establish-subscription | > > > |----------------------------->| > > > | RPC Reply: OK, id = 23 | > > > |<-----------------------------| > > > | | > > > | | > > > | notification message (for 22)| > > > |<-----------------------------| > > > > > > NEW: > > > > > > | establish-subscription | > > > |----------------------------->| > > > | notification message (for 22)| > > > |<-----------------------------| > > > | RPC Reply: OK, id = 23 | > > > |<-----------------------------| > > > | | > > > | | > > > | notification message (for 22)| > > > |<-----------------------------| > > Added > > > > o Appendix A.2.1 > > > > > > The example in Figure 3 is not correct. > > > > > > The example in Fixgure 5 is not correct wrt namespace. > > > > > > Hmm, it seems many examples are wrong. I strongly suggest that you > > > set up automatic testing of all your examples. If you for some > > > reason don't do that, please let me (and the WG) know so that we can > > > validate all examples in detail manually. Meanwhile, I will not > > > check all examples. > > Einar has since built an automated testbed for the examples. Results > are included in independent directories in the git repository: > > https://github.com/netconf-wg/notif-netconf > > Eric /martin
- Re: [Netconf] mbj's WGLC comments on netconf-even… Eric Voit (evoit)
- Re: [Netconf] mbj's WGLC comments on netconf-even… Martin Bjorklund
- Re: [Netconf] mbj's WGLC comments on netconf-even… Martin Bjorklund
- Re: [Netconf] mbj's WGLC comments on netconf-even… Martin Bjorklund
- Re: [Netconf] mbj's WGLC comments on netconf-even… Eric Voit (evoit)
- Re: [Netconf] mbj's WGLC comments on netconf-even… Kent Watsen
- [Netconf] mbj's WGLC comments on netconf-event-no… Martin Bjorklund