Subject: [netconf] YANG Push drafts comments

From: William Lupton <[email-masked]>
Date: Tue, 29 Jan 2019 11:40:30 +0000

All,

I'm sorry that the comments below are so late. I did ask the chairs what I
should do with them, and they said to send them to the list despite the
lateness from a process perspective.

The comments are based on the four WG consensus drafts as of late November.
I haven't revisited them to see which may have been addressed by subsequent
discussion.

Nearly all of the comments relate to
the draft-ietf-netconf-subscribed-notifications draft, plus there are some
general remarks, so it didn't seem worth splitting them into a message per
draft.

Cheers,
William

----

   1. General:
      1. Why is the subscription id optional? Without it, will the receiver
      always be able to tell how to handle an event message?
      2. YANG Push is not referred to consistently across the document set
      (presence or absence of hyphen, "yang" capitalization, "push"
      capitalization). There should be a single, consistently-used term.
   2. draft-ietf-netconf-subscribed-notifications:
      1. I wonder why it was decided that dynamic subscriptions wouldn't
      allow the receiver to be different from the subscriber. It seems like two
      different concepts are being mixed: dynamic/configured subscription and
      embedded/external receiver.
      2. Event occurrence time: This isn't quite clear. What if there was
      already a timestamp (from a lower-level system) when the event was
      identified? Should that be used?
      3. Event record:
         1. So does the "event record" term apply to all the drafts? If so,
         does "event" apply to all the drafts? (This note is in the
context of a
         later note about the fact that "event stream" appears to be
specific to
         this draft.)
         2. YANG notifications are explicitly defined in YANG models (e.g.
         the ietf-hardware YANG module defines a hardware-state-change
         notification). I thought that a major part of YANG Push is to
effectively
         allow notifications to be defined dynamically.
            1. Later: I think perhaps the point is that event streams are
            indeed restricted to YANG notifications, but that other
drafts (e.g. YANG
            Push) define mechanisms beyond event streams. Section 2.4 (dynamic
            subscriptions) says "These RPCs have been designed
extensibly so that they
            may be augmented for subscription targets beyond event streams".
            2. Even later: Ah! YANG Push defines generic "push-update" and
            "push-change-update" notifications. So it's all OK.
            4. Event stream:
         1. Having now read most of the YANG Push draft, I realize that
         "Event streams" are a legacy thing and are not used by YANG Push. This
         could be made clear. Otherwise it looks like these are a base
concept that
         will be built on.
         2. Perhaps the truly cleanest solution would be to move the legacy
         aspects to a separate document, or clearly indicate they're
legacy, or use
         less "core" names, or move this material to an Annex or Appendix.
         3. So referring to them as sets of Events (rather than sets of
         Event streams) seems inaccurate.
         5. Notification message:
         1. I don't find "The time the event was generated by the event
         source" quite clear. Is it the time at which the change
occurred or was
         detected (event occurrence time), or the time at which the message was
         created? Probably the former?
         6. Subscription:
         1. The section 2.2 description is very unclear. It appears that
         filters are on subscriptions, refer to records, and cause
"event messages"
         to be excluded from delivery. But "event message" isn't a defined term.
         3. draft-ietf-netconf-yang-push:
   1. YANG Push defines a new "datastore" subscription target that's
      independent of the existing "stream" subscription target. (Assuming that
      this is correct, I'd have liked some explanation of _why_ YANG Push isn't
      able to extend the base mechanism.)
      4. Editorial (there might be some duplication with earlier comments):
      1. Some YANG descriptions have incorrect indentation on the second
      and subsequent lines (such lines should begin under the first
character of
      the first line, i.e. one character to the right of the opening double
      quote). No doubt the YANG Doctors will catch such things!
      2. draft-ietf-netconf-subscribed-notifications-18
         1. Event stream definition: "Event stream: A continuous,
         chronologically ordered set of events" ... but should this be
"...of event
         records"
         2. s/intantiation/instantiation/ (two occurrences)
         3. The "subscription state change notifications" section name may
         have been subject to a s/Subscription/subscription/ edit?
Should capitalise
         "subscription" both here and where it's used at the start of sentences
         (there's at least one case of this)
      3. draft-ietf-netconf-yang-push-20
         1. There's a reference to "Custom Subscription to Event Streams".
         Is this the old title of "Subscription to YANG Event
Notifications"? If so,
         it needs to be updated
         2. Spurious "6 record" in the YANG (should be "record")
         3. In the data model introductory section, the "sn:" prefix isn't
         explained (I know it's sort of obvious, but the "yp:" prefix
(also obvious)
         _is_ explained)

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Tue, 29 Jan 2019 20:31:30 +0000

Hi William,

Thanks for the interest, thoughts in-line...

From: netconf <[email-masked]> On Behalf Of William Lupton
Sent: Tuesday, January 29, 2019 6:41 AM
To: [email-masked]
Subject: [netconf] YANG Push drafts comments

All,

I'm sorry that the comments below are so late. I did ask the chairs what I should do with them, and they said to send them to the list despite the lateness from a process perspective.

The comments are based on the four WG consensus drafts as of late November. I haven't revisited them to see which may have been addressed by subsequent discussion.

Nearly all of the comments relate to the draft-ietf-netconf-subscribed-notifications draft, plus there are some general remarks, so it didn't seem worth splitting them into a message per draft.

Cheers,
William

----
1.      General:
1.      Why is the subscription id optional? Without it, will the receiver always be able to tell how to handle an event message?
I assume you are referring to the pushed notification messages carrying event records.  The RFC-5277 one way notification doesn’t support a subscription-id object in its current event schema, so we can’t embed this object for subscribed-notifications.  This is one reason that draft-ietf-netconf-notification-messages was adopted.  For YANG push, we do have a subscription-id in the notification definition, which is then carried within the RFC-5277 one-way notification.
2.      YANG Push is not referred to consistently across the document set (presence or absence of hyphen, "yang" capitalization, "push" capitalization). There should be a single, consistently-used term.
I think these was caught/covered with other reviews.

2.      draft-ietf-netconf-subscribed-notifications:
1.      I wonder why it was decided that dynamic subscriptions wouldn't allow the receiver to be different from the subscriber. It seems like two different concepts are being mixed: dynamic/configured subscription and embedded/external receiver.
Mostly we were attempting to make attempting a DoS attack with this infrastructure harder.  I.e., by forcing the dynamic subscription to also be the receiver, it is harder to attack a third party receiver with an overwhelming amount of subscribed traffic.   To accomplish a DoS with configured subscriptions, you need to have configuration credentials.  Beyond this, there are other difficulties distributed session management didn’t want to take on.   If someone wants to go down this path, we are happy to advise.

2.      Event occurrence time: This isn't quite clear. What if there was already a timestamp (from a lower-level system) when the event was identified? Should that be used?
Lower-level system times can be used.  There is nothing which says where on the publisher this time might come from, just that the time should be as close to the actual time as possible.

3.      Event record:
1.      So does the "event record" term apply to all the drafts? If so, does "event" apply to all the drafts? (This note is in the context of a later note about the fact that "event stream" appears to be specific to this draft.)
Subscribing to an event stream is different than a subscribing to a datastore.  For event streams, some publisher process has already placed an event record on a time ordered series, which is then filtered per-subscriber.  For datastore subscriptions there is no such time ordered series.  Instead you need a trigger (periodic or on-change, the name of the datastore, the name of the objects, etc.) to define the custom extract which becomes the time ordered series.   Below is a chart from a presentation on this.  I think it was in the material from IETF 100.
So you can subscribe to either an event stream or a datastore.
2.      YANG notifications are explicitly defined in YANG models (e.g. the ietf-hardware YANG module defines a hardware-state-change notification). I thought that a major part of YANG Push is to effectively allow notifications to be defined dynamically.
1.      Later: I think perhaps the point is that event streams are indeed restricted to YANG notifications, but that other drafts (e.g. YANG Push) define mechanisms beyond event streams. Section 2.4 (dynamic subscriptions) says "These RPCs have been designed extensibly so that they may be augmented for subscription targets beyond event streams".
2.      Even later: Ah! YANG Push defines generic "push-update" and "push-change-update" notifications. So it's all OK.
4.      Event stream:
1.      Having now read most of the YANG Push draft, I realize that "Event streams" are a legacy thing and are not used by YANG Push. This could be made clear. Otherwise it looks like these are a base concept that will be built on.
I think event streams are useful.  I don’t consider them legacy.
2.      Perhaps the truly cleanest solution would be to move the legacy aspects to a separate document, or clearly indicate they're legacy, or use less "core" names, or move this material to an Annex or Appendix.
This was why we have a fully separate document for subscribed-notifications.  The WG requested that the authors retrofit the infrastructure documents in this way to uplevel what could be done with event streams.
3.      So referring to them as sets of Events (rather than sets of Event streams) seems inaccurate.
I don’t see any text on either sets of events, or sets of event streams.
5.      Notification message:
1.      I don't find "The time the event was generated by the event source" quite clear. Is it the time at which the change occurred or was detected (event occurrence time), or the time at which the message was created? Probably the former?
I can’t find the exact text quote:  "The time the event was generated by the event source" .   Where is this?
6.      Subscription:
1.      The section 2.2 description is very unclear. It appears that filters are on subscriptions, refer to records, and cause "event messages" to be excluded from delivery. But "event message" isn't a defined term.
I will refine “event message” into “event record” in the next document version release.
3.      draft-ietf-netconf-yang-push:
1.      YANG Push defines a new "datastore" subscription target that's independent of the existing "stream" subscription target. (Assuming that this is correct, I'd have liked some explanation of _why_ YANG Push isn't able to extend the base mechanism.)
It was recommended several years ago by members of the WG that subscribing to datastores and streams was sufficiently different, and that different targets were needed.  Some of the reasons are listed above.  Beyond what is listed above, another reason is that filters on a stream target exclude member of an already generated set of event records.  This is quite different from a datastore target, where the filters choose which nodes are of interest (i.e., include).

4.      Editorial (there might be some duplication with earlier comments):
1.      Some YANG descriptions have incorrect indentation on the second and subsequent lines (such lines should begin under the first character of the first line, i..e. one character to the right of the opening double quote). No doubt the YANG Doctors will catch such things!
2.      draft-ietf-netconf-subscribed-notifications-18
1.      Event stream definition: "Event stream: A continuous, chronologically ordered set of events" ... but should this be "...of event records"
It will certainly be implemented that way.  Early in the process, some people said that it is possible to have a pointer to the event records within the event stream.  So we left it with the more abstracted term ‘events’.
2.      s/intantiation/instantiation/ (two occurrences)
Fixed.
3.      The "subscription state change notifications" section name may have been subject to a s/Subscription/subscription/ edit? Should capitalise "subscription" both here and where it's used at the start of sentences (there's at least one case of this)
Fixed
3.      draft-ietf-netconf-yang-push-20
1.      There's a reference to "Custom Subscription to Event Streams". Is this the old title of "Subscription to YANG Event Notifications"? If so, it needs to be updated
2.      Spurious "6 record" in the YANG (should be "record")
3.      In the data model introductory section, the "sn:" prefix isn't explained (I know it's sort of obvious, but the "yp:" prefix (also obvious) _is_ explained)
<Eric> I will note these two Alex, who is hoping to post an update tonight.

Thanks!
Eric

---

From: William Lupton <[email-masked]>
Date: Wed, 30 Jan 2019 09:54:01 +0000

Thanks Eric. Please see some follow-ups below (I've removed items where I
have no further comment). Cheers, W.

On Tue, 29 Jan 2019 at 20:31, Eric Voit (evoit) <[email-masked]> wrote:

> *From:* netconf <[email-masked]> *On Behalf Of *William Lupton
> *Sent:* Tuesday, January 29, 2019 6:41 AM
> *To:* [email-masked]
> *Subject:* [netconf] YANG Push drafts comments
>
>
> 1.      General:
>
> 1.      Why is the subscription id optional? Without it, will the
> receiver always be able to tell how to handle an event message?
>
> I assume you are referring to the pushed notification messages carrying
> event records.  The RFC-5277 one way notification doesn’t support a
> subscription-id object in its current event schema, so we can’t embed this
> object for subscribed-notifications.  This is one reason that
> draft-ietf-netconf-notification-messages was adopted.  For YANG push, we do
> have a subscription-id in the notification definition, which is then
> carried within the RFC-5277 one-way notification.
>

I would expect this question to arise in the minds of many readers. A note
might be useful?

> does "event" apply to all the drafts? (This note is in the context of a
> later note about the fact that "event stream" appears to be specific to
> this draft.)
>
> Subscribing to an event stream is different than a subscribing to a
> datastore. [...]
>

This was a terminology comment. I felt that it wasn't always clear which
terms were generic and would apply to all drafts. "event record", "event"
and "event stream" caused me particular problems. It was just a comment...
if people don't think there's a need for clarification (or greater rigor in
the use of terms) then that's fine.

> could be made clear. Otherwise it looks like these are a base concept that
> will be built on.
>

I didn't mean "legacy" pejoratively. Reading the draft I was thinking "Ah,
event streams are a core concept that will be leveraged by the other
drafts". So I was surprised to discover that YANG Push doesn't use them.
Assuming that I'm correct here, I think that a note (either here or in YANG
Push) could be helpful.

> I don’t see any text on either sets of events, or sets of event streams.

Sorry, part of this got lost in translation. I intended to refer to the
fact that:

   - Event stream is defined (1.2) as "A continuous, chronologically
   ordered set of events..."
   - And then later (2.1) it seems to be suggested that an event stream is
   a set of event records (rather than a set of events)

(But re-reading it just now, I'm not sure whether it really says this! I
also note that you commented on this point later. So we can drop this one.)

> source" quite clear. Is it the time at which the change occurred or was

Sorry. The actual text is from RFC 5277, describing eventTime. Obviously
I'm not suggesting that RFC 5277 be changed!