Subject: [i2rs] AD review of draft-ietf-i2rs-ephemeral-state-18

From: Alia Atlas <[email-masked]>
Date: Tue, 4 Oct 2016 22:37:04 -0400

Hi,

As is customary, I have done my AD review
of draft-ietf-i2rs-ephemeral-state-18.  First, I would like to thank Sue
and Jeff for their hard work pulling this document together in an effort to
add clarity to the requirements.

I do have a number of comments - many relatively minor.  Assuming a fast
turn-around, as usual from I2RS, we should be able to have this on the Oct
27 telechat - which will mean it needs to enter IETF Last Call before the
end of this week.

Here is my review:

Major:

1) Ephemeral-REQ-12:  This specifies that a notification be sent to the
original client, regardless of whether it won or lost the priority
collision.
I had assumed that the notification would go to either the requesting client
or the original client depending on which one lost the priority comparison.
I have some concerns about an indirect flood of notifications caused by a
requesting client that has the lower priority.  Regardless, clarifying that
the lower-priority client is notified is important.

Minor:
a) Intro: Remove "3 suggest protocol strawman" as something that
   the I2RS requirements must do.  I know that is how the process
   has been working out - but it isn't dictated by the technology
   at all - as the other 2 are.  Similarly, replace the following
   paragraph "The purpose of these requirements and the suggested
   protocol strawman is to provide a quick turnaround on creating
   the I2RS protocol." with something like "The purpose of these
   requirements is to ensure clarity during I2RS protocol creation."

b) Section 2:  "The following are ten requirements that [RFC7921]
   contains which provide context for the ephemeral data state
   requirements given in sections 3-8:"
      How about "The following are requirements distilled from [RFC7921]
     that provide context for..."

    1)  Not relevant for ephemeral - this matters for pub/sub (suggest
removal)
    2)  Relevant for ephemeral b/c of vague performance requirements on
        possible solutions.
    3)  What changes if the data model is protocol dependent?  Is this just
that
        the model may be an augmentation/extension of an existing module?
    4)  Absolutely - keep
    5)  Absolutely - keep
    6)  Remove - not relevant for ephemeral just security requirements
    7)  Remove - not relevant for ephemeral just security requirements
    8)  Absolutely - keep (but says storing secondary identity on deletion
when
        that isn't mentioned for (4) b/c it's about priority - so clarify
slightly)
    9)  Absolutely - keep
   10)  Remove - not relevant for ephemeral

c) Sec 3.3 bullet 2:  This refers to YANG data model instead of YANG module
as
   in bullet 1.  If there's a reason for the difference, please clarify and
otherwise
   make consistent.

d) Sec 5 & 6 for NETCONF and RESTCONF are the same requirements.  Please
consolidate into a section of "The changes to NETCONF and the conceptual
changes to RESTCONF are"

e) Sec 8:  I found this pull-out unclear.  "multiple operations in one
      or more messages; though errors in
      message or operation will have no effect on other messages or
      commands even they are related."

     I think you mean "Multiple operations in one message can be sent.
However
     an error in one operation MUST NOT stop additional operations from
being
     carried out nor can it cause previous operations in the same message to
     be rolled back."

Nits:

i) Abstract: "attempting to meet I2RS needs has to provide"/
"attempting to meet the needs of I2RS has to provide"

ii) 3.2: "MPLS LSP-ID or BGP IN-RIB"  please expand acronyms

iii) Sec 5 last sentence:  Either missing a ( or has an unneeded ).

iv) Ephemeral-REQ-11:  "I2RS Protocol I2RS Protocol" repeated

---

From: "Joel M. Halpern" <[email-masked]>
Date: Wed, 5 Oct 2016 03:25:37 -0400

We probably should tweak the wording on REQ-12.  The notification is 
only needed when the new operation succeeds.
When the new operation fails, the requester will receive an error, and 
the original state is still there, so no notification is needed.  I 
should have realized that in my earlier review.

Suggested fix, add text at left margin:
    Ephemeral-REQ-12: When a collision occurs as two clients are trying
    to write the same data node, this collision is considered an error
    and priorities were created to give a deterministic result.  When
    there is a collision,
and the data node is changed,
       a notification (which includes indicating data
    node the collision occurred on) MUST BE sent to the original client
    to give the original client a chance to deal with the issues
    surrounding the collision.  The original client may need to fix their
    state.

Yours,
Joel

On 10/4/16 10:37 PM, Alia Atlas wrote:
> and Jeff for their hard work pulling this document together in an effort
> to add clarity to the requirements.
>
> turn-around, as usual from I2RS, we should be able to have this on the
> Oct 27 telechat - which will mean it needs to enter IETF Last Call
> before the end of this week.
>
> Here is my review:
>
> Major:
>
>     3)  What changes if the data model is protocol dependent?  Is this
> just that
>     8)  Absolutely - keep (but says storing secondary identity on
> deletion when
>         that isn't mentioned for (4) b/c it's about priority - so
> clarify slightly)
>     9)  Absolutely - keep
> c) Sec 3.3 bullet 2:  This refers to YANG data model instead of YANG
> module as
>    in bullet 1.  If there's a reason for the difference, please clarify
> and otherwise
>    make consistent.
>
> _______________________________________________
> i2rs mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/i2rs
>

---

From: Alia Atlas <[email-masked]>
Date: Wed, 5 Oct 2016 08:53:00 -0400

LGTM

On Oct 5, 2016 3:25 AM, "Joel M. Halpern" <[email-masked]> wrote:

> We probably should tweak the wording on REQ-12.  The notification is only
> needed when the new operation succeeds.
> When the new operation fails, the requester will receive an error, and the
> original state is still there, so no notification is needed.  I should have
> realized that in my earlier review.
>

---

From: "Susan Hares" <[email-masked]>
Date: Wed, 5 Oct 2016 09:19:57 -0400

Alia:

I’ve updated version 19 with the changes. The only change I did not implement was to combine section 5 and 6.   The NETCONF group asked us not to combine these two sections.  I left these two sections intact.   Does this work for you?  

Sue 

From: Alia Atlas [mailto:[email-masked]] 
Sent: Tuesday, October 4, 2016 10:37 PM
To: [email-masked]; [email-masked]
Subject: AD review of draft-ietf-i2rs-ephemeral-state-18

Hi,

As is customary, I have done my AD review of draft-ietf-i2rs-ephemeral-state-18.  First, I would like to thank Sue and Jeff for their hard work pulling this document together in an effort to add clarity to the requirements.

I do have a number of comments - many relatively minor.  Assuming a fast turn-around, as usual from I2RS, we should be able to have this on the Oct 27 telechat - which will mean it needs to enter IETF Last Call before the end of this week.

Here is my review:

Major:

original client, regardless of whether it won or lost the priority collision.

 

Minor:

    1)  Not relevant for ephemeral - this matters for pub/sub (suggest removal)

        possible solutions.

    3)  What changes if the data model is protocol dependent?  Is this just that

    4)  Absolutely - keep

    5)  Absolutely - keep

    8)  Absolutely - keep (but says storing secondary identity on deletion when

        that isn't mentioned for (4) b/c it's about priority - so clarify slightly)

    9)  Absolutely - keep

c) Sec 3.3 bullet 2:  This refers to YANG data model instead of YANG module as

   in bullet 1.  If there's a reason for the difference, please clarify and otherwise

   make consistent.

consolidate into a section of "The changes to NETCONF and the conceptual changes to RESTCONF are"

     I think you mean "Multiple operations in one message can be sent.  However

     an error in one operation MUST NOT stop additional operations from being

     be rolled back."

Nits:

---

From: "Susan Hares" <[email-masked]>
Date: Wed, 5 Oct 2016 09:30:13 -0400

Joel: 

WFM - see inclusion in version 19. 

Sue 

-----Original Message-----
From: Joel M. Halpern [mailto:[email-masked]] 
Sent: Wednesday, October 5, 2016 3:26 AM
To: Alia Atlas; [email-masked]; [email-masked]
Subject: Re: [i2rs] AD review of draft-ietf-i2rs-ephemeral-state-18 - REQ-12

Yours,
Joel

On 10/4/16 10:37 PM, Alia Atlas wrote:
> Hi,
>
> As is customary, I have done my AD review of 
> draft-ietf-i2rs-ephemeral-state-18.  First, I would like to thank Sue 
> and Jeff for their hard work pulling this document together in an 
> effort to add clarity to the requirements.
>
> I do have a number of comments - many relatively minor.  Assuming a 
> fast turn-around, as usual from I2RS, we should be able to have this 
> on the Oct 27 telechat - which will mean it needs to enter IETF Last 
> Call before the end of this week.
>
> Here is my review:
>
> Major:
>
> 1) Ephemeral-REQ-12:  This specifies that a notification be sent to 
> the original client, regardless of whether it won or lost the priority 
> collision.
> I had assumed that the notification would go to either the requesting 
> client or the original client depending on which one lost the priority
comparison.
> I have some concerns about an indirect flood of notifications caused 
> by a requesting client that has the lower priority.  Regardless, 
> clarifying that the lower-priority client is notified is important.
>
>
>
> Minor:
> d) Sec 5 & 6 for NETCONF and RESTCONF are the same requirements.  
> Please consolidate into a section of "The changes to NETCONF and the 
> conceptual changes to RESTCONF are"
>

---

From: Alia Atlas <[email-masked]>
Date: Wed, 5 Oct 2016 10:14:06 -0400

Sue,

This looks good - thanks.  I will put it into IETF Last Call.

Regards,
Alia

On Wed, Oct 5, 2016 at 9:19 AM, Susan Hares <[email-masked]> wrote:

> Alia:
>
>
>
> I’ve updated version 19 with the changes. The only change I did not
> implement was to combine section 5 and 6.   The NETCONF group asked us not
> to combine these two sections.  I left these two sections intact.   Does
> this work for you?
>
>
>
>
>
> Sue
>
>
>
> *From:* Alia Atlas [mailto:[email-masked]]
> *Sent:* Tuesday, October 4, 2016 10:37 PM
> *To:* [email-masked]; [email-masked]
> *Subject:* AD review of draft-ietf-i2rs-ephemeral-state-18
>
>
>
> Hi,
>
>
>
> As is customary, I have done my AD review of draft-ietf-i2rs-ephemeral-state-18.
> First, I would like to thank Sue and Jeff for their hard work pulling this
> document together in an effort to add clarity to the requirements.
>
>
>
> client

---

From: Joe Clarke <[email-masked]>
Date: Wed, 5 Oct 2016 10:51:43 -0400

> WFM - see inclusion in version 19.

Agreed.  And I agree with Alia's comment.  The overwriting client should 
just succeed.  Why let them know (again) that it worked via a notification.

Joe