Re: [Netconf] mbj's WGLC comments on subscribed-notifications-10

Martin Bjorklund <mbj@tail-f.com> Thu, 22 March 2018 19:13 UTC

Return-Path: <mbj@tail-f.com>
X-Original-To: netconf@ietfa.amsl.com
Delivered-To: netconf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 9355112741D for <netconf@ietfa.amsl.com>; Thu, 22 Mar 2018 12:13:54 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.911
X-Spam-Level:
X-Spam-Status: No, score=-1.911 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7vS8e_1UtZ9w for <netconf@ietfa.amsl.com>; Thu, 22 Mar 2018 12:13:50 -0700 (PDT)
Received: from mail.tail-f.com (mail.tail-f.com [46.21.102.45]) by ietfa.amsl.com (Postfix) with ESMTP id 903F4126E64 for <netconf@ietf.org>; Thu, 22 Mar 2018 12:13:49 -0700 (PDT)
Received: from localhost (h-80-27.A165.priv.bahnhof.se [212.85.80.27]) by mail.tail-f.com (Postfix) with ESMTPSA id 41F4C1AE0310; Thu, 22 Mar 2018 20:13:47 +0100 (CET)
Date: Thu, 22 Mar 2018 19:13:46 +0000
Message-Id: <20180322.191346.2265316234275728148.mbj@tail-f.com>
To: evoit@cisco.com
Cc: netconf@ietf.org
From: Martin Bjorklund <mbj@tail-f.com>
In-Reply-To: <9d58ffac319648c48ee6b2ab04e238a1@XCH-RTP-013.cisco.com>
References: <20180315.101146.1606096356347320501.mbj@tail-f.com> <9d58ffac319648c48ee6b2ab04e238a1@XCH-RTP-013.cisco.com>
X-Mailer: Mew version 6.7 on Emacs 24.5 / Mule 6.0 (HANACHIRUSATO)
Mime-Version: 1.0
Content-Type: Text/Plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/wI6akVDCqd9kHv6OsuD2l8nwHi8>
Subject: Re: [Netconf] mbj's WGLC comments on subscribed-notifications-10
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Network Configuration WG mailing list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 22 Mar 2018 19:13:56 -0000

Hi,

[pruning to open issues]


"Eric Voit (evoit)" <evoit@cisco.com> wrote:
> Hi Martin,
> 
> Thanks very much for the comments.  Thoughts are in line.  Also a few
> key areas marked with "****"
> 
> Note that changes shown below are also included in this working
> version
> https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-11.txt
> 
> > Martin Bjorklund, March 15, 2018 5:12 AM
> > 
> > Hi,
> > 
> > Here are my WGLC comments on
> > draft-ietf-netconf-subscribed-notifications-10.
> > 
> > In general, I think ths document has improved a lot, it is now easier
> > to follow.
> > 
> > 
> > 
> > Major issue
> > -----------
> > 
> > o  list receiver
> > 
> >    In the YANG model, the "receiver" list is indexed by ip and port.
> >
> >    How is the server supposed to use this simple list of ip/port?
> > 
> >    We get a hint from the document in 2.5.2:
> > 
> >      transport specific
> >      call-home operations will be used to establish the connection
> > 
> >    But it is not clear that this list of ip/port helps. 
> 
> Actually, the address is of type Host.  Of course for everyone must
> eventually resolve to an IP address to make a connection.
> 
> However what I think what you are ultimately trying to do is make sure
> the choice of index doesn't preclude future mechanisms for referencing
> receivers beyond address+Port.  And this likely is a good idea.
> 
> >    The document
> >    draft-ietf-netconf-netconf-client-server provides configuration for
> >    call home for NETCONF.  In that document, the receiver is
> >    identified with a "netconf-client" name and an "endpoint" name, not
> >    ip/port. (draft-ietf-netconf-restconf-client-server has a similar
> >    structure for RESTCONF).
> 
> Yes, having alternative, indirect ways of *ultimately* getting to IP
> address + port would help flexibility.
> 
> >    The best solution would have been to have leafrefs to these
> >    datamodels, but if we do that now, it would introduce a delay b/c
> >    of the dependency.
> > 
> >    The second best solution (I think) is to change this list to have
> >    just a "name" as the single key, and nothing else, and write that
> >    transport-specific modules can add to this in the future.
> 
> I am fine with name as a single key.  However "nothing else" injects a
> few problems:
> 
> (1) our traffic counters are transport agnostic.  Displacing them to a
> transport draft reduces utility.

Yes; my mistake, I didn't mean to exclude the counters / state /
action.  Just host/port.

> (2) forcing the acquisition of host or ip address only from the
> transport draft creates unnecessary complexity for people who are able
> to simply configure host+Port, should it be available in that form
> already?  So why not simply leave those in as a transport agnostic
> option of last resort should it be available to populate?

I think it is a bad idea to include open-ended objects just b/c "maybe
someone would like to use them for something".  For interoperability,
it is important that all objects we define have clear semantics, and
that it clear how to use them.

In this case, none of the standard transports will need these nodes
(unless the client-server models are severly changed).  If someone
defines a transport in the future that needs these objects, it is
trivial to do, and it will be done in a more robust way.

This said, if the WG strongly believes that these nodes are important,
here's a design that might work:

  identity inline-address {
    description
      "A transport identity can derive from this identity in order
       to allow inline definition of the host address in the
       'receiver' list";
    }
  }

  ...

  leaf address {
    when 'derived-from(../../../transport, "sn:inline-address")';
    ...
  }

The you can define your transport like this:

  identity my-udp-based-transport {
    base sn:transport;
    base sn:inline-address;
  }

I would prefer to simply leave this out though.
    
> **** A reasonable approach seems to be:
> 
> (a) add "name" as integer under receivers.  Make this the index.

Yes, but as a string.

> (b) leave address + port as optional objects

See above.

> (c) note that future augmentations mechanisms for identifying
> receivers are encouraged.

Yes, I think it is important to explain what the intention is.  I
suggest you include an example for how to do this.

> I will put this question up in front of the WG during the Tuesday
> session to get more feedback.
> 
> >    Once the client-server docs are ready, we need to revise some
> >    document to add the leafrefs in this list.
> > 
> >    See also below about my comments about transport identities.  I
> >    think we should introduce a YANG module in both the
> >    transport-specific documents, that would define the transport
> >    identity, and augment the receiever list with the appropriate
> >    leafrefs to call home.
> 
> I have no problem with new modules for transport being identified.  I
> don't think they are needed yet.

If new modules just define the identity now, they will be updated to
include receiver-specific parameters later, right?  And we have a
place to add more transport-specific params in the future.  See below.

> My suggestion would be to introduce
> a new module and augment the receiver list will the leafrefs to call
> home once those drafts make it through WGLC.  Otherwise we create a
> dependency per above.

Yes, I think that's what I suggested as well.

[...]

> > o  Section 1.4
> > 
> >   You write:
> > 
> >    o  the data model in this document replaces the data model in
> >       [RFC5277].
> > 
> >   Does this mean that in an implementation that supports both 5277 and
> >   this new document, the set of streams in the two data models are
> >   guaranteed to be equal?
> 
> No
> 
> >   I would assume that an implementation should be allowed to keep the
> >   5277 stuff as-is, and just add future streams to this new mechanism.
> 
> I have updated the text to say:
> 
> the data model in this document provides an alternative structure the
> Notification Management Schema of [RFC5277], Section 3.4.

I don't think this is quite right.  If this is an alternative, does it
mean that I can use this with <create-subscription etc from 5277,
instead of using the structure in 5277?  I don't think this is the
intention. 

Maybe say:

   o  the RPC operations in this draft are intended to replace
      the operation "create-subscription" defined in [RFC5277],
      section 4.

   o  the data model in this document is used instead of the data model
      in [RFC5277] for the new operations.

... or something along those lines.

[...]

> > o  Section 1.4
> > 
> >   You write:
> > 
> >    o  a publisher MAY implement both the data model and RPCs defined in
> >       [RFC5277] and this new document concurrently, in order to support
> >       old clients.  However the use of both alternatives on a single
> >       transport session is prohibited.
> > 
> >   What exactly does the last sentence mean?  And why do you have this
> >   restriction?   This is just the Introduction, but I can't find any
> >   details about this restriction in the rest of the document.  I
> >   suggest you remove this last sentence.  If not, you should work out
> >   the details later in the doc, and add a reference to that new new
> >   text in section 1.4.
> 
> All this was trying to saying is that you can't use this specification
> and RFC-5277's create-subscription on the same NETCONF transport
> session.
> 
> So what I did was to remove the last sentence.  And then I added the
> point above into a new section 6.3 in the
> draft-ietf-netconf-netconf-event-notifications draft.  This section is
> titled "Concurrent Support of NETCONF Subscription specifications",
> and has a single sentence: "A single NETCONF transport session MUST
> NOT support both this specification and a subscription established by
> [RFC5277]'s "create-subscription" RPC."

"MUST NOT support this specification" doesn't sound quite right.  I
think what you mean is that an attempt to request a
"establish-sublish-subscription" must fail, if there's an active
subscription started by "create-subscription", and vice versa.


> Done
>  
> > o  Section 2.3
> > 
> >   I think it would be useful to have "dscp" covered by a separate
> >   feature than "dependency" and "weighting".  The former is quite
> >   simple to implement, but the latter seems to require a very separate
> >   implementation; thus I think there should be two separate features.
> 
> ****
> Dscp should be easy enough to build, why don't we make it part of the
> base specification?  I can make that a question for the WG on Tuesday.
> I am happy with whatever answer is chosen.

It might be safest to have two separate features.  My main point is to
not bundle them into the same feature though.

> >   Also, you write that dependency MUST work identically to 7540, but
> >   7540 defines an "exclusive dependency" that is not covered here.
> >   Maybe it is worth mentioning that this is excluded.
> 
> ****
> Based on Kent's comments, the normative relationship to 7540 as been
> eliminated.  In fact, the non-normative text should likely be removed
> as well.  The text has been simplified to now read:
> 
> Dequeuing of notification messages across independent subscriptions to
> a receiver SHOULD be allocated bandwidth proportionally based on each
> subscription's weight.

I don't understad this.  How can different subscriptions to the same
reciever on the same transport session be allocated different bandwidth?

> If a subscription has a dependency, then any buffered notification
> messages containing event records selected by the parent subscription
> SHOULD be dequeued prior to the notification messages of the dependent
> subscription.  If notification messages have dependencies on each
> other, the notification message queued the longest MUST go first.

Why is the first one a SHOULD but the second a MUST?

> If a
> dependency included within an RPC references a subscription which does
> not exist or is no longer visible to that subscriber, that dependency
                            ^^^^^^^
accessible?

> may be silently removed.
  ^^^^^^

is?


> Based on that, "exclusive dependency" is fully out of scope.
> 
> Does this work for you?

In the sense that I think I know how to implement it - yes.  But I
wonder if this added complexity is necessary, or if we should leave it
out.

[...]

> > o  Section 2.4.2
> > 
> >   You write:
> > 
> >    It MUST be possible to
> >    support multiple establish subscription RPC requests made within the
> >    same transport session.
> > 
> >   I think you mean:
> > 
> >    A server MUST support multiple "establish-subscription" requests
> >    made within the same transport session.
> 
> Agree.  With Kent's comments also integrated, the text now says:
> 
> The transport selected by the subscriber to reach the publisher MUST
> be able to support multiple "establish-subscription" requests made
> within the same transport session.

Ok.  But I wonder if this really is a requirement of the base
mechanism, or if it should be moved to the transport-specific
documents instead.  For example, how is this supposed to work on
non-session based transports?

> > o  Section 2.4.2
> > 
> >   You write:
> > 
> >    And it MUST be possible to support the
> >    interleaving of RPC requests made on independent subscriptions.
> > 
> >   I think you mean:
> > 
> >    Additionally, a server MUST support interleaving of RPC requests
> >    with outgoing notification messages.
> > 
> 
> This is true.  But the ultimate point is also about the pipelining of
> RPC requests as well.  There are good reasons (like speed to
> establishment after a reboot, and reducing the number of transport
> sessions) to require pipeline support for subscriptions.  So the
> updated text currently says:
> 
> The transport session MUST support the pipelining of RPC requests made
> on independent subscriptions.
> 
> Is this good with you?

First of all, I don't understand what an "RPC on a subscription"
means.

Secondly, I think that this should also be done in the transport
document.  For NETCONF, it is not needed, since 6241 already requires
pipelining.

> > o  Section 2.4.2
> > 
> >   (clarification)
> > 
> >   OLD:
> > 
> >    If the publisher can satisfy the "establish-subscription" request, it
> >    provides an identifier for the subscription, and immediately starts
> >    streaming notification messages.
> > 
> >   NEW:
> > 
> >    If the publisher can satisfy the "establish-subscription" request,
> >    it replies with an identifier for the subscription, and then
> >    immediately starts streaming notification messages.
> 
> Kent made a similar comment.  Now it says:
> 
> If the publisher can satisfy the "establish-subscription" request, it
> replies with an identifier for the subscription, and immediately
> starts streaming notification messages.

I still think you need "and *then* starts ...".  Otherwise it seems it
could be ok to send a notif on this new subscription before sending
the reply.

> > o  Section 2.4.6
> > 
> >   OLD:
> > 
> >    1.  yang-data establish-subscription-error-stream: This MUST be
> > 
> >    NEW:
> > 
> >    1.  "establish-subscription-error-stream": This MUST be
> 
> Done
> 
> >    And same for the other two.
> 
> Yes
>  
> >    Also, I suggest you change the name to
> >    "establish-subscription-error".
> 
> ****
> I cannot do that as there is a different structure which is provided
> when hints are being returned for a datastore subscription.

Ok.  You also presented this in the session in London.

I suggest you use:

  establish-subscription-stream-error-info
  establish-subscription-datastore-error-info

etc.

> > o  Section 2.5.1
> > 
> >   (explicitily name the states)
> > 
> >   OLD:
> > 
> >    subscription may also transition to a concluded state if a configured
> > 
> >   NEW:
> > 
> >    subscription may also transition to the "concluded" state if a
> >    configured
> 
> Per comments from Kent and others, Transitioned all states to
> UPPERCASE across the two diagrams and associated text within the
> document for consistency.  E.g., this text now reads:
> 
> A VALID subscription may become INVALID on one of two ways.  First, it

Shouldn't this be:

  A subscription in the VALID state may transition to the INVALID
  state in on of two ways ...

etc, since VALID is the name of a state.

> may be modified in a way which fails a re-evaluation.  See (1) in the
> diagram. Second, the publisher itself might determine that the
> subscription is no longer supportable.  See (2) in the diagram.  In
> either case, a "subscription-terminated" notification is sent to any
> active or suspended receivers.  A VALID subscription may also
> transition to a CONCLUDED state via (4) if a configured stop time has
                ^^^^^^^^^^^

Several of my suggestions earlier was to not write "a XXXX state", but
rather "the XXXX state".  There is just one.

> been reached.  In this case, a "subscription-concluded" is sent to any
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^

This should be:  a "subscription-concluded" notification is sent

> ACTIVE or SUSPENDED receivers.  Finally, a subscription may be deleted
> by configuration (3).
> 
> >   OLD:
> > 
> >    subscription, and is only relevant when the configured subscription
> >    itself is determined to be valid.
> > 
> >   NEW:
> > 
> >    subscription, and is only relevant when the configured subscription
> >    itself is in the "valid" state.
> 
> Ok.  Consistent with above it says 
> 
> ...itself is in the VALID state.
> 
> >   OLD:
> > 
> >    When a subscription first becomes valid, the operational state of
> >    each receiver is initialized to connecting.  Individual receivers are
> >    moved to an active state when a "subscription-started" state change
> >    notification is successfully passed to that receiver.  Configured
> >    receivers remain active if transport connectivity is not lost, and
> >    event records are not being dropped due to a publisher buffer
> >    overflow.  A configured subscription's receiver MUST be moved to
> >    connecting if transport connectivity is lost, or if the receiver is
> >    reset via configuration operations.
> > 
> >   NEW:
> > 
> >    When a subscription first becomes valid, the "state" leaf of
> >    each receiver is initialized to "connecting".  Individual receivers
> >    are
> >    moved to the "active" state when a "subscription-started" state change
> >    notification is successfully passed to that receiver.  Configured
> >    receivers remain active if transport connectivity is not lost, and
> >    event records are not being dropped due to a publisher buffer
> >    overflow.  A configured subscription's receiver MUST be moved to the
> >    "connecting" state if transport connectivity is lost, or if the
> >    receiver is
> >    reset via configuration operations.
> 
> Made the change:
> 
> When a subscription first becomes VALID, the operational state of each
> receiver is initialized to CONNECTING.

You missed my point above.  Please check the whole diff to see if you
agree with all my changes.  If not, let me know.  This applies to all
such comments.

In this case, it is not clear what "the operational state" means,
hence I proposed: the "state" leaf.

> If transport connectivity is
> not available to any receiver, a transport session is established
> (e.g., through [RFC8071]). Individual receivers are moved to an ACTIVE
> state when a "subscription-started" state change notification is
> successfully passed to that receiver (1). Configured receivers remain
> ACTIVE if both transport connectivity can be verified to the receiver,
> and event records are not being dropped due to a publisher buffer
> overflow. The result is that a receiver will remain ACTIVE on the
> publisher as long as events aren't being lost, or the receiver cannot
> be reached.  However if there is buffer overflow, or the publisher
> cannot generate events for a receiver, the receiver MUST be moved to
> SUSPENDED (2).  In addition, a configured subscription's receiver MUST
> be moved to CONNECTING if transport connectivity cannot be achieved,
> or if the receiver is reset via configuration (3).
> 
> >   OLD:
> > 
> >    A configured subscription's receiver MUST be moved to a suspended
> >    state if there is transport connectivity between the publisher and
> >    receiver, but notification messages are not being generated for that
> >    receiver.  A configured subscription receiver MUST be returned to an
> >    active state from the suspended state when notification messages are
> >    again being generated and a receiver has successfully been sent a
> >    "subscription-resumed" or a "subscription-modified".
> > 
> >   NEW:
> > 
> >    A configured subscription's receiver MUST be moved to the "suspended"
> >    state if there is transport connectivity between the publisher and
> >    receiver, but notification messages are not being generated for that
> >    receiver.  A configured subscription receiver MUST be returned to the
> >    "active" state from the "suspended" state when notification messages
> >    are
> >    again being generated and a receiver has successfully been sent a
> >    "subscription-resumed" or a "subscription-modified" notification.
> 
> Now:
> 
> A configured subscription's receiver MUST be moved to a SUSPENDED

again you missed some of my changes; specifically "*the* SUSPENDED state".

> state if there is transport connectivity between the publisher and
> receiver, but notification messages are not able to be generated for

You missed "not able to" as well.

It is ok with me if you disagree with my proposed changes, but please
write that so that we can close these issues.  As it is now I don't
know if you just missed them, or didn't like them.

> that receiver.  A configured subscription receiver MUST be returned to
> an ACTIVE state from the SUSPENDED state...
> 
> > o  Section 2.5.1
> > 
> >   You write:
> > 
> >    A configured subscription's receiver MUST be moved to
> >    connecting if transport connectivity is lost, or if the receiver is
> >    reset via configuration operations.
> > 
> >   What does "reset via configuration operations" mean?  I see an
> >   action called "reset", is that involved here?
> 
> Yes.
> Based on Kent's comments, a reset section now exists later in this
> section.
> 
> 
> <section title="Resetting a Configured Receiver" anchor="reset">
> It is possible that a configured subscription to a receiver needs to
> be reset.  This is accomplished via the "reset" action within the YANG
> model at "/subscriptions/subscription/receivers/receiver/reset". This
> re-initialization may be useful in cases where a publisher has timed
> out trying to reach a receiver. When such a reset occurs, a transport
> session will be initiated if necessary, and a new
> "subscription-started" notification will be sent.
> </section>
> 
> I placed a reference to that new section in 2.5.1:
> 
> For more on reset, see <xref target="reset"/>.


Ok.  I'll guess I'll review the new text when you publish an updated
version.  I want to make sure that you addressed my point, which was
that it is not clear (or rather it is wrong) to talk about "reset via
configuration operations"; reference present or not.

[...]

> > o  Section 2.5.3
> > 
> >   If a suspended receiver exists, and the subscription is modified
> >   several times, does the server have to buffer all
> >   "subscription-modified" notifs?  I think this behavior should be
> >   clarified.
> 
> Since all subscription parameters are included in the notification, it
> is not necessary to buffer anything but the last modification.  Added
> the last sentence in the note below:
> 
> 
> (Note: in this case, the "subscription-modified" notification informs
> the receiver that the subscription has been resumed, so no additional
> "subscription-resumed" need be sent.  Also note that if multiple
> modifications have occurred during the suspension, only the latest one
> need be sent to the receiver.)

Ok.

> > o  Section 2.5.5
> > 
> >   Add text that explains that the "reset" action is used for this.
> 
> There is a new section added for this.   The text is:
> 
> 2.5.5.  Resetting a Configured Receiver
> 
>    It is possible that a configured subscription to a receiver needs to
>    be reset.  This is accomplished via the "reset" action within the
>    YANG model at "/subscriptions/subscription/receivers/receiver/reset".
>    This re-initialization may be useful in cases where a publisher has
>    timed out trying to reach a receiver.  When such a reset occurs, a
>    transport session will be initiated if necessary, and a new
>    "subscription-started" notification will be sent.

Ok.

> > o  Section 2.6
> > 
> >   I can't parse this:
> > 
> >    A subscription's events MUST NOT be sent to a receiver
> >    until after a corresponding RPC response or state-change notification
> >    has been passed receiver indicating that events should be expected.
> > 
> >   Maybe you mean:
> > 
> >    A subscription's events MUST NOT be sent to a receiver unless the
> >    receiver is in the "active" state.
> 
> 
> ****
> Based on Kent's comments, this is now:
> 
>    A subscription's events MUST NOT be sent to a receiver
>    until after a corresponding RPC response (in the case of a dynamic
>    subscription) or state-change notification (in the case of a
>    configured subscription) has been passed receiver indicating that
>    events should be expected.
> 
> Does this work for you?

s/events/event records/

I understand what you want to say, but I don't think text does a good
job of explainig it.  What is a "corresponding RPC response"?
Corresponding to what?  Maybe be more explict, along the lines of:

  When a dynamic subscription has been started or modified, with
  "establish-subscription" or "modify-subcription" respectively, event
  records matching the new filter criteria MUST NOT be sent until
  after the RPC reply has been sent.

  When a configuried subscription has been started or modified, event
  records matching the new filter criteria MUST NOT be sent until
  after the "subscription-started" or "subscription-modified"
  notifications has been sent, respectively.

[...]

> >   OLD:
> > 
> >    This [RFC5277] section 4 one-way operation has the drawback of not
> >    including useful header information such as a subscription
> >    identifier.
> > 
> >   NEW:
> > 
> >    The <notification> message defined in [RFC5277] section 4 has the
> >    drawback of not including useful header information such as a
> >    subscription identifier.
> 
> Kent has asked that this text be removed for now, as he believes this
> might impact IESG reviews.  So this and the notification-messages
> draft reference text has been removed.

Ok.

> > o  Section 2.7
> > 
> >   Perhaps write that the subscription state notifications are not
> >   stored in replay buffers.
> 
> Now says:
> 
> Subscription state notifications cannot be filtered out, they cannot
> be stored in replay buffers, and they are delivered only to impacted
> receiver(s) of a subscription.

Ok.

> > o  Section 2.7.4
> > 
> >   You write:
> > 
> >    The
> >    two conditions where is this possible are "insufficient-resources"
> >    and "unsupportable-volume".
> > 
> >   What exactly is the difference between these two?  If I can't handle
> >   the volume I probably have insufficient resources?
> 
> I updated the text to:
> 
> The two conditions where is this possible are:
>  1. "insufficient-resources" when a publisher is unable to produce the
>  requested stream of notification messages, and
>  2. "unsupportable-volume" when the bandwidth needed to get generated
>  notification messages to a receiver exceeds a threshold.
> 
> This additional feedback / differentiation is useful as a subscriber
> prepares for another attempt at subscription.
> 
> I have also tweaked the definitions of the identities to better
> support this.

Ok.

> > o  For the transport identities, add RFC editor notes to replace the
> >    draft strings.
> > 
> >    For the http1.1 and http2 transport, you have listed these docs as
> >    "informative refererences".  I understand this; otherwise it would
> >    have been a downref, but it is really correct?   An alternative
> >    would be to specify the transport-specific identities in the
> >    transport-specific documents.
> 
> ****
> 
> Having a YANG module for a transport identity seems like overkill.

I agree.  But see my other comments above; I think such a module will
likely define receiver params (address, authentication info, timers
etc).  I think we agree that transport-specific receiver parameters
will be needed, so where will they be defined?   It seems logical to
define the transport identity and corresponding receiver params in the
same module.

> Plus with things like CBOR, UDP, etc. coming, replicating model names
> for this seems excessive.  It would be great to leave as is.  I of
> course will respect whatever IETF process requires.

I would like to hear the opinion from others in the WG about this
one.

> > o  typedef stream-ref
> > 
> >     description
> >       "This type is used to reference a system-provided datastream.";
> > 
> >     What is a "datastream"?   This word occur twice in the document,
> >     w/o further explanation.
> 
> Renamed to stream

Ok.  I would probably have used "event stream", but stream is fine.

> > o  leaf protocol / encoding
> > 
> >    Why is "protocol" only for the feature "configured", but "encoding"
> >    is not?
> 
> ****
> 
> There have been requests for an establish-subscription RPC to request
> an alternative encoding (e.g., binary with CBOR).  Since there are
> lots of instances for which this is possible, trying not to exclude
> it.

Hmm, ok.  It would have been nice to not have this leaf for transports
that don't support it, and even better if the encoding would depend on
the transport.  This *can* be done with some new identities:

  identity configurable-encoding {
    description
      "If a transport identity derives from this identity, it means
       that it supports configurable encodings.";
  }

  identity http-2 {
    base transport;
    base configurable-encoding;
  }

  leaf encoding {
    when 'derived-from(../transport, "sn:configurable-encoding")';
  }

You can also have transport-specific encodings, instead of generic
encondings, in order to make it impossible to set e.g. "xml" encoding
on a comi transport (if that is a transport...).

> > o  For all rpcs:
> > 
> >    In order to make it clear to implementors what to do in the case of
> >    errors, add a paragraph to the description:
> > 
> >      If an error occurs, the server replies with an 'rpc-error' where
> >      the 'error-info' field MAY contain an instantation of the
> >      'establish-subscription-error' structure.
> 
> ****
> I did this for delete and kill RPCs.
> 
> There is already text for establish and modify subscription RPCs.  But
> it cannot be boilerplate, referring just to this yang-data within this
> yang module.  The reasoning here is that we need to have different
> yang-data structures for different targets (datastore vs stream).
> This is due to different hints.

Ok.


/martin