Re: [RTG-DIR] RtgDir review: draft-ietf-netconf-netconf-event-notifications-17

["Eric Voit (evoit)" <[e-mail]>]

Hi Dhruv,
Hi Kent,

Thanks very much for the comments Dhruv.  Thoughts in-line, with one question to Kent...

> From: Dhruv Dhody, April 30, 2019 12:53 AM
>
> Hello,
> 
> I have been selected as the Routing Directorate reviewer for this draft. The
> Routing Directorate seeks to review all routing or routing-related drafts as they
> pass through IETF last call and IESG review, and sometimes on special request.
> The purpose of the review is to provide assistance to the Routing ADs. For more
> information about the Routing Directorate, please see
> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
> 
> Although these comments are primarily for the use of the Routing ADs, it would
> be helpful if you could consider them along with any other IETF Last Call
> comments that you receive, and strive to resolve them through discussion or by
> updating the draft.
> 
> Document: draft-ietf-netconf-netconf-event-notifications-17
> Reviewer: Dhruv Dhody
> Review Date: 2019-04-29
> IETF LC End Date: 2019-04-12
> Intended Status: Standards Track
> 
> Summary:
> --------
> I have some minor concerns about this document that I think should be resolved
> before publication.
> 
> Comments:
> ---------
> This document provides a binding for events streamed over the NETCONF for
> dynamic subscriptions. This is a companion document to draft-ietf-netconf-
> subscribed-notifications and this capability for RESTCONF is defined in draft-ietf-
> netconf-restconf-notif.
> 
> The document is overall well written, it makes an assumption that the reader is
> well versed in this area and thus sparse in providing details in the Introduction
> section. The appendix provides good examples.
> 
> I don't see any Routing Yang model specific issue.
> 
> Major Issues:
> -------------
> Note - An IETF process issue, but worth handling right away.
> 
> Section 11 says -
> 
> 11.  Notes to the RFC Editor
> 
>    This section can be removed by the RFC editor after the requests have
>    been performed.
> 
> It further says -
> 
>    RFC 6241 needs to be updated based on the needs of this draft.
>    RFC-6241 section 1.2 bullet "(2)" targets RFC-5277 (actually it
>    identifies RFC 5717, but that was an error fixed after RFC
>    publication).  Anyway the current phrasing in RFC-5277 says that a
>    notification message can only be sent after a successful "create-
>    subscription".  Therefore the reference text must be modified to also
>    allow notification messages be sent after a successful "establish-
>    subscription".  Proposed text for bullet (2) of RFC-6241 would be:
> 
>      (2)  The Messages layer provides a simple, transport-independent
>           framing mechanism for encoding RPCs and notifications.
>           Section 4 documents the RPC messages, [RFC5277] documents
>           Notifications sent as a result of a <create-subscription> RPC,
>           and [RFC xxxx] documents Notifications sent as a result of
>           an <establish-subscription> RPC.
> 
>       (where xxxx is replaced with this RFC number)
> 
> I am not sure if this is correct. I don't think RFC editor can do the action you are
> asking them to do on their own. They would need an errata (which is not correct
> here) or another document that updates RFC 6241. In my view this document
> should just update RFC 6241 (and mark that in this document's
> header) and do necessary text changes to reflect that.

I am happy to follow whatever process is cleanest.   

Kent, are you comfortable with this document directly revising  wording of RFC-6241 section 1.2 bullet "(2)" above?    If yes, it would be great to have your thoughts on what needs to go into this document.   Especially as RFC-6241 section 1.2 bullet "(2)" already had a fix applied against it. 

> Minor Issues:
> -------------
> (1) Abstract & Introduction, It is not clear what does the 'binding' mean and who
> are the parties to this binding? If this is the document that mentions 'binding'
> first, so please add some more clarifying text.
> 
> (2) Section 3, since you use MUST in the error handling, isn't it better to use
> normative in below sentence as well -
> OLD:
>                                                        However a single
>    NETCONF transport session cannot support both this specification and
>    a subscription established by [RFC5277]'s "create-subscription" RPC.
> NEW:
>                                                        However a single
>    NETCONF transport session MUST NOT support both this specification and
>    a subscription established by [RFC5277]'s "create-subscription" RPC.

Makes sense.  I have made the change, and will post the update when the "Major issue" from above is resolved.

> (3) Section 6, You have -
> 
>    And per [RFC5277]'s "eventTime" object definition, the
>    "eventTime" MUST be populated with the event occurrence time.
> 
> Is this a new requirement, or just re-stating RFC5277? RFC5277 says -
> 
>       eventTime
> 
>          The time the event was generated by the event source.  This
>          parameter is of type dateTime and compliant to [RFC3339].
>          Implementations must support time zones.
> 
>       Also contains notification-specific tagged content, if any.  With
>       the exception of <eventTime>, the content of the notification is
>       beyond the scope of this document.
> 
> Maybe remove MUST? If you are trying to refine the text from RFC5277, then
> please re-word.

You are correct.  The MUST is redundant with RFC-5277's XSD definition.   Therefore I have removed "MUST be".

> Nits:
> -----
> (1) Abstract
> 
>    RFC Editor note: please replace the four references to pre-RFC
>    normative drafts with the actual assigned RFC numbers.
> 
> I see two drafts in the reference section. Why four?

You are correct.  I removed the word "four".  

> Also, since those two are normative references, these would be published as a
> cluster as a part of normal RFC editor processing right?

Yes.
 
> (2) Regarding NETCONF, the RFC editor says [1] -
> 
> NETCONF    - Network Configuration Protocol (NETCONF)
>                [Not typically expanded in titles, but expand in abstract]
> 
> Please expand.

Done.

> (3) s/[I-D.draft-ietf-netconf-subscribed-notifications]
>      /[I-D.ietf-netconf-subscribed-notifications]

Actually I changed the [I-D.ietf-netconf-yang-push]  to  [I-D.draft-ietf-netconf-yang-push]

This makes it consistent across all four drafts.

Thanks again!
Eric

> Just so that you have the same style of draft reference in the document. I get
> that it would be replaced with a RFC number anyways :)
> 
> [1] https://www.rfc-editor.org/materials/abbrev.expansion.txt
> 
> Thanks!
> Dhruv