Subject: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-21

From: Andy Bierman <[email-masked]>
Date: Mon, 14 Jan 2019 17:07:51 -0800

Reviewer: Andy Bierman
Review result: Ready with Issues

based on draft-21
ietf-subscribed-notifications@2018-12-19

General Comments:

  -- the document is well-written and the feature-set is
     very comprehensive. There are a lot of YANG features (10)
     but they are defined to allow for a large set of
     publisher platforms.

  -- the extensibility mechanisms for filters, transport,
     and encoding should be valuable over time. There is
     a risk of proprietary solutions that do not interoperate
     but this base YANG module provides enough functionality.
     Reliance on future augmentations is new but should be OK

  -- advanced use of groupings, refine-stmt and augment-stmt
     within groupings makes the module difficult for humans
     to fully understand without additional tools, and without
     the normative text outside the YANG module.

Issues:

I1) 1.4.  Relationship to RFC 5277

  -- this section should mention that stop-time is not coupled to
     notification replay like RFC 5277. This parameter can be used
     on any stream, even if replay is not supported at all.

I2) sec 2.1 para 5:
   If subscriber permissions change during the lifecycle of a
   subscription and event stream access is no longer permitted, then the
   subscription MUST be terminated.

  -- Why can't server suspend until NACM configured
   (i.e., MUST be terminated or suspended). Should be clear somewhere that
   suspend is for CPU and other resources, and NACM not considered
   to be a resource.

I3) sec 2.1 para 6:
   Event records MUST NOT be delivered to a receiver in a different
   order than they were placed onto an event stream.

  -- does this apply to subscription-state? Think not, they are not events
    placed in event stream. Need to allow ended or suspended to be sent
    head-of-line whenever state changes
  -- Maybe another table or more text should be added listing the
     notifications that indicates
       (a) sent-for-configured
       (b) sent-for-dynamic
       (c) inserted at end of event stream, middle, or sent head-of-line

     replay-completed         Y  Y  end
     subscription-completed   Y  N  end
     subscription-modified    Y  Y  middle [1]
     subscription-resumed     Y  Y  head [2]
     subscription-started     Y  N  head
     subscription-suspended   Y  Y  head
     subscription-terminated  Y  Y  head [3]

  -- [1] not clear where in the event stream subscription-modified is sent.
     Maybe implementation-dependent? e.g. does filter-change inserted
     at slot N means all events N+i are done with new filter?
  -- [2] not clear where in the event stream subscription-resumed is sent.
     There may be events ready to send. Clearly resumed is sent before
     any of these after transition to active state
  -- [3] not clear that subscription terminated right away then all
     events for all receivers are deleted and only subscription-terminated
     is sent to all receivers; sec 2.7.3 says this but not in module
  -- insert point may be specific to each receiver, not each subscription
  -- how long does the server wait for a receiver when sending
     subscription-terminated?

I4) sec 2.4.6: RPC Failures
  -- concern about a subscription-specific error reporting system
     must make sure protocol error reporting system is used correctly

  -- The error-tag value needs to be identified for each 'reason' identity

   2.  "modify-subscription-stream-error-info": This MUST be returned
       with the leaf "reason" populated if an RPC error reason has not
       been placed elsewhere within the transport portion of a failed
       "modify-subscription" RPC response.  This MUST be sent if hints

  -- all 3 paragraphs like this; unclear what "placed elsewhere"
      text means; not appropriate for MUST;  Only 3 fields seem
      to be relevant (error-tag, error-app-tag, error-info).
      Protcol operations are expected to document server requirements
      for these 3 fields, if applicable.  Only the error-tag
      is mandatory-to-use.

  -- the error assignments are extremely specific. e.g., it is not
     possible for <kill-subscription> to fail with an
     'insufficient-resources' error; Do not agree that scoping each
     identity to specific RPC operations is a good idea.

  -- how are errors in these parameters reported for configured
     subscriptions when <edit-config> is the RPC that has the error?
     How are the yang-data structs used for edit-config or commit errors?

I5) sec 2.5

   Multiple configured subscriptions MUST be supportable over
   a single transport session.

  -- why is this a MUST instead of SHOULD? explain harm to
     interoperability if not supported

I6) sec 2.5, para 3:

   On a receiver of a
   configured subscription, support for dynamic subscriptions is
   optional except where replaying missed event records is required.

  -- confusing because text in 1.3:
     Note that there is no mixing-and-matching of dynamic and configured
     operations on a single subscription.  Specifically, a configured
  -- clarify the receiver may have multiple subscriptions here
  -- not clear what "except where replaying..." text means

I7) leaf stream-xpath-filter: [multiple uses]

           The expression is evaluated in the following XPath context:

             o   The set of namespace declarations is the set of prefix
                 and namespace pairs for all YANG modules implemented
                 by the server, where the prefix is the YANG module
                 name and the namespace is as defined by the
                 'namespace' statement in the YANG module.

  -- This prefix processing is not done anywhere else in NETCONF
     or RESTCONF.  IMO a bad precedent.  Only the XML prefixes
     should be required for processing of XML encoding.  YANG
     module prefixes are not required to be unique, unlike
     the prefix mappings in XML
  -- NMDA allows the same module to appear in multiple module-sets
     and different in each datastore. This text about "implemented by
     the server" does not work for NMDA

I8) leaf /subscriptions/subscription/encoding {
      when 'not(../transport) or derived-from(../transport,
      "sn:configurable-encoding")';
      type encoding;

  -- when-stmt is odd; there are no configurable-encodings specified
  -- there should be an example of a configurable encoding provided
  -- maybe add some text noting this is not the "encoding" leaf in
     establish-subscription.  Text is confusing since not clear how
     a dynamic subscription would use this leaf inside subscription-policy
     grouping
  -- it is only clear in the YANG tree that modify-subscription does
     not allow encoding to be changed.  Is this worth mentioning in
     the establish-subscription/input/encoding leaf?

I9) leaf /streams/stream/description {
        type string;
        mandatory true;

  -- it is odd to see an admin-string be mandatory. should add explanation
     why this is mandatory (in config false even more odd)

I10) leaf /subscriptions/subscription/configured-subscription-state
        if-feature "configured";
          enum concluded {
            value 3;
              description
                "A subscription is inactive as it has hit a stop time,
                but not yet been removed from configuration.";
          }

  -- it is also possible for stop-time to be reached but not all
     events delivered to all receivers on the subscription.
     Is this a different state or part of 'concluded'?

I11) extension subscription-state-notification {

       This statement is not for use
       outside of this YANG module.";

  -- this text should be removed. There is no value in limiting
     the scope of this extension.  It prevents even this WG from
     creating a module that uses the extension again.

I12)   notification replay-completed {

    description
      "This notification is sent to indicate that all of the replay
        notifications have been sent. It must not be sent for any other
       reason.";

  -- 2nd sentence should be removed. It is implied that the notification
     is only sent for its defined purpose.  No other notifications
     have this type of text.

I13)   notification subscription-started {
    sn:subscription-state-notification;
    if-feature "configured";
    description
      "This notification indicates that a subscription has started and
        notifications are beginning to be sent. This notification shall
       only be sent to receivers of a subscription; it does not
       constitute a general-purpose notification.";

  -- 2nd sentence is confusing; all notifications are sent to
     receivers of a subscription. last part is redundant since
     the sn:subscription-state-notification extension is used

I14)   rc:yang-data modify-subscription-stream-error-info {

      leaf filter-failure-hint {
        type string;
          description
            "Information describing where and/or why a provided filter
             was unsupportable for a subscription.";
      }

  -- rpc-error already allows more precise error reporting
     It uses error-tag, error-path, error-string, and error-info extensions
     to identify which parameters/conditions caused the RPC to be rejected.
     This error reporting will continue to be used, Not sure this failure-hint
     has any standards value. Perhaps real-use example can be added

I15)  notification subscription-completed {
    sn:subscription-state-notification;
    if-feature "configured";
    description
      "This notification is sent to indicate that a subscription has
       finished passing event records, as the 'stop-time' has been
       reached.";

  -- Could be more clear:
     A) replay in use and stop-time has past:
        * send replay-completed
        * send subscription-completed
     B) no replay and stop-time has past:
        * send subscription-completed

I16) leaf replay-previous-event-time {

  -- It is not clear why this leaf is needed
     How does this relate to replay-log-creation-time
     and replay-log-aged-time?

Nits:

2.3:
This document provide for several QoS parameters
  -- s/provide/provides

2.4, para 1:
  -- term 'target' usage is confusing, inconsistent with sec 1.2

reference-stmt:
    "RFC XXXX:Customized Subscriptions to a Publisher's Event Streams";
 -- Does not match document title

feature configured:
      "This feature indicates that configuration of subscription is
      supported.";
 -- s/subscription/subscriptions/

refine "target/stream/replay-start-time" {
  description
    "Indicates the time that a replay using for the streaming of

 -- sentence seems mangled; needs repair

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 16 Jan 2019 18:35:07 +0000

Hi Andy,

Thanks for the review.  Some thoughts....

> From: Andy Bierman, January 14, 2019 8:08 PM
> 
> Reviewer: Andy Bierman

If you are ok with the following text, I can add the a new clarification bullet to this section:
"A subscription "stop-time" can be specified as part of a notification replay.  This supports an analogous capability to the stopTime parameter of [RFC 5277].  However in this specification, a "stop-time" parameter can also be applied without replay." 

>    (i.e., MUST be terminated or suspended).

The original reason for 'MUST be terminated' was that it was more deterministic / simpler option.  And it seems a reasonable solution considering permissions changes that collide with existing subscription is something which is likely to be a rare corner case.   If an implementation does also want to support suspended, then the implementation will then have to monitor permissions to get that subscription out of suspension.  And adding support for this would add more text, complexity, and NACM integration to the specification.  The best option of course would be to consider NACM implications for subscriptions in flight prior to making permissions changes.  With this, adding the extra complexity of suspension wouldn't be necessary.

>    Should be clear somewhere that

If NACM is active, it needs to be followed.  The text we have for NACM is in Section 5.4.  Do you see something else specific to subscription suspension needed here?  (Maybe I am not getting your point.)
 
>     placed in event stream. 

Agree that they are not on the event stream.  So they do not violate this requirement.  

Additionally there is supporting text in "Section 2.7: subscription state notifications", including...

" Instead, they are inserted (as defined in this section) within the sequence of notification messages sent to a particular receiver."

>     Need to allow ended or suspended to be sent

I am not sure that suspended should always be sent head-of-line.   Consider that implementation might want to let the existing queue of filtered event records be sent if is filter complexity causing the CPU issue.   That could be different than if it is a bandwidth issue driving the suspension, and you definitely want the 'subscription-suspended' to be placed at the head of line.

>      subscription-terminated  Y  Y  head [3]

See thinking on this table below at ***table thoughts***
 
>      at slot N means all events N+i are done with new filter?

Yes.  The text which indicates this is in Section 2.5.1...

"A "subscription-modified" subscription state change notification will
   be sent to all active receivers, immediately followed by notification
   messages conforming to the new parameters."

There is also text in the YANG model "notification subscription-modified" saying

      "Notification messages sent from this point on will 
       conform to the modified terms of the subscription."

Beyond that it is up to the implementation to decide if some un-transmitted queue of event records should be flushed and reprocessed based on the modification.  I do not expect this would popular, as a replay subscription could accomplish this same functional need.

>      any of these after transition to active state.

Yes.  The text on this is in Section 2.7.5 should help.  There is also text in the YANG module...

   "Subscribed event records generated after the issuance of this
   subscription state change notification may now be sent."

Thinking more on your point, it might be worth tweaking a couple words to allow for head-of-line placement of "subscription-suspended".

   "Subscribed event records queued for sending after the issuance of this
   subscription state change notification may now be sent."

Are you good with this suggested change?

>      is sent to all receivers; sec 2.7.3 says this but not in module

I am not sure if the module is the best place to embed this.  My thinking was the module should defining the meaning of the notification and its relationship to other model elements, and the behavior of the publisher prior to the issuance of the notification can be included in the specification text.  As a result beyond Section 2.7.3, there is also related text in Section 2.5.4...

   "Immediately after a subscription is successfully deleted, the
   publisher sends to all receivers of that subscription a subscription
   state change notification stating the subscription has ended (i.e.,
   "subscription-terminated")."

>   -- insert point may be specific to each receiver, not each subscription

This is true, and is a natural function of the text is Section 2.5.4.

>   -- how long does the server wait for a receiver when sending

Per Section 2.5.4. there is no wait.

***table thoughts*** The table does provide an interesting summary view.   And do I appreciate you putting it together.  Based on the notes for [1], [2], and [3], my perception is that the requirements are covered within the existing text.  Is it an absolute requirement from the YANG Doctors that this summary table be embedded as a new section of the draft (perhaps as a Section 2.7.8?)  I am hoping to avoid major new text additions at this point.

>      must make sure protocol error reporting system is used correctly

Yes.  We have done our best to integrate with the embedded NETCONF and RESTCONF mechanisms.  There is much additional information in the transport drafts here.
 
>   -- The error-tag value needs to be identified for each 'reason' identity

This is done in the transport drafts.   E.g., see draft-ietf-netconf-netconf-event-notifications Section 7
 
>       text means; not appropriate for MUST;  

Instead of "placed elsewhere", how about: "placed in subscription transport document defined object".  Would this be sufficient?

In any case, the -v21 wording results from the attempted balancing the WG requests for:
* merging with transport protocol error mechanisms
* WG leadership guidance to provide requirements for transport documents

>      Only 3 fields seem

Hopefully these are covered sufficiently when this document is coupled with the NETCONF and RESTCONF Notif transport documents.   For other transports, the tags you identify about would not be applicable.
 
>      'insufficient-resources' error; 

This is the intent of the base specification, e.g., we don't believe a kill-subscription should fail for an insufficient-resources reason.   But vendors might desire more specificity.  As a result is certainly ok for vendor implementations to add new error identities.

>      Do not agree that scoping each

This level of specificity was not the author's original plans.  Nor was this level of specificity part of earlier draft versions up through -v08.   However members of the WG made it clear that such specificity was necessary for draft progression.
 
>      How are the yang-data structs used for edit-config or commit errors?

None of these yang-data structures are specified for use with <edit-config> operations.   For <edit-config>, the change to a configured subscription would be written to the datastore if it were semantically valid.   At this point the subscription enters the [evaluate] points of Figure 8.  Issues from this point out would be reported with a vendor specific construct such as SYSLOG.
 
>      interoperability if not supported

From the receiver side, a receiver must be able to differentiate between state notifications for different subscriptions.  If a publisher can't support more than one, it doesn't actually hurt interoperability with a receiver.  It just doesn't meet the minimum criteria set for feature support for the capability as defined in this document.
 
>   -- not clear what "except where replaying..." text means

How about the following tweak:

"On a receiver of a configured subscription, support for dynamic subscriptions is optional.  However if replaying missed event records is required for a configured subscription, support for dynamic subscription is highly recommended.  In this case, a separate dynamic subscription can be established to retransmit the missing event records."

>      the prefix mappings in XML

This text was proposed by Martin as a result of the "xpath expressions in JSON" thread last October in NETMOD.

I am happy to incorporate whatever text is appropriate.  I was hoping that the suggested text was sufficient for now.  Kent has already incorporated this as an issue for yang-next
https://github.com/netmod-wg/yang-next/issues/55 
So hopefully there is no final precedent being claimed. 

>      the server" does not work for NMDA

I am happy to adopt whatever text meets YANG doctor approval.   Can you suggest? 

>   -- when-stmt is odd; there are no configurable-encodings specified

This constraint was suggested by Martin last June in the thread:
https://www.ietf.org/mail-archive/web/netconf/current/msg14402.html 
more details can be seen at:
https://mailarchive.ietf.org/arch/msg/netconf/wHQjZuJnh7asJvpE1nIovPDnESM 

>   -- there should be an example of a configurable encoding provided

I am happy to enhance the definition YANG model's identity definition of "configurable-encoding".  I could do this by adding the following additional text to the description: "An example of a configurable encoding might be a new identity such as 'encode-cbor'.  Such an identity could use 'configurable-encoding' as its base.  This would allow a dynamic subscription encoded in JSON [RFC-8259] to request notification messages be encoded via CBOR [RFC-7049].  Further details for any specific configurable encoding would be explored in a transport document based on this specification."   Does this meet your ask?

>      a dynamic subscription would use this leaf inside subscription-policy

Dynamic subscription is possible.  This is the intent of drafts like 
draft-birkholz-yang-core-telemetry-01

>      the establish-subscription/input/encoding leaf?

I don't think the encoding leaf of establish-subscription needs to note that it cannot be modified.  There are other objects which cannot be modified as well, they are not explicitly called out.   In the end, the current model matches the intended behavior per the text above.  And it is only possible to request an encoding for delivered event records be different from that of the RPC during the establish subscription.

>      why this is mandatory (in config false even more odd)

Thinking about this some, it certainly should be there.  But does not need to be mandatory.  So I have changed the leaf description so that it is no longer mandatory.

>      Is this a different state or part of 'concluded'?

I have no problem supporting your request for refinement/specificity.  As a result, I have tweaked the definition as follows...

                "A subscription is inactive as it has hit a stop time, 
                it no longer has receivers in the 'receiver active' or 
                'receiver suspended' state, but not yet been 
                removed from configuration.";

>      creating a module that uses the extension again.

This was the subject of significant debate in the WG.  The authors did not want this restriction either.  

To be allowed to progress the document, we inserted the document.  If this really is mandatory-to-remove from a YANG doctor point-of-view, what is the process for quick closure on this issue between WG leadership and the YANG doctors?
 
>      is only sent for its defined purpose.  No other notifications

Done
 
>      the sn:subscription-state-notification extension is used

There is no issue with removing this second sentence completely.  If I did that, would this address your concern?

>      has any standards value. Perhaps real-use example can be added

Per your thoughts on rpc-error...   For NETCONF and RESTCONF, you point to error structures which historically been used with those transports.  Of course we were looking to have all subscription hints supportable across transports via a single portable YANG data structure.  So the value is that a single string object exists so to transport whatever the vendor thinks would be useful as a hint in this case.  I.e., there has been no attempt to standardize the contents of this string.  If operational experiences drive a desire for such structuring, this could provide the basis for a new draft building off of this starting point.  

>         * send subscription-completed

A "replay-completed" might have already been sent if the subscription started with replay, but the replay concluded before the "stop-time".  Based on this, are you good with the following clarification sentence at the end of the current description?....

       "If this is a replay subscription, and a 'replay-completed' subscription state notification has not been sent, the 'replay-completed' must be sent prior to the 'subscription-completed'." 

>   -- It is not clear why this leaf is needed

The purpose of the leaf is included in the second paragraph of the description

            If a receiver previously received event records for this 
            configured subscription, it can compare this time to the
            last event record previously received.  If the two are not 
            the same (perhaps due to a reboot), then a dynamic replay 
            can be initiated to acquire any missing event records."

More on the leaf is also described in Section 2.5.6., paragraph 4.    

>      and replay-log-aged-time?

It does not relate directly to those objects.  But indirectly if the replay log is empty (or timed out), then the first paragraph of the description applies...

            "If there is at least one event in the replay buffer prior 
            to 'replay-start-time', this gives the time of the event 
            generated immediately prior to the 'replay-start-time'.

If it helps, I could add one more sentence to the first paragraph:

             "If there is no prior event in the replay buffer, this object MUST NOT be populated."

But this seems like over-specifying to me. 

> This document provide for several QoS parameters

Fixed

>   -- term 'target' usage is confusing, inconsistent with sec 1.2

Are you ok if I tweak the text to:
"These RPCs have been designed extensibly so that they may be augmented for use beyond event streams."

>  -- Does not match document title

Fixed to:
"RFC XXXX:Subscription to YANG Event Notifications"

>  -- s/subscription/subscriptions/

Done

>  -- sentence seems mangled; needs repair

Made it:
"replay is using"

Thanks again for doing a very complete YANG Doctor review.

Eric

---

From: Andy Bierman <[email-masked]>
Date: Thu, 17 Jan 2019 10:38:41 -0800

On Wed, Jan 16, 2019 at 10:35 AM Eric Voit (evoit) <[email-masked]> wrote:

> "A subscription "stop-time" can be specified as part of a notification
> replay.  This supports an analogous capability to the stopTime parameter of
> [RFC 5277].  However in this specification, a "stop-time" parameter can
> also be applied without replay."
>
>

OK

> The original reason for 'MUST be terminated' was that it was more
> deterministic / simpler option.  And it seems a reasonable solution
> considering permissions changes that collide with existing subscription is
> something which is likely to be a rare corner case.   If an implementation
> does also want to support suspended, then the implementation will then have
> to monitor permissions to get that subscription out of suspension.  And
> adding support for this would add more text, complexity, and NACM
> integration to the specification.  The best option of course would be to
> consider NACM implications for subscriptions in flight prior to making
> permissions changes.  With this, adding the extra complexity of suspension
> wouldn't be necessary.
>
>
OK to leave as-is. Agree it is more complex.

> If NACM is active, it needs to be followed.  The text we have for NACM is
> in Section 5.4.  Do you see something else specific to subscription
> suspension needed here?  (Maybe I am not getting your point.)
>
>
No -- OK to leave NACM as terminate-if-loss-of-rights
(Is there an error identity for this event?)

> " Instead, they are inserted (as defined in this section) within the
> sequence of notification messages sent to a particular receiver."
>
> I am not sure that suspended should always be sent head-of-line.
>  Consider that implementation might want to let the existing queue of
> filtered event records be sent if is filter complexity causing the CPU
> issue.   That could be different than if it is a bandwidth issue driving
> the suspension, and you definitely want the 'subscription-suspended' to be
> placed at the head of line.
>
>

It is up to the publisher to decide when to stop sending events on a
subscription.
Obviously the publisher cannot wait until the subscription is idle.
The reason it is getting suspended is it is far from idle

So also up to the publisher wrt/ what to do with any events that have not
been delivered yet on a subscription.  Could delete them or save them for
when more bandwidth available (for example)

> Yes.  The text which indicates this is in Section 2.5.1...

OK

> Beyond that it is up to the implementation to decide if some
> un-transmitted queue of event records should be flushed and reprocessed
> based on the modification.  I do not expect this would popular, as a replay
> subscription could accomplish this same functional need.
>

Agreed that an implementation can drop at any time and increment the
appropriate counters. It will try to to do this, but no requirements except
maybe subscription events like 'replay-completed' cannot be dropped

>    subscription state change notification may now be sent."
OK

> Thinking more on your point, it might be worth tweaking a couple words to
> allow for head-of-line placement of "subscription-suspended".
>
Not sure -- it needs to be clear that subscription-suspended is the
last event sent before suspending and subscription-resumed is
the first event sent after transition from suspended to active.
The next event could also be subscription-terminated.

Implementation details do not need to be mentioned

> I am not sure if the module is the best place to embed this.  My thinking
> was the module should defining the meaning of the notification and its
> relationship to other model elements, and the behavior of the publisher
> prior to the issuance of the notification can be included in the
> specification text.  As a result beyond Section 2.7.3, there is also
> related text in Section 2.5.4...
>
>
OK to leave as-is

> This is true, and is a natural function of the text is Section 2.5.4.
OK

> Per Section 2.5.4. there is no wait.
OK

> ***table thoughts*** The table does provide an interesting summary view.
>  And do I appreciate you putting it together.  Based on the notes for [1],
> [2], and [3], my perception is that the requirements are covered within the
> existing text.  Is it an absolute requirement from the YANG Doctors that
> this summary table be embedded as a new section of the draft (perhaps as a
> Section 2.7.8?)  I am hoping to avoid major new text additions at this
> point.
>
>
there is no need to add a new table

> Yes.  We have done our best to integrate with the embedded NETCONF and
> RESTCONF mechanisms.  There is much additional information in the transport
> drafts here.
>
> This is done in the transport drafts.   E.g., see
> draft-ietf-netconf-netconf-event-notifications Section 7
>
>
I do not agree this is a good idea.
Each error identity should simply state the required "error-tag"
that is associated with the error.  This is expected of protocol operations
that are added to NETCONF and RESTCONF.

Both NETCONF and RESTCONF use a compatible error reporting data structure.
The "error-tag" is used in both of them.  IMO client developers do not
want a different set of error codes for the same error conditions.

I agree that transport drafts could define their own error identities,
which would document the expected error-tag there.

> Instead of "placed elsewhere", how about: "placed in subscription
> transport document defined object".  Would this be sufficient?
>
>
No -- NETCONF and RESTCONF have well-defined error reporting.
The server requirements for this error reporting must be documented.

I agree with the following approach:
  - each operation MUST identify the error-tags that are expected for
    various error conditions (such s is done in RFC 6241)
  - the server MUST return the specified error-tags. If a condition not
explicitly
    defined then the server MUST pick the appropriate error-tag from RFC
6241
 - the server MAY include the specified rc:yang-data in the <error-info>
data structure
 - the server MUST use the appropriate rc:yang-data to report hints
 - for protocols other than NETCONF and RESTCONF, they can map error-tag or
ignore it,
   but the document defining the protocol operation MUST provide

In any case, the -v21 wording results from the attempted balancing the WG
> requests for:
> Hopefully these are covered sufficiently when this document is coupled
> with the NETCONF and RESTCONF Notif transport documents.   For other
> transports, the tags you identify about would not be applicable.
>
> This is the intent of the base specification, e.g., we don't believe a
> kill-subscription should fail for an insufficient-resources reason.   But
> vendors might desire more specificity.  As a result is certainly ok for
> vendor implementations to add new error identities.
>
>
IMO anything can fail for insufficient resources. That is very
implementation-specific.

> This level of specificity was not the author's original plans.  Nor was
> this level of specificity part of earlier draft versions up through -v08.
>  However members of the WG made it clear that such specificity was
> necessary for draft progression.
>
> None of these yang-data structures are specified for use with
> <edit-config> operations.   For <edit-config>, the change to a configured
> subscription would be written to the datastore if it were semantically
> valid.   At this point the subscription enters the [evaluate] points of
> Figure 8.  Issues from this point out would be reported with a vendor
> specific construct such as SYSLOG.
>
>
So how are hints reported for configured subscriptions?

> From the receiver side, a receiver must be able to differentiate between
> state notifications for different subscriptions.  If a publisher can't
> support more than one, it doesn't actually hurt interoperability with a
> receiver.  It just doesn't meet the minimum criteria set for feature
> support for the capability as defined in this document.
>
>
OK - the vendor can use deviations to change it if they want

> "On a receiver of a configured subscription, support for dynamic
> subscriptions is optional.  However if replaying missed event records is
> required for a configured subscription, support for dynamic subscription is
> highly recommended.  In this case, a separate dynamic subscription can be
> established to retransmit the missing event records."
>
>
OK

> I am happy to incorporate whatever text is appropriate.  I was hoping that
> the suggested text was sufficient for now.  Kent has already incorporated
> this as an issue for yang-next
I do not agree that this YANG module should define a new way to encode XPath
into XML instance documents. This will require significant changes to server
implementations.  YANG module prefixes are not even required to be unique
so the set of prefixes used by the server in XML instance documents may be
different,
since it must be unique.

> I am happy to adopt whatever text meets YANG doctor approval.   Can you

Remove all text about YANG prefixes and continue using XML encoding without
modification

> https://mailarchive.ietf.org/arch/msg/netconf/wHQjZuJnh7asJvpE1nIovPDnESM
OK

> I am happy to enhance the definition YANG model's identity definition of
> "configurable-encoding".  I could do this by adding the following
> additional text to the description: "An example of a configurable encoding
> might be a new identity such as 'encode-cbor'.  Such an identity could use
> 'configurable-encoding' as its base.  This would allow a dynamic
> subscription encoded in JSON [RFC-8259] to request notification messages be
> encoded via CBOR [RFC-7049].  Further details for any specific configurable
> encoding would be explored in a transport document based on this
> specification."   Does this meet your ask?
>
>

OK

> I don't think the encoding leaf of establish-subscription needs to note
> that it cannot be modified.  There are other objects which cannot be
> modified as well, they are not explicitly called out.   In the end, the
> current model matches the intended behavior per the text above.  And it is
> only possible to request an encoding for delivered event records be
> different from that of the RPC during the establish subscription.
>
>
OK

> Thinking about this some, it certainly should be there.  But does not need
> to be mandatory.  So I have changed the leaf description so that it is no
> longer mandatory.
>
>
OK

> I have no problem supporting your request for refinement/specificity.  As
> a result, I have tweaked the definition as follows...
>

OK

> To be allowed to progress the document, we inserted the document.  If this
> really is mandatory-to-remove from a YANG doctor point-of-view, what is the
> process for quick closure on this issue between WG leadership and the YANG
> doctors?
>
>

The YANG language makes no restrictions about exporting statements.
I guess I missed that debate so I will just say OK and wonder what problem
this is supposed to solve. I guess the WG wants to give YANG Doctors more
things to check. (This is what we called a CLR in SNMP-land ;-)

> that, would this address your concern?
OK

> Per your thoughts on rpc-error...   For NETCONF and RESTCONF, you point to
> error structures which historically been used with those transports.  Of
> course we were looking to have all subscription hints supportable across
> transports via a single portable YANG data structure.  So the value is that
> a single string object exists so to transport whatever the vendor thinks
> would be useful as a hint in this case.  I.e., there has been no attempt to
> standardize the contents of this string.  If operational experiences drive
> a desire for such structuring, this could provide the basis for a new draft
> building off of this starting point.
>
>
I guess I do not consider NETCONF and RESTCONF "historic" quite yet.
There are many implementations using the rpc-error reporting with no intent
to replace it with something else.

I was just asking for an example, since I have no idea what an implementor
would put in this leaf.

> A "replay-completed" might have already been sent if the subscription
> started with replay, but the replay concluded before the "stop-time".
> Based on this, are you good with the following clarification sentence at
> the end of the current description?....
>
>
OK -- no changes needed

>        "If this is a replay subscription, and a 'replay-completed'
> subscription state notification has not been sent, the 'replay-completed'
> must be sent prior to the 'subscription-completed'."
>
OK

> It does not relate directly to those objects.  But indirectly if the
> replay log is empty (or timed out), then the first paragraph of the
> description applies...
>
OK -- leave unchanged

> Thanks again for doing a very complete YANG Doctor review.

Andy

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Fri, 18 Jan 2019 19:53:17 +0000

Hi Andy,

Thanks.  I have incorporated items where there was agreement.  I have removed the items below where you were ok.  

Remaining below are the open items, with responses.

> If NACM is active, it needs to be followed.  The text we have for NACM is in
> Section 5.4.  Do you see something else specific to subscription suspension
> needed here?  (Maybe I am not getting your point.)
> 

The identity which applies here is "stream-unavailable".  This is the same identity which would be used if a subscriber had never sufficient permissions in the first place.  I don't believe we would want to return an identity specific to when NACM when permissions have just been changed.

> " Instead, they are inserted (as defined in this section) within the sequence of
> notification messages sent to a particular receiver."
> 
> I am not sure that suspended should always be sent head-of-line.   Consider
> that implementation might want to let the existing queue of filtered event
> records be sent if is filter complexity causing the CPU issue.   That could be
> different than if it is a bandwidth issue driving the suspension, and you
> definitely want the 'subscription-suspended' to be placed at the head of line.
> 
> 

Agree fully with this.  Is there text required in the draft here?

... 
> Beyond that it is up to the implementation to decide if some un-transmitted
> queue of event records should be flushed and reprocessed based on the
> modification.  I do not expect this would popular, as a replay subscription could
> accomplish this same functional need.
> 

Have put a minor tweak into Section 2.7:

[old]  subscription state change notifications cannot be filtered out

[new] subscription state change notifications cannot be dropped or filtered out

...
> The next event could also be subscription-terminated.

I do think this possibility is covered in the text.  For Section 2.7.4 subscription-suspended the current text is:

"No further notification will be sent until the subscription resumes or is terminated."

And Section 2.7.5 subscription-resumed says":
"Subscribed event records generated after the issuance of this subscription state change notification may now be sent."

Based on the discussion, I can make it:

"Subscribed event records are again permitted to be sent following this subscription state change notification."

Is this sufficient for you?

...
> that are added to NETCONF and RESTCONF.

In draft-ietf-netconf-netconf-event-notifications, section 7, the required "error-tag" is identified as "operation-failed".   If we instead placed that "error-tag" information in the YANG model, then we have tied the YANG model to the RESTCONF and NETCONF transports.

> want a different set of error codes for the same error conditions.

draft-ietf-netconf-restconf-notif Section 3.3 also requires an "error-tag" node of "operation-failed".  So we used the transport drafts rather than the YANG model to support the same error codes for the same error conditions.
 
> Instead of "placed elsewhere", how about: "placed in subscription transport
> document defined object".  Would this be sufficient?
> 
>   - the server MUST return the specified error-tags. If a condition not explicitly
>     defined then the server MUST pick the appropriate error-tag from RFC 6241
>  - the server MAY include the specified rc:yang-data in the <error-info> data
> structure

Functionally, everything you ask for is fully covered when you include consider draft-ietf-netconf-netconf-event-notifications (section 7) and draft-ietf-netconf-restconf-notif (section 3.3).

My read of the issue is that you believe "error-tag" must be specified in the YANG model.  I believe that "error-tag" shouldn't be in the YANG model because that would tie the model to a transport type.

Any thoughts on how we might close this?    If absolutely required I could place a new comment line in the YANG model under
/* Identities for RPC and Notification errors */

The comment would be something like:
/* When used with NETCONF and RESTCONF RPCs:
    "error-type" node to be used is "application"
     "error-tag" must be "operation-failed".  */

This seems incongruous.  Just throwing it out as a suggestion.

> Hopefully these are covered sufficiently when this document is coupled with
> the NETCONF and RESTCONF Notif transport documents.   For other transports,
> the tags you identify about would not be applicable.
> 
> This is the intent of the base specification, e.g., we don't believe a kill-
> subscription should fail for an insufficient-resources reason.   But vendors might
> desire more specificity.  As a result is certainly ok for vendor implementations
> to add new error identities.
> 
> IMO anything can fail for insufficient resources. That is very implementation-
> specific.

Instead of implementation specific I would call it application specific.   Right now we don't have a catch-all error-identity of 'other-error'.  We preferred that error conditions beyond the current ones listed could be included by vendors as needed.  Further deployment experience could result in new error identities surfacing for standardization should this draft catch on.

> This level of specificity was not the author's original plans.  Nor was this level of
> specificity part of earlier draft versions up through -v08.   However members of
> the WG made it clear that such specificity was necessary for draft progression.
> 
> None of these yang-data structures are specified for use with <edit-config>
> operations.   For <edit-config>, the change to a configured subscription would
> be written to the datastore if it were semantically valid.   At this point the
> subscription enters the [evaluate] points of Figure 8.  Issues from this point out
> would be reported with a vendor specific construct such as SYSLOG.
> 

There is nothing in the specification which requires this.   An implementation could choose to place these in some form of SYSLOG.
...
> "On a receiver of a configured subscription, support for dynamic subscriptions
> is optional.  However if replaying missed event records is required for a
> configured subscription, support for dynamic subscription is highly
> recommended.  In this case, a separate dynamic subscription can be established
> to retransmit the missing event records."
> 
> OK

Change made.

> I am happy to incorporate whatever text is appropriate.  I was hoping that the
> suggested text was sufficient for now.  Kent has already incorporated this as an
> issue for yang-next

See next note

> Remove all text about YANG prefixes and continue using XML encoding without

As a different YANG doctor has required the current text modification, I believe this is a blocker.  What is the process for YANG model reviews in such a case.  I am happy to accept whatever here.  Any suggestions on next steps?

...
> "configurable-encoding".  I could do this by adding the following additional text
> to the description: "An example of a configurable encoding might be a new
> identity such as 'encode-cbor'.  Such an identity could use 'configurable-
> encoding' as its base.  This would allow a dynamic subscription encoded in JSON
> [RFC-8259] to request notification messages be encoded via CBOR [RFC-
> 7049].  Further details for any specific configurable encoding would be explored
> in a transport document based on this specification."   Does this meet your ask?
> 
> 
> OK

Added

> things to check. (This is what we called a CLR in SNMP-land ;-)

Thanks.  No action taken.

Done

> error structures which historically been used with those transports.  Of course
> we were looking to have all subscription hints supportable across transports via
> a single portable YANG data structure.  So the value is that a single string
> object exists so to transport whatever the vendor thinks would be useful as a
> hint in this case.  I.e., there has been no attempt to standardize the contents of
> this string.  If operational experiences drive a desire for such structuring, this
> could provide the basis for a new draft building off of this starting point.
> 

Here is an example from our implementation.  Say you mistype an extra "\" to an xpath filter:
/if:interfaces-state/interface[name="GigabitEthernet0/0"]/oper-status
As a result, the filter is passed to the publisher is:
/if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/oper-status

What we would return in the failure-hint string is:
Invalid expression: offset(9) in '/if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/oper-status'

Eric

---

From: "Mehmet Ersue" <[email-masked]>
Date: Sat, 19 Jan 2019 00:15:15 +0100

Hi Eric,

once again I appreciate your tremendous efforts and engagement to get the notification drafts done.

There is nothing like YANG Doctors approval. The YANG Doctor in charge does the review and suggests changes following RFC6087bis and the YANG language standard. The YANG doctors (reviewers) are actually supposed to help the OPS AD responsible for Network Management to review YANG documents that are destined to be on the IESG agenda.

As  draft-ietf-netconf-subscribed-notifications is a Netconf WG document, I think the WG needs decide on issues which are not very clear and require some discussion on how to change. 

Cheers,
Mehmet

> -----Original Message-----
> From: netconf <[email-masked]> On Behalf Of Eric Voit (evoit)
> Sent: Friday, January 18, 2019 8:53 PM
> To: Andy Bierman <[email-masked]>
> Cc: [email-masked]; [email-masked]; draft-ietf-netconf-subscribed-
> [email-masked]
> Subject: Re: [netconf] Yangdoctors last call review of draft-ietf-netconf-
> subscribed-notifications-21
> 
> Hi Andy,
> 
> Thanks.  I have incorporated items where there was agreement.  I have
> removed the items below where you were ok.
> 
> > If NACM is active, it needs to be followed.  The text we have for NACM
> > is in Section 5.4.  Do you see something else specific to subscription
> > No -- OK to leave NACM as terminate-if-loss-of-rights (Is there an
> > error identity for this event?)
> 
> The identity which applies here is "stream-unavailable".  This is the same
> identity which would be used if a subscriber had never sufficient permissions
> in the first place.  I don't believe we would want to return an identity specific
> to when NACM when permissions have just been changed.
> 
> > > I3) sec 2.1 para 6:
> > issue.   That could be different than if it is a bandwidth issue
> > driving the suspension, and you definitely want the 'subscription-
> suspended' to be placed at the head of line.
> >
> >
> > So also up to the publisher wrt/ what to do with any events that have
> > not been delivered yet on a subscription.  Could delete them or save
> > them for when more bandwidth available (for example)
> 
> > un-transmitted queue of event records should be flushed and
> > reprocessed based on the modification.  I do not expect this would
> > popular, as a replay subscription could accomplish this same functional
> need.
> >
> > appropriate counters. It will try to to do this, but no requirements
> > except maybe subscription events like 'replay-completed' cannot be
> > dropped
> 
> > Thinking more on your point, it might be worth tweaking a couple words
> > to allow for head-of-line placement of "subscription-suspended".
> >
> > last event sent before suspending and subscription-resumed is the
> > first event sent after transition from suspended to active.
> "Subscribed event records generated after the issuance of this subscription
> state change notification may now be sent."
> 
> > RESTCONF mechanisms.  There is much additional information in the
> > transport drafts here.
> >
> > draft-ietf-netconf-netconf-event- notifications Section 7
> >
> > that is associated with the error.  This is expected of protocol
> > operations that are added to NETCONF and RESTCONF.
> 
> In draft-ietf-netconf-netconf-event-notifications, section 7, the required
> "error-tag" is identified as "operation-failed".   If we instead placed that
> "error-tag" information in the YANG model, then we have tied the YANG
> model to the RESTCONF and NETCONF transports.
> 
> draft-ietf-netconf-restconf-notif Section 3.3 also requires an "error-tag"
> node of "operation-failed".  So we used the transport drafts rather than the
> YANG model to support the same error codes for the same error conditions.
> 
> >  - the server MAY include the specified rc:yang-data in the
> > <error-info> data structure
> >  - for protocols other than NETCONF and RESTCONF, they can map
> > error-tag or ignore it,
> Functionally, everything you ask for is fully covered when you include
> consider draft-ietf-netconf-netconf-event-notifications (section 7) and draft-
> ietf-netconf-restconf-notif (section 3.3).
> 
> My read of the issue is that you believe "error-tag" must be specified in the
> YANG model.  I believe that "error-tag" shouldn't be in the YANG model
> because that would tie the model to a transport type.
> 
> Any thoughts on how we might close this?    If absolutely required I could
> place a new comment line in the YANG model under
> > kill- subscription should fail for an insufficient-resources reason.
> > But vendors might desire more specificity.  As a result is certainly
> > ok for vendor implementations to add new error identities.
> >
> > implementation- specific.
> 
> Instead of implementation specific I would call it application specific.   Right
> now we don't have a catch-all error-identity of 'other-error'.  We preferred
> that error conditions beyond the current ones listed could be included by
> vendors as needed.  Further deployment experience could result in new
> error identities surfacing for standardization should this draft catch on.
> 
> > This level of specificity was not the author's original plans.  Nor
> > was this level of specificity part of earlier draft versions up
> > through -v08.   However members of the WG made it clear that such
> specificity was necessary for draft progression.
> >
> > <edit-config> operations.   For <edit-config>, the change to a
> > configured subscription would be written to the datastore if it were
> > semantically valid.   At this point the subscription enters the
> > [evaluate] points of Figure 8.  Issues from this point out would be reported
> with a vendor specific construct such as SYSLOG.
> >
> There is nothing in the specification which requires this.   An implementation
> could choose to place these in some form of SYSLOG.
> ...
> > > I6) sec 2.5, para 3:
> > >
> > >    On a receiver of a
> > subscriptions is optional.  However if replaying missed event records
> > is required for a configured subscription, support for dynamic
> > subscription is highly recommended.  In this case, a separate dynamic
> > subscription can be established to retransmit the missing event records."
> >
> > OK
> 
> Change made.
> 
> > This text was proposed by Martin as a result of the "xpath expressions in
> JSON"
> > thread last October in NETMOD.
> >
> > I am happy to incorporate whatever text is appropriate.  I was hoping
> > that the suggested text was sufficient for now.  Kent has already
> > incorporated this as an issue for yang-next
> > I do not agree that this YANG module should define a new way to encode
> > XPath into XML instance documents. This will require significant
> > changes to server implementations.  YANG module prefixes are not even
> > required to be unique so the set of prefixes used by the server in XML
> > instance documents may be different, since it must be unique.
> 
> See next note
> 
> > Remove all text about YANG prefixes and continue using XML encoding
> > without modification
> 
> As a different YANG doctor has required the current text modification, I
> believe this is a blocker.  What is the process for YANG model reviews in such
> a case.  I am happy to accept whatever here.  Any suggestions on next steps?
> 
> ...
> > I am happy to enhance the definition YANG model's identity definition
> > of "configurable-encoding".  I could do this by adding the following
> > additional text to the description: "An example of a configurable
> > encoding might be a new identity such as 'encode-cbor'.  Such an
> > identity could use 'configurable- encoding' as its base.  This would
> > allow a dynamic subscription encoded in JSON [RFC-8259] to request
> > notification messages be encoded via CBOR [RFC- 7049].  Further
> > details for any specific configurable encoding would be explored in a
> transport document based on this specification."   Does this meet your ask?
> >
> >
> > OK
> 
> Added
> 
> > To be allowed to progress the document, we inserted the document.  If
> > this really is mandatory-to-remove from a YANG doctor point-of-view,
> > what is the process for quick closure on this issue between WG
> > leadership and the YANG doctors?
> >
> >
> > I guess I missed that debate so I will just say OK and wonder what
> > problem this is supposed to solve. I guess the WG wants to give YANG
> > Doctors more things to check. (This is what we called a CLR in
> > SNMP-land ;-)
> 
> Thanks.  No action taken.
> 
> > There is no issue with removing this second sentence completely.  If I
> > did that, would this address your concern?
> >
> > OK
> 
> Done
> 
> > Per your thoughts on rpc-error...   For NETCONF and RESTCONF, you
> > point to error structures which historically been used with those
> > transports.  Of course we were looking to have all subscription hints
> > supportable across transports via a single portable YANG data
> > structure.  So the value is that a single string object exists so to
> > transport whatever the vendor thinks would be useful as a hint in this
> > case.  I.e., there has been no attempt to standardize the contents of
> > this string.  If operational experiences drive a desire for such structuring,
> this could provide the basis for a new draft building off of this starting point.
> >
> > There are many implementations using the rpc-error reporting with no
> > intent to replace it with something else.
> >
> > I was just asking for an example, since I have no idea what an
> > implementor would put in this leaf.
> 
> Invalid expression: offset(9) in '/if:inte\rfaces-
> state/interface[name="GigabitEthernet0/0"]/oper-status'
> 
> Eric
> 
> > Andy
> 
> _______________________________________________
> netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

---

From: Andy Bierman <[email-masked]>
Date: Sat, 19 Jan 2019 08:17:10 -0800

On Fri, Jan 18, 2019 at 11:53 AM Eric Voit (evoit) <[email-masked]> wrote:

> is in
> identity which would be used if a subscriber had never sufficient
> permissions in the first place.  I don't believe we would want to return an
> identity specific to when NACM when permissions have just been changed.
>
>
OK

> sequence of
>  Consider
no - I do not have any text to suggest

...
> un-transmitted
> except
Not sure this is a good addition because the event might get dropped
further down the stack out of the server process that sent the event.

...
> I do think this possibility is covered in the text.  For Section 2.7.4
> subscription-suspended the current text is:
>
OK

> Is this sufficient for you?
old text is OK

> transport
> "error-tag" is identified as "operation-failed".   If we instead placed
> that "error-tag" information in the YANG model, then we have tied the YANG
The subscribed-notifications draft contains rpc-stmts.
Every other RFC with such statements specifies the error handling details
for them.
This document creates a new error reporting system just for 3 or 4 RPC
operations,
and fails to specify even the mandatory fields for the error reporting
system
for NETCONF and RESTCONF.

> node of "operation-failed".  So we used the transport drafts rather than
> the YANG model to support the same error codes for the same error
> conditions.
>
> transport
> data
> consider draft-ietf-netconf-netconf-event-notifications (section 7) and
> draft-ietf-netconf-restconf-notif (section 3.3).
>
> My read of the issue is that you believe "error-tag" must be specified in
> the YANG model.  I believe that "error-tag" shouldn't be in the YANG model
The RFC containing the protocol operation defined with the rpc-stmt
needs to document at least the mandatory-to-implement "error-tag" field.

This is a WG or IESG review issue and maybe the IESG will not care at all
about it.

> This seems incongruous.  Just throwing it out as a suggestion.
Usually the normative text contains the error-tag info.
No other operations use a YANG module to define their own error reporting
system

Andy

> with
> kill-
> implementation-
> Instead of implementation specific I would call it application specific.
>  Right now we don't have a catch-all error-identity of 'other-error'.  We
> preferred that error conditions beyond the current ones listed could be
> included by vendors as needed.  Further deployment experience could result
> in new error identities surfacing for standardization should this draft
> catch on.
>
> this level of
> <edit-config>
> There is nothing in the specification which requires this.   An
> implementation could choose to place these in some form of SYSLOG.
> ...
> > > I6) sec 2.5, para 3:
> > >
> > >    On a receiver of a
> subscriptions
> > This text was proposed by Martin as a result of the "xpath expressions
> in JSON"
> that the
> XPath
> without
> believe this is a blocker.  What is the process for YANG model reviews in
> such a case.  I am happy to accept whatever here.  Any suggestions on next
> steps?
>
> ...
> additional text
> > This was the subject of significant debate in the WG.  The authors did
> not want
> > this restriction either.
> >
> this
> problem
> course
> Invalid expression: offset(9) in
> '/if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/oper-status'
>
> Eric
>
> > Andy
>
>

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 21 Jan 2019 10:49:51 +0100 (CET)

> > > From: Andy Bierman, January 14, 2019 8:08 PM

[...]

> > JSON" thread last October in NETMOD.
> >
> into XML instance documents.

This mechanism was first used in RFC 8040, section 4.8.4, in order to
have a transport-agnostic encoding of XPath expressions (same for XML
and JSON).

There was long debate on the ML for this issue, and this was the
outcome that the WG decided.

> This will require significant changes to server

YANG module prefixes are not used here.  The module *name* is used as
the identifier prefix in XPath expressions:

  /ietf-interfaces:interfaces/ietf-interfaces:interface/...

/martin

---

From: Andy Bierman <[email-masked]>
Date: Mon, 21 Jan 2019 11:17:02 -0800

On Mon, Jan 21, 2019 at 1:49 AM Martin Bjorklund <[email-masked]> wrote:

> in
> that
OK -- no need to change anything then.
I do not agree that "xmlns" attributes pose an implementation challenge for
clients
but servers already need custom XPath to process YANG so this just adds to
that list.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Mon, 21 Jan 2019 11:26:08 -0800

Hi,

I think the error-tag issue can be resolved by including 1 extra sentence
in each error identity.
I know this is NETCONF and RESTCONF centric but those are the only 2
standard protocols
supported for the YANG language right now.

       If the 'error-tag' field is used in error reporting,
       then the value '<correct error-tag>' MUST be used.

For example:

OLD:

  identity insufficient-resources {
    base establish-subscription-error;
    base modify-subscription-error;
    base subscription-suspended-reason;
    description
      "The publisher has insufficient resources to support the
       requested subscription.  An example might be that allocated CPU
       is too limited to generate the desired set of notification
       messages.";
  }

NEW:

  identity insufficient-resources {
    base establish-subscription-error;
    base modify-subscription-error;
    base subscription-suspended-reason;
    description
      "The publisher has insufficient resources to support the
       requested subscription.  An example might be that allocated CPU
       is too limited to generate the desired set of notification
       messages. If the 'error-tag' field is used in error reporting,
       then the value 'resource-denied' MUST be used.";
  }

Andy

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Tue, 22 Jan 2019 22:25:18 +0000

Hi Andy,

Looking at your proposal...  My reading is that it takes the transport specific error info contained in draft-ietf-netconf-netconf-event-notifications section 7, and then replicates that info within 12 separate description objects of the transport independent ietf-subscribed-notifications.yang.    The value you are asserting is that RFCs containing YANG models containing the rpc-stmt have traditionally document the mandatory-to-implement "error-tag" field within the model.  And presumably you are concerned that developers should not have to look elsewhere for this information.

If the YANG doctors require this, it can be inserted.  A similar text change would be needed for quite a few error identities within YANG Push.   Personally I don’t like that YANG models should be required to embed this information.  But I will make the change if you really want this, and nobody else objects.

Other than that, I am not aware of any other open issues in the YANG Doctor review.  Do you know of anything else?

Eric

From: Andy Bierman, January 21, 2019 2:26 PM

Hi,

I think the error-tag issue can be resolved by including 1 extra sentence in each error identity.
I know this is NETCONF and RESTCONF centric but those are the only 2 standard protocols
supported for the YANG language right now.

For example:

OLD:

NEW:

Andy

On Fri, Jan 18, 2019 at 11:53 AM Eric Voit (evoit) <[email-masked]<mailto:[email-masked]>> wrote:
Hi Andy,

...

...

...

Change made.

See next note

...

Added

Thanks.  No action taken.

Done

Eric

---

From: Andy Bierman <[email-masked]>
Date: Tue, 22 Jan 2019 16:43:12 -0800

On Tue, Jan 22, 2019 at 2:25 PM Eric Voit (evoit) <[email-masked]> wrote:

> Hi Andy,
>
>
>
> Looking at your proposal...  My reading is that it takes the transport
> specific error info contained in
> draft-ietf-netconf-netconf-event-notifications section 7, and then
> replicates that info within 12 separate description objects of the
> transport independent ietf-subscribed-notifications.yang.    The value you
> are asserting is that RFCs containing YANG models containing the rpc-stmt
> have traditionally document the mandatory-to-implement "error-tag" field
> within the model.  And presumably you are concerned that developers should
> not have to look elsewhere for this information.
>
>
>
> If the YANG doctors require this, it can be inserted.  A similar text
> change would be needed for quite a few error identities within YANG Push.
> Personally I don’t like that YANG models should be required to embed this
> information.  But I will make the change if you really want this, and
> nobody else objects.
>
>
>

YANG Doctors does not make requirements, just comments.

> Other than that, I am not aware of any other open issues in the YANG
> Doctor review.  Do you know of anything else?
>
>
>

No

Andy

>
>
> *From:* Andy Bierman, January 21, 2019 2:26 PM
>
> Hi,
>
>
>

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 23 Jan 2019 09:31:35 +0100 (CET)

Hi,

"Eric Voit (evoit)" <[email-masked]> wrote:
> transport independent ietf-subscribed-notifications.yang.  The value
> you are asserting is that RFCs containing YANG models containing the
> rpc-stmt have traditionally document the mandatory-to-implement
> "error-tag" field within the model.  And presumably you are concerned
> that developers should not have to look elsewhere for this
> information.

I think that maybe there are two separate issues here.

The first issue is that for each error identity defined, there needs
to be a mapping to the protocol-specific error handling.  Andy
suggests that this info is added to this document, but currently this
information is available in the protcol-mapping documents
(netconf-notif and restconf-notif).  Personally, I think that the
current split of text between documents is fine.

The second issue is that currently, both netconf-notif and
restconf-notif say that *all* these errors use the error-tag
"operation-failed".  Essentially it means that we bypass the error
handling in the protocols.  As Andy points out below, the error
"insufficient-resources" should be mapped to "resource-denied" in
NETCONF and RESTCONF (they mean the same thing).  So it might make
sense to carefully go through the list of errors and map them to the
correct error-tag (but specifiy this in the transport drafts).

/martin

> change would be needed for quite a few error identities within YANG
> Push.  Personally I don’t like that YANG models should be required to
> embed this information.  But I will make the change if you really want
> this, and nobody else objects.
> 
> On Fri, Jan 18, 2019 at 11:53 AM Eric Voit (evoit)
> <[email-masked]<mailto:[email-masked]>> wrote:
> Hi Andy,
> 
> The identity which applies here is "stream-unavailable".  This is the
> same identity which would be used if a subscriber had never sufficient
> permissions in the first place.  I don't believe we would want to
> return an identity specific to when NACM when permissions have just
> been changed.
> 
> > > I3) sec 2.1 para 6:
> > Agree that they are not on the event stream.  So they do not violate
> > this
> > requirement.
> >
> > Additionally there is supporting text in "Section 2.7: subscription
> > state
> > notifications", including...
> >
> > not
> > to
> "Subscribed event records are again permitted to be sent following
> this subscription state change notification."
> 
> In draft-ietf-netconf-netconf-event-notifications, section 7, the
> required "error-tag" is identified as "operation-failed".  If we
> instead placed that "error-tag" information in the YANG model, then we
> have tied the YANG model to the RESTCONF and NETCONF transports.
> 
> draft-ietf-netconf-restconf-notif Section 3.3 also requires an
> "error-tag" node of "operation-failed".  So we used the transport
> drafts rather than the YANG model to support the same error codes for
> the same error conditions.
> 
> >   - explicitly
> >  - data
> >  - for protocols other than NETCONF and RESTCONF, they can map error-tag
> >  - or
> > ignore it,
> consider draft-ietf-netconf-netconf-event-notifications (section 7)
> and draft-ietf-netconf-restconf-notif (section 3.3).
> 
> My read of the issue is that you believe "error-tag" must be specified
> in the YANG model.  I believe that "error-tag" shouldn't be in the
> YANG model because that would tie the model to a transport type.
> 
> Any thoughts on how we might close this?  If absolutely required I
> could place a new comment line in the YANG model under
> Instead of implementation specific I would call it application
> specific.  Right now we don't have a catch-all error-identity of
> 'other-error'.  We preferred that error conditions beyond the current
> ones listed could be included by vendors as needed.  Further
> deployment experience could result in new error identities surfacing
> for standardization should this draft catch on.
> 
> > was this level of
> As a different YANG doctor has required the current text modification,
> I believe this is a blocker.  What is the process for YANG model
> reviews in such a case.  I am happy to accept whatever here.  Any
> suggestions on next steps?
> 
> ...
> > of

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 23 Jan 2019 12:35:18 +0000

> From: Martin Bjorklund, January 23, 2019 3:32 AM
> 
> Hi,
> 
> The first issue is that for each error identity defined, there needs to be a
> mapping to the protocol-specific error handling.  Andy suggests that this info is
> added to this document, but currently this information is available in the
> protcol-mapping documents (netconf-notif and restconf-notif).  Personally, I
> think that the current split of text between documents is fine.
> 
> The second issue is that currently, both netconf-notif and restconf-notif say
> that *all* these errors use the error-tag "operation-failed".  Essentially it means
> that we bypass the error handling in the protocols.  As Andy points out below,
> the error "insufficient-resources" should be mapped to "resource-denied" in
> NETCONF and RESTCONF (they mean the same thing).  So it might make sense
> to carefully go through the list of errors and map them to the correct error-tag
> (but specifiy this in the transport drafts).

I am completely good with this.   Does this work for you Andy?

Eric

> > standard protocols supported for the YANG language right now.
> >
> > > If NACM is active, it needs to be followed.  The text we have for
> > > NACM is in Section 5.4.  Do you see something else specific to
> > > subscription suspension needed here?  (Maybe I am not getting your
> > > point.)
> > >
> > > this requirement.
> > > state notifications", including...
> > >
> > > that implementation might want to let the existing queue of filtered
> > > event records be sent if is filter complexity causing the CPU issue.
> > > That could be different than if it is a bandwidth issue driving the
> > > suspension, and you definitely want the 'subscription-suspended' to
> > > be placed at the head of line.
> > >
> > >
> > > So also up to the publisher wrt/ what to do with any events that
> > > have not been delivered yet on a subscription.  Could delete them or
> > > save them for when more bandwidth available (for example)
> >
> > > popular, as a replay subscription could accomplish this same
> > > functional need.
> > >
> > > Thinking more on your point, it might be worth tweaking a couple
> > > words to allow for head-of-line placement of
> > > "subscription-suspended".
> > >
> > > Yes.  We have done our best to integrate with the embedded NETCONF
> > > and RESTCONF mechanisms.  There is much additional information in
> > > the transport drafts here.
> > >
> > > The "error-tag" is used in both of them.  IMO client developers do
> > > not want a different set of error codes for the same error conditions.
> >
> > > I agree that transport drafts could define their own error
> > > identities, which would document the expected error-tag there.
> > >
> > >
> > > <error-info>
> > > error-tag
> > > Hopefully these are covered sufficiently when this document is
> > > coupled with the NETCONF and RESTCONF Notif transport documents.
> > > For other transports, the tags you identify about would not be
> > > applicable.
> > >
> > > subscription should fail for an insufficient-resources reason.  But
> > > vendors might desire more specificity.  As a result is certainly ok
> > > for vendor implementations to add new error identities.
> > >
> > > [evaluate] points of Figure 8.  Issues from this point out would be
> > > reported with a vendor specific construct such as SYSLOG.
> > >
> > > subscriptions is optional.  However if replaying missed event
> > > records is required for a configured subscription, support for
> > > dynamic subscription is highly recommended.  In this case, a
> > > separate dynamic subscription can be established to retransmit the
> > > missing event records."
> > >
> > > OK
> >
> > Change made.
> >
> > > This text was proposed by Martin as a result of the "xpath
> > > expressions in JSON"
> > > I am happy to incorporate whatever text is appropriate.  I was
> > > hoping that the suggested text was sufficient for now.  Kent has
> > > already incorporated this as an issue for yang-next
> > > I do not agree that this YANG module should define a new way to
> > > encode XPath into XML instance documents. This will require
> > > significant changes to server implementations.  YANG module prefixes
> > > are not even required to be unique so the set of prefixes used by
> > > the server in XML instance documents may be different, since it must
> > > be unique.
> >
> > See next note
> >
> > > I am happy to enhance the definition YANG model's identity
> > > definition of "configurable-encoding".  I could do this by adding
> > > the following additional text to the description: "An example of a
> > > configurable encoding might be a new identity such as 'encode-cbor'.
> > > Such an identity could use
> > > 'configurable-
> > > encoding' as its base.  This would allow a dynamic subscription
> > > encoded in JSON [RFC-8259] to request notification messages be
> > > encoded via CBOR [RFC- 7049].  Further details for any specific
> > > configurable encoding would be explored in a transport document
> > > based on this specification."  Does this meet your ask?
> > >
> > >
> > > OK
> >
> > Added
> >
> > > To be allowed to progress the document, we inserted the document.
> > > If this really is mandatory-to-remove from a YANG doctor
> > > point-of-view, what is the process for quick closure on this issue
> > > between WG leadership and the YANG doctors?
> > >
> > >
> > > There is no issue with removing this second sentence completely.  If
> > > I did that, would this address your concern?
> > >
> > > OK
> >
> > Done
> >
> > > transports.
> > > Of course
> > > we were looking to have all subscription hints supportable across
> > > transports via a single portable YANG data structure.  So the value
> > > is that a single string object exists so to transport whatever the
> > > vendor thinks would be useful as a hint in this case.  I.e., there
> > > has been no attempt to standardize the contents of this string.  If
> > > operational experiences drive a desire for such structuring, this

---

From: Andy Bierman <[email-masked]>
Date: Wed, 23 Jan 2019 09:42:16 -0800

On Wed, Jan 23, 2019 at 4:35 AM Eric Voit (evoit) <[email-masked]> wrote:

> restconf-notif say
This is better.
I'm glad no other drafts are creating their own error reporting system for
each rpc-stmt.
This is a bad precedent and likely to be skipped in implementations.

Andy

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Wed, 23 Jan 2019 20:48:50 +0000

Hi Andy,

Thanks very much for the thorough YANG Doctor review.   I have included the agreed upon comments, and uploaded to:

draft-ietf-netconf-subscribed-notifications-22

a summary of the clarifications made is at the end of the document.  Let me know if there anything else needed to conclude the YANG doctor review of this document.

Also as the result of the ‘error-tag’ discussion with you and Martin, we need to perform the refinement of the ‘error-tag’ mapping within both draft-ietf-netconf-netconf-event-notifications Section 7, and draft-ietf-netconf-restconf-notif Section 3.3.   Directly below is some text and proposed error-tag mappings for those documents.

    o  An "error-tag" node with the value being a string that

       corresponds to an identity associated with the error.  This

       "error-tag" will correspond to the error identities within

       [I-D.draft-ietf-netconf-subscribed-notifications] section

       2.4.6 for general subscription errors:

          error identity         uses error-tag

          ---------------------- --------------

          dscp-unavailable       invalid-value

          encoding-unsupported   invalid-value

          filter-unavailable     invalid-value

         filter-unsupported     invalid-value

          insufficient-resources resource-denied

          no-such-subscription   invalid-value

          replay-unsupported     operation-not-supported

          stream-unavailable     invalid-value

          suspension-timeout     operation-failed

          unsupportable-volume   too-big

       Or this "error-tag" will correspond to the error identities

       within [I-D.ietf-netconf-yang-push] Appendix A.1 for

       subscription errors specific to YANG datastores:

          error identity              uses error-tag

          ----------------------      --------------

          cant-exclude                operation-not-supported

          datastore-not-subscribable  operation-not-supported

          no-such-subscription-resync invalid-value

          on-change-unsupported       operation-not-supported

          on-change-sync-unsupported  operation-not-supported

          period-unsupported          invalid-value

          update-too-big              too-big

          sync-too-big                too-big

          unchanging-selection        operation-failed

Do you (or anyone else in this thread) have any suggestions on the text or proposed mappings?  If this turns out to be ok, Alex will need to remove the NETCONF error-tag specifics from draft-ietf-netconf-yang-push Sections 4.4.1 & 4.4.2

Also Reshad will have to do some work because he is the YANG doctor of netconf-netconf-event-notifications, and he will want to include the same information within draft-ietf-netconf-restconf-notif.

Thanks,

Eric

From: Andy Bierman <[email-masked]>

Sent: Wednesday, January 23, 2019 12:42 PM

To: Eric Voit (evoit) <[email-masked]>

Cc: Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]; [email-masked]

Subject: Re: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-21

> "Eric Voit (evoit)" <mailto:[email-masked]> wrote:

This is better.

I'm glad no other drafts are creating their own error reporting system for each rpc-stmt.

Eric

Andy

> > I think the error-tag issue can be resolved by including 1 extra

> > sentence in each error identity.

> > <mailto:[email-masked]<mailto:mailto:[email-masked]>> wrote:

> > [new] subscription state change notifications cannot be dropped or

> > filtered out

> > >    "Subscribed event records queued for sending after the issuance of

> > "No further notification will be sent until the subscription resumes

> > or is terminated."

> > > >      must make sure protocol error reporting system is used

> > > > correctly

> > > >   -- The error-tag value needs to be identified for each 'reason'

> > > > identity

> > > draft-ietf-netconf-netconf-event-

> > > notifications Section 7

> > > Both NETCONF and RESTCONF use a compatible error reporting data

> > > structure.

> > > >        "modify-subscription" RPC response.  This MUST be sent if

> > > > hints

> > > In any case, the -v21 wording results from the attempted balancing

> > > the WG requests for:

> > > * WG leadership guidance to provide requirements for transport

> > > documents

> > > This is the intent of the base specification, e.g., we don't believe

> > > a

> > > I am happy to adopt whatever text meets YANG doctor approval.  Can

> > > you suggest?

> > > >   -- there should be an example of a configurable encoding

> > > > provided

> > > This was the subject of significant debate in the WG.  The authors

> > > did not want this restriction either.

> > > >      It uses error-tag, error-path, error-string, and error-info

> > > >      extensions

> > > >      to identify which parameters/conditions caused the RPC to be

> > > >      rejected.

> > > >      This error reporting will continue to be used, Not sure this

> > > >      failure-hint

> > > >      has any standards value. Perhaps real-use example can be

> > > > added

> > > could provide the basis for a new draft building off of this

> > > starting point.

> > Here is an example from our implementation.  Say you mistype an extra

> > "\" to an xpath filter:

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 24 Jan 2019 14:16:40 +0100 (CET)

"Eric Voit (evoit)" <[email-masked]> wrote:
> a summary of the clarifications made is at the end of the document.
> Let me know if there anything else needed to conclude the YANG
> doctor review of this document. 
> 
> 
> 
> Also as the result of the ‘error-tag’ discussion with you and
> Martin, we need to perform the refinement of the ‘error-tag’ mapping
> within both draft-ietf-netconf-netconf-event-notifications Section
> 7, and draft-ietf-netconf-restconf-notif Section 3.3.   Directly
> below is some text and proposed error-tag mappings for those
> documents. 
> 
> 
> 

Ok.  But it is not clear to me when this error is actually supposed to
be generated?  The leaf and identity have the same if-feature, so it
isn't a special errro code for "unsupported leaf", which is good!

Then I have to assume it is supposed to be some kind of runtime error?

Thinking some more, what is supposed to happen if the client on the
same session sends first an establish-subscription with dscp 42, and
then another establish-subscription with dscp 10?

>           encoding-unsupported   invalid-value

Ok.  But this identity doesn't give more information than the standard
error:

  error-tag: invalid-value
  error-path: /rpc/establish-subscription/encoding

>           filter-unavailable     invalid-value

This is a "subscription-terminated-reason", which will never be sent
in an rpc-error, and thus should not be mapped to an error-tag.

>          filter-unsupported     invalid-value

Ok.  But this identity doesn't give more information than the standard
error:

  error-tag: invalid-value
  error-path: /rpc/establish-subscription/stream-xpath-filter

>           insufficient-resources resource-denied

Ok.  But this identity doens't give more information than the standard
error in the case of establish-subscription and modify-subscription.

>           no-such-subscription   invalid-value

Ok.  But this identity doens't give more information than the standard
error in the case of establish-subscription and modify-subscription.

>           replay-unsupported     operation-not-supported

Ok.  But this identity doesn't give more information than the standard
error.

>           stream-unavailable     invalid-value

This is a "subscription-terminated-reason", which will never be sent
in an rpc-error, and thus should not be mapped to an error-tag.

>           suspension-timeout     operation-failed

This is a "subscription-terminated-reason", which will never be sent
in an rpc-error, and thus should not be mapped to an error-tag.

>           unsupportable-volume   too-big

This is a "subscription-terminated-reason", which will never be sent
in an rpc-error, and thus should not be mapped to an error-tag.

>           error identity              uses error-tag

I think that this should be invalid-value.

>           no-such-subscription-resync invalid-value

Ok, but again the value of having this is unclear.

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 24 Jan 2019 13:55:26 +0000

> From: Martin Bjorklund, January 24, 2019 8:17 AM
> 
> > Let me know if there anything else needed to conclude the YANG doctor
> > review of this document.
> >
> >
> >
> > Also as the result of the ‘error-tag’ discussion with you and Martin,
> > we need to perform the refinement of the ‘error-tag’ mapping within
> > both draft-ietf-netconf-netconf-event-notifications Section
> Ok.  But it is not clear to me when this error is actually supposed to be
> generated?  The leaf and identity have the same if-feature, so it isn't a special
> errro code for "unsupported leaf", which is good!
> 

Yes.    A publisher, nor the network to which is connects does not have to:
(a) support all DSCP values, nor 
(b) allow a particular value requested by a particular subscriber,
So this condition allows a publisher to reject a request for a DSCP value where is knows the value will not be respected.
 
> Thinking some more, what is supposed to happen if the client on the same
> session sends first an establish-subscription with dscp 42, and then another
> establish-subscription with dscp 10?

This would be allowed.   The interesting part comes with bundling the event records.   The initial versions of draft-ietf-netconf-notification-messages required that all event records in a bundle had a common dscp.  At this point, that seems overly restrictive to the marshalling process, so for now that requirement is not in the document.

> This is a "subscription-terminated-reason", which will never be sent in an rpc-
> error, and thus should not be mapped to an error-tag.

Yes, forgot to remove those.  It is now out.

> Ok.  But this identity doens't give more information than the standard error in
> the case of establish-subscription and modify-subscription.
> 
> Ok.  But this identity doens't give more information than the standard error in
> the case of establish-subscription and modify-subscription.
> 
> Ok.  But this identity doesn't give more information than the standard error.
> 
> This is a "subscription-terminated-reason", which will never be sent in an rpc-
> error, and thus should not be mapped to an error-tag.

Yes, forgot to remove those.  It is now out.

> This is a "subscription-terminated-reason", which will never be sent in an rpc-
> error, and thus should not be mapped to an error-tag.

Yes, forgot to remove those.  It is now out.

> This is a "subscription-terminated-reason", which will never be sent in an rpc-
> error, and thus should not be mapped to an error-tag.

Yes, forgot to remove those.  It is now out.

> I think that this should be invalid-value.

Ok

/Eric

> > Do you (or anyone else in this thread) have any suggestions on the
> > text or proposed mappings?  If this turns out to be ok, Alex will need
> > to remove the NETCONF error-tag specifics from
> > draft-ietf-netconf-yang-push Sections 4.4.1 & 4.4.2
> >
> >
> >
> > Also Reshad will have to do some work because he is the YANG doctor of
> netconf-netconf-event-notifications, and he will want to include the same
> information within draft-ietf-netconf-restconf-notif.
> >
> >
> >
> > Thanks,
> >
> > Eric
> >
> >
> >
> >
> >
> > Cc: Martin Bjorklund <[email-masked]>; [email-masked];
> > [email-masked];
> > [email-masked]
> >
> > Subject: Re: [yang-doctors] Yangdoctors last call review of
> > draft-ietf-netconf-subscribed-notifications-21
> >
> >
> >
> >
> >
> >
> >
> > > to be a
> > > sense
> > > > > the

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 24 Jan 2019 15:39:38 +0100 (CET)

"Eric Voit (evoit)" <[email-masked]> wrote:
> > > Thanks very much for the thorough YANG Doctor review.  I have included
> > > the
> > agreed upon comments, and uploaded to:
> > >
> > be
> So this condition allows a publisher to reject a request for a DSCP
> value where is knows the value will not be respected.

Good explanation, I wish it was part of the "leaf dscp" in the
module :)

The dscp-unavailable identity doesn't add any addition value compared
to the standard error.

> > same
> This would be allowed.

On linux at least this is a sockopt, i.e., the option applies to the
socket, which means all packets on the session.  So how is this
supposed to be implemented if different messages on the session should
have different dscp values?  Or is the idea that you send the msg,
flush all data from ssh/tls to tcp, then flush the tcp buffers (not
that easy...)?

Even if there's just one establish-subscription with a dscp value,
since it applies to the session it means that all normal rpcs on this
session will get the same dscp value.  It is not clear that this is
the intention?

/martin

> The interesting part comes with bundling the
> event records.  The initial versions of
> draft-ietf-netconf-notification-messages required that all event
> records in a bundle had a common dscp.  At this point, that seems
> overly restrictive to the marshalling process, so for now that
> requirement is not in the document.
> 
> > in an rpc-
> > error in
> > error in
> > in an rpc-
> > in an rpc-
> > in an rpc-
> > > On Wed, Jan 23, 2019 at 4:35 AM Eric Voit (evoit)
> > > <mailto:[email-masked]>
> > wrote:
> > >

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 24 Jan 2019 15:22:58 +0000

> From: Martin Bjorklund, January 24, 2019 9:40 AM
> 
> > > > Thanks very much for the thorough YANG Doctor review.  I have
> > > > included the
> > > > Martin, we need to perform the refinement of the ‘error-tag’
> > > > mapping within both draft-ietf-netconf-netconf-event-notifications
> Section
> > > Ok.  But it is not clear to me when this error is actually supposed
> > > to be generated?  The leaf and identity have the same if-feature, so
> > > it isn't a special errro code for "unsupported leaf", which is good!
> > >
> > (b) allow a particular value requested by a particular subscriber, So
> > this condition allows a publisher to reject a request for a DSCP value
> > where is knows the value will not be respected.
> 
> Good explanation, I wish it was part of the "leaf dscp" in the module :)
> 
> The dscp-unavailable identity doesn't add any addition value compared to the
> standard error.

For NETCONF and RESTCONF, this is the case.
 
> On linux at least this is a sockopt, i.e., the option applies to the socket, which
> means all packets on the session.  So how is this supposed to be implemented if
> different messages on the session should have different dscp values?  Or is the
> idea that you send the msg, flush all data from ssh/tls to tcp, then flush the tcp
> buffers (not that easy...)?
> 
> Even if there's just one establish-subscription with a dscp value, since it applies
> to the session it means that all normal rpcs on this session will get the same
> dscp value.  It is not clear that this is the intention?

For a NETCONF session, I agree that an implementation need not try to attempt to support more than one DSCP for that session.  And the error identity dscp-unavailable is a valid response here.

For RESTCONF and other transports, there options which can more flexibly support different DSCP values.  This is one reason I was pushing hard in 2016 to leverage HTTP2.

Eric

> /martin
> 
> 
> > The interesting part comes with bundling the event records.  The
> > initial versions of draft-ietf-netconf-notification-messages required
> > that all event records in a bundle had a common dscp.  At this point,
> > that seems overly restrictive to the marshalling process, so for now
> > that requirement is not in the document.
> >
> > > in an rpc- error, and thus should not be mapped to an error-tag.
> >
> > > Ok.  But this identity doens't give more information than the
> > > standard error in the case of establish-subscription and
> > > modify-subscription.
> > >
> > > Ok.  But this identity doens't give more information than the
> > > standard error in the case of establish-subscription and
> > > modify-subscription.
> > >
> > > in an rpc- error, and thus should not be mapped to an error-tag.
> >
> > > in an rpc- error, and thus should not be mapped to an error-tag.
> >
> > > in an rpc- error, and thus should not be mapped to an error-tag.
> >
> > > > text or proposed mappings?  If this turns out to be ok, Alex will
> > > > need to remove the NETCONF error-tag specifics from
> > > netconf-netconf-event-notifications, and he will want to include the
> > > same information within draft-ietf-netconf-restconf-notif.
> > > >
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Eric
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > > I know this is NETCONF and RESTCONF centric but those are the
> > > > > > only
> > > > > > 2
> > > >
> > > > > > I do think this possibility is covered in the text.  For
> > > > > > Section
> > > > > > 2.7.4
> > > >
> > > > > > consider draft-ietf-netconf-netconf-event-notifications
> > > > > > (section
> > > > > > 7)
> > > >
> > > > > > > >      How are the yang-data structs used for edit-config or
> > > > > > > > commit
> > > errors?
> > > >
> > > > > > >
> > > >
> > > > > > /if:interfaces-state/interface[name="GigabitEthernet0/0"]/oper
> > > > > > -sta
> > > > > > tus
> > > >
> > > > > > /if:inte\rfaces-state/interface[name="GigabitEthernet0/0"]/ope
> > > > > > r-st
> > > > > > atus
> > > >
> > > > > >
> > > >

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 24 Jan 2019 16:29:45 +0100 (CET)

"Eric Voit (evoit)" <[email-masked]> wrote:
> > to the

And comi.  The point is, what makes the rpcs in this module so special
that they have to invent a new error reporting scheme?   If we do that
for these rpcs, why not for all other rpc in all other modules?

> > socket, which
> > since it applies
> For a NETCONF session, I agree that an implementation need not try to
> attempt to support more than one DSCP for that session.  And the error
> identity dscp-unavailable is a valid response here.

Then that should be explained (preferrably in the leaf dscp).

> For RESTCONF and other transports, there options which can more
> flexibly support different DSCP values.  This is one reason I was
> pushing hard in 2016 to leverage HTTP2.

We're talking about dynamic subscriptions here.  I don't think anyone
has suggested HTTP2 for dynamic subscriptions.

For configured subscriptions, this is not an issue, not even for
NETCONF.

/martin

> > > > > > > > >   -- The error-tag value needs to be identified for each
> > > > > > > > >   -- 'reason'
> > > > >
> > > > > > > > > identity
> > > > >
> > > > > > > >
> > > > >
> > > > > > > > >   -- all 3 paragraphs like this; unclear what "placed
> > > > > > > > >   -- elsewhere"
> > > > >

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 24 Jan 2019 17:04:53 +0000

> > > > > > mapping within both
> > > > > > draft-ietf-netconf-netconf-event-notifications
> > > Section
> > > > > Ok.  But it is not clear to me when this error is actually
> > > > > supposed to be generated?  The leaf and identity have the same
> > > > > if-feature, so it isn't a special errro code for "unsupported leaf", which is
> good!
> > > > >
> > > > Yes.  A publisher, nor the network to which is connects does not
> > > > have
> > > > to:
> > > > So this condition allows a publisher to reject a request for a
> > > > DSCP value where is knows the value will not be respected.
> > >
> > > module
> > > :)
> > > The dscp-unavailable identity doesn't add any addition value
> > > compared to the standard error.
> >

The current RPC error mechanisms ties YANG RPCs to NETCONF and RESTCONF transports.   Over many years there has been lots of work to align the subscription drafts to these existing mechanisms, while maintaining as much transport independence for hints as possible.

> > > > > Thinking some more, what is supposed to happen if the client on
> > > > > the same session sends first an establish-subscription with dscp
> > > > > 42, and then another establish-subscription with dscp 10?
> > > >
> > > > This would be allowed.
> > >
> > > supposed to be implemented if different messages on the session
> > > should have different dscp values?
> > > Or is the
> > > idea that you send the msg, flush all data from ssh/tls to tcp, then
> > > flush the tcp buffers (not that easy...)?
> > >
> > > since it applies to the session it means that all normal rpcs on
> > > this session will get the same dscp value.  It is not clear that
> > > this is the intention?
> >
> We're talking about dynamic subscriptions here.  I don't think anyone has
> suggested HTTP2 for dynamic subscriptions.

I have always believed that HTTP2 for dynamic subscriptions was a useful industry target.   This driver was the original reason I wrote the original draft which became draft-ietf-netconf-restconf-notif, and many variations on dynamic subscriptions with HTTP2.    In the meantime, GRPC subscriptions (based on HTTP2) has grown into this space.

Eric

> For configured subscriptions, this is not an issue, not even for NETCONF.
> 
> 
> /martin
> 
> 
> 
> >
> > Eric
> >
> > > /martin
> > >
> > >
> > > > initial versions of draft-ietf-netconf-notification-messages
> > > > required that all event records in a bundle had a common dscp.  At
> > > > this point, that seems overly restrictive to the marshalling
> > > > process, so for now that requirement is not in the document.
> > > >
> > > > > This is a "subscription-terminated-reason", which will never be
> > > > > sent in an rpc- error, and thus should not be mapped to an error-tag.
> > > >
> > > > > This is a "subscription-terminated-reason", which will never be
> > > > > sent in an rpc- error, and thus should not be mapped to an error-tag.
> > > >
> > > > > This is a "subscription-terminated-reason", which will never be
> > > > > sent in an rpc- error, and thus should not be mapped to an error-tag.
> > > >
> > > > > This is a "subscription-terminated-reason", which will never be
> > > > > sent in an rpc- error, and thus should not be mapped to an error-tag.
> > > >
> > > > > > Do you (or anyone else in this thread) have any suggestions on
> > > > > > the text or proposed mappings?  If this turns out to be ok,
> > > > > > Alex will need to remove the NETCONF error-tag specifics from
> > > > > netconf-netconf-event-notifications, and he will want to include
> > > > > the same information within draft-ietf-netconf-restconf-notif.
> > > > > >
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Eric
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > > I know this is NETCONF and RESTCONF centric but those are
> > > > > > > > the only
> > > > > > > > 2
> > > > > >
> > > > > > > > > (Is there an
> > > > > > > > > >      How are the yang-data structs used for
> > > > > > > > > > edit-config or commit
> > > > > errors?
> > > > > >
> > > > > > > > >
> > > > > >
> > > > > > > > > I guess I do not consider NETCONF and RESTCONF
> > > > > > > > > "historic" quite
> > > yet.
> > > > > >
> > > > > > > > '/if:inte\rfaces-state/interface[name="GigabitEthernet0/0"
> > > > > > > > ]/oper-
> > > status'
> > > > > >
> > > > > > > >
> > > > > >
> > > > > > > > Eric
> > > > > >
> > > > > > > >
> > > > > >
> > > > > > > > > Andy

---

From: Andy Bierman <[email-masked]>
Date: Thu, 24 Jan 2019 09:29:32 -0800

On Thu, Jan 24, 2019 at 9:04 AM Eric Voit (evoit) <[email-masked]> wrote:

> The current RPC error mechanisms ties YANG RPCs to NETCONF and RESTCONF
> transports.   Over many years there has been lots of work to align the
> subscription drafts to these existing mechanisms, while maintaining as much
> transport independence for hints as possible.
>
>

The subscription draft changes error reporting in significant ways, for the
worse, not the better.
There are a set of error-tag values that are used for all protocol
operations defined with rpc-stmt.
It was first defined in NETCONF but it has been applied to RESTCONF as well.

This draft uses identities to replace the set of common error-tags.
Instead, every single
rpc-stmt can have its own set of error codes. Even worse, a different set
of error codes
depending on the protocol that was used.

Why should a 'resource-denied' error be something different in RESTCONF vs.
NETCONF?
Or even in CoMI or some unknown protocol?  Why would 'invalid-value' be
different,
depending on the operation?

It is 1 thing to change the documentation of YANG-based protocol operations.
It is another to omit mandatory information needed to work with NETCONF and
RESTCONF.
If the error-tag values are properly documented and the NETCONF and
RESTCONF mappings are identical
then there should not be any problems using standard error reporting.

Andy

> I have always believed that HTTP2 for dynamic subscriptions was a useful
> industry target.   This driver was the original reason I wrote the original
> draft which became draft-ietf-netconf-restconf-notif, and many variations
> on dynamic subscriptions with HTTP2.    In the meantime, GRPC subscriptions
> (based on HTTP2) has grown into this space.
>
> Eric
>
> > > > > > > > > Thanks.  I have incorporated items where there was
> agreement.
> > > > > > > > > I have
> > > > > > >
> > > > > > > > > > suspension, and you definitely want the
> 'subscription-suspended'
> > > > > > > > > > to
> > > > > > >
> > > > > > > > > > subscription should fail for an insufficient-resources
> reason.
> > > > > > > > > > But
> > > > > > >
> > > > > > > > > > This level of specificity was not the author's original
> plans.
> > > > > > > > > > Nor
> > > > > > >
> > > > > > > > > > >      subscriptions when <edit-config> is the RPC that
> has the
> > > > > > > > > > >      error?
> > > > > > >
> > > > > > > > > > >    optional except where replaying missed event
> records is
> > > > > > > > > > >    required.
> > > > > > >
> > > > > > > > > > >
> > > > > > >
> > > > > > > > > > >            The expression is evaluated in the
> following XPath
> > > > > > > > > > >            context:
> > > > > > >
> > > > > > > > > > >
> > > > > > >
> > > > > > > > > > >              o The set of namespace declarations is
> the set of
> > > > > > > > > > >              prefix
> > > > > > >
> > > > > > > > > > I am happy to adopt whatever text meets YANG doctor
> approval.
> > > > > > > > > > Can
> > > > > > >
> > > > > > > > > > you suggest?
> > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > > > > > reviews in such a case.  I am happy to accept whatever
> here.
> > > > > > > > > Any
> > > > > > >
> > > > > > > > > > There is no issue with removing this second sentence
> completely.
> > > > > > > > > > If
> > > > > > >
> > > > > > > > > > has been no attempt to standardize the contents of this
> string.
> > > > > > > > > > If
> > > > > > >

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 24 Jan 2019 20:14:15 +0100 (CET)

"Eric Voit (evoit)" <[email-masked]> wrote:
> > > > > > if-feature, so it isn't a special errro code for "unsupported leaf",
> > > > > > which is good!
> > > > > >
> The current RPC error mechanisms ties YANG RPCs to NETCONF and
> RESTCONF transports.  Over many years there has been lots of work to
> align the subscription drafts to these existing mechanisms, while
> maintaining as much transport independence for hints as possible.

I'm not talking about the "hints", but the error codes.

Or are you saying that you think that *all* rpcs defined today should
have their own error reporting mechanism?

> > > > this session will get the same dscp value.  It is not clear that

Did you miss these questions?

> > has
> I have always believed that HTTP2 for dynamic subscriptions was a
> useful industry target.  This driver was the original reason I wrote
> the original draft which became draft-ietf-netconf-restconf-notif, and
> many variations on dynamic subscriptions with HTTP2.  In the meantime,
> GRPC subscriptions (based on HTTP2) has grown into this space.

This is irrelevant for the problem.  The problem is maintaining
several dscp values on the same session.  Any solution that introduces
a single session obviously avoids the problem.  And GRPC has also
nothing to do with this.

/martin

> > > > > > > > > > >      subscriptions when <edit-config> is the RPC that has
> > > > > > > > > > >      the
> > > > > > > > > > >      error?
> > > > > > >
> > > > > > > > > > >            The expression is evaluated in the following
> > > > > > > > > > >            XPath
> > > > > > > > > > >            context:
> > > > > > >
> > > > > > > > > > >
> > > > > > >
> > > > > > > > > > >              o The set of namespace declarations is the set
> > > > > > > > > > >              of
> > > > > > > > > > >              prefix
> > > > > > >

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 24 Jan 2019 23:04:55 +0000

> have their own error reporting mechanism?

I just want to do whatever gets this done.

> Did you miss these questions?

With NETCONF, one DSCP will apply to single TCP session.  So there is no issue.

For RESTCONF, there is no requirement that there is a single TCP session between publisher and subscriber.  Where there are different DSCPs requested, a different logical connection can be established should the publisher want to concurrently support several DSCPs.
 
> a single session obviously avoids the problem.  And GRPC has also

I was responding to your point that 'I don't think anyone has suggested HTTP2 for dynamic subscriptions'.   Up through draft-ietf-netconf-restconf-notif v08, the benefits of HTTP2 for dynamic subscriptions was explicitly laid out.

Eric

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Thu, 24 Jan 2019 23:55:58 +0000

On Thu, Jan 24, 2019 at 9:04 AM Eric Voit (evoit) <[email-masked]<mailto:[email-masked]>> wrote:
> "Eric Voit (evoit)" <[email-masked]<mailto:[email-masked]>> wrote:
> > > "Eric Voit (evoit)" <[email-masked]<mailto:[email-masked]>> wrote:
> > > > > "Eric Voit (evoit)" <[email-masked]<mailto:[email-masked]>> wrote:
> > > > > > Hi Andy,
> > > > > >
> > > > > >
> > > > > >

The subscription draft changes error reporting in significant ways, for the worse, not the better.
There are a set of error-tag values that are used for all protocol operations defined with rpc-stmt.
It was first defined in NETCONF but it has been applied to RESTCONF as well.

This draft uses identities to replace the set of common error-tags.  Instead, every single
rpc-stmt can have its own set of error codes. Even worse, a different set of error codes
depending on the protocol that was used.

Why should a 'resource-denied' error be something different in RESTCONF vs. NETCONF?
Or even in CoMI or some unknown protocol?  Why would 'invalid-value' be different,
depending on the operation?

It is 1 thing to change the documentation of YANG-based protocol operations.
It is another to omit mandatory information needed to work with NETCONF and RESTCONF.
If the error-tag values are properly documented and the NETCONF and RESTCONF mappings are identical
then there should not be any problems using standard error reporting.

<Eric> I believe that with this thread, that the error-tag values will be well documented and identical between NETCONF and RESTCONF transport drafts.

Eric

Andy

---

From: Andy Bierman <[email-masked]>
Date: Thu, 24 Jan 2019 16:02:43 -0800

On Thu, Jan 24, 2019 at 3:56 PM Eric Voit (evoit) <[email-masked]> wrote:

> <Eric> I believe that with this thread, that the error-tag values will be
> well documented and identical between NETCONF and RESTCONF transport
> drafts.
>
>
>

I agree.

Using the correct error-tag values plus identity names for error-app-tag
values allows
both generic clients and clients specialized for this application to both
work.

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 25 Jan 2019 09:39:38 +0100 (CET)

[...]

> With NETCONF, one DSCP will apply to single TCP session.  So there is no issue.

The problem is if a client sends two establish-subscriptions
with different values for dscp on the same session.  Or even if the
client sends one establish-subscription with som dscp value, then the
dscp will be applied to all packets from other rpcs as well.

... and even worse, with SSH channels you can have multiple NETCONF
session on the same TCP session, which means that the dscp value
applies to all packets in all NETCONF sessions sharing the same TCP
session.

I don't know what the right thing to do is.  Probably first agree that
this is in fact a problem, then maybe simply document this somehow.

> For RESTCONF, there is no requirement that there is a single TCP
> session between publisher and subscriber.

I agree that this is not a problem with RESTCONF, since there is no
multiplexing going on.  When the server accepts the SSE GET, it can
safely set the dscp value.

> Where there are different
> DSCPs requested, a different logical connection can be established
> should the publisher want to concurrently support several DSCPs.

I don't think is an issue, see above.

/martin

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Fri, 25 Jan 2019 15:14:36 +0000

> From: Martin Bjorklund, January 25, 2019 3:40 AM
> 
> > > > > > > > > Thinking some more, what is supposed to happen if the
> > > > > > > > > client on the same session sends first an
> > > > > > > > > establish-subscription with dscp 42, and then another establish-
> subscription with dscp 10?
> > > > > > > >
> > > > > > > > This would be allowed.
> > > > > > >
> > > > > > > On linux at least this is a sockopt, i.e., the option
> > > > > > > applies to the socket, which means all packets on the
> > > > > > > session.  So how is this supposed to be implemented if
> > > > > > > idea that you send the msg, flush all data from ssh/tls to
> > > > > > > tcp, then flush the tcp buffers (not that easy...)?
> > > > > > >
> > > > > > > Even if there's just one establish-subscription with a dscp
> > > > > > > value, since it applies to the session it means that all
> > > > > > > normal rpcs on this session will get the same dscp value.
> > > > > > > It is not clear that this is the intention?
> > >
> The problem is if a client sends two establish-subscriptions with different
> values for dscp on the same session.  Or even if the client sends one establish-
> subscription with som dscp value, then the dscp will be applied to all packets
> from other rpcs as well.
> 
> ... and even worse, with SSH channels you can have multiple NETCONF session
> on the same TCP session, which means that the dscp value applies to all
> packets in all NETCONF sessions sharing the same TCP session.
> 
> I don't know what the right thing to do is.  Probably first agree that this is in
> fact a problem, then maybe simply document this somehow.

I hadn't thought about multiple NETCONF SSH channels.  I agree something to guide developers is needed here.  The easiest fix is to recommend not supporting feature "dscp" with NETCONF.  (It shouldn't be absolutely prohibited as DSCP related RFC's such as RFC7657 embed text like "a single DSCP should be used for all packets in a TCP connection".)

Here is the text I suggest is put into draft-ietf-netconf-netconf-event-notifications, Section 5:

"The feature "dscp" SHOULD NOT be supported over NETCONF.  This will avoid the potential for out-of-order packet delivery of the set of all traffic within the TCP session."

With this text, a RESTCONF publisher can support the feature dscp.  And if someone attempts to use dscp with a NETCONF subscription to the same publisher, the error "dscp-unavailable" can be sent.

Eric 

...

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 25 Jan 2019 16:29:41 +0100 (CET)

"Eric Voit (evoit)" <[email-masked]> wrote:
> > > > > > > > different messages on the session should have different dscp
> > > > > > > > values?
> > > > > > > > Or is the
> > > With NETCONF, one DSCP will apply to single TCP session.  So there is
> > > no
> > issue.
> > 
> > session
> > this is in
> I hadn't thought about multiple NETCONF SSH channels.  I agree
> something to guide developers is needed here.  The easiest fix is to
> recommend not supporting feature "dscp" with NETCONF.  (It shouldn't
> be absolutely prohibited as DSCP related RFC's such as RFC7657 embed
> text like "a single DSCP should be used for all packets in a TCP
> connection".)
> 
> Here is the text I suggest is put into
> draft-ietf-netconf-netconf-event-notifications, Section 5:
> 
> "The feature "dscp" SHOULD NOT be supported over NETCONF.  This will
> avoid the potential for out-of-order packet delivery of the set of all
> traffic within the TCP session."

I think this is a bit too limiting.  For configured subscriptions it
can be perfectly fine, since the server can ensure that there's a
single TCP session to the receiver in this case.  (I know we don't
support configured subscriptions at the moment).

Perhaps we can say in the SN document:

  If a server that supports the "dscp" feature cannot guarantee that
  the only packets sent on an underlying transport session are from
  the subscription, then it should reject the subscription with a
  "dscp-unavailable" error.  This will avoid the potential for
  out-of-order packet delivery of the set of all traffic within the
  TCP session.

I think such text belongs to the SN document, since it may apply to
other transports than just NETCONF.

/martin

> With this text, a RESTCONF publisher can support the feature dscp.
> And if someone attempts to use dscp with a NETCONF subscription to the
> same publisher, the error "dscp-unavailable" can be sent.
> 
> Eric 
> 
> 
> ...
> > /martin
>

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Fri, 25 Jan 2019 15:41:47 +0000

> From: Martin Bjorklund, January 25, 2019 10:30 AM
> 
> > > > > > > > > > > Thinking some more, what is supposed to happen if
> > > > > > > > > > > the client on the same session sends first an
> > > > > > > > > > > establish-subscription with dscp 42, and then
> > > > > > > > > > > another
> > > > > > > > > > > establish-
> > > > > > > > > different messages on the session should have different
> > > > > > > > > dscp values?
> > > > > > > > > Or is the
> > > > > > > > > idea that you send the msg, flush all data from ssh/tls
> > > > > > > > > to tcp, then flush the tcp buffers (not that easy...)?
> > > > > > > > >
> > > > > > > > > Even if there's just one establish-subscription with a
> > > > > > > > > dscp value, since it applies to the session it means
> > > > > > > > > that all normal rpcs on this session will get the same dscp value.
> > > > With NETCONF, one DSCP will apply to single TCP session.  So there
> > > > is no
> > > issue.
> > >
> > > The problem is if a client sends two establish-subscriptions with
> > > different values for dscp on the same session.  Or even if the
> > > client sends one
> > > establish-
> > > subscription with som dscp value, then the dscp will be applied to
> > > all packets from other rpcs as well.
> > >
> > > I don't know what the right thing to do is.  Probably first agree
> > > that this is in fact a problem, then maybe simply document this
> > > somehow.
> >
> I think this is a bit too limiting.  For configured subscriptions it can be perfectly
> fine, since the server can ensure that there's a single TCP session to the receiver
> in this case.  (I know we don't support configured subscriptions at the moment).
> 

This works for me.    I will put this in Section 2.3, directly after the sentence: 

   If the publisher supports the "dscp" feature, then a subscription
   with a "dscp" leaf MUST result in a corresponding [RFC2474] DSCP
   marking being placed within the IP header of any resulting
   notification messages and subscription state change notifications.

 unless I hear any objections on this thread.

Eric
 
> I think such text belongs to the SN document, since it may apply to other
> transports than just NETCONF.
> 
> 
> /martin
> 
> 
>

---

From: Per Hedeland <[email-masked]>
Date: Fri, 25 Jan 2019 19:19:30 +0100

>   unless I hear any objections on this thread.

I'm not sure why the "avoid out-of-order" rationale was presented.
There is no order "of the set of all traffic within the TCP session"
to be out-of. Regardless of DSCP, SSH peers will send data on the
channels (NETCONF sessions) in an order that depends, among other
things, on the rate of consumption of that data. They will preserve
order on any given channel, but will not consider order with respect
to other channels (arguably, not having to do that is the whole point
of having flow control at the SSH level).

I suggest simply dropping the "This will avoid ..." sentence.

--Per

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Fri, 25 Jan 2019 21:15:31 +0000

> From: Per Hedeland, January 25, 2019 1:20 PM
> 
> >>>>>>>>>> On linux at least this is a sockopt, i.e., the option applies
> >>>>>>>>>> to the socket, which means all packets on the session.  So
> >>>>>>>>>> how is this supposed to be implemented if different messages
> >>>>>>>>>> on the session should have different dscp values?
> >>>>>>>>>> Or is the
> >>> avoid the potential for out-of-order packet delivery of the set of
> >>> all traffic within the TCP session."
> >>
> >> single TCP session to the receiver in this case.  (I know we don't support
> configured subscriptions at the moment).
> >>
> to be out-of. Regardless of DSCP, SSH peers will send data on the channels
> (NETCONF sessions) in an order that depends, among other things, on the rate
> of consumption of that data. They will preserve order on any given channel, but
> will not consider order with respect to other channels (arguably, not having to
> do that is the whole point of having flow control at the SSH level).

Hi Per,

What you say is absolutely true.  You likely know this, but Martin asked to include the out-of-order rationale because if a TCP session carries traffic consisting of a set of DSCP values, then the network elements between the publisher and receiver might reorder the packets.   While the order might be restored at the receiver (per your point), this might also be seen as loss.  The result to the application is that higher priority dscp packets will be seen no faster those with lower priority.  Again you likely know this, but there is more on this general topic in places like RFC-7657, Section 5.2.

To cover what Martin wants, and reduce the non-normative text, how about the following?...

"Where TCP is used, a publisher which supports the "dscp" feature SHOULD ensure that a subscription's notification messages are returned within a single TCP transport session where all traffic shares the subscription's requested "dscp" leaf value.   Where this cannot be guaranteed, any "establish subscription" RPC request SHOULD be rejected with a "dscp-unavailable" error."

Eric

> >>> And if someone attempts to use dscp with a NETCONF subscription to
> >>> the same publisher, the error "dscp-unavailable" can be sent.
> >>>
> >>> Eric
> >>>
> >>>
> >>> ...
> >>>> /martin
> >>>
> >

---

From: Per Hedeland <[email-masked]>
Date: Fri, 25 Jan 2019 22:31:53 +0100

On 2019-01-25 22:15, Eric Voit (evoit) wrote:
> What you say is absolutely true.  You likely know this, but Martin asked to include the out-of-order rationale because if a TCP session carries traffic consisting of a set of DSCP values, then the network elements between the publisher and receiver might reorder the packets.

Hm, from the e-mail thread, it seems that the issue of reordering was
introduced by you, and Martin's text just copied your text regarding
that issue, while relaxing the requirement from completely forbidding
the use of the DSCP parameter to disallowing it where it could have
undesired effects.

> While the order might be restored at the receiver (per your point), this might also be seen as loss.  The result to the application is that higher priority dscp packets will be seen no faster those with lower priority.  Again you likely know this, but there is more on this general topic in places like RFC-7657, Section 5.2.
> 

This seems fine to me. The point, as far as I can see, is that a DSCP
parameter must not be applied to a session (or subscription) that
didn't "ask" for it. This is enough of a rationale IMHO, no need to go
into the details of what the result may or may not be as far as
reordering of packets goes.

--Per

---

From: "Eric Voit (evoit)" <[email-masked]>
Date: Fri, 25 Jan 2019 21:37:44 +0000

> From: Per Hedeland, January 25, 2019 4:32 PM
> To: Eric Voit (evoit) <[email-masked]>; Martin Bjorklund <[email-masked]>
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: [netconf] [yang-doctors] Yangdoctors last call review of draft-ietf-
> netconf-subscribed-notifications-21
> 
> >>>>>>> With NETCONF, one DSCP will apply to single TCP session.  So
> >>>>>>> there is no
> >>>>>> issue.
> >>>>>>
> >>>>>> subscription with som dscp value, then the dscp will be applied
> >>>>>> to all packets from other rpcs as well.
> >>>>>>
> >>>>>> ... and even worse, with SSH channels you can have multiple
> >>>>>> NETCONF session on the same TCP session, which means that the
> >>>>>> dscp value applies to all packets in all NETCONF sessions sharing
> >>>>>> the same TCP session.
> >>>>>>
> >>>>> something to guide developers is needed here.  The easiest fix is
> >>>>> to recommend not supporting feature "dscp" with NETCONF.  (It
> >>>>> shouldn't be absolutely prohibited as DSCP related RFC's such as
> >>>>> RFC7657 embed text like "a single DSCP should be used for all
> >>>>> packets in a TCP
> >>>>> connection".)
> >>>>>
> >>>>> "The feature "dscp" SHOULD NOT be supported over NETCONF.  This
> >>>>> will avoid the potential for out-of-order packet delivery of the
> >>>>> set of all traffic within the TCP session."
> >>>>
> >>>> I think this is a bit too limiting.  For configured subscriptions
> >>>> it can be perfectly fine, since the server can ensure that there's
> >>>> a single TCP session to the receiver in this case.  (I know we
> >>>> don't support
> >> to other channels (arguably, not having to do that is the whole point of
> having flow control at the SSH level).
> >
> > Hi Per,
> >
> > What you say is absolutely true.  You likely know this, but Martin asked to
> include the out-of-order rationale because if a TCP session carries traffic
> consisting of a set of DSCP values, then the network elements between the
> publisher and receiver might reorder the packets.
> 
> introduced by you, and Martin's text just copied your text regarding that issue,
> while relaxing the requirement from completely forbidding the use of the DSCP
> parameter to disallowing it where it could have undesired effects.
> 
> > While the order might be restored at the receiver (per your point), this might
> also be seen as loss.  The result to the application is that higher priority dscp
> packets will be seen no faster those with lower priority.  Again you likely know
> this, but there is more on this general topic in places like RFC-7657, Section 5.2.
> >
> > "Where TCP is used, a publisher which supports the "dscp" feature SHOULD
> ensure that a subscription's notification messages are returned within a single
> TCP transport session where all traffic shares the subscription's requested
> "dscp" leaf value.   Where this cannot be guaranteed, any "establish
> subscription" RPC request SHOULD be rejected with a "dscp-unavailable"
> error."
> 
> This seems fine to me. The point, as far as I can see, is that a DSCP parameter
> must not be applied to a session (or subscription) that didn't "ask" for it. This is
> enough of a rationale IMHO, no need to go into the details of what the result
> may or may not be as far as reordering of packets goes.

Thanks Per, this works for me.

Eric