Re: [netconf] [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-21
Martin Bjorklund <mbj@tail-f.com> Wed, 23 January 2019 08:31 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 77F16130E59; Wed, 23 Jan 2019 00:31:43 -0800 (PST)
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 qk40rQH-EHl2; Wed, 23 Jan 2019 00:31:40 -0800 (PST)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id C2739128D52; Wed, 23 Jan 2019 00:31:39 -0800 (PST)
Received: from localhost (unknown [173.38.220.45]) by mail.tail-f.com (Postfix) with ESMTPSA id 7CE961AE028C; Wed, 23 Jan 2019 09:31:36 +0100 (CET)
Date: Wed, 23 Jan 2019 09:31:35 +0100
Message-Id: <20190123.093135.970106755262082435.mbj@tail-f.com>
To: evoit@cisco.com
Cc: andy@yumaworks.com, yang-doctors@ietf.org, netconf@ietf.org, draft-ietf-netconf-subscribed-notifications.all@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <2ff23fa29204403489b6d69fdc5ecd74@XCH-RTP-013.cisco.com>
References: <0181b187e85a4ab1a41e5adb65d64d4e@XCH-RTP-013.cisco.com> <CABCOCHQMRxX0f3e0x49N7-fwoxFbt-kKkxyouCQaEJxKSGNe1A@mail.gmail.com> <2ff23fa29204403489b6d69fdc5ecd74@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="utf-8"
Content-Transfer-Encoding: base64
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/CBvijQVi4g2cf116o4DU2pYId0M>
Subject: Re: [netconf] [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-21
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETCONF WG 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: Wed, 23 Jan 2019 08:31:44 -0000
Hi, "Eric Voit (evoit)" <evoit@cisco.com> wrote: > Hi Andy, > > Looking at your proposal... My reading is that it takes the transport > specific error info contained in > draft-ietf-netconf-netconf-event-notifications section 7, and then > replicates that info within 12 separate description objects of the > transport independent ietf-subscribed-notifications.yang. The value > you are asserting is that RFCs containing YANG models containing the > rpc-stmt have traditionally document the mandatory-to-implement > "error-tag" field within the model. And presumably you are concerned > that developers should not have to look elsewhere for this > information. I think that maybe there are two separate issues here. The first issue is that for each error identity defined, there needs to be a mapping to the protocol-specific error handling. Andy suggests that this info is added to this document, but currently this information is available in the protcol-mapping documents (netconf-notif and restconf-notif). Personally, I think that the current split of text between documents is fine. The second issue is that currently, both netconf-notif and restconf-notif say that *all* these errors use the error-tag "operation-failed". Essentially it means that we bypass the error handling in the protocols. As Andy points out below, the error "insufficient-resources" should be mapped to "resource-denied" in NETCONF and RESTCONF (they mean the same thing). So it might make sense to carefully go through the list of errors and map them to the correct error-tag (but specifiy this in the transport drafts). /martin > > If the YANG doctors require this, it can be inserted. A similar text > change would be needed for quite a few error identities within YANG > Push. Personally I don’t like that YANG models should be required to > embed this information. But I will make the change if you really want > this, and nobody else objects. > > Other than that, I am not aware of any other open issues in the YANG > Doctor review. Do you know of anything else? > > Eric > > > From: Andy Bierman, January 21, 2019 2:26 PM > > Hi, > > I think the error-tag issue can be resolved by including 1 extra > sentence in each error identity. > I know this is NETCONF and RESTCONF centric but those are the only 2 > standard protocols > supported for the YANG language right now. > > If the 'error-tag' field is used in error reporting, > then the value '<correct error-tag>' MUST be used. > > For example: > > > OLD: > > identity insufficient-resources { > base establish-subscription-error; > base modify-subscription-error; > base subscription-suspended-reason; > description > "The publisher has insufficient resources to support the > requested subscription. An example might be that allocated CPU > is too limited to generate the desired set of notification > messages."; > } > > > NEW: > > identity insufficient-resources { > base establish-subscription-error; > base modify-subscription-error; > base subscription-suspended-reason; > description > "The publisher has insufficient resources to support the > requested subscription. An example might be that allocated CPU > is too limited to generate the desired set of notification > messages. If the 'error-tag' field is used in error reporting, > then the value 'resource-denied' MUST be used."; > } > > > Andy > > > On Fri, Jan 18, 2019 at 11:53 AM Eric Voit (evoit) > <evoit@cisco.com<mailto:evoit@cisco.com>> wrote: > Hi Andy, > > Thanks. I have incorporated items where there was agreement. I have > removed the items below where you were ok. > > Remaining below are the open items, with responses. > > > > Should be clear somewhere that > > > suspend is for CPU and other resources, and NACM not considered > > > to be a resource. > > > > If NACM is active, it needs to be followed. The text we have for NACM > > is in > > Section 5.4. Do you see something else specific to subscription > > suspension > > needed here? (Maybe I am not getting your point.) > > > > No -- OK to leave NACM as terminate-if-loss-of-rights > > (Is there an error identity for this event?) > > The identity which applies here is "stream-unavailable". This is the > same identity which would be used if a subscriber had never sufficient > permissions in the first place. I don't believe we would want to > return an identity specific to when NACM when permissions have just > been changed. > > > > I3) sec 2.1 para 6: > > > Event records MUST NOT be delivered to a receiver in a different > > > order than they were placed onto an event stream. > > > > > > -- does this apply to subscription-state? Think not, they are not events > > > placed in event stream. > > > > Agree that they are not on the event stream. So they do not violate > > this > > requirement. > > > > Additionally there is supporting text in "Section 2.7: subscription > > state > > notifications", including... > > > > " Instead, they are inserted (as defined in this section) within the > > sequence of > > notification messages sent to a particular receiver." > > > > > Need to allow ended or suspended to be sent > > > head-of-line whenever state changes > > > > I am not sure that suspended should always be sent head-of-line. > > Consider > > that implementation might want to let the existing queue of filtered > > event > > records be sent if is filter complexity causing the CPU issue. That > > could be > > different than if it is a bandwidth issue driving the suspension, and > > you > > definitely want the 'subscription-suspended' to be placed at the head > > of line. > > > > > > It is up to the publisher to decide when to stop sending events on a > > subscription. > > Obviously the publisher cannot wait until the subscription is idle. > > The reason it is getting suspended is it is far from idle > > > > So also up to the publisher wrt/ what to do with any events that have > > not > > been delivered yet on a subscription. Could delete them or save them > > for > > when more bandwidth available (for example) > > Agree fully with this. Is there text required in the draft here? > > ... > > Beyond that it is up to the implementation to decide if some > > un-transmitted > > queue of event records should be flushed and reprocessed based on the > > modification. I do not expect this would popular, as a replay > > subscription could > > accomplish this same functional need. > > > > Agreed that an implementation can drop at any time and increment the > > appropriate counters. It will try to to do this, but no requirements > > except > > maybe subscription events like 'replay-completed' cannot be dropped > > Have put a minor tweak into Section 2.7: > > [old] subscription state change notifications cannot be filtered out > > [new] subscription state change notifications cannot be dropped or > filtered out > > ... > > Thinking more on your point, it might be worth tweaking a couple words > > to > > allow for head-of-line placement of "subscription-suspended". > > > > "Subscribed event records queued for sending after the issuance of > > this > > subscription state change notification may now be sent." > > > > Are you good with this suggested change? > > > > Not sure -- it needs to be clear that subscription-suspended is the > > last event sent before suspending and subscription-resumed is > > the first event sent after transition from suspended to active. > > The next event could also be subscription-terminated. > > I do think this possibility is covered in the text. For Section 2.7.4 > subscription-suspended the current text is: > > "No further notification will be sent until the subscription resumes > or is terminated." > > And Section 2.7.5 subscription-resumed says": > "Subscribed event records generated after the issuance of this > subscription state change notification may now be sent." > > Based on the discussion, I can make it: > > "Subscribed event records are again permitted to be sent following > this subscription state change notification." > > Is this sufficient for you? > > ... > > > I4) sec 2.4.6: RPC Failures > > > -- concern about a subscription-specific error reporting system > > > must make sure protocol error reporting system is used correctly > > > > Yes. We have done our best to integrate with the embedded NETCONF and > > RESTCONF mechanisms. There is much additional information in the > > transport > > drafts here. > > > > > -- The error-tag value needs to be identified for each 'reason' identity > > > > This is done in the transport drafts. E.g., see > > draft-ietf-netconf-netconf-event- > > notifications Section 7 > > > > I do not agree this is a good idea. > > Each error identity should simply state the required "error-tag" > > that is associated with the error. This is expected of protocol > > operations > > that are added to NETCONF and RESTCONF. > > In draft-ietf-netconf-netconf-event-notifications, section 7, the > required "error-tag" is identified as "operation-failed". If we > instead placed that "error-tag" information in the YANG model, then we > have tied the YANG model to the RESTCONF and NETCONF transports. > > > Both NETCONF and RESTCONF use a compatible error reporting data > > structure. > > The "error-tag" is used in both of them. IMO client developers do not > > want a different set of error codes for the same error conditions. > > draft-ietf-netconf-restconf-notif Section 3.3 also requires an > "error-tag" node of "operation-failed". So we used the transport > drafts rather than the YANG model to support the same error codes for > the same error conditions. > > > I agree that transport drafts could define their own error identities, > > which would document the expected error-tag there. > > > > > > > 2. "modify-subscription-stream-error-info": This MUST be returned > > > with the leaf "reason" populated if an RPC error reason has not > > > been placed elsewhere within the transport portion of a failed > > > "modify-subscription" RPC response. This MUST be sent if hints > > > > > > -- all 3 paragraphs like this; unclear what "placed elsewhere" > > > text means; not appropriate for MUST; > > > > Instead of "placed elsewhere", how about: "placed in subscription > > transport > > document defined object". Would this be sufficient? > > > > No -- NETCONF and RESTCONF have well-defined error reporting. > > The server requirements for this error reporting must be documented. > > > > I agree with the following approach: > > - each operation MUST identify the error-tags that are expected for > > various error conditions (such s is done in RFC 6241) > > - the server MUST return the specified error-tags. If a condition not > > - explicitly > > defined then the server MUST pick the appropriate error-tag from RFC > > 6241 > > - the server MAY include the specified rc:yang-data in the <error-info> > > - data > > structure > > - the server MUST use the appropriate rc:yang-data to report hints > > - for protocols other than NETCONF and RESTCONF, they can map error-tag > > - or > > ignore it, > > but the document defining the protocol operation MUST provide > > Functionally, everything you ask for is fully covered when you include > consider draft-ietf-netconf-netconf-event-notifications (section 7) > and draft-ietf-netconf-restconf-notif (section 3.3). > > My read of the issue is that you believe "error-tag" must be specified > in the YANG model. I believe that "error-tag" shouldn't be in the > YANG model because that would tie the model to a transport type. > > Any thoughts on how we might close this? If absolutely required I > could place a new comment line in the YANG model under > /* Identities for RPC and Notification errors */ > > The comment would be something like: > /* When used with NETCONF and RESTCONF RPCs: > "error-type" node to be used is "application" > "error-tag" must be "operation-failed". */ > > This seems incongruous. Just throwing it out as a suggestion. > > > In any case, the -v21 wording results from the attempted balancing the > > WG > > requests for: > > * merging with transport protocol error mechanisms > > * WG leadership guidance to provide requirements for transport documents > > > > > Only 3 fields seem > > > to be relevant (error-tag, error-app-tag, error-info). > > > Protcol operations are expected to document server requirements > > > for these 3 fields, if applicable. Only the error-tag > > > is mandatory-to-use. > > > > Hopefully these are covered sufficiently when this document is coupled > > with > > the NETCONF and RESTCONF Notif transport documents. For other > > transports, > > the tags you identify about would not be applicable. > > > > > -- the error assignments are extremely specific. e.g., it is not > > > possible for <kill-subscription> to fail with an > > > 'insufficient-resources' error; > > > > This is the intent of the base specification, e.g., we don't believe a > > kill- > > subscription should fail for an insufficient-resources reason. But > > vendors might > > desire more specificity. As a result is certainly ok for vendor > > implementations > > to add new error identities. > > > > IMO anything can fail for insufficient resources. That is very > > implementation- > > specific. > > Instead of implementation specific I would call it application > specific. Right now we don't have a catch-all error-identity of > 'other-error'. We preferred that error conditions beyond the current > ones listed could be included by vendors as needed. Further > deployment experience could result in new error identities surfacing > for standardization should this draft catch on. > > > > Do not agree that scoping each > > > identity to specific RPC operations is a good idea. > > > > This level of specificity was not the author's original plans. Nor > > was this level of > > specificity part of earlier draft versions up through -v08. However > > members of > > the WG made it clear that such specificity was necessary for draft > > progression. > > > > > -- how are errors in these parameters reported for configured > > > subscriptions when <edit-config> is the RPC that has the error? > > > How are the yang-data structs used for edit-config or commit errors? > > > > None of these yang-data structures are specified for use with > > <edit-config> > > operations. For <edit-config>, the change to a configured > > subscription would > > be written to the datastore if it were semantically valid. At this > > point the > > subscription enters the [evaluate] points of Figure 8. Issues from > > this point out > > would be reported with a vendor specific construct such as SYSLOG. > > > > So how are hints reported for configured subscriptions? > > There is nothing in the specification which requires this. An > implementation could choose to place these in some form of SYSLOG. > ... > > > I6) sec 2.5, para 3: > > > > > > On a receiver of a > > > configured subscription, support for dynamic subscriptions is > > > optional except where replaying missed event records is required. > > > > > > -- confusing because text in 1.3: > > > Note that there is no mixing-and-matching of dynamic and configured > > > operations on a single subscription. Specifically, a configured > > > -- clarify the receiver may have multiple subscriptions here > > > -- not clear what "except where replaying..." text means > > > > How about the following tweak: > > > > "On a receiver of a configured subscription, support for dynamic > > subscriptions > > is optional. However if replaying missed event records is required > > for a > > configured subscription, support for dynamic subscription is highly > > recommended. In this case, a separate dynamic subscription can be > > established > > to retransmit the missing event records." > > > > OK > > Change made. > > > > I7) leaf stream-xpath-filter: [multiple uses] > > > > > > The expression is evaluated in the following XPath context: > > > > > > o The set of namespace declarations is the set of prefix > > > and namespace pairs for all YANG modules implemented > > > by the server, where the prefix is the YANG module > > > name and the namespace is as defined by the > > > 'namespace' statement in the YANG module. > > > > > > -- This prefix processing is not done anywhere else in NETCONF > > > or RESTCONF. IMO a bad precedent. Only the XML prefixes > > > should be required for processing of XML encoding. YANG > > > module prefixes are not required to be unique, unlike > > > the prefix mappings in XML > > > > This text was proposed by Martin as a result of the "xpath expressions > > in JSON" > > thread last October in NETMOD. > > > > I am happy to incorporate whatever text is appropriate. I was hoping > > that the > > suggested text was sufficient for now. Kent has already incorporated > > this as an > > issue for yang-next > > https://github.com/netmod-wg/yang-next/issues/55 > > So hopefully there is no final precedent being claimed. > > > > I do not agree that this YANG module should define a new way to encode > > XPath > > into XML instance documents. This will require significant changes to > > server > > implementations. YANG module prefixes are not even required to be > > unique > > so the set of prefixes used by the server in XML instance documents > > may be > > different, > > since it must be unique. > > See next note > > > > -- NMDA allows the same module to appear in multiple module-sets > > > and different in each datastore. This text about "implemented by > > > the server" does not work for NMDA > > > > I am happy to adopt whatever text meets YANG doctor approval. Can you > > suggest? > > > > > > Remove all text about YANG prefixes and continue using XML encoding > > without > > modification > > As a different YANG doctor has required the current text modification, > I believe this is a blocker. What is the process for YANG model > reviews in such a case. I am happy to accept whatever here. Any > suggestions on next steps? > > ... > > > -- there should be an example of a configurable encoding provided > > > > I am happy to enhance the definition YANG model's identity definition > > of > > "configurable-encoding". I could do this by adding the following > > additional text > > to the description: "An example of a configurable encoding might be a > > new > > identity such as 'encode-cbor'. Such an identity could use > > 'configurable- > > encoding' as its base. This would allow a dynamic subscription > > encoded in JSON > > [RFC-8259] to request notification messages be encoded via CBOR [RFC- > > 7049]. Further details for any specific configurable encoding would > > be explored > > in a transport document based on this specification." Does this meet > > your ask? > > > > > > OK > > Added > > > > I11) extension subscription-state-notification { > > > > > > This statement is not for use > > > outside of this YANG module."; > > > > > > -- this text should be removed. There is no value in limiting > > > the scope of this extension. It prevents even this WG from > > > creating a module that uses the extension again. > > > > This was the subject of significant debate in the WG. The authors did > > not want > > this restriction either. > > > > To be allowed to progress the document, we inserted the document. If > > this > > really is mandatory-to-remove from a YANG doctor point-of-view, what > > is the > > process for quick closure on this issue between WG leadership and the > > YANG > > doctors? > > > > > > The YANG language makes no restrictions about exporting statements. > > I guess I missed that debate so I will just say OK and wonder what > > problem > > this is supposed to solve. I guess the WG wants to give YANG Doctors > > more > > things to check. (This is what we called a CLR in SNMP-land ;-) > > Thanks. No action taken. > > > > I13) notification subscription-started { > > > sn:subscription-state-notification; > > > if-feature "configured"; > > > description > > > "This notification indicates that a subscription has started and > > > notifications are beginning to be sent. This notification shall > > > only be sent to receivers of a subscription; it does not > > > constitute a general-purpose notification."; > > > > > > -- 2nd sentence is confusing; all notifications are sent to > > > receivers of a subscription. last part is redundant since > > > the sn:subscription-state-notification extension is used > > > > There is no issue with removing this second sentence completely. If I > > did that, > > would this address your concern? > > > > OK > > Done > > > > I14) rc:yang-data modify-subscription-stream-error-info { > > > > > > leaf filter-failure-hint { > > > type string; > > > description > > > "Information describing where and/or why a provided filter > > > was unsupportable for a subscription."; > > > } > > > > > > -- rpc-error already allows more precise error reporting > > > It uses error-tag, error-path, error-string, and error-info > > > extensions > > > to identify which parameters/conditions caused the RPC to be > > > rejected. > > > This error reporting will continue to be used, Not sure this > > > failure-hint > > > has any standards value. Perhaps real-use example can be added > > > > Per your thoughts on rpc-error... For NETCONF and RESTCONF, you point > > to > > error structures which historically been used with those transports. > > Of course > > we were looking to have all subscription hints supportable across > > transports via > > a single portable YANG data structure. So the value is that a single > > string > > object exists so to transport whatever the vendor thinks would be > > useful as a > > hint in this case. I.e., there has been no attempt to standardize the > > contents of > > this string. If operational experiences drive a desire for such > > structuring, this > > could provide the basis for a new draft building off of this starting > > point. > > > > I guess I do not consider NETCONF and RESTCONF "historic" quite yet. > > There are many implementations using the rpc-error reporting with no > > intent > > to replace it with something else. > > > > I was just asking for an example, since I have no idea what an > > implementor > > would put in this leaf. > > Here is an example from our implementation. Say you mistype an extra > "\" to an xpath filter: > /if:interfaces-state/interface[name="GigabitEthernet0/0"]/oper-status > As a result, the filter is passed to the publisher is: > /if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/oper-status > > What we would return in the failure-hint string is: > Invalid expression: offset(9) in > '/if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/oper-status' > > Eric > > > Andy
- [Netconf] Yangdoctors last call review of draft-i… Andy Bierman
- Re: [Netconf] Yangdoctors last call review of dra… Eric Voit (evoit)
- Re: [Netconf] Yangdoctors last call review of dra… Andy Bierman
- Re: [netconf] Yangdoctors last call review of dra… Eric Voit (evoit)
- Re: [netconf] Yangdoctors last call review of dra… Mehmet Ersue
- Re: [netconf] Yangdoctors last call review of dra… Andy Bierman
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Andy Bierman
- Re: [netconf] Yangdoctors last call review of dra… Andy Bierman
- Re: [netconf] Yangdoctors last call review of dra… Eric Voit (evoit)
- Re: [netconf] Yangdoctors last call review of dra… Andy Bierman
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Andy Bierman
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Andy Bierman
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Andy Bierman
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Martin Bjorklund
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Per Hedeland
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Per Hedeland
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)
- Re: [netconf] [yang-doctors] Yangdoctors last cal… Eric Voit (evoit)