Re: [netconf] [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-21

[Martin Bjorklund <[e-mail]>]

"Eric Voit (evoit)" <[e-mail]> wrote:
> > From: Martin Bjorklund, January 24, 2019 8:17 AM
> > 
> > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > Hi Andy,
> > >
> > >
> > >
> > > Thanks very much for the thorough YANG Doctor review.  I have included
> > > the
> > agreed upon comments, and uploaded to:
> > >
> > > draft-ietf-netconf-subscribed-notifications-22
> > >
> > > a summary of the clarifications made is at the end of the document.
> > > Let me know if there anything else needed to conclude the YANG doctor
> > > review of this document.
> > >
> > >
> > >
> > > Also as the result of the ‘error-tag’ discussion with you and Martin,
> > > we need to perform the refinement of the ‘error-tag’ mapping within
> > > both draft-ietf-netconf-netconf-event-notifications Section
> > > 7, and draft-ietf-netconf-restconf-notif Section 3.3.   Directly
> > > below is some text and proposed error-tag mappings for those
> > > documents.
> > >
> > >
> > >
> > >     o  An "error-tag" node with the value being a string that
> > >
> > >        corresponds to an identity associated with the error.  This
> > >
> > >        "error-tag" will correspond to the error identities within
> > >
> > >        [I-D.draft-ietf-netconf-subscribed-notifications] section
> > >
> > >        2.4.6 for general subscription errors:
> > >
> > >
> > >
> > >           error identity         uses error-tag
> > >
> > >           ---------------------- --------------
> > >
> > >           dscp-unavailable       invalid-value
> > 
> > Ok.  But it is not clear to me when this error is actually supposed to
> > be
> > generated?  The leaf and identity have the same if-feature, so it
> > isn't a special
> > errro code for "unsupported leaf", which is good!
> > 
> > Then I have to assume it is supposed to be some kind of runtime error?
> 
> Yes.  A publisher, nor the network to which is connects does not have
> to:
> (a) support all DSCP values, nor 
> (b) allow a particular value requested by a particular subscriber,
> So this condition allows a publisher to reject a request for a DSCP
> value where is knows the value will not be respected.

Good explanation, I wish it was part of the "leaf dscp" in the
module :)

The dscp-unavailable identity doesn't add any addition value compared
to the standard error.

> > Thinking some more, what is supposed to happen if the client on the
> > same
> > session sends first an establish-subscription with dscp 42, and then
> > another
> > establish-subscription with dscp 10?
> 
> This would be allowed.

On linux at least this is a sockopt, i.e., the option applies to the
socket, which means all packets on the session.  So how is this
supposed to be implemented if different messages on the session should
have different dscp values?  Or is the idea that you send the msg,
flush all data from ssh/tls to tcp, then flush the tcp buffers (not
that easy...)?

Even if there's just one establish-subscription with a dscp value,
since it applies to the session it means that all normal rpcs on this
session will get the same dscp value.  It is not clear that this is
the intention?



/martin


> The interesting part comes with bundling the
> event records.  The initial versions of
> draft-ietf-netconf-notification-messages required that all event
> records in a bundle had a common dscp.  At this point, that seems
> overly restrictive to the marshalling process, so for now that
> requirement is not in the document.
> 
> > >           encoding-unsupported   invalid-value
> > 
> > Ok.  But this identity doesn't give more information than the standard
> > error:
> > 
> >   error-tag: invalid-value
> >   error-path: /rpc/establish-subscription/encoding
> > 
> > 
> > >           filter-unavailable     invalid-value
> > 
> > This is a "subscription-terminated-reason", which will never be sent
> > in an rpc-
> > error, and thus should not be mapped to an error-tag.
> 
> Yes, forgot to remove those.  It is now out.
> 
> > >          filter-unsupported     invalid-value
> > 
> > Ok.  But this identity doesn't give more information than the standard
> > error:
> > 
> >   error-tag: invalid-value
> >   error-path: /rpc/establish-subscription/stream-xpath-filter
> > 
> > 
> > >           insufficient-resources resource-denied
> > 
> > 
> > Ok.  But this identity doens't give more information than the standard
> > error in
> > the case of establish-subscription and modify-subscription.
> > 
> > >           no-such-subscription   invalid-value
> > 
> > Ok.  But this identity doens't give more information than the standard
> > error in
> > the case of establish-subscription and modify-subscription.
> > 
> > >           replay-unsupported     operation-not-supported
> > 
> > Ok.  But this identity doesn't give more information than the standard
> > error.
> > 
> > >           stream-unavailable     invalid-value
> > 
> > This is a "subscription-terminated-reason", which will never be sent
> > in an rpc-
> > error, and thus should not be mapped to an error-tag.
> 
> Yes, forgot to remove those.  It is now out.
> 
> > >           suspension-timeout     operation-failed
> > 
> > This is a "subscription-terminated-reason", which will never be sent
> > in an rpc-
> > error, and thus should not be mapped to an error-tag.
> 
> Yes, forgot to remove those.  It is now out.
> 
> > >           unsupportable-volume   too-big
> > 
> > This is a "subscription-terminated-reason", which will never be sent
> > in an rpc-
> > error, and thus should not be mapped to an error-tag.
> 
> Yes, forgot to remove those.  It is now out.
> 
> > >        Or this "error-tag" will correspond to the error identities
> > >
> > >        within [I-D.ietf-netconf-yang-push] Appendix A.1 for
> > >
> > >        subscription errors specific to YANG datastores:
> > >
> > >
> > >
> > >           error identity              uses error-tag
> > >
> > >           ----------------------      --------------
> > >
> > >           cant-exclude                operation-not-supported
> > >
> > >           datastore-not-subscribable  operation-not-supported
> > 
> > I think that this should be invalid-value.
> 
> Ok
> 
> /Eric
> 
> > >           no-such-subscription-resync invalid-value
> > 
> > Ok, but again the value of having this is unclear.
> > 
> > >           on-change-unsupported       operation-not-supported
> > >
> > >           on-change-sync-unsupported  operation-not-supported
> > >
> > >           period-unsupported          invalid-value
> > >
> > >           update-too-big              too-big
> > >
> > >           sync-too-big                too-big
> > >
> > >           unchanging-selection        operation-failed
> > 
> > 
> > 
> > /martin
> > 
> > 
> > 
> > >
> > >
> > >
> > >
> > >
> > > Do you (or anyone else in this thread) have any suggestions on the
> > > text or proposed mappings?  If this turns out to be ok, Alex will need
> > > to remove the NETCONF error-tag specifics from
> > > draft-ietf-netconf-yang-push Sections 4.4.1 & 4.4.2
> > >
> > >
> > >
> > > Also Reshad will have to do some work because he is the YANG doctor of
> > netconf-netconf-event-notifications, and he will want to include the
> > same
> > information within draft-ietf-netconf-restconf-notif.
> > >
> > >
> > >
> > > Thanks,
> > >
> > > Eric
> > >
> > >
> > >
> > >
> > >
> > > From: Andy Bierman <[e-mail]>
> > >
> > > Sent: Wednesday, January 23, 2019 12:42 PM
> > >
> > > To: Eric Voit (evoit) <[e-mail]>
> > >
> > > Cc: Martin Bjorklund <[e-mail]>; [e-mail];
> > > [e-mail];
> > > [e-mail]
> > >
> > > Subject: Re: [yang-doctors] Yangdoctors last call review of
> > > draft-ietf-netconf-subscribed-notifications-21
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > On Wed, Jan 23, 2019 at 4:35 AM Eric Voit (evoit)
> > > <mailto:[e-mail]>
> > wrote:
> > >
> > > > From: Martin Bjorklund, January 23, 2019 3:32 AM
> > >
> > > >
> > >
> > > > Hi,
> > >
> > > >
> > >
> > > > "Eric Voit (evoit)" <mailto:[e-mail]> 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).
> > >
> > >
> > >
> > > I am completely good with this.   Does this work for you Andy?
> > >
> > >
> > >
> > > This is better.
> > >
> > > I'm glad no other drafts are creating their own error reporting system
> > > for
> > each rpc-stmt.
> > >
> > > This is a bad precedent and likely to be skipped in implementations.
> > >
> > >
> > >
> > > Eric
> > >
> > >
> > >
> > > > /martin
> > >
> > > >
> > >
> > >
> > >
> > > Andy
> > >
> > >
> > >
> > >
> > >
> > > >
> > >
> > > >
> > >
> > > >
> > >
> > > > >
> > >
> > > > > 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)
> > >
> > > > > <mailto:[e-mail]<mailto:mailto:[e-mail]>> 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-sta
> > > > > tus
> > >
> > > > > As a result, the filter is passed to the publisher is:
> > >
> > > > > /if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/oper-st
> > > > > atus
> > >
> > > > >
> > >
> > > > > 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