Subject: Re: [netconf] RFC 6241 Ambiguity

From: Andy Bierman <[email-masked]>
Date: Mon, 3 Jun 2019 08:24:03 -0700

On Fri, May 31, 2019 at 3:19 PM Mahesh Jethanandani <[email-masked]>
wrote:

>
> Several attempts have been made to clarify the role of confirmed commit in
> RFC 6241 vis-a-vis Section 7.8 <close-session> and Section 7.9
> <kill-session>, and the text in Section 8.4 Confirmed Commit Capability.
>
> We, (the chairs) believe that the best way to resolve issues with the
> current set of erratum that already exist or are being proposed is to
> minimize re-explaining the role of confirmed commit in both Section 7.8 and
> 7.9 and defer the explanation to Section 8.4 of the RFC. With that in mind,
> we are proposing that Section 7.8 and 7.9 should ultimately look as
> follows. Note, the highlights in all the sections are to enable identifying
> the changes.
>
>
> OLD (as in original RFC 6241)
>
> *7.8* <https://tools.ietf.org/html/rfc6241#section-7.8>*.
> <close-session>*
>
>    Description:  Request graceful termination of a NETCONF session.
>
>       When a NETCONF server receives a <close-session> request, it will
>       gracefully close the session.  The server will release any locks
>       and resources associated with the session and gracefully close any
>       associated connections.  Any NETCONF requests received after a
>       <close-session> request will be ignored.
>
>    Positive Response:  If the device was able to satisfy the request, an
>       <rpc-reply> is sent that includes an <ok> element.
>
>    Negative Response:  An <rpc-error> element is included in the
>       <rpc-reply> if the request cannot be completed for any reason.
>
>    Example:
>
>      <rpc message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <close-session/>
>      </rpc>
>
>      <rpc-reply message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <ok/>
>      </rpc-reply>
>
> NEW
>
> *7.8* <https://tools.ietf.org/html/rfc6241#section-7.8>*.
> <close-session>*
>
>    Description:  Request graceful termination of a NETCONF session.
>
>       When a NETCONF server receives a <close-session> request, it will
>       gracefully close the session.  The server will release any locks
>       and resources associated with the session and gracefully close any
>       associated connections.  Any NETCONF requests received after a
>       <close-session> request will be ignored.
>
>       For details on what happens if a NETCONF server receives a
>       <close-session> request while processing a confirmed commit,
>       please refer to Section 8.4
> <https://tools.ietf.org/html/rfc6241#section-8.4>.
>
>    Positive Response:  If the device was able to satisfy the request, an
>       <rpc-reply> is sent that includes an <ok> element.
>
>    Negative Response:  An <rpc-error> element is included in the
>       <rpc-reply> if the request cannot be completed for any reason.
>
>    Example:
>
>      <rpc message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <close-session/>
>      </rpc>
>
>      <rpc-reply message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <ok/>
>      </rpc-reply>
>
> OLD (as in original RFC 6241).
>
> *7.9* <https://tools.ietf.org/html/rfc6241#section-7.9>*.  <kill-session>*
>
>    Description:  Force the termination of a NETCONF session.
>
>       When a NETCONF entity receives a <kill-session> request for an
>       open session, it will abort any operations currently in process,
>       release any locks and resources associated with the session, and
>       close any associated connections.
>
>       If a NETCONF server receives a <kill-session> request while
>       processing a confirmed commit (Section 8.4
> <https://tools.ietf.org/html/rfc6241#section-8.4>), it MUST restore the
>       configuration to its state before the confirmed commit was issued.
>
>       Otherwise, the <kill-session> operation does not roll back
>       configuration or other device state modifications made by the
>       entity holding the lock.
>
>    Parameters:
>
>       session-id:  Session identifier of the NETCONF session to be
>          terminated.  If this value is equal to the current session ID,
>          an "invalid-value" error is returned.
>
>    Positive Response:  If the device was able to satisfy the request, an
>       <rpc-reply> is sent that includes an <ok> element.
>
>    Negative Response:  An <rpc-error> element is included in the
>       <rpc-reply> if the request cannot be completed for any reason.
>
>
>    Example:
>
>      <rpc message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <kill-session>
>          <session-id>4</session-id>
>        </kill-session>
>      </rpc>
>
>      <rpc-reply message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <ok/>
>      </rpc-reply>
>
> NEW
>
> *7.9* <https://tools.ietf.org/html/rfc6241#section-7.9>*.  <kill-session>*
>
>    Description:  Force the termination of a NETCONF session.
>
>       When a NETCONF entity receives a <kill-session> request for an
>       open session, it will abort any operations currently in process,
>       release any locks and resources associated with the session, and
>       close any associated connections.
>
>       For details on what happens if a NETCONF server receives a
>       <kill-session> request while processing a confirmed commit,
>       please refer to Section 8.4
> <https://tools.ietf.org/html/rfc6241#section-8.4>.
>
>       Otherwise, the <kill-session> operation does not roll back
>       configuration or other device state modifications made by the
>       entity holding the lock.
>
>    Parameters:
>
>       session-id:  Session identifier of the NETCONF session to be
>          terminated.  If this value is equal to the current session ID,
>          an "invalid-value" error is returned.
>
>    Positive Response:  If the device was able to satisfy the request, an
>       <rpc-reply> is sent that includes an <ok> element.
>
>    Negative Response:  An <rpc-error> element is included in the
>       <rpc-reply> if the request cannot be completed for any reason.
>
>
>    Example:
>
>      <rpc message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <kill-session>
>          <session-id>4</session-id>
>        </kill-session>
>      </rpc>
>
>      <rpc-reply message-id="101"
>           xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>        <ok/>
>      </rpc-reply>
>
>
> In addition, the following paragraph in Section 8.4.1 would be modified.
>
> OLD:
>
>    If the device reboots for any reason before the confirm timeout
>    expires, the server MUST restore the configuration to its state
>    before the confirmed commit was issued.
>
>
> NEW:
>
>    If the device reboots for any reason before the confirm timeout
>    expires, the server MUST restore the configuration to its state
>    before the confirmed commit was issued, unless the confirmed commit
>    also included a <persist> element.
>
>

I do not agree at all that the original text says or implies that a server
MUST support
continuation of a confirmed commit across a reboot.
Is that what this new text is meant to convey?

The new text could mean that if <persist> was provided that the server MAY
or SHOULD restore the configuration
and terminate the confirmed commit procedure.

(There is text in multiple places that assumes the reader knows that
restoring the configuration
also terminates the CC)

Current Erratums for RFC 6241 will be adjusted to accommodate these changes.
>
> Mahesh & Kent (co-chairs)
>
>
>

Andy

> _______________________________________________
> netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf
>

---

From: Mahesh Jethanandani <[email-masked]>
Date: Tue, 4 Jun 2019 12:47:56 -0700

Hi Andy,

Please provide alternative text in the form of OLD/NEW, if you do not agree with what is being proposed.

Thanks

> On Jun 3, 2019, at 8:24 AM, Andy Bierman <[email-masked]> wrote:
> 
> 
> 
> On Fri, May 31, 2019 at 3:19 PM Mahesh Jethanandani <[email-masked] <mailto:[email-masked]>> wrote:
> 
> 
> Several attempts have been made to clarify the role of confirmed commit in RFC 6241 vis-a-vis Section 7.8 <close-session> and Section 7.9 <kill-session>, and the text in Section 8.4 Confirmed Commit Capability.
> 
> We, (the chairs) believe that the best way to resolve issues with the current set of erratum that already exist or are being proposed is to minimize re-explaining the role of confirmed commit in both Section 7.8 and 7.9 and defer the explanation to Section 8.4 of the RFC. With that in mind, we are proposing that Section 7.8 and 7.9 should ultimately look as follows. Note, the highlights in all the sections are to enable identifying the changes.
> 
>  
> 7.8 <https://tools.ietf.org/html/rfc6241#section-7.8>.  <close-session>
> 
> 7.8 <https://tools.ietf.org/html/rfc6241#section-7.8>.  <close-session>
> 
>       please refer to Section 8.4 <https://tools.ietf.org/html/rfc6241#section-8.4>.
> 
> 7.9 <https://tools.ietf.org/html/rfc6241#section-7.9>.  <kill-session>
> 
>       processing a confirmed commit (Section 8.4 <https://tools.ietf.org/html/rfc6241#section-8.4>), it MUST restore the
> 7.9 <https://tools.ietf.org/html/rfc6241#section-7.9>.  <kill-session>
> 
>       please refer to Section 8.4 <https://tools.ietf.org/html/rfc6241#section-8.4>.
> 
> I do not agree at all that the original text says or implies that a server MUST support 
> The new text could mean that if <persist> was provided that the server MAY or SHOULD restore the configuration
> (There is text in multiple places that assumes the reader knows that restoring the configuration
> also terminates the CC)
> 
> 
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/netconf <https://www.ietf.org/mailman/listinfo/netconf>
Mahesh Jethanandani
[email-masked]

---

From: Andy Bierman <[email-masked]>
Date: Tue, 4 Jun 2019 13:16:56 -0700

Hi,

I prefer to leave the old text in 8.4.1 alone and not change it
I don't agree there is any text to support this significant change in
server behavior.
An Errata should not make existing implementations non-compliant.
I don't see any text that indicates the original intent was to make a
confirmed commit
survive a reboot.

If a <persist> parameter is provided by the client session, and then that
client session
terminates, there is no way for a superuser (or any user) to invoke the
<cancel-commit>
operation.  The only remedy is to wait for the timeout or reboot the device.
If the timeout is long or unknown) then waiting it out may not be a good
option.

With the new text, rebooting the device does not clear this situation

Andy

On Tue, Jun 4, 2019 at 12:48 PM Mahesh Jethanandani <[email-masked]>
wrote:

> On Fri, May 31, 2019 at 3:19 PM Mahesh Jethanandani <
> [email-masked]> wrote:
>
>>
>> Several attempts have been made to clarify the role of confirmed commit
>> in RFC 6241 vis-a-vis Section 7.8 <close-session> and Section 7.9

---

From: Kent Watsen <[email-masked]>
Date: Tue, 11 Jun 2019 20:16:58 +0000

Hi Andy,

So, by exclusion, is it fair to say that you support the NEW 7.8 and 7.9 text?

As for the 8.4.1 text, do you agree that there is a disconnect between what's in 8.4.1 and the description for the 'persist' leaf (e.g., current text says "the only way to abort a persistent confirmed commit is to let the timer expire, or to use the <cancel-commit> operation.").  If so, then a clarifying statement is needed, the only question is what it is. The choices are:

  a)  'persist' MUST span reboots.
  b)  'persist' MUST NOT span reboots.
  c) ' it is an implementation decision as to if 'persist' spans reboots.

Personally, I'm okay with any of these as it seems the primary benefit for 'persist' is still reaped (i.e., to survive a client-disconnect that may occur due to the config being committed) and given that most devices do not need to reboot in order to commit a configuration.  

That said, some (IoT?) devices may need to reboot in order to apply any configuration.  If this ability does not support such devices, then they would never be able to support 'commit confirmed' (with or without 'persist') and hence may become bricks in some scenarios.  Do we care?

Kent  // contributor

> On Jun 4, 2019, at 4:16 PM, Andy Bierman <[email-masked]> wrote:
> 
> Hi,
> 
> I don't agree there is any text to support this significant change in server behavior.
> I don't see any text that indicates the original intent was to make a confirmed commit
> survive a reboot. 
> 
> If a <persist> parameter is provided by the client session, and then that client session
> terminates, there is no way for a superuser (or any user) to invoke the <cancel-commit>
> If the timeout is long or unknown) then waiting it out may not be a good option.
> 
> On Tue, Jun 4, 2019 at 12:48 PM Mahesh Jethanandani <[email-masked] <mailto:[email-masked]>> wrote:
> Hi Andy,
> 
>> On Jun 3, 2019, at 8:24 AM, Andy Bierman <[email-masked] <mailto:[email-masked]>> wrote:
>> 
>> 
>> 
> [email-masked] <mailto:[email-masked]>
> 
> 
>

---

From: Andy Bierman <[email-masked]>
Date: Tue, 11 Jun 2019 14:19:05 -0700

> So, by exclusion, is it fair to say that you support the NEW 7.8 and 7.9
They defer to section 8.4 which fine

> As for the 8.4.1 text, do you agree that there is a disconnect between
> what's in 8.4.1 and the description for the 'persist' leaf (e.g., current
> text says "the only way to abort a persistent confirmed commit is to let
> the timer expire, or to use the <cancel-commit> operation.").  If so, then
> a clarifying statement is needed, the only question is what it is. The
> choices are:
>
> Personally, I'm okay with any of these as it seems the primary benefit for
> 'persist' is still reaped (i.e., to survive a client-disconnect that may
> occur due to the config being committed) and given that most devices do not
> need to reboot in order to commit a configuration.
>
>
IMO the text most strongly supports (b) [para 6].
It could be argued that [para 5] supports (a) because a server reboot does
cause a session
termination, but IMO this was not the intent of para 5. It suggests the
server is still running
but the client session is terminated

[para 5]

   If the session issuing the confirmed commit is terminated for any
   reason before the confirm timeout expires, the server MUST restore
   the configuration to its state before the confirmed commit was
   issued, unless the confirmed commit also included a <persist>
   element.

[para 6]

I think (c) is OK since it not 100% clear what the scope of "unless"
is in para 5.

That said, some (IoT?) devices may need to reboot in order to apply any
> configuration.  If this ability does not support such devices, then they
> would never be able to support 'commit confirmed' (with or without
> 'persist') and hence may become bricks in some scenarios.  Do we care?
>
> Kent  // contributor
>
>
>
Andy

> On Tue, Jun 4, 2019 at 12:48 PM Mahesh Jethanandani <

---

From: "Jonathan Hansford" <[email-masked]>
Date: Wed, 12 Jun 2019 11:16:05 +0000

On 11/06/2019 22:19:05, "Andy Bierman" <[email-masked]> wrote:

>>what's in 8.4.1 and the description for the 'persist' leaf (e.g., 
>>current text says "the only way to abort a persistent confirmed commit 
>>is to let the timer expire, or to use the <cancel-commit> 
>>operation.").  If so, then a clarifying statement is needed, the only 
>>question is what it is. The choices are:
>>
>>Personally, I'm okay with any of these as it seems the primary benefit 
>>for 'persist' is still reaped (i.e., to survive a client-disconnect 
>>that may occur due to the config being committed) and given that most 
>>devices do not need to reboot in order to commit a configuration.
>
With (c), how does a client know what each server has implemented, and 
how does a server know what each client has implemented? And what if 
your clients and servers have all implemented, say, (a) and then you 
need to add a client or server that has implemented (b)?
>It could be argued that [para 5] supports (a) because a server reboot 
>does cause a session
>I think (c) is OK since it not 100% clear what the scope of "unless" is in para 5.
>
>
>>That said, some (IoT?) devices may need to reboot in order to apply 
>>any configuration.  If this ability does not support such devices, 
>>then they would never be able to support 'commit confirmed' (with or 
>>without 'persist') and hence may become bricks in some scenarios.  Do 
>>we care?
>>
>>Kent  // contributor
>>
>>
>
>Andy
>
>>
>>>On Tue, Jun 4, 2019 at 12:48 PM Mahesh Jethanandani 
>>><[email-masked]> wrote:
>>>>Hi Andy,
>>>>
>>>>>On Fri, May 31, 2019 at 3:19 PM Mahesh Jethanandani 
>>>>><[email-masked]> wrote:
>>>>>>
>>>>>>Several attempts have been made to clarify the role of confirmed 
>>>>>>commit in RFC 6241 vis-a-vis Section 7.8 <close-session> and 
>>>>>>Section 7.9 <kill-session>, and the text in Section 8.4 Confirmed 
>>>>>>Commit Capability.
>>>>>>
>>>>>>We, (the chairs) believe that the best way to resolve issues with 
>>>>>>the current set of erratum that already exist or are being 
>>>>>>proposed is to minimize re-explaining the role of confirmed commit 
>>>>>>in both Section 7.8 and 7..9 and defer the explanation to Section 
>>>>>>8.4 of the RFC. With that in mind, we are proposing that Section 
>>>>>>7.8 and 7.9 should ultimately look as follows.. Note, the 
>>>>>>highlights in all the sections are to enable identifying the 
>>>>>>changes.
>>>>>>
>>>>>>
>>>>>The new text could mean that if <persist> was provided that the 
>>>>>server MAY or SHOULD restore the configuration

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

---

From: Kent Watsen <[email-masked]>
Date: Wed, 12 Jun 2019 13:02:34 +0000

>   c) it is an implementation decision as to if 'persist' spans reboots.
> 
> [...]
> 
> It could be argued that [para 5] supports (a) because a server reboot does cause a session
> termination, but IMO this was not the intent of para 5. It suggests the server is still running
> [...]

If (a) is not possible, and (b) excludes [IoT] devices that must reboot for each commit, then (c) becomes the front runner.

The text could be curt and just state “it is an implementation decision as to if 'persist' spans reboots.”

But it might be helpful to elaborate along the lines of “devices that require a reboot in order to commit SHOULD..., other devices MAY...”. 

As to how a client can determine the server’s behavior, given this is just Errata, it would be inappropriate to define a ‘feature’.   How about:

“This document does not provide a mechanism enabling a client to determine if a server supports the ‘persist’ behavior over reboots, or if a server requires a reboot in order to effect configuration changes.“

???

Kent // contributor

---

From: "Jonathan Hansford" <[email-masked]>
Date: Wed, 12 Jun 2019 13:39:35 +0000

On 12/06/2019 14:02:34, "Kent Watsen" <[email-masked]> wrote:

>If (a) is not possible, and (b) excludes [IoT] devices that must reboot 
>for each commit, then (c) becomes the front runner.
If (a) is not possible, how can it be an implementation decision whether 
'persist' spans reboots? If (a) is not possible, only (b) is left and 
IoT devices that must reboot for each commit are excluded.
>But it might be helpful to elaborate along the lines of “devices that 
>require a reboot in order to commit SHOULD..., other devices MAY...”.
>
>As to how a client can determine the server’s behavior, given this is 
>just Errata, it would be inappropriate to define a ‘feature’.   How 
>about:
>
>“This document does not provide a mechanism enabling a client to 
>determine if a server supports the ‘persist’ behavior over reboots, or 
>if a server requires a reboot in order to effect configuration 
>changes.“
>
>???
>
>Kent // contributor

---

---

From: Andy Bierman <[email-masked]>
Date: Wed, 12 Jun 2019 06:54:25 -0700

On Wed, Jun 12, 2019 at 6:02 AM Kent Watsen <[email-masked]> wrote:

> As to how a client can determine the server’s behavior, given this is just
> Errata, it would be inappropriate to define a ‘feature’.   How about:
>
> “This document does not provide a mechanism enabling a client to determine
> if a server supports the ‘persist’ behavior over reboots, or if a server
> requires a reboot in order to effect configuration changes.“
>
>
>

I object to changing RFC 6241 with an Errata if it attempts to enforce
protocol behavior
that the original RFC does not actually specify.  (c) is correct for an
Errata because the original RFC
is under-specified.  Or perhaps: (d) 'persist' MAY span reboots

Andy

---

From: Kent Watsen <[email-masked]>
Date: Wed, 12 Jun 2019 20:13:25 +0000

Hi Jonathan,

> If (a) is not possible, how can it be an implementation decision whether 'persist' spans reboots?

(a) is not possible only from an errata perspective (not an implementation perspective).

Kent // contributor

---

From: Kent Watsen <[email-masked]>
Date: Wed, 12 Jun 2019 20:16:49 +0000

Hi Andy,

> I object to changing RFC 6241 with an Errata if it attempts to enforce protocol behavior
> that the original RFC does not actually specify.  (c) is correct for an Errata because the original RFC

It sounds like we're in agreement.  Are you objecting to something I wrote in particular?

Kent // contributor

---

From: Andy Bierman <[email-masked]>
Date: Wed, 12 Jun 2019 14:36:45 -0700

Hi,

I think (c) or (d) is OK.
I would object to (a) or (b).
Adding a MUST in an Errata would only be OK if it was unanimous that was
the original intent.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Wed, 12 Jun 2019 14:47:50 -0700

Hi,

This is the new text in question:
It appears your intent is (c) since there is no mention at all what happens
if
<persist> was included.  The text just says what does not happen in this
case.

>>    also included a <persist> element.

How about adding at the end:

OLD:

element.

NEW:

element, in which case the server MAY continue the confirmed commit
procedure.

Andy

---

From: Mahesh Jethanandani <[email-masked]>
Date: Tue, 25 Jun 2019 11:02:47 -0700

Summarizing the list of changes that we seem to have agreed upon:

For the first set of changes in section 7.8 and 7.9, we seem to agree on the original set of changes I had sent out earlier, which essentially deferred the discussion of confirmed commit to section 8.4 of the RFC. The second set of changes updates Section 8.4.1 to explain what happens to a confirmed commit that includes a <persist> element.

The highlights in the sections should enable identifying the changes.

CHANGE 1 START

   Example:

     <rpc message-id="101"

NEW

   Example:

     <rpc message-id="101"

   Parameters:

   Example:

     <rpc message-id="101"

NEW

   Parameters:

   Example:

     <rpc message-id="101"

CHANGE 1 END

CHANGE 2 START
The following paragraph in Section 8.4.1 would be modified.

OLD:

NEW:

   If the device reboots for any reason before the confirm timeout
   expires, the server MUST restore the configuration to its state
   before the confirmed commit was issued, unless the confirmed commit 
   also included a <persist> element, in which case the server MAY continue
   the confirmed commit procedure.

CHANGE 2 END

Finally, looking at the current set of Errata that are already open against the RFC, we (the chairs) believe that Errata number 5397 should be deleted and (a new Errata with these contents) be posted. This is being done to avoid stacking one Errata on top of another.

Unless there is objection to these changes within the next two weeks, we will proceed with submitting them to the RFC Editor.

> On Jun 12, 2019, at 2:47 PM, Andy Bierman <[email-masked]> wrote:
> 
> Hi,
> 
> It appears your intent is (c) since there is no mention at all what happens if
> <persist> was included.  The text just says what does not happen in this case.
> 
>>> 
>> element, in which case the server MAY continue the confirmed commit procedure.
>> 
>> 
>> Andy
>>  
> 
> On Wed, Jun 12, 2019 at 1:16 PM Kent Watsen <[email-masked] <mailto:[email-masked]>> wrote:
> 
> Hi Andy,
> 

Mahesh Jethanandani
[email-masked]

---

From: Andy Bierman <[email-masked]>
Date: Tue, 25 Jun 2019 15:56:58 -0700

Hi,

these changes look OK to me.

Andy

On Tue, Jun 25, 2019 at 11:03 AM Mahesh Jethanandani <
[email-masked]> wrote:

> For the first set of changes in section 7.8 and 7.9, we seem to agree on
> the original set of changes I had sent out earlier, which essentially
> deferred the discussion of confirmed commit to section 8.4 of the RFC. The
> second set of changes updates Section 8.4.1 to explain what happens to a
> confirmed commit that includes a <persist> element.
>
> *CHANGE 1 START*
> *CHANGE 1 END*
> *CHANGE 2 START*
> *CHANGE 2 END*
> Finally, looking at the current set of Errata that are already open
> against the RFC, we (the chairs) believe that Errata number 5397 should be
> deleted and (a new Errata with these contents) be posted. This is being
> done to avoid stacking one Errata on top of another.
>
> Unless there is objection to these changes within the next two weeks, we
> will proceed with submitting them to the RFC Editor.
>