Subject: [Netconf] mbj's WGLC review of subscribed-notifications-16

From: Martin Bjorklund <[email-masked]>
Date: Mon, 10 Sep 2018 19:41:52 +0200 (CEST)

Hi,

I have reviewed this document, and I think it is almost ready.  Here
are my comments:

o  Section 1.4

  s/RFC-5277/RFC 5277/

o  Section 2.1

  The text says:

    It is out of the scope of this document
    to identify [...] b) how event records are defined

  Is this really what we want?  Shouldn't this document specify that
  an event record is an instance of a YANG "notification" statement?
  Do we want to support event records that are not defined in YANG?

  This was discussed in the thread "comments on
  draft-ietf-netconf-subscribed-notifications-12", and it seems that
  noone really have any use case for non-YANG-defined event records,
  accessed with YANG-defined operations.

  I think that this document should state that an event record is an
  instance of a YANG "notification" statement.  If not, it is unclear
  both how subtree/xpath filters and xml/json/... encodings would work.

o  Section 2.1

  The text says:

    Among
    these included NETCONF XML event records are individual YANG 1.1
    notifications described in section 7.16 of [RFC7950].

  First of all, does this mean that there can be event records on the
  "NETCONF" stream that are *not* defined with a "notification"
  statement?

  Second, this seems to imply that *all* notifications defined in all
  YANG modules end up in the "NETCONF" stream, but I don't think this
  is the intention.  At least I hope so.  ("yp:push-update" is an
  example of a notification that will not be sent on the "NETCONF"
  stream.)

  Further, the text says:

    Each of these
    YANG 1.1 notifications will be treated as a distinct event record.

  This seems to confuse the schema definition with specific instances;
  it is not the schema definition that is treated as a disctinct event
  record, but the notification instances.

  So, I suggest that these two sentences are removed.

o  Section 2.4.1

  OLD:

   o  A delete or kill RPC will end the subscription, as will the
      reaching of a "stop-time".

  NEW:

   o  A "delete-subscription" or "kill-subscription" RPC will end the
      subscription, as will the reaching of a "stop-time".

o  Section 2.4.1

  OLD:

      Successful
      modification returns the subscription to an active state.

  NEW:

      Successful
      modification returns the subscription to the active state.

o  Section 2.4.2

  The text says:

    The transport selected by the subscriber
    to reach the publisher MUST be able to support multiple "establish-
    subscription" requests made within the same transport session.

  This seems to indicate that RESTCONF cannot be used with this
  specification, since RESTCONF doesn't have sessions.

  This makes me wonder, why does this document specify this
  requirement?  I agree that this is the behaviour we want from
  NETCONF, but then I think that should be defined in the
  netconf-notif draft.

o  Section 2.4.2

  s/"start-time"/"replay-start-time"/  (3 occurrences)

  Also, the text says that the replay-start-time MUST be in the past.

  Suppose the server receives a "replay-start-time" of 12:00:00.001,
  and the replay buffer is empty.

  A) If the current time is 12:00:00.002, the request is accepted, and
     a "replay-completed" notif is sent.

  B) If the current time is 12:00:00.000, the request is rejected with
     an error.

  Why is this important?  Why not simply just send the
  replay-completed notif in both cases?

o  Section 2.4.2.1

  OLD:

   If the time the replay starts is later than the time marked within
   any event records retained within the replay buffer, then the
   publisher MUST send a "replay-completed" notification immediately
   after a successful establish-subscription RPC response.

  NEW:

   If the given "replay-start-time" is later than the time marked within
   any event records retained within the replay buffer, then the
   publisher MUST send a "replay-completed" notification immediately
   after a successful establish-subscription RPC response.

o  Section 2.4.2.1

  There is no text that explains what happens in the "succesful" case;
  i.e., when there are buffered event records with time greater than
  or equal to the requested "replay-start-time".

  Maybe the first paragraph is supposed to cover this, but it just
  says "meets the timeframe criteria", which is a bit vague.

o  Section 2.4.5

  OLD:

    A publisher MUST terminate any dynamic
    subscription identified by RPC request.

  NEW:

    A publisher MUST terminate the dynamic
    subscription identified by the "identifier" parameter in the RPC
    request, if such a subscription exists.

o  Section 2.4.6

  The text says:

    1.  "establish-subscription-stream-error-info": This MUST be returned
        if an RPC error reason has not been placed elsewhere within the
        transport portion of a failed "establish-subscription" RPC
        response.

  (and similar for the other errors)

  What exactly does "if an RPC error reason has not been placed
  elsewhere" mean?

o  Section 2.5.1

  In the second diagram (for the valid state), the states are called
  "receiver connecting", "reciever timeout" etc.  I find this a bit
  confusing; it is not the *receiver* that is connecting, it is the
  publisher.  Ok, the legend then says:

     dashed boxes which include the word 'receiver' show the possible
     states for an individual receiver

 But since all dashed boxes include the word "receiver", and the
 diagram specifically is the state machine *per receiver*, maybe we
 can simply remove the word "receiver" from the boxes?

o  Section 2.5.2

  s/details."/details./

o  Section 2.5.6

  I know that this feature was presented in Montreal.  I think it is
  problematic, and suggest it is removed.

  For example, what happens if the transport session is lost and
  re-established; are all event records in the buffer replayed again?

  What happens if a new receiver is added to a subscription that
  already has a receiver, will all events in the buffer be replayed to
  the new receiver?

  When a new subscription is configured with configured-replay set,
  the publisher will immediately try to connect to the publisher and
  send all buffered event from the last reboot.  Is this really
  useful?

  If the mechansim relies on the subscriber being able to also use
  dynamic subscriptions for replay of all event records, maybe we
  could simplify things by always relying on dynamic subscriptions for
  replay for configured subscriptions.  Specifically, remove
  "configured-replay" etc, but keep the "replay-previous-event-time"
  in the "subscription-started" notification.  Then a subscriber can
  simply check this value to see if anything is missing, and if it is,
  use a dynamic subscription to request replay from the latest time he
  knows about.

o  Section 2.6

  It just occurred to me that this text about using the <notification>
  message shouldn't be in this document, but in the netconf-notif (and
  restconf-notif) document.  It is highly transport and encoding
  specific.

o  Section 2.7

  This section talks about "subscription state" notifications.  But in
  many places in the document, you use the term "state change"
  notification.  I suggest pick one term and use it consistently, to
  avoid confusion.

o  Section 2.8

  s/operational datastore/operational state datastore/

 (four occurrences)

o  Section 4

          action reset {
            if-feature "configured";
            description
              "Allows the reset of this configured subscription receiver

  This action will be available also for dynamic subscription, if the
  server supports the "configured" feature.  Maybe add:

    when "../../../configured-subscription-state";

o  Section 4

  Should the rpc kill-subscription be marked with
  nacm:default-deny-all?  Otherwise, by default, anyone can terminate
  any other's subscriptions.

o  Section 4

  I suggest that the leaf /filters/stream-filter/identifier is renamed
  to "name", for consistency, and that it is of type "string", and
  that the type "filter-id" is removed, for consistency and
  simplicity.

  Also, I suggest that the leaf /subscriptions/subscription/target/
  stream-filter/by-reference/stream-filter-ref is renamed to
  "stream-filter", or maybe "stream-filter-name".

o  Section 4

  I suggest that the subscription's key leaf "identifier" is
  renamed to simply "id".  This would match the name of the type which
  is "subscription-id".

  (it is also consistent with other (not all) IETF models, where a
  "name" is used for an arbitrary string key, and "id" for a more
  strict key, often some integer type)

  NOTE: yang push should also be changed to use the same naming
  convention

o  Section 4

  I suggest the counters "count-sent" and "count-exlcuded" are renamed
  to something more descriptive, perhaps "out-event-records" or
  "sent-event-records", and "excluded-event-records" or
  "dropped-event-records", respectively.

o  Section 4

  I suggest you consistently format the YANG module wrt. spaces etc.
  Also, some lines are longer than 69 characters and should be
  adjusted.

  You can use pyang --ietf --max-line-length 69 to help with the
  latter, and pyang -f yang --keep-comments to help with the former.
  (note that you may need to manually fix the output of pyang -f,
  e.g. if some lines are too long)

  (The RFC editor checks this, so it is better to do it now)

o  Section 4

  OLD:

    "This feature indicates support for xpath filtering.";

  NEW:

    "This feature indicates support for XPath filtering.";

o  Section 4

  With the inclusion of "replay-start-time-revision", shouldn't the
  identity "history-unavailable" be removed?

o  Section 4

  identity no-such-subscription {
    base modify-subscription-error;
    base delete-subscription-error;
    base subscription-terminated-reason;
    description
     "Referenced subscription doesn't exist. This may be as a result of
      a non-existent subscription ID, an ID which belongs to another
      subscriber, or an ID for configured subscription.";
  }

  s/ID/id/

o  Section 5.4

   s/publisher"/publisher/

o  Appendix A

  Since this appendix defines an example module, it needs to be named
  accordingly.  Specifically, the module should be called
  "example-foo-subscribed-notifications", and the namespace should be
  "urn:example:foo-subscribed-notifications".

  (with this change, you can also remove the note to the RFC editor)

o  Appendix A

  The name of Figure 21 should be changed:

  OLD:

           Figure 21: Example Transport Augmentation for NETCONF

  NEW:
           Figure 21: Example Transport Augmentation for the
           fictitious protocol foo

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Tue, 11 Sep 2018 19:04:07 +0000

Thanks for the comments Martin,  thoughts in-line...

> From: Martin Bjorklund, September 10, 2018 1:42 PM
> 
> Hi,
> 
> I have reviewed this document, and I think it is almost ready.  Here are my
> comments:
> 
> 
> o  Section 1.4
> 
>   s/RFC-5277/RFC 5277/

Done

>   accessed with YANG-defined operations.

Yes this was discussed previously.  My reading was that others wanted to support subscription to non-YANG events.   
E.g., see Andy & Tianran's thoughts in
https://www.ietf.org/mail-archive/web/netconf/current/msg14764.html 

>   both how subtree/xpath filters and xml/json/... encodings would work.

Applying xpath filters against XML based event records seems viable to me.  Of course it will be up to the definition of any particular stream to articulate the event record contents sufficiently for filters to be created.

>   "NETCONF" stream that are *not* defined with a "notification"

For the NETCONF stream, personally I only care about things YANG notification statement.  But looking again at Andy's comment from:
https://www.ietf.org/mail-archive/web/netconf/current/msg14764.html 
he says "I would say SHOULD be defined in YANG, for the "NETCONF" stream."   So for -v16 I updated the text in a way that doesn't preclude this.  
 
>   example of a notification that will not be sent on the "NETCONF"

The NETCONF stream has a very broad/inclusive definition in RFC-5277.  This definition is: "This stream contains all NETCONF XML event notifications supported by the NETCONF server".

To me, thinking about how that statement and applying it to YANG, this translates as YANG 1.1 notifications should go into the NETCONF stream, unless explicitly excluded.  This provides a very simple way determining which types of notifications should be placed on the stream.  (Otherwise, how is a developer of a YANG notification supposed to know whether it should go into the NETCONF stream or not?  The alternative would seem to be having each notification opt-in to the NETCONF stream.)

So if you agree with this reasoning, how about:

The "NETCONF" event stream contains all instances of YANG 1.1 notifications generated by a publisher, except where a type of notification has explicitly been excluded from the stream. 
 
>   So, I suggest that these two sentences are removed.

Removed.

>       subscription, as will the reaching of a "stop-time".

updated

>       modification returns the subscription to the active state.

updated

>   specification, since RESTCONF doesn't have sessions.

The desired behavior is that a subscriber can request multiple subscriptions without each requiring a separate transport session establishment.  I think we are ok here with RESTCONF because underneath RESTCONF is HTTP, TLS, TCP.  And each of those lower layers can have sessions.  

To clarify further, I have tweaked the text to:

"The transport selected by the subscriber to reach the publisher MUST be able to support multiple "establish-subscription" requests without each requiring a separate persistent transport connection."

>   NETCONF, but then I think that should be defined in the

By saying something in SN about this requirement, I believe we can preclude an implementation which can only support one subscription per subscriber.  In any case, the specific "how" details will still need to be articulated in each transport draft.

>   s/"start-time"/"replay-start-time"/  (3 occurrences)

Fixed

>   replay-completed notif in both cases?

For (B), if an event occurs at 12:00:00.0005, should it be sent?  Should it be sent before or after the "replay-start-time"?  How long should we allow the clocks to drift before we reject the subscription?    As we don't support future/pending subscriptions, we don't have to worry about these questions with the current definition.  

>    after a successful establish-subscription RPC response.

Change made

>   says "meets the timeframe criteria", which is a bit vague.

This was supposed to be covered by the first paragraph.  To help clarify, I have updated the text to:

"In other words, as the subscription initializes itself, it sends any event records within the target event stream which meet the filter criteria, which have an event time which is after the "replay-start-time", and which have an event time before the "stop-time" should this "stop-time" exist."

>     request, if such a subscription exists.

updated

>   What exactly does "if an RPC error reason has not been placed

The intent is to match how NETCONF & RESTCONF historically handles errors.  As you know with NETCONF, each error identity will be inserted as the "error-app-tag".  And therefore placing the YANG data "establish-subscription-stream-error-info" would be redundant.  For NETCONF,  "establish-subscription-stream-error-info" are only needed when hints to be provided.

Therefore assuming you are looking for an alternative way to make this point, I have tweaked the text to: 
"This MUST be returned if an RPC error reason has not been mapped down into transport specific error objects contained within a failed "establish-subscription" RPC response."

>  But since all dashed boxes include the word "receiver", and the  diagram
> specifically is the state machine *per receiver*, maybe we  can simply remove
> the word "receiver" from the boxes?

There have been some discussions on this.  I agree it is certainly valid to remove "receiver" from these four boxes.  However I have found that getting readers to really internalize that the state machine is actually per-receiver rather than per-subscription, it seems to go faster/cleaner when receiver is shown in the four boxes.  

Nevertheless, if you absolutely want "receiver" removed from the boxes, I can do that.   

>   s/details."/details./

Fixed

>   problematic, and suggest it is removed.

This issue was discussed in Montreal.    The feature is of course is optional.  There are a large number of downsides for someone who chooses not to implement for certain use cases.  Please see the Montreal slides and the long threads with Kent for more details.  If you want to pursue this further it is probably best to start a new thread dedicated to this topic alone.

>   re-established; are all event records in the buffer replayed again?

No.  The publisher will not lose context of what was sent.
 
>   already has a receiver, will all events in the buffer be replayed to

Yes.  (To clarify, I added "to each configured receiver" at the end of the last sentence of the first paragraph.)

>   send all buffered event from the last reboot.  Is this really

Yes.  See the security cases reviewed on the earlier threads with Kent.  If someone doesn't care about log entries since boot, they simply don't have to implement this optional feature.

>   use a dynamic subscription to request replay from the latest time he

This is a valid approach, and can be used as an alternative to this optional feature.  However there are still many downsides to the alternative.  See the slides from Montreal, as well as the previous thread discussions for details.

>   restconf-notif) document.  It is highly transport and encoding

I think you are referring to both the sentence: "This notification message MUST be encoded in a <notification> message as defined within [RFC5277], Section 4.  And per [RFC5277]'s "eventTime" object definition, the "eventTime" is populated with the event occurrence time."  As well as to the example compliant message in Figure 10.

While the text could be extracted and placed in the netconf-notif document, extracting it fully from SN would leave no baseline message format.  Perhaps others will have a problem if the SN solution does not have some pre-defined structure to hold event records?

I can move this if I hear no objection.   As you know, I am hoping that draft-ietf-netconf-notification-messages at some point displaces the <notification> message of [RFC5277], Section 4.    And this is easier to do with fewer references to RFC5277 within SN.

>   notification.  I suggest pick one term and use it consistently, to

Updated to subscription state change notification

>   s/operational datastore/operational state datastore/

updated

>     when "../../../configured-subscription-state";

This would be nice to have.  But action doesn't support "when" as an allowed sub-statement.

>   any other's subscriptions.

This is a good idea.  Added.

>   that the type "filter-id" is removed, for consistency and

Done

>   "stream-filter", or maybe "stream-filter-name".

Done.   (It is now 'stream-filter-name')

>   NOTE: yang push should also be changed to use the same naming

Change made.   Alex knows to make this to YANG Push as well.

>   "dropped-event-records", respectively.

Changed to "sent-event-records" and "excluded-event-records".

>   Also, some lines are longer than 69 characters and should be

All lines are no longer that 69 characters now.
 
>   (The RFC editor checks this, so it is better to do it now)

Done. 

>     "This feature indicates support for XPath filtering.";

Updated.

>   identity "history-unavailable" be removed?

Removed.

>       subscriber, or an ID for configured subscription.";

Updated

>    s/publisher"/publisher/

Updated

>   (with this change, you can also remove the note to the RFC editor)

Updated

>            Figure 21: Example Transport Augmentation for the

Done.   And thanks!

Eric

> /martin
> 
> _______________________________________________
> Netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 12 Sep 2018 11:34:46 +0200 (CEST)

> > From: Martin Bjorklund, September 10, 2018 1:42 PM

[...]

> Yes this was discussed previously.  My reading was that others wanted
> to support subscription to non-YANG events.

Yes, but read on in that thread, see
https://www.ietf.org/mail-archive/web/netconf/current/msg14766.html
for a clarification.

AFAIK, noone has presented a use case for sending non-YANG-defined
data here.

> Applying xpath filters against XML based event records seems viable to
> me.

Where does it say that it has to be XML?  Whatabout mixed-content?
What if such a notification is sent when the encoding is JSON?

If you want to make this specification open for other data modeling
languages, I think you need to carefully define how that is supposed
to work; essentially similar to how we handle transports; we have a
set of requirement on what a transport document must define.  IMO this
is super complicated and overkill.  This draft is complex enough.
Don't add even more complexity.

> Of course it will be up to the definition of any particular
> stream to articulate the event record contents sufficiently for
> filters to be created.
> 
> > o  Section 2.1
> > 
> >   The text says:
> > 
> >     Among
> For the NETCONF stream, personally I only care about things YANG
> notification statement.  But looking again at Andy's comment from:
> he says "I would say SHOULD be defined in YANG, for the "NETCONF"
> stream."

I strongly disagree.

> So for -v16 I updated the text in a way that doesn't
> preclude this.

See above.  If all events records are defined in YANG we can remove
this.

> The NETCONF stream has a very broad/inclusive definition in RFC-5277.
> This definition is: "This stream contains all NETCONF XML event
> notifications supported by the NETCONF server".

This is very underspecified.  IIRC the original intention was
notifications related to NETCONF, hence "*NETCONF* XML event
notifications".

> To me, thinking about how that statement and applying it to YANG, this
> translates as YANG 1.1 notifications should go into the NETCONF
> stream, unless explicitly excluded.

I disagree.  This is not how 5277 works.  Until we have worked out a
useful way to assign notifications to streams, we shouldn't restrict
anything in this regard.

> This provides a very simple way
> determining which types of notifications should be placed on the
> stream.  (Otherwise, how is a developer of a YANG notification
> supposed to know whether it should go into the NETCONF stream or
> not?

They don't.  But note that it might be an operator decision for how to
assign notifications to streams.

> The alternative would seem to be having each notification opt-in to
> the NETCONF stream.)
> 
> The "NETCONF" event stream contains all instances of YANG 1.1
> notifications generated by a publisher, except where a type of
> notification has explicitly been excluded from the stream.

I think that this document should keep the definition of the NETCONF
stream as defined in RFC 5277.

Also, I think it should state that an event record is an instantiation
of a YANG "notification" statement (any version of YANG).

[...]

> The desired behavior is that a subscriber can request multiple
> subscriptions without each requiring a separate transport session
> establishment.  I think we are ok here with RESTCONF because
> underneath RESTCONF is HTTP, TLS, TCP.  And each of those lower layers
> can have sessions.
> 
> "The transport selected by the subscriber to reach the publisher MUST
> be able to support multiple "establish-subscription" requests without
> each requiring a separate persistent transport connection."
> 
> By saying something in SN about this requirement, I believe we can
> preclude an implementation which can only support one subscription per
> subscriber.  In any case, the specific "how" details will still need
> to be articulated in each transport draft.

I think you are mixing the specification and implementation.  If a
transport document says that an implemention MUST support multiple
subscriptions per session (in a transport-specific way), then an
implementation cannot just support a sinlge subscription.

My point is that this a transport-specific requirement, and it doesn't
belong in this document.

> For (B), if an event occurs at 12:00:00.0005, should it be sent?
> Should it be sent before or after the "replay-start-time"?  How long
> should we allow the clocks to drift before we reject the subscription?
> As we don't support future/pending subscriptions, we don't have to
> worry about these questions with the current definition.

I would simply say that if the replay-start-time is later than any
buffered notif, then "replay-complete" is immediately sent.  It dont'
matter if it is one microsecond or one year later.

The problem otherwise is how to decide if something is "in the past".
In the past when the client sent the request?  When the server parsed
the XML?  When the instrumentation code runs?

But this is not a big issue.

[...]

> "In other words, as the subscription initializes itself, it sends any
> event records within the target event stream which meet the filter
> criteria, which have an event time which is after the
> "replay-start-time", and which have an event time before the
> "stop-time" should this "stop-time" exist."

Ok.

[...]

> The intent is to match how NETCONF & RESTCONF historically handles
> errors.  As you know with NETCONF, each error identity will be
> inserted as the "error-app-tag".

It could also be in "error-tag" and/or "error-message".  My point is
that I don't think that you mean that if an implementation populate
say "error-message", then it should not popluate error-info with
establish-subscription-stream-error-info.

> And therefore placing the YANG data
> "establish-subscription-stream-error-info" would be redundant.  For
> NETCONF, "establish-subscription-stream-error-info" are only needed
> when hints to be provided.
> 
> "This MUST be returned if an RPC error reason has not been mapped down
> into transport specific error objects contained within a failed
> "establish-subscription" RPC response."

Howabout simply removing this sentence?

> >  diagram
> There have been some discussions on this.  I agree it is certainly
> valid to remove "receiver" from these four boxes.  However I have
> found that getting readers to really internalize that the state
> machine is actually per-receiver rather than per-subscription, it
> seems to go faster/cleaner when receiver is shown in the four boxes.
> 

Maybe you can remove receiver from the individual boxes and then
change the word "valid" from the outer box to "per-receiver state
machine"?

Or maybe remove the outer box and add a Figure title: 'per-receiver
state machine for the subscription's "valid" state'

Also not a big deal, but I found it confusing.

[...]

> This issue was discussed in Montreal.  The feature is of course is
> optional.  There are a large number of downsides for someone who
> chooses not to implement for certain use cases.  Please see the
> Montreal slides and the long threads with Kent for more details.

I did watch the Montreal session, but I didn't see any discussion on
this issue (just your presentation).   Can you send a pointer to the
email thread with Kent?

> If
> you want to pursue this further it is probably best to start a new
> thread dedicated to this topic alone.
> 

This is not clear from the text.  In fact, the text explicitly says:

   The leading event record sent will be the first event record
   subsequent to the latest of three different times: the
   "replay-log-creation-time", "replay-log-aged- time", or the most
   recent publisher boot time.

> Yes.  (To clarify, I added "to each configured receiver" at the end of
> the last sentence of the first paragraph.)
> 
> Yes.  See the security cases reviewed on the earlier threads with
> Kent.  If someone doesn't care about log entries since boot, they
> simply don't have to implement this optional feature.
> 
> This is a valid approach, and can be used as an alternative to this
> optional feature.  However there are still many downsides to the
> alternative.  See the slides from Montreal, as well as the previous
> thread discussions for details.
> 
> > o  Section 2.6
> > 
> I think you are referring to both the sentence: "This notification
> message MUST be encoded in a <notification> message as defined within
> [RFC5277], Section 4.  And per [RFC5277]'s "eventTime" object
> definition, the "eventTime" is populated with the event occurrence
> time."  As well as to the example compliant message in Figure 10.
> 
> While the text could be extracted and placed in the netconf-notif
> document, extracting it fully from SN would leave no baseline message
> format.

Things like this are inevitable when you have a transport-independent
specification.  Of course, you can require that all transports MUST
use the 5277 spec of a notification.  But then I think you rule out
transports like CoMI and possibly other future binary protocols.

> Perhaps others will have a problem if the SN solution does
> not have some pre-defined structure to hold event records?
> 
> I can move this if I hear no objection.  As you know, I am hoping that
> draft-ietf-netconf-notification-messages at some point displaces the
> <notification> message of [RFC5277], Section 4.  And this is easier to
> do with fewer references to RFC5277 within SN.

[...]

> This would be nice to have.  But action doesn't support "when" as an

Ha!  That's unfortunate... I wonder why we didn't allow this...

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 12 Sep 2018 18:35:35 +0000

Hi Martin,

Much incorporated.  Latest is at:
https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-17.txt

Specific responses in-line...

> From: Martin Bjorklund, September 12, 2018 5:35 AM
> 
> AFAIK, noone has presented a use case for sending non-YANG-defined data
> here.

As anything can be YANG wrapped, per the thread above.  I am fine with this.

> If you want to make this specification open for other data modeling languages,
> I think you need to carefully define how that is supposed to work; essentially
> similar to how we handle transports; we have a set of requirement on what a
> transport document must define.  IMO this is super complicated and overkill.
> This draft is complex enough.

If you are just saying that we can put everything into a YANG wrapper, that is fine with me.  To clarify this, I have tweaked the first sentence of Section 2.1 to:
"An event stream is a named entity on a publisher which exposes a continuously updating set of YANG encoded event records."

And I have tweaked the first sentence of the third paragraph to:
"As YANG encoded event records are created by a system,..."

> > Of course it will be up to the definition of any particular stream to
> > articulate the event record contents sufficiently for filters to be
> > created.
> >
> > > o  Section 2.1
> > >
> > >   The text says:
> > >
> > >     Among
> > So for -v16 I updated the text in a way that doesn't preclude this.
> 
> See above.  If all events records are defined in YANG we can remove this.

Per above, I have included text above so that YANG encapsulation is required.  I think YANG Data should be an allowable structure (so I didn't specify just a "Notification").  If you think that YANG Data should be excluded from the allowed event records. I can refine the words.

> This is very underspecified.  IIRC the original intention was notifications related
> to NETCONF, hence "*NETCONF* XML event notifications".
> 
> I disagree.  This is not how 5277 works.  Until we have worked out a useful way
> to assign notifications to streams, we shouldn't restrict anything in this regard.

Ok.  Effectively we are naming the stream NETCONF without improving the definition of what membership in that stream should be.  And that is fine.  As people seem to understand what this is supposed to be, I will return to the original definition.  See text, and let me know if you have issues.
 
> > supposed to know whether it should go into the NETCONF stream or not?
> 
> They don't.  But note that it might be an operator decision for how to assign
> notifications to streams.

Ok.  A stream membership definition is out-of-scope for all other streams, we can leave it this way for the "NETCONF" stream.  Someone else can pick up and define this in another draft the WG feels it necessary.
 
> I think that this document should keep the definition of the NETCONF stream as
> defined in RFC 5277.

Ok

> Also, I think it should state that an event record is an instantiation of a YANG
> "notification" statement (any version of YANG).

What about a YANG Data?  (Per above)

> I think you are mixing the specification and implementation.  If a transport
> document says that an implemention MUST support multiple subscriptions per
> session (in a transport-specific way), then an implementation cannot just
> support a sinlge subscription.
> 
> My point is that this a transport-specific requirement, and it doesn't belong in
> this document.

I can see why you don't see it applicable to Section 2.4.2.    However Section 5.3 provides guidelines for transport document developers (this is a section Kent requested.)  And so I have moved the requirement there.  Is that sufficient. 

> I would simply say that if the replay-start-time is later than any buffered notif,
> then "replay-complete" is immediately sent.  It dont'

Tweaked the sentence:

"If the given "replay-start-time" is later than the time marked within any event records retained within the replay buffer, then the publisher MUST send a "replay-completed" notification immediately after a successful establish-subscription RPC response."
 
> In the past when the client sent the request?  When the server parsed the XML?
> When the instrumentation code runs?
> 

Hopefully with the tweak, we can leave it as modified.

[..]
> It could also be in "error-tag" and/or "error-message".  My point is that I don't
> think that you mean that if an implementation populate say "error-message",
> then it should not popluate error-info with establish-subscription-stream-
> error-info.

I do mean that.  I don't think we should send: "establish-subscription-stream-error-info" for NETCONF when there is no hint included.  All the necessary error info is already in the existing message.

Done

> Maybe you can remove receiver from the individual boxes and then change the
> word "valid" from the outer box to "per-receiver state machine"?
> 
> Or maybe remove the outer box and add a Figure title: 'per-receiver state
> machine for the subscription's "valid" state'
> 

All are viable.  But I have found the current diagram useable. Do you have a major issue if we leave it as it is?
 
> this issue (just your presentation).   Can you send a pointer to the

Some minutes on the topic can be seen at:
https://datatracker.ietf.org/doc/minutes-102-netconf/ 
With the WG vote in Montreal, I would love not to re-open.  

The previous exchange was long.  Some items include:
https://www.ietf.org/mail-archive/web/netconf/current/msg14575.html
https://www.ietf.org/mail-archive/web/netconf/current/msg14902.html 

>    recent publisher boot time.

I see your point.   I have tweaked the text to:

" If the publisher does not know the next record intended for the receiver, the leading event record..."
 
I have also made a corresponding tweak to the YANG model.

> specification.  Of course, you can require that all transports MUST use the 5277
> spec of a notification.  But then I think you rule out transports like CoMI and
> possibly other future binary protocols.

Ok, I am fine with the move.  I have moved the identified text to NETCONF-Notif.  (Actually the text was already there too.  So I just tweaked it.)
 
Thanks,
Eric

> > Perhaps others will have a problem if the SN solution does not have
> > some pre-defined structure to hold event records?
> >

---

From: Andy Bierman <[email-masked]>
Date: Wed, 12 Sep 2018 11:42:28 -0700

On Wed, Sep 12, 2018 at 11:35 AM, Eric Voit (evoit) <
evoit=[email-masked]> wrote:

> https://github.com/netconf-wg/rfc5277bis/blob/master/draft-
> ietf-netconf-subscribed-notifications-17.txt
>

I think my concern was simply that I take notice any time anyone
uses the term MUST in an I-D.  I ask myself "What harm to interoperability
will result if this MUST is not followed?  If I cannot think of anything
(as in this case) then MUST is probably a SHOULD instead.

The use-case I have in mind -- protobuffs -- I don't see why it is
important to
require a YANG module for every .proto file. Even worse, why is the
subscription draft making this requirement?

Andy

> languages,
> If you are just saying that we can put everything into a YANG wrapper,
> that is fine with me.  To clarify this, I have tweaked the first sentence
> of Section 2.1 to:
> "An event stream is a named entity on a publisher which exposes a
> continuously updating set of YANG encoded event records."
>
> Per above, I have included text above so that YANG encapsulation is
> required.  I think YANG Data should be an allowable structure (so I didn't
> specify just a "Notification").  If you think that YANG Data should be
> excluded from the allowed event records. I can refine the words.
>
> notifications related
> useful way
> Ok.  Effectively we are naming the stream NETCONF without improving the
> definition of what membership in that stream should be.  And that is fine.
> As people seem to understand what this is supposed to be, I will return to
> the original definition.  See text, and let me know if you have issues.
>
> Ok.  A stream membership definition is out-of-scope for all other streams,
> we can leave it this way for the "NETCONF" stream.  Someone else can pick
> up and define this in another draft the WG feels it necessary.
>
> of a YANG
> transport
> belong in
> I can see why you don't see it applicable to Section 2.4.2.    However
> Section 5.3 provides guidelines for transport document developers (this is
> a section Kent requested.)  And so I have moved the requirement there.  Is
> that sufficient.
>
> > > > o  Section 2.4.2
> > > >
> buffered notif,
> "If the given "replay-start-time" is later than the time marked within any
> event records retained within the replay buffer, then the publisher MUST
> send a "replay-completed" notification immediately after a successful
> establish-subscription RPC response."
>
> that I don't
> I do mean that.  I don't think we should send: "establish-subscription-stream-error-info"
> for NETCONF when there is no hint included.  All the necessary error info
> is already in the existing message.
>
> Ok, I am fine with the move.  I have moved the identified text to
> NETCONF-Notif.  (Actually the text was already there too.  So I just
> tweaked it.)
>
> Thanks,
> Eric
>

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 12 Sep 2018 20:02:09 +0000

From: Andy Bierman, September 12, 2018 2:42 PM

On Wed, Sep 12, 2018 at 11:35 AM, Eric Voit (evoit) <evoit=[email-masked]<mailto:evoit=[email-masked]>> wrote:
Hi Martin,

> "Eric Voit (evoit)" <[email-masked]<mailto:[email-masked]>> wrote:

The use-case I have in mind -- protobuffs -- I don't see why it is important to
require a YANG module for every ..proto file. Even worse, why is the
subscription draft making this requirement?

<eric>  I am good with whatever works for people.  I don’t see the wrapper as necessary, which is why it wasn’t in the document through –v16.  If your argument on the .proto file sways Martin, I am happy to go back to the earlier text.

Eric

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 12 Sep 2018 22:08:05 +0200 (CEST)

> (as in this case) then MUST is probably a SHOULD instead.

As I wrote earlier in the thread, if this is supposed to work with any
(or just other?) DMLs, I think the document has to be explain how it
is supposed to work, and describe the requirements for such a DML.

One thing that would harm interoperability is the requirement to be
able to use multiple encodings; today both XML and JSON are defined,
maybe there will be more in the future.

> subscription draft making this requirement?

Can you describe this use case?  The SN doc simply says that the event
records that you can subscribe to and filter and apply NACM rules to
are defined using YANG's "notification" statement.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Wed, 12 Sep 2018 13:50:02 -0700

On Wed, Sep 12, 2018 at 1:08 PM, Martin Bjorklund <[email-masked]> wrote:

> data

I don't have any real objection to the requirement, since it is
unenforceable anyway.
One can easily construct a YANG model using an empty container or "anydata"
as a wrapper.

Andy

> > > > document says that an implemention MUST support multiple
> subscriptions
> > > per
> > > > > >   A) If the current time is 12:00:00.002, the request is
> accepted,
> > > and
> > > > > >         if an RPC error reason has not been placed elsewhere
> within
> > > the
> > > > > Therefore assuming you are looking for an alternative way to make
> this
> > > > > point, I have tweaked the text to:
> change
> > > the
> state
> > > All are viable.  But I have found the current diagram useable. Do you
> have
> > > a major issue if we leave it as it is?
> > >
> > > > [...]
> > > >
> > > > > > o  Section 2.5.6
> > > > > >
> > > > > >   already has a receiver, will all events in the buffer be
> replayed
> > > to
> > > > > >   the new receiver?
> > > > >
> > > > > >   could simplify things by always relying on dynamic
> subscriptions
> > > for
> > > > > >   simply check this value to see if anything is missing, and if
> it
> > > is,
> > > > > >   use a dynamic subscription to request replay from the latest
> time
> > > he
> > > > > >   knows about.
> > > > >
> use
> > > the 5277
> > > > spec of a notification.  But then I think you rule out transports
> like
> > > CoMI and

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 13 Sep 2018 09:21:34 +0200 (CEST)

Hi,

Thank you for addressing my comments.  More inline.

"Eric Voit (evoit)" <[email-masked]> wrote:
> that is fine with me.  To clarify this, I have tweaked the first
> sentence of Section 2.1 to:

Howabout:

OLD:

   An event stream is a named entity on a publisher which exposes a
   continuously updating set of event records.  Each event stream is
   available for subscription.  It is out of the scope of this document
   to identify a) how streams are defined (other than the NETCONF
   stream), b) how event records are defined/generated, and c) how event
   records are assigned to streams.

NEW:

   An event stream is a named entity on a publisher which exposes a
   continuously updating set of event records.  An event record is
   defined with the "notification" YANG statement.  Each event stream is
   available for subscription.  It is out of the scope of this document
   to identify a) how streams are defined (other than the NETCONF
   stream), b) how event records are generated, and c) how event
   records are assigned to streams.

> "As YANG encoded event records are created by a system,..."

I think the original sentence is better.

Also, some tweaks are needed to the filter leafs in the YANG module:

OLD:

          The subtree filter is applied to the representation of
          individual, delineated event records as contained within the
          event stream.  For example, if the notification message
          contains an instance of a notification defined in YANG, then
          the top-level element is the name of the YANG notification.

NEW:

          The subtree filter is applied to the representation of
          individual, delineated event records as contained within the
          event stream.  The top-level element in the event record is
          the name of the YANG notification.

and

OLD:

           The XPath expression is evaluated on the representation of
           individual, delineated event records as contained within
           the event stream.  For example, if the notification message
           contains an instance of a notification defined in YANG,
           then the top-level element is the name of the YANG
           notification, and the root node has this top-level element
           as the only child.

NEW:

           The XPath expression is evaluated on the representation of
           individual, delineated event records as contained within
           the event stream.  The top-level element in the event
           record is the name of the YANG notification, and the root
           node has this top-level element as the only child.

(I think it would be useful with some filtering examples, like in
section 5 of RFC 5277, but I think you have all examples in
netconf-notif, so they would go into that draft.  (re-reading an old
thread I found that examples for filters were requested before)).

> required.  I think YANG Data should be an allowable structure (so I
> didn't specify just a "Notification").  If you think that YANG Data
> should be excluded from the allowed event records. I can refine the
> words.

Yes I think yang-data must be excluded (actually this was one of the
concerns, raised by Andy I think; that people start to use yang-data
for things that we already have proper YANG statements for...)

> Ok.  Effectively we are naming the stream NETCONF without improving
> the definition of what membership in that stream should be.  And that
> is fine.  As people seem to understand what this is supposed to be, I
> will return to the original definition.  See text, and let me know if
> you have issues.

Maybe one small tweak from the original text:

OLD:

  The "NETCONF" event stream contains all NETCONF XML event
  record information supported by the publisher

NEW:

  The "NETCONF" event stream contains all NETCONF event
  record information supported by the publisher

(since it actually is possible to get them in JSON over RESTCONF)

> Ok.  A stream membership definition is out-of-scope for all other
> streams, we can leave it this way for the "NETCONF" stream.  Someone
> else can pick up and define this in another draft the WG feels it
> necessary.

Yes.

> What about a YANG Data?  (Per above)

See above.

> Section 5.3 provides guidelines for transport document developers
> (this is a section Kent requested.)  And so I have moved the
> requirement there.  Is that sufficient.

Ok, I agree that the text belongs to section 5.3 rather than 2.4.2.  I
just don't agree with the text ;-)  But I can accept it.

> "If the given "replay-start-time" is later than the time marked within
> after a successful establish-subscription RPC response."
>  

Ok.

> I do mean that.  I don't think we should send:
> "establish-subscription-stream-error-info" for NETCONF when there is
> no hint included.  All the necessary error info is already in the
> existing message.

But is this really true?  In the
"establish-subscription-stream-error-info" there is a leaf called
"reason", which is an identity with a detailed error reason, for
example "history-unavailable".  This info is not present anywhere else
in the rpc-error, so it needs to be present here.

> > change the
> have a major issue if we leave it as it is?

No.  As I said, I found it confusing, but if others think it is more
helpful without my suggested changes, we should leave it as is.

> https://www.ietf.org/mail-archive/web/netconf/current/msg14902.html 

Ok, if you believe that this thread shows that the WG supports this
feature, I won't pursue this.

> receiver, the leading event record..."

Hmm, not sure this helps.  How about:

OLD:

   As a result, when any configured subscription receivers become
   active, buffered event records will be sent immediately after the
   "subscription-started" notification.  If the publisher does not know
   the next record intended for the receiver, the leading event record
   sent will be the first event record subsequent to the latest of three
   different times: the "replay-log-creation-time", "replay-log-aged-
   time", or the most recent publisher boot time.

NEW:

   As a result, when any configured subscription receivers become
   active, buffered event records will be sent immediately after the
   "subscription-started" notification.  The leading event record
   sent will be the first event record subsequent to the latest of four
   different times: the "replay-log-creation-time", "replay-log-aged-
   time", the most recent publisher boot time, and the time of the
   last event record sent to the reciever, if any.

> I have also made a corresponding tweak to the YANG model.

In the leaf "configured-replay"?  I couldn't find this text.

> > use the 5277

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 13 Sep 2018 16:21:17 +0200 (CEST)

Hi,

One problematic thing below.

>            node has this top-level element as the only child.

This text is problematic b/c it doesn't work well with inline
notifications.

Consider the example in section 7.16.3 in RFC 7950:

       container interfaces {
         list interface {
           key "name";
           leaf name {
             type string;
           }
           notification interface-enabled {
             leaf by-user {
               type string;
             }
           }
         }
       }
       
and corresponding XML example:

     <notification
       xmlns="urn:ietf:params:xml:ns:netconf:notification:1.0">
       <eventTime>2008-07-08T00:01:00Z</eventTime>
       <interfaces xmlns="urn:example:interface-module">
         <interface>
           <name>eth1</name>
           <interface-enabled>
             <by-user>fred</by-user>
           </interface-enabled>
         </interface>
       </interfaces>
     </notification>

What, in this example, is the "event record"?  I assume it is this
part:

       <interfaces xmlns="urn:example:interface-module">
         <interface>
           <name>eth1</name>
           <interface-enabled>
             <by-user>fred</by-user>
           </interface-enabled>
         </interface>
       </interfaces>

But the element that is the name of the YANG notification is this:

           <interface-enabled>
             <by-user>fred</by-user>
           </interface-enabled>

(With draft-ietf-netconf-notification-message, the "event record"
would be placed in the <notification-contents> anydata)

A problem is that the containment hierarchy is defined in RFC 7950 in
the XML encoding text.  There is no text in RFC 7951 or RFC 8040 for
how to do this in JSON for RESTCONF.  It is natural to do a pure
JSON-translation of the XML example, but this isn't specified
anywhere.

So I propose the following tweaked changes:

OLD:

NEW:

   An event stream is a named entity on a publisher which exposes a
   continuously updating set of event records.  An event record is an
   intantiation of a "notification" YANG statement.  If the
   "notification" is defined as a child to a data node, the
   intantiation includes the hierarchy of nodes that identifies the
   data node in the datastore (see Section 7.16.2 of RFC 7950).  Each
   event stream is available for subscription.  It is out of the scope
   of this document to identify a) how streams are defined (other than
   the NETCONF stream), b) how event records are generated, and c) how
   event records are assigned to streams.

and in the YANG module:

OLD:

NEW:

          event stream.

and

OLD:

NEW:

           the event stream.

> thread I found that examples for filters were requested before)).

And I strongly think that we should add examples in an appendix, even
though this is a protocol and encoding neutral document.  Maybe one
example with NETCONF XML and one with RESTCONF JSON.  One example can
show a subtree filter and a top-level notif, and the other an XPath
filter with an inline notif.

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 13 Sep 2018 15:08:11 +0000

Hi Martin,

I am fine with the three text clarifications proposed, and will incorporate.   More in-line...

> From: Martin Bjorklund, September 13, 2018 10:21 AM
> 
> Hi,
> 
> > > > > Yes this was discussed previously.  My reading was that others
> > > > > wanted to support subscription to non-YANG events.
> > > > > Applying xpath filters against XML based event records seems
> > > > > viable to me.
> > > >
> > > > If you want to make this specification open for other data
> > > > modeling languages, I think you need to carefully define how that
> > > > is supposed to work; essentially similar to how we handle
> > > > transports; we have a set of requirement on what a transport
> > > > document must define.  IMO this is super complicated and overkill.
> > > If you are just saying that we can put everything into a YANG
> > > wrapper, that is fine with me.  To clarify this, I have tweaked the
> > > first sentence of Section 2.1 to:
> This text is problematic b/c it doesn't work well with inline notifications.
> 
> A problem is that the containment hierarchy is defined in RFC 7950 in the XML
> encoding text.  There is no text in RFC 7951 or RFC 8040 for how to do this in
> JSON for RESTCONF.  It is natural to do a pure JSON-translation of the XML
> example, but this isn't specified anywhere.
> 
> 
> And I strongly think that we should add examples in an appendix, even though
> this is a protocol and encoding neutral document.  Maybe one example with
> NETCONF XML and one with RESTCONF JSON.  One example can show a
> subtree filter and a top-level notif, and the other an XPath filter with an inline
> notif.

We have worked very hard to keep the document protocol agnostic.  Even the Appendix A example protocol for configured subscriptions is 'foo'.

If you are ok with it, I am completely fine with adding two such NETCONF XML filtering examples into a new Appendix section A.4 in NETCONF-Notif.  Perhaps Reshad would be able to do the same for RESTCONF JSON within RESTCONF-Notif.

Eric

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 13 Sep 2018 16:38:55 +0000

Hi Martin,

> From: Martin Bjorklund, September 13, 2018 3:22 AM
> 
> Hi,
> 
[...]
> > > set of requirement on what a transport document must define.  IMO
> > > this is super complicated and overkill.
> (I think it would be useful with some filtering examples, like in section 5 of RFC
> 5277, but I think you have all examples in netconf-notif, so they would go into
> that draft.  (re-reading an old thread I found that examples for filters were
> requested before)).

This portion of this thread resulted in updated text based on the updated thread you sent out.  We should continue discussions there if there are questions.

> > > > Of course it will be up to the definition of any particular stream
> > > > to articulate the event record contents sufficiently for filters
> > > > to be created.
> > > >
> > > > > o  Section 2.1
> > > > >
> > > > >   The text says:
> > > > >
> > > > >     Among
> > > > https://www.ietf.org/mail-archive/web/netconf/current/msg14764.htm
> > > > l he says "I would say SHOULD be defined in YANG, for the
> > > > "NETCONF"
> > > > stream."
> > >
> > > I strongly disagree.
> > >
> Yes I think yang-data must be excluded (actually this was one of the concerns,
> raised by Andy I think; that people start to use yang-data for things that we
> already have proper YANG statements for...)

Ok.  With the text in the other thread, this should be covered.

> > > > To me, thinking about how that statement and applying it to YANG,
> > > > this translates as YANG 1.1 notifications should go into the
> > > > NETCONF stream, unless explicitly excluded.
> > >

Updated

[...]
> > > > underneath RESTCONF is HTTP, TLS, TCP.  And each of those lower
> > > > layers can have sessions.
> > > >
> > > > "The transport selected by the subscriber to reach the publisher
> > > > MUST be able to support multiple "establish-subscription" requests
> > > > without each requiring a separate persistent transport connection."
> > > >
> > > > preclude an implementation which can only support one subscription
> > > > per subscriber.  In any case, the specific "how" details will
> > > > still need to be articulated in each transport draft.
> > >
> > > My point is that this a transport-specific requirement, and it
> > > doesn't belong in this document.
> >
> Ok, I agree that the text belongs to section 5.3 rather than 2.4.2.  I just don't
> agree with the text ;-)  But I can accept it.

Thanks.

> > > > Should it be sent before or after the "replay-start-time"?  How
> > > > long should we allow the clocks to drift before we reject the
> subscription?
> "establish-subscription-stream-error-info" there is a leaf called "reason", which
> is an identity with a detailed error reason, for example "history-unavailable".
> This info is not present anywhere else in the rpc-error, so it needs to be
> present here.

For NETCONF, this info should be mapped into <error-app-tag>.  See section 7 of NETCONF-Notif, as well as Figure 10 for an example.  Or section 3 of RESTCONF-Notif, with Figure 7.

This was done to align things with existing NETCONF & RESTCONF error mechanisms.

> > > > "establish-subscription-stream-error-info" would be redundant.
> > > > For NETCONF, "establish-subscription-stream-error-info" are only
> > > > needed when hints to be provided.
> > > >
> > > > this point, I have tweaked the text to:
> > > > "This MUST be returned if an RPC error reason has not been mapped
> > > > down into transport specific error objects contained within a
> > > > failed "establish-subscription" RPC response."
> > >
> > > > > diagram specifically is the state machine *per receiver*, maybe
> > > > > we can simply remove the word "receiver" from the boxes?
> > > >
> No.  As I said, I found it confusing, but if others think it is more helpful without
> my suggested changes, we should leave it as is.

Will see if others chime in.
 
> Ok, if you believe that this thread shows that the WG supports this feature, I
> won't pursue this.

Thanks

>    last event record sent to the reciever, if any.

The reason I didn't word it this way is if there are multiple events with the same timestamp.  How about the text below...

NEW 

If the publisher knows the last event record sent to a receiver, and the publisher has not rebooted, the next event record on the stream which meets filtering criteria will be the leading event record sent.  Otherwise, the leading event record will be the first event record meeting filtering criteria subsequent to the latest of three different times: the "replay-log-creation-time", "replay-log-aged-time", or the most recent publisher boot time.

> In the leaf "configured-replay"?  I couldn't find this text.

Not in the configured-replay leaf, that looks good to me.   The first time this object is configured, this is when replay starts.   If it turns out there are multiple subscription-started notifications due to transport loss, the YANG model won't change.

To find the updated text, look for: refine "target/stream/replay-start-time"

Eric

[..]

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 13 Sep 2018 19:25:40 +0200 (CEST)

"Eric Voit (evoit)" <[email-masked]> wrote:
> > the XML
> > though
> We have worked very hard to keep the document protocol agnostic.  Even
> the Appendix A example protocol for configured subscriptions is 'foo'.
> 
> If you are ok with it, I am completely fine with adding two such
> NETCONF XML filtering examples into a new Appendix section A.4 in
> NETCONF-Notif.  Perhaps Reshad would be able to do the same for
> RESTCONF JSON within RESTCONF-Notif.

I think that having an example in one encoding for one protocol (or
two of them) wouldn't make the document less protocol agnostic.  For
example RFC 8342 (the NMDA architecture document) has some XML
examples.

See also section 3.12 of rfc6087bis.  (at least once I added examples
to a document during the AD review).

Examples also help verify that the text is clear.

This said, I won't object to the document moving forward w/o
examples.

/martin

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Thu, 13 Sep 2018 17:28:01 +0000

PSI.

On 2018-09-13, 11:08 AM, "Eric Voit (evoit)" <[email-masked]> wrote:

    Hi Martin,

    

    Eric
    
    <RR> Yes  if it gets added to NETCONF-Notif, I agree to add similar examples tp RESTCONF-Notif.

Regards,
Reshad.

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 13 Sep 2018 19:41:30 +0200 (CEST)

Hi,

[trimming...]

> > "reason", which
> For NETCONF, this info should be mapped into <error-app-tag>.  See
> section 7 of NETCONF-Notif, as well as Figure 10 for an example.  Or
> section 3 of RESTCONF-Notif, with Figure 7.
> 

Ok.  I thought the special error-app-tag thing was removed.  If we
have the error-app-tag value, why do we also have the "reason" leaf?

Since I thought that the error-app-tag was removed, I asked Alex to
remove them from the examples in YANG push (that doc has examples ;).
These examples in YANG push need to put back the error-app-tag.

So now we're back to the main issue.  Is it clear from the SN text
when to include the error-info yang-data stuff or not?

[...]

> The reason I didn't word it this way is if there are multiple events
> with the same timestamp.  How about the text below...
> 
> NEW 
> 
> If the publisher knows the last event record sent to a receiver, and
> the publisher has not rebooted, the next event record on the stream
> which meets filtering criteria will be the leading event record sent.
> Otherwise, the leading event record will be the first event record
> meeting filtering criteria subsequent to the latest of three different
> times: the "replay-log-creation-time", "replay-log-aged-time", or the
> most recent publisher boot time.

Ok.

/martin

> Not in the configured-replay leaf, that looks good to me.  The first
> time this object is configured, this is when replay starts.  If it
> turns out there are multiple subscription-started notifications due to
> transport loss, the YANG model won't change.
>

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 13 Sep 2018 18:25:52 +0000

Hi Martin,

> From: Martin Bjorklund, September 13, 2018 1:42 PM
> 
> Hi,
> 
> > > > > > The intent is to match how NETCONF & RESTCONF historically
> > > > > > handles errors.  As you know with NETCONF, each error identity
> > > > > > will be inserted as the "error-app-tag".
> > > > >
> > > > > It could also be in "error-tag" and/or "error-message".  My
> > > > > point is that I don't think that you mean that if an
> > > > > implementation populate say "error-message", then it should not
> > > > > popluate error-info with
> > > > "establish-subscription-stream-error-info" for NETCONF when there
> > > > is no hint included.  All the necessary error info is already in
> > > > the existing message.
> > >
> > > example "history-unavailable".
> > > This info is not present anywhere else in the rpc-error, so it needs
> > > to be present here.
> >
> Ok.  I thought the special error-app-tag thing was removed.  If we have the
> error-app-tag value, why do we also have the "reason" leaf?

Because of the need to be transport agnostic.  Other protocols are unlikely to have a pre-existing "error-app-tag" construct.  And the current YANG data structure can be inserted where needed and contain

> Since I thought that the error-app-tag was removed, I asked Alex to remove
> them from the examples in YANG push (that doc has examples ;).
> So now we're back to the main issue.  Is it clear from the SN text when to
> include the error-info yang-data stuff or not?

I have added a new sentence which hopefully clarifies this intent within section 2.4.  I have also re-added the second sentence of each bullet to something close to the original text of -v16.  It now reads:

   There is one final set of transport independent RPC error elements
   included in the YANG model.  These are three yang-data structures
   which enable the publisher to provide to the receiver that error
   information which does not fit into existing transport layer RPC
   structures.  These three yang-data structures are:

   1.  "establish-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
       "establish-subscription" RPC response.  This MUST be sent if
       hints on how to overcome the RPC error are included.

   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
       on how to overcome the RPC error are included.

   3.  "delete-subscription-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
       "delete-subscription" or "kill-subscription" RPC response

Eric

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 13 Sep 2018 20:43:30 +0200 (CEST)

"Eric Voit (evoit)" <[email-masked]> wrote:
> Because of the need to be transport agnostic.  Other protocols are
> unlikely to have a pre-existing "error-app-tag" construct.

... which is a reason to not do the error-app-tag trick; it would then
work the same across all protocols.

But are you sure that these "other protocols" even has a mechanism to
insert the new error-info structures into their errors?

> And the
> current YANG data structure can be inserted where needed and contain
> 

Ok, I don't like this solution, but I can accept it.

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 13 Sep 2018 20:37:06 +0000

> From: Martin Bjorklund, September 13, 2018 2:44 PM
> 
> > > > > > > > handles errors.  As you know with NETCONF, each error
> > > > > > > > identity will be inserted as the "error-app-tag".
> > > > > > >
> > > > > > > implementation populate say "error-message", then it should
> > > > > > > not popluate error-info with
> > > > > > > establish-subscription-stream- error-info.
> > > > > >
> > > > > > "establish-subscription-stream-error-info" for NETCONF when
> > > > > > there is no hint included.  All the necessary error info is
> > > > > > already in the existing message.
> > > > >
> > > > > "establish-subscription-stream-error-info" there is a leaf
> > > > > called "reason", which is an identity with a detailed error
> > > > > reason, for example "history-unavailable".
> > > > > This info is not present anywhere else in the rpc-error, so it
> > > > > needs to be present here.
> > > >
> > > > section 7 of NETCONF-Notif, as well as Figure 10 for an example.
> > > > Or section 3 of RESTCONF-Notif, with Figure 7.
> > > >
> ... which is a reason to not do the error-app-tag trick; it would then work the
> same across all protocols.

Yes, that was our original proposal.   The WG requested this change about a year ago.  (I can find the threads if necessary.)
 
> But are you sure that these "other protocols" even has a mechanism to insert
> the new error-info structures into their errors?

We will need to look at them one-by-one.  But I am hoping yes.
 
> > I have added a new sentence which hopefully clarifies this intent within
> section 2.4.  I have also re-added the second sentence of each bullet to
> something close to the original text of -v16.  It now reads:
> >
> >

Thanks.

Eric

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Mon, 17 Sep 2018 15:26:09 +0000

I have placed Xpath and subtree examples into section A.4 of:
https://github.com/netconf-wg/notif-netconf/blob/master/draft-ietf-netconf-netconf-event-notifications-12.txt

I will upload NETCONF-Notif v12 once comments on subscribed-notifications slow...

Eric

> From: Reshad Rahman, September 13, 2018 1:28 PM
> 
> PSI.
> 
>     > > > > > https://www.ietf.org/mail-
> archive/web/netconf/current/msg14764.h
>     > > > > > tml
>     > > > >
>     > > > > https://www.ietf.org/mail-
> archive/web/netconf/current/msg14766.htm
>     > > > > l
>     > > > > for a clarification.
>     > > > >
>     If you are ok with it, I am completely fine with adding two such NETCONF
> XML filtering examples into a new Appendix section A.4 in NETCONF-Notif.
> Perhaps Reshad would be able to do the same for RESTCONF JSON within
> RESTCONF-Notif.
> 
>     Eric
>