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

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

Hi Dhruv,
Hi Kent,

From: Kent Watsen, May 1, 2019 6:39 PM
On Apr 30, 2019, at 1:44 PM, Eric Voit (evoit) <[e-mail]<mailto:[e-mail]>> wrote:

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.


Dhruv is correct, the RFC Editor cannot make the requested change, and an Errata is also not correct, as there is nothing wrong with the way that RFC 6241 was written at the time it was written.

To answer the question, if this draft is to "update" RFC 6241, the steps are 1) declare the update in the draft's header (see Section 2.33.10 of RFC 7749), 2) so so in the Abstract, and 3) have a new section (either before of after the current Section 3 called "Update to RFC 6241" that describes the desired modification.

However, it is unclear if an "update" is appropriate either, given that the "update" is informative in nature and, while it is true that this document cannot be used without RFC 6241, that is also true for so many documents that don't update RFC 6241.

>From now obsolete RFC 2223 (I couldn't find a current reference):

   Updates

      To be used as a reference from a new item that cannot be used
      alone (i.e., one that supplements a previous document), to refer
      to the previous document.  The newer publication is a part that
      will supplement or be added on to the existing document; e.g., an
      addendum, or separate, extra information that is to be added to
      the original document.

and from expired draft-rfc-editor-rfc2223bis-08:

         Updates

            Specifies an earlier document whose contents are modified or
            augmented by the new document.  The new document cannot be
            used alone, it can only be used in conjunction with the
            earlier document.

Is an "update" overreaching?

Another option is to not update RFC 6241 and instead just mention it, similar to what was done here: https://tools.ietf.org/html/rfc8071#section-1.4.   Note that RFC 8071 originally "updated" RFC 4253, but Benoit (AD at the time) recommended downgrading it to what it is now.


A couple more thoughts:

1) if an update is needed, would it be better coming from draft-ietf-netconf-subscribed-notifications, which defines the "establish-subscription" RPC?


2) whatever the outcome of this discussion, it seems that a similar outcome should be applied to draft-ietf-netconf-restconf-notif and its relation to RFC 8040.


<eric> Based on my reading of your process suggestions Kent, I like best the "mention" approach which you used for RFC-8071, Section 1.4.

What I think could be done to cover this is:

(A)  Remove Section 11: Notes to the RFC Editor

(B)  Per Kent's desire to also cover draft-ietf-netconf-restconf-notif, place the following statement into draft-ietf-netconf-subscribed-notifications directly after Figure 10

[RFC-5277] Section 2.2.1 states that a notification message is to be sent to a subscriber which initiated a "create-subscription".   With this specification, this RFC-5277 statement should be more broadly interpreted to mean that notification messages can also be sent to a subscriber which initiated an "establish-subscription", or a configured receiver which has been sent a "subscription-started".

Does this work for both of you?


[also see my comment below]



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.


The issue isn't consistency so much as meeting expectations, per `xml2rfc`, the document should have something like the following in the References section, which then auto-expands to the correct reference text, as well as defining the anchor "I-D.ietf-netconf-subscribed-notifications":

        <?rfc include="reference.I-D.ietf-netconf-subscribed-notifications"?>

<Eric> That definitely makes things easier than what I have been doing.  I am hitting an xml2rfc error trying this right now, but I will get there.

Eric



Kent // shepherd



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