Re: [Netconf] LC on netconf-event-notifications-08

[Mahesh Jethanandani <[e-mail]>]

Eric,

> On Mar 8, 2018, at 10:06 AM, Eric Voit (evoit) <[e-mail]> wrote:
> 
> Hi Kent,
>  
> Thanks for the review.    In-line...
>  
>  
> > From: Kent Watsen, March 7, 2018 9:05 PM
> >
> > Abstract:
> >   - too terse.  Also, what does "binding" really mean?  I suggest rephrasing
> >     and expanding.
>  
> The current abstract was suggested by Mahesh during his previous review.  He asked for this simplification in the following way:   "Since you repeat what is included in the abstract in the introduction section also, you can shorten the abstract to say that it provides NETCONF bindings for subscribed notifications and yang-push - period”

>  
> I agree with you and also believe earlier versions of the abstract were more complete.  If you and Mahesh are fine with it, I would love to revert to one of the previous abstracts such as:
>  
>    This document provides a NETCONF binding for subscription to event notifications and YANG datastores.  Included are:
>  
>    o  transport mappings for subscription RPCs, state change notifications, and notification messages,
>    o  functional requirements, and
>    o  examples 
>  

Even if we were to restore it, what does “binding” mean? I would rather steal part of the introduction for the abstract. How about:

"This document defines mechanisms for asynchronous message notification delivery as part of the NETCONF [RFC6241] protocol. This optional capability is built on top of existing NETCONF notification mechanism, and provides a superset of the functionality defined in [RFC5277]."

> > Introduction:
> >   - again, what does "binding" really mean?  - can you be more concrete  about what all is included?
>  
> The word ‘binding’ is a recent change which suggested several reviews ago.  If we want to avoid saying binding, and we don’t want to repeat text from the abstract (per Mahesh’s comment), perhaps we could return to something close to the text from –v04.  This text would make the intro:
>  
> This document defines mechanisms that provide an asynchronous message notification delivery service for the NETCONF protocol [RFC6241] based on [I-D.draft-ietf-netconf-subscribed-notifications].  This is an optional capability built on top of the base NETCONF definition.
>  
> The document [I-D.draft-ietf-netconf-subscribed-notifications] plus this document provides a superset of the functionality defined in [RFC5277]. 
>  
> In addition, [I-D.ietf-netconf-yang-push] plus the capabilities of this document provide a mechanism for a NETCONF client to maintain a subset/extract of an actively changing YANG datastore.
>  
>  
> >   - does this document actually "enable" something?
>  
> The suggest Intro text just above should cover the “enable” part.
>  
> >   - are the events "from" a datastore, or "for" a datastore?
>  
> “from” is correct.
>  
> > Section 2:
> >   - missing reference to RFC 8174
>  
> Hmm.  I used to XML to RFC tool.   Not sure why it didn’t insert this.   I can manually insert.
>  
> > Section 3:
> >   - the 2nd sentence is not needed, remove?
>  
> Will do.
>  
> >   - the last sentence seems odd, and perhaps unnecessary, as this builds
> >     on top RFC5277 notifications, for which the :interleave capability already
> >     specifies this behavior.  I think that you might be trying to say it in a
> >     way that extends to notification-messages, but it doesn't read that way
> >     to me.  Do you mean "operations"?
>  
> Based on Martin’s reading of 5277 section 6 (interleave) he thought it worthwhile to explicitly say that interleave works needs the work with the subscribed-notifications.  For example: the interleave definition in RFC5277 section 6.1 hints there is only one subscription, and section 6.5 points out interactions with <create-subscription>.  For more on this discussion see:
> https://www.ietf.org/mail-archive/web/netconf/current/msg14186.html <https://www.ietf.org/mail-archive/web/netconf/current/msg14186.html>   
> As you can see from that thread, I addressed Martin’s point with text in Section 4.
>  
> So actually with the text is Section I do think this particular sentence can be safely removed.    Anyone have an objection to my removing it?
>  
> > Section 4:
> >   - the first sentence compares apples to oranges (a draft vs an RPC)
>  
> The was text was generalized to cover support for both configured subscriptions and RPCs in subscribed-notifications.  I can reword too:
>  
> A publisher is allowed to concurrently support configured subscription and dynamic subscription RPCs of [I-D.draft-ietf-netconf-subscribed-notifications], as well as [RFC5277]'s "create-subscription" RPC.
>  
> >   - in first sentence, "this support" should be reworded
>  
> I can turn the second sentence into:
>  
> However supporting independent subscriptions from these two specifications MUST NOT happen on a single transport NETCONF session.
>  
> >   - the first bullet point confuses me.  I get the gist of what it's trying
> >     to say, but the word "subscription" is ambiguous in this context
>  
> I can tweak the words to:
>  
> A solution must reply with the [RFC6241] error "operation-not-supported" if a "create-subscription" RPC is received on a NETCONF session where any other[I-D.draft-ietf-netconf-subscribed-notifications] or [RFC5277] subscription exists.
>  
> >   - is the second bullet point needed? I mean, how could this happen?
>  
> Yes.   What if someone configures a subscription, and there is an active session from <create-subscription> there.  And actually that case brings up a good point, I could add text within parenthesis below for the following corner-case text to Section 6.2:
>  
> In cases where a configured subscription has a receiver in the connecting state and the protocol configured as NETCONF, but no NETCONF transport session exists to that receiver (or all existing NETCONF transport sessions are currently supporting [RFC5277] subscriptions), the publisher MUST initiate a transport session via NETCONF call home [RFC8071], section 4.1 to that receiver.
>  
>  
> >   - again, the word "subscription" is ambiguous in this context
>  
> Will make “... [RFC5277] created subscription.”
>  
> And likewise on the following bullet, can make it:
>  
> “...if any [I-D.draft-ietf-netconf-subscribed-notifications] or [RFC5277] subscription is...”
> > Section 5:
> >   - why is the first sentence a normative statement?  isn't it given from
> >     RFC 6241?  Maybe rephrase to indicate from which RFC it comes from?
>  
> XML support is just optional for subscribed notifications, so I thought it would be good to make this explicit.   I can either remove, or reword to:
>  
> The "encode-xml" feature of [I-D.draft-ietf-netconf-subscribed-notifications] is mandatory to support.  This indicates that XML is a valid encoding for RPCs, state change notifications, and subscribed content.
>  
> >   - for the second sentence, already 5277 comes with NETCONF stream, so
> >     is this trying to cover the case when 5277 is NOT supported? - but
> >     then I think there needs to be more normative text to extend this
> >     definition.
>  
> The NETCONF stream is redefined in subscribed-notifications, section 2.1.   The normative text there allows the NETCONF stream to be available over other transports.  It also improves the definition of the stream. 
>  
> >   - for the third sentence, is the same true for any publisher
> >     supporting [I-D.ietf-netconf-yang-push]?
>  
> Not necessarily.
>  
> > Section 6:
> >   - the section is called "transport connectivity", but has subsections for
> >     configured/dynamic *subscriptions*.   As such, it makes me think this
> >     section should be called "Subscription Types".  Perhaps the subsection
> >     titles should be renamed (e.g., Related to --- Subscriptions?)
> >   - a segue sentence before Section 6.1 would help.
>  
> How about: 
>  
> “NETCONF connectivity and the subscription lifecycle”
>  
> With text before section 6.1 which says:
>  
> This section describes how the availability of NETCONF transport impacts the establishment and lifecycle of different types of [I-D.draft-ietf-netconf-subscribed-notifications] subscriptions.
>  
> > Section 6.2:
> >   - I'm expecting this info to be in subscribed-notifications.  However,
> >     searching for "8071" in subscribed-notifications finds nothing.
>  
> Transports other than NETCONF and RESTCONF are considering the use of subscribed-notifications and YANG push.   As a result, it seems like the actual mechanism for call home should be included within the transport dependent document.
>  
> >     Even if there is a good reason for why it's not normative in subscribed-
> >     notifications, that draft should say something like "bindings must
> >     enable publishers to proactively establish connections with subscribers
> >     (e.g., RFC8071).
>  
> The could be included in subscribed-notifications.    In section 2.5.1, text could be inserted:
>  
> “... initialized to connecting.  If transport connectivity is not available to any receiver, a transport session is established (e.g., through [RFC8071]).  Individual receivers are...”
>  
> >   - second to last paragraph, why is "Transport" capitalized here?
>  
> Will make it a small ‘t’.
>  
> >   - the second to last paragraph is not needed.  It's already a SHOULD in
> >     RFC 8071, S7.  Perhaps you want to make a non-normative statement, or
> >     clearly indicate that the normative statement is coming from RFC 8071?
>  
> I will make it non-normative:  “ the publisher restarts”
>  
> >   - in this whole section, when referring to the ""connecting" state", it
> >     would help if the text indicated that this is from the subscribed-
> >     notifications draft.
>  
> Will refine to: “"connecting" state as described in [I-D.draft-ietf-netconf-subscribed-notifications], section 2.6.1.”
>  
> > Section 7:
> >   - what does "NETCONF" mean here?  - the protocol, the stream, the WG?
>  
> With the two previous words being “transported over”,  and the section being “Notification Messages” I didn’t see a need.  Nevertheless I will say “NETCONF protocol”
>  
> > Section 8:
> >   - first sentence, isn't it just subscribed-notifications, per what's in
> >     the introduction?
>  
> I simplified the intro based on Mahesh’s suggestion.  When we return to the earlier introduction as I suggest above, this concern should drop.
>  
> >    Also, are there specific sections that can be
> >     referenced?
>  
> I will make it “...[I-D.ietf-netconf-yang-push] 4.4 and [I-D.draft-ietf-netconf-subscribed-notifications], section 2.4 “
>  
> >   - 1st bullet point, s/"error-type"/an "error-type" node/?
> >   - 2nd bullet point, s/"error-tag"/an "error-tag" node/?
> >   - 3rd bullet point, s/Optionally,//
>  
> Will tweak for these three bullets.
>  
> >   - 4th bullet point, a wall of text, consider formatting (bullet points),
> >     and the ", respectively" at the end is not needed.
>  
> Will break into a table underneath the main bullet as follows:
>  
>    o  "error-app-tag" with the value being a string that corresponds to identity associated with the error, as defined in [I-D.draft-ietf-netconf-subscribed-notifications] section 2.4.6 for general subscriptions, and [I-D.ietf-netconf-yang-push] Appendix A.1, for datastore subscriptions. The tag to use depends on the RPC for which the error occurred.  Viable errors for different RPCs are as follows:
>  
>          RPC                     use base identity
>          ----------------------  ----------------------------
>          establish-subscription  establish-subscription-error    
>          modify-subscription     modify-subscription-error
>          delete-subscription     delete-subscription-error   
>          kill-subscription       kill-subscription-error
>          resynch-subscription    resynch-subscription-error
>  
> >   - 5th bullet point, s/optionally, "error-info"/ optionally, an
> >     "error-info" node/.   Another wall of text, consider formatting
> >     (bullet points)
>  
> Will break into a table underneath the main bullet as follows:
>  
> o  In case of error responses to an establish-subscription or modify-subscription request there is the option of including an "error-info" node.  This node may contain XML-encoded data with hints for parameter settings that might lead to successful RPC requests in the future.   Following are the yang-data structures which may be returned
>  
>          RPC made on a datastore  return hints in yang-data structure
>         -----------------------  -----------------------------------        
>          establish-subscription   establish-subscription-error-datastore
>          modify-subscription      modify-subscription-error-datastore
>  
>          RPC made on a stream     return hints in yang-data structure
>         -----------------------  -----------------------------------        
>          establish-subscription   establish-subscription-error-stream
>          modify-subscription      modify-subscription-error-stream
>  
>  
> >   - "These yang-data that is" - what is "yang-data" here?  also s/is/are/
> >     and s/"error-info"/the "error-info" node/?
> >   - last sentence, s/"error-path"/an "error-path" node/?
>  
> Will change the sentence to:
> “When the yang-data structures above are returned as part of an RPC response, the optional leaf "error-reason" SHOULD NOT be included.   Providing this leaf would be redundant with the information that is already placed within the  error-app-tag.”
>  
> > Section 9:
> >   - 3rd paragraph, what does this mean, since this draft doesn't define
> >     a module?  Perhaps instead say "this draft does not define a YANG
> >     module and therefore doesn't have any YANG-related Security
> >     Considerations."?   - and remove the rfc6536bis reference.
>  
> Yes, will do that.
>  
> > Appendix A.
> >   - the text should state if the section is normative or non-normative.
>  
> Will say non-normative.
>  
> >   - as I don't see any RFC 2119 language, I'll assume non-normative
> >     and stop my review here.  Given the number of issues found above, I
> >     suggest reviewing this section carefully.
> >   - looking forward to the YANG Doctor and the Shepherd reviews, please
> >     post a script that validates each example listed in this document.
>  
> I will ask Einar to do this.
>  
> If you are good with these change, let me know and I will post an interim version on the github site.
> 
> Eric
>  
> > Kent
> >
> >
> >
> > ===== original message =====
> >
> > WG,
> >
> > The authors of netconf-event-notifications have indicated privately that they
> > believe this document is now ready for Last Call.
> >
> > This starts a 2.5-week Last Call on draft-ietf-netconf-netconf-event-
> > notifications-08 [1] The LC will end on March 17, or when active threads peter
> > out.
> >
> > Please send your comments on this thread. Reviews of the document, and
> > statement of support, are particularly helpful to the authors.  If you have
> > concerns about the document, please state those too.
> >
> > Authors please indicate if you are aware of any IPR on the document.
> >
> >
> > [1] https://urldefense.proofpoint.com/v2/url?u=https- <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent-2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB-cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e>
> > 3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent- <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent-2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB-cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e>
> > 2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK- <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent-2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB-cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e>
> > ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=G <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent-2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB-cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e>
> > TTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB- <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent-2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB-cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e>
> > cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetconf-2Dnetconf-2Devent-2Dnotifications-2D08&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=1UJkiYEXB-cdME1VDg8JuvfeiHrpo6wjx4FWOJUTR28&e>=
> >
> >
> > Kent & Mahesh
> >
> >
> >
> >
> > _______________________________________________
> > Netconf mailing list
> > [e-mail] <mailto:[e-mail]>
> > https://urldefense.proofpoint.com/v2/url?u=https- <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e>
> > 3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6S <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e>
> > cbfh0UjBXeMK- <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e>
> > ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=G <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e>
> > TTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v- <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e>
> > pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=GTTYeioeUTd_V-M93k5RNRCFhoaoNiS4kY-v-pIV0kE&s=XTJIg4iBdppRM_RXJCeh51GZFGMJD2S66B2uPmHtx0E&e>=
> >
> >
> > _______________________________________________
> > Netconf mailing list
> > [e-mail] <mailto:[e-mail]>
> > https://www.ietf.org/mailman/listinfo/netconf <https://www.ietf.org/mailman/listinfo/netconf>_______________________________________________
> Netconf mailing list
> [e-mail] <mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/netconf <https://www.ietf.org/mailman/listinfo/netconf>
Mahesh Jethanandani
[e-mail]