Re: [i2rs] AD review of draft-ietf-i2rs-ephemeral-state-18 - REQ-12

Alia Atlas <akatlas@gmail.com> Wed, 05 October 2016 12:53 UTC

Return-Path: <akatlas@gmail.com>
X-Original-To: i2rs@ietfa.amsl.com
Delivered-To: i2rs@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 3382B1296E8; Wed, 5 Oct 2016 05:53:05 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102.699
X-Spam-Level:
X-Spam-Status: No, score=-102.699 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, USER_IN_WHITELIST=-100] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wdGmSsv3ZiQp; Wed, 5 Oct 2016 05:53:02 -0700 (PDT)
Received: from mail-yw0-x22d.google.com (mail-yw0-x22d.google.com [IPv6:2607:f8b0:4002:c05::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 5B6511296D8; Wed, 5 Oct 2016 05:53:02 -0700 (PDT)
Received: by mail-yw0-x22d.google.com with SMTP id t193so78800645ywc.2; Wed, 05 Oct 2016 05:53:02 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=gfdNVwj59YNeKmNmAWKL3thu9l3SJevybPsCfug6QAM=; b=0jq7Lm6GCo4yQTo/5nHkm4hmfZe/J4+4T0bud3ujWwfpWP9J9Xvf2XC0ylFfco/wn7 0W+0UsNaEax+tx7qvqhug9TTjx4ojVKjbtJKhbRyxEtCHk63erj5nLH1Cy4b2PUoJnFb h7XA6bKJOMjF7u3k00cH274fu7iBWQWU73ZeS4GrQQBCBkqyvv3X+RHd6nGfM37HjgVN uHX5BtCwzbldKk8RlPhsik0NFpL/pEVXLfOvLrfgMc9Kwn9hGC3ALLEf+cvgbz3kuX2+ i67kx5zgrOFfgMk57RmfNSCAX4efQzMSj5EVEWUO71bsFuZsv1EfzMggVE6xEELKvod8 Oo5w==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=gfdNVwj59YNeKmNmAWKL3thu9l3SJevybPsCfug6QAM=; b=QKLNUnaR1G/DQ2GTZn6nIzTHlBgee7rb9dzS+AFzHB8VrqJLB69kEE1z9KYYUbnWRV aMx3a58gHDPTiyKZI/hvTNSSVAu4z128aMO7DI5P343N2rPOSCu4KMH6wk/QvHeN+Tkm d6M5pde8ojwgRwBvt+LcfRZDAWQFZWN1EKas91tDpeLGNrJvCqx+8q2Sk0k4JXTdCeeO E6/fZ5LDufIB3OtVg3Kzyku75kF0CYY0qcqyZinGJw7nDIMz78iPJevyAAydalKzSeZy SF3LjZSCbB+y2lcZ217vYy/9jdDGsf72cgSYgnepHjxwbdvRUJ/fvqD+J92S0B9V0QuU Qp5Q==
X-Gm-Message-State: AA6/9RmlbYT69T2wdNq7U9Jct3P1Hy0jSOmDTC877C52d8A307d3m79wayxCjgwO3AMjL9TlF0mvKDH8bvogJg==
X-Received: by 10.37.196.197 with SMTP id u188mr6763887ybf.19.1475671981560; Wed, 05 Oct 2016 05:53:01 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.129.56.133 with HTTP; Wed, 5 Oct 2016 05:53:00 -0700 (PDT)
Received: by 10.129.56.133 with HTTP; Wed, 5 Oct 2016 05:53:00 -0700 (PDT)
In-Reply-To: <f931ee98-583a-d67a-02e7-66a5e1681e1b@joelhalpern.com>
References: <CAG4d1rccNuy1OuUHkhQok=jrnVnqR06TmBR5sV6OoqxaWMj31Q@mail.gmail.com> <f931ee98-583a-d67a-02e7-66a5e1681e1b@joelhalpern.com>
From: Alia Atlas <akatlas@gmail.com>
Date: Wed, 5 Oct 2016 08:53:00 -0400
Message-ID: <CAG4d1rdAO-BPDvLMGa0eBv7121muPhgwfE9vTo8Nu9bEkf8Mbg@mail.gmail.com>
To: "Joel M. Halpern" <jmh@joelhalpern.com>
Content-Type: multipart/alternative; boundary=94eb2c054820441953053e1da844
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2rs/Ze62d45amKuch3xc59F5Hx_cGjQ>
Cc: i2rs@ietf.org, draft-ietf-i2rs-ephemeral-state@ietf.org
Subject: Re: [i2rs] AD review of draft-ietf-i2rs-ephemeral-state-18 - REQ-12
X-BeenThere: i2rs@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: "Interface to The Internet Routing System \(IRS\)" <i2rs.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2rs>, <mailto:i2rs-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2rs/>
List-Post: <mailto:i2rs@ietf.org>
List-Help: <mailto:i2rs-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2rs>, <mailto:i2rs-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 05 Oct 2016 12:53:05 -0000

LGTM

On Oct 5, 2016 3:25 AM, "Joel M. Halpern" <jmh@joelhalpern.com> 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.
>
> 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:
>
>> 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
>>
>>
>>
>> _______________________________________________
>> i2rs mailing list
>> i2rs@ietf.org
>> https://www.ietf.org/mailman/listinfo/i2rs
>>
>>