Re: [Netconf] two-week review of drafts related to notifications and subscriptions

Hi Kent,

Thanks very much for the review.  Thoughts in-line.

BTW: I do appreciate you calling the two week review.  This has worked well.  I have blocked my calendar for the next week so that I can interact (and hopefully incorporate) items as quickly as feasible.   

The good news is that we have had more comments on the drafts in the last week than in the two years since draft adoption, I would love to keep the WG focus here in the coming weeks to close these items.

> From: Kent Watsen, October 18, 2017 10:03 PM
> 
> As a contributor, here is my review.  I focused on the three drafts that the
> authors have stated a desire to see go to last call:
> 
>    o  draft-ietf-netconf-subscribed-notifications
>    o  draft-ietf-netconf-yang-push
>    o  draft-ietf-netconf-netconf-event-notifications
> 
> Note: the below do not represent full reviews.  I was literally jumping around
> and just noticing what caught my eye.  Random sampling, though I tended to
> spend a fair amount of time on the tree diagrams and YANG modules.
> 
> 
> 
> draft-ietf-netconf-subscribed-notifications-05
> ------------------------------------------------------
> status: not ready yet.  

Hopefully not for long  ;-)

> very complicated (many details).

If there anything you see as unnecessary, or perhaps should be optional?

>         flow difficult to read.

Anything specific?  I am happy to re-order as suggested.

> * what are "subscription control plane operations bindings"?

Fixed that based on Martin's comments

> * are "pushed event instances" different than the above?

No.  Also fixed that based on Martin's comments.

> * what is "promise theory based interaction model"?  (reference?)

Yes, I will add the reference.  The best on-line one I know is:
http://markburgess.org/PromiseMethod.pdf 

> * I see transport identities for netconf and http2, but why not for restconf too?
> (I don't see it defined in restconf-notif-03 either)

RESTCONF is not involved with configured subscriptions (see restconf-notif Section 3.1.3 for HTTP2 flow).  So there is no reason for an identity for this purpose.

While is it true that RESTCONF is used for dynamic subscriptions, RESTCONF is never involved with the transport of the one-way notifications.  These always go as a POST on a different TCP connection.   See restconf-notif for 3.1.1 via HTTP2, and 3.1.2 for HTTP1.1.   So it is not necessary to have a RESTCONF identity as a transport for even for operational reporting.

> * what to do when transport=http2 isn't clear

Based on Martin's comments I have added http1.1 as an identity for operational reporting.   Either HTTP1.1 or HTTP2 can be associated with the dynamic subscription established via RESTCONF (as specified in restconf-notif for 3.1.1 via HTTP2, and 3.1.2 for HTTP1.1).

> * Is the "NETCONF" event stream same as in 5277 s3.2.3?

Yes.

And we don't have identities for streams anymore.  So the definition of the NETCONF stream from 5277 s3.2.3 is listed in section 2.1, and the mandatory support for the NETCONF stream when NETCONF transport is used is in notif-netconf section 3.2.   (In the version of subscribed-notifications you read, there was also a steam definition housed in the now nonexistent NETCONF identity description).

> * what is the relationship between streams here and in 5277?

This document doesn't discuss the details of how streams are generated within the publisher like 5277 does.  This detail is not necessary for the external interface definition.

That said,  how to handle/encode names of the streams, the managed objects, the ability to filter, etc. are identical.

> * streams here are identities, unlike "streamNameType" (aka xs:string) in 5077.
> Is this an issue?

Based on Martin's comments that he desires to retain the ability for streams to be spun up possibly during run-time, we have returned to using string for streams (as we did in earlier versions of the draft).   Our switch to identities several months ago was one attempt we made to simplify things, but it didn't work out.    This is an issue and a resolution we have long been anticipating.

> * use of "<edit-config> operations" for configured subscriptions is confusing.

It is just a normal configuration.  I will happily delete the example if you think it unnecessary or redundant.

> * the example in s5.1 seems to be more about how to create a configured
> subscription than establishing a subscription connection

Exactly.  How to establish the subscription connection is transport specific, and therefore in the netconf-notif draft.  Specifically in Section 3.4.3

> * Section 7 says "This notification message MAY be encoded as one-way
> notification element of [RFC5277], Section 4."  If this is a MAY, how does the
> client know what it received?  And how does the publisher know what format
> to send?

I was trying to lay the groundwork for a future support of notification-messages in a way that this document wouldn't have to be updated.  I have given up on that.  Support is now a MUST.  And this will need to be revised in the future when we get notification-messages to an RFC state.

> * xpath-filter is not a feature-based option?

Not feature based.  Of course if the syntax is not supported on a filter, the subscription will be rejected with the error " filter-type-unsupported ".
At this point, I don't recall anyone suggesting xpath should not be a mandatory for event filtering though.  

> * I see top-level /subscription-config and /subscriptions - is this NMDA
> compliant?

As this pre-dates NMDA, we are awaiting the "you MUST make the conversion" directive.  And if this comes, we will gladly do that.

> * for configured subscriptions, how is security established between endpoints?

Via existing mechanisms for secure transport establishment.  E.g., Call home being used for NETCONF as per netconf-notif section 3.4.

> * for dynamic subscriptions, why is encoding an option, and not inherited from
> the transport instead?

If a RESTCONF establish-subscription and a NETCONF establish-subscription are coming to the same stream name (which under-the-covers is encoded in only one way, and will not be trans-codable) must we reject one of them as unsupportable?  What if you want a push of binary encoded content, must you make that request over a binary transport?  This is supposed to allow variations here.

> * having "hints" for what might work better seems complicated...

In the end, it is necessary.  And not just because people have asked for it:
https://www.ietf.org/mail-archive/web/netconf/current/msg12154.html 

When you get to YANG-Push, there are enough knobs that simply rejecting a subscription without guidance will lead to very poor subscribing client behavior.  The poor behavior will take the form of many unnecessary signaling interactions as subscribers will be making guesses blindly.

> * are event-notif and restconf-notif Informative references?

If you mean netconf-notif, then Yes.  I don't see them being normative, as they don't depend on this document.  Am I missing something?

> * draft-title seems odd

This is a result of history more than anything else.  There are only so many words to go around.  If "Custom Subscription to Event Streams" feels better, I have no issue with the change.

> draft-ietf-netconf-yang-push-09
> ------------------------------------------------------
> status: better, but could use a couple clean-up passes
> 
> * title: "[Subscribing to] YANG Datastore based Event Notifications"?

If we were to change, I think the simpler "Subscribing to YANG datastores" is probably cleaner

> * "rapid visibility" - sounds like marketing...

Will delete "rapid"

> * "On-change subscriptions allow subscribers to subscribe to updates" - it
> should be "receive updates", right?

Yes, that is cleaner.  Will fix.

> * why does s3.5.1 discuss XML/JSON encoding rules?
> * dscp, weighting, dependency - are these really needed now?

If it were just NETCONF, then no.  But there is real need for congestion management into Telemetry sever receiver clusters.  These clusters are a current choke point.  And head-of-line blocking from queued subscriptions is avoidable when HTTP2 is used.

> * s3.9: I have an issue with post-filtering?

I don't understand.  The sequence of when NACM is applied and when the selection filter is applied is not specified.  Just that the result is the same as with a GET.

> * shouldn't 'notifiable-on-change' be metadata?

Yes.   Is it sufficient just to say this explicitly in section 3.10, or are you looking for something else.

> draft-ietf-netconf-netconf-event-notifications-05
> -------------------------------------------------
> status: unclear purpose.  mostly seems to be NETCONF-based examples for
> stuff defined in [subscribe] without any normative text.  I'm unsure we need
> another draft just for this...

It is true that most of this is examples.  Examples which we can't place in subscribed-notifications without tying that draft to a specific transport.

That said, there is some normative elements
- use of call home
- timing of the message flows with respect to call home.  (Many reader might assume the correct timing, but we thought it better to be explicit.)
- Required availability of the NETCONF stream
- the making of the NETCONF stream as mandatory.  (For IoT use, there really isn't a reason for the NETCONF stream to be required.)
- how should retries work on a failed configured subscription

> * what is the "the base NETCONF definition"?

Should just be NETCONF RFC 6241.

> * in the examples in s3.1 (and many other sections in the draft), is the xmlns
> correct? should it be: urn:ietf:params:xml:ns:yang:ietf-subscribed-
> notifications?  

I didn't write these, but I did review them.   In my review my belief was that as yang-push imports ietf-yang-push, examples with that namespace should be equally valid.  Am I missing something?

> But, more importantly, why is this here at all, shouldn't it be in
> [subscribe]?

This is an example of a NETCONF get on the steams.  It is probably excessive to have this in either draft.  Section 3.1 can safely be cut out completely.

> * how often are the examples validated?  - in all the drafts?

I defer to Alberto.  I gave an eyeball.

> * s3.2 - also should be in [subscribe]

If IoT devices use subscribed-notifications for pushing SYSLOG extracts, must they support the NETCONF stream?   This is why we felt it better to put it here

> * let's not use "data plane" as a term

Agree.  The three instances of this need to be reworded.

> * There's an "Open Items" section in Appendix A.  The comment should read
> "(To be removed by Authors prior to Last Call)"  ;)

Very true.   All these elements are out-of-scope now, and it can be removed.

> Also, where is it that the solution relies on the server parsing capability strings
> from the client's <hello> message?  - I didn't see it this time, but recall
> discussing it with you before...
> 
> Lastly, just skimming these three drafts, I don't yet have a good feeling for how
> the new [notification-messages] based notification messages are brought in.

At this point, Martin has convinced me that notification-messages should be minimally highlighted so as not to confuse readers prior to its availability.  And that when that works gets done, we make revisions to the three drafts here.

Key items which will get touched here are:

Subscribed-notifications
- update section 7 to use the new notification messages if receiver has indicated support
- update to the establish-subscription RPC so that the subscriber indicates has the option of adding headers

Yang-push
- revision to notifications push-update and push-change-update to use the common headers 

Notif-netconf
- discovery mechanism for the receiver support of the new messaged

Notif-restconf (await the notification-messages draft to publish)
- configured subscriptions and HTTP2 must use the new format
- we probably should just make the format mandatory for the whole draft

Eric

> Thanks,
> Kent // contributor
> 
> 
> --
> 
> We’d like to direct the WG attention to the set of drafts related to notifications
> and subscriptions, with the goal of getting a pulse on if the WG thinks they are
> ready to progress now.
> 
> To enable you get an overview and to see how the documents relate to each
> other, this draft is a good place to start:  https://tools.ietf.org/html/draft-voit-
> netconf-subscription-and-notif-overview-00.
> 
> The drafts related to this review are:
>    -  draft-ietf-netconf-subscribed-notifications
>    -  draft-ietf-netconf-yang-push
>    -  draft-ietf-netconf-netconf-event-notifications
>    -  draft-ietf-netconf-restconf-notif
>    -  draft-ietf-netconf-notification-messages
> 
> While this is not an official WGLC, we will issue one if the response is positive.
> Please provide comments within two weeks.
> 
> Thanks,
> Mahesh and Kent
> 
> 
> 
> 
> 
> _______________________________________________
> Netconf mailing list
> Netconf@ietf.org
> https://www.ietf.org/mailman/listinfo/netconf