Re: [Netconf] Review of draft-ietf-netconf-netconf-event-notifications-15

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

Hi Qin,

Thank you very much for assisting with the Shepherd role here.   Some thoughts in-line...

> From: Eric Voit, January 8, 2019 6:37 PM
> 
> 
> 
> From: Netconf <[e-mail]> On Behalf Of Qin Wu
> Sent: Tuesday, January 8, 2019 12:44 AM
> To: [e-mail]
> Subject: [Netconf] Review of draft-ietf-netconf-netconf-event-notifications-15
> 
> Hi, All:
> I am assigned as acting shepherd to assist Kent to review draft-ietf-netconf-
> netconf-event-notifications-15
> and have the following comments:
> 1. I see event record can be sent via notification for dynamic subscription based
> on section 2.6 of draft-ietf-netconf-subscribed-notifications-19, but I didn't see
> subscription state notifications can be sent for dynamic subscription based on
> section 2.7 of draft-ietf-netconf-subscribed- notifications-19. If this is true,
> Configured subscription example should be cleaned up in the appendix.

There are several places in draft-ietf-netconf-subscribed-notifications-19 where it describes that subscription state notifications may be sent for dynamic subscriptions.   However it is good to add more text on this.  As a result, I have updates a paragraph in 1.3 with more info on this to say...

"Note that there is no mixing-and-matching of dynamic and configured operations on a single subscription.  Specifically, a configured subscription cannot be modified or deleted using RPCs defined in this document.  Similarly, a dynamic subscription cannot be directly modified or deleted by configuration operations.  It is however possible to perform a configuration operation which indirectly impacts a dynamic subscription. By changing value of a pre-configured filter referenced by an existing dynamic subscription, the selected event records passed to a receiver might change."

This text is reflected in the just-uploaded version of draft-ietf-netconf-netconf-event-notifications.

> 2. By reading section 10, it is not clear which document should be updated to
> support notification after a successful "establish-subscription", RFC6241 itself,
> or this document?

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". 

As a result I have updated bullet (2) in Section 10 to say:

   (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 should be replaced with this RFC number)


> 3. Running nits tools, there are the following errors and warnings:
> "
>   Checking nits according to https://www.ietf.org/id-info/checklist :
>   ----------------------------------------------------------------------------
> 
>   ** The document seems to lack an IANA Considerations section.  (See Section
>      2.2 of https://www.ietf.org/id-info/checklist for how to handle the case
>      when there are no actions for IANA.)

I have added the requested section stating that there are no IANA Considerations

>   Miscellaneous warnings:
>   ----------------------------------------------------------------------------
> 
>   == The copyright year in the IETF Trust and authors Copyright Line does not
>      match the current year

This looks to have corrected itself automatically with the new version.
 
>   == Line 195 has weird spacing: '...ription  estab...'
> 
>   == Line 199 has weird spacing: '...ription    res...'
> 
>   == Line 211 has weird spacing: '... stream   esta...'
> 
>   == Line 214 has weird spacing: '...ription    ret...'
> 
>   == Line 216 has weird spacing: '... stream   modi...'

These five items are on table formatting.   Things look much cleaner as currently shown in the document when the extra spaces are included.

> 4. RFC6241 needs to be updated but it is not listed on the title page header.

The update to the RFC editor for RFC6241 is noted in a separate section titled: " Notes to the RFC Editor".   Adding this information to the header seemed redundant.

Thanks,
Eric
 
> Note that I have talked with authors on most of these comments, I believe a
> new version will come soon to address these comments.
> 
> -Qin