Re: [Dime] WGLC #1 for draft-ietf-dime-agent-overload-05

Steve Donovan <srdonovan@usdonovans.com> Mon, 06 June 2016 01:58 UTC

Return-Path: <srdonovan@usdonovans.com>
X-Original-To: dime@ietfa.amsl.com
Delivered-To: dime@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 68F5012D0C7 for <dime@ietfa.amsl.com>; Sun, 5 Jun 2016 18:58:42 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.12
X-Spam-Level:
X-Spam-Status: No, score=-1.12 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_NEUTRAL=0.779] autolearn=no autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3kg-JfROKXfF for <dime@ietfa.amsl.com>; Sun, 5 Jun 2016 18:58:41 -0700 (PDT)
Received: from biz131.inmotionhosting.com (biz131.inmotionhosting.com [74.124.197.190]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 0228312B01B for <dime@ietf.org>; Sun, 5 Jun 2016 18:58:40 -0700 (PDT)
Received: from cpe-97-99-50-102.tx.res.rr.com ([97.99.50.102]:52827 helo=Steves-MacBook-Air.local) by biz131.inmotionhosting.com with esmtpsa (TLSv1.2:DHE-RSA-AES256-SHA:256) (Exim 4.86_1) (envelope-from <srdonovan@usdonovans.com>) id 1b9joH-000RPE-KG; Sun, 05 Jun 2016 18:58:39 -0700
From: Steve Donovan <srdonovan@usdonovans.com>
To: "A. Jean Mahoney" <mahoney@nostrum.com>, jouni.nospam@gmail.com, "dime@ietf.org" <dime@ietf.org>, "Lionel.morand@orange.com" <Lionel.morand@orange.com>
References: <5bba2470-8921-f7db-0f1b-aad280eae684@gmail.com> <9cf5ab38-2e42-c747-98ba-52b61aa959f8@nostrum.com>
Message-ID: <3c01a953-9308-f48a-2ecf-e450c46a3320@usdonovans.com>
Date: Sun, 5 Jun 2016 20:58:34 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.1.1
MIME-Version: 1.0
In-Reply-To: <9cf5ab38-2e42-c747-98ba-52b61aa959f8@nostrum.com>
Content-Type: multipart/alternative; boundary="------------9FD7FABFB57BE571E54798FF"
X-OutGoing-Spam-Status: No, score=-1.0
X-AntiAbuse: This header was added to track abuse, please include it with any abuse report
X-AntiAbuse: Primary Hostname - biz131.inmotionhosting.com
X-AntiAbuse: Original Domain - ietf.org
X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12]
X-AntiAbuse: Sender Address Domain - usdonovans.com
X-Get-Message-Sender-Via: biz131.inmotionhosting.com: authenticated_id: srdonovan@usdonovans.com
X-Authenticated-Sender: biz131.inmotionhosting.com: srdonovan@usdonovans.com
X-Source:
X-Source-Args:
X-Source-Dir:
Archived-At: <https://mailarchive.ietf.org/arch/msg/dime/WzlEUsbdwwDBw9SVoYOLTequcKI>
Subject: Re: [Dime] WGLC #1 for draft-ietf-dime-agent-overload-05
X-BeenThere: dime@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Diameter Maintanence and Extentions Working Group <dime.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/dime>, <mailto:dime-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/dime/>
List-Post: <mailto:dime@ietf.org>
List-Help: <mailto:dime-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/dime>, <mailto:dime-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 06 Jun 2016 01:58:42 -0000

Jean,

Thanks for the review.

Please find my comments inline.

Regards,

Steve

On 6/1/16 6:11 PM, A. Jean Mahoney wrote:
> Hi Steve,
>
> I think the draft is in pretty good shape, but I have a few comments 
> (mostly nits) and I did get confused in Section 5.1.2 --
>
> Minor issues:
>
> Section 5.1.1
>
>   "When relaying a request that includes a SourceID AVP in the OC-
>    Supported-Features AVP, a DOIC node that supuports the OC_PEER_REPORT
>    feature must remove the received SourceID AVP and replace it with a
>    SourceID AVP containing its own Diameter identity."
>
> Shouldn't "must" be "MUST"?
SRD> Yes.  I've made the change.
>
>
> Section 5.1.2
>
>   "When relaying an answer message, a reporting node that supports the
>    OC_PEER_REPORT feature MUST strip any SourceID AVP from the OC-
>    Supported-Features AVP."
>
> And replace it with a SourceID AVP containing its own Diameter 
> identity? Or does an answer with an OC-Supported-Features AVP that 
> does not have a SourceID AVP mean the peer is an agent? Section 5 
> doesn't cover how to interpret an OC-Supported-Features AVP without a 
> SourceID, only when a SourceID doesn't match the peer.
SRD> The next two paragraphs address when the SourceID is included.  
Maybe I need to change the next paragraph to the following:

From:

    When sending an answer message, a reporting node that supports the
    OC_PEER_REPORT feature MUST determine if the peer to which the answer
    is to be sent supports the OC_PEER_REPORT feature.

To:

    When sending *or relaying* an answer message, a reporting node that 
supports the
    OC_PEER_REPORT feature MUST determine if the peer to which the answer
    is to be sent supports the OC_PEER_REPORT feature.
>
>
> Section 5.1.2
>
>   "If the peer supports the OC_PEER_REPORT feature then the reporting
>    node MUST indicate support for the feature in the Supported-Features
>    AVP."
>
> Should be "OC-Supported-Features".
SRD> Agreed.
>
>
> Section 6.1
>
> Should the AVP Code for OC-Supported-Features be 621 instead of TBD1? 
> (I wasn't sure if, when extending an AVP of type Grouped, you needed 
> to give the AVP a new AVP Code, and I couldn't find guidance in RFC 
> 6733 or RFC 7423) Also, AVP Code TBD1 is used for OC-Peer-Algo AVP in 
> Section 6.1.2 and for SourceID in Section 6.4. TBD2 is used for OC-OLR 
> in 6.2 (should it be 623?), SourceID in 6.3, and OC-Peer-Algo in 6.4.
SRD> Yes, good catch.  This was a cut and paste from before the DOIC AVP 
codes were assigned.  I've cleaned it up to have TBD1 apply to 
OC-Peer-Algo and TBD2 apply to SourceID.
>
>
>
> Section 6.2
>
>   "The overload report must also include the Diameter identity of the
>    agent that generated the report."
>
> Shouldn't "must" be "MUST"?
SRD> Yes.
>
>
> Nits:
>
> Section 3 - s/"suited controlling traffic"/"suited to controlling 
> traffic"
SRD> This was fixed when rewording this section based on Maria Cruz's 
comments.
>
> Section 5.2.4 - s/"reporting nodes transaction state"/"reporting 
> node's transaction state"
>
> Section 6.1 - s/"handling by the agents peer."/"handling by the 
> agent's peer."
>
> Section 6.1.2 - s/"reused in for this AVP."/"reused for this AVP."
>
> Section 6.2 - s/non supporting/non-supporting
SRD> Changes made for the above.
>
>
> Thanks!
>
> Jean
>
>
> On 5/23/16 2:12 PM, Jouni Korhonen wrote:
>> Folks,
>>
>> This email starts the WGLC #1 for draft-ietf-dime-agent-overload-05.
>> Please, review the document, post your comments to the mailing list and
>> also insert them into the Issue Tracker with your proposed resolution.
>>
>> WGLC starts: 5/23/2016
>>        ends: 6/6/2016 EOB PDT
>>
>> - Jouni & Lionel
>>
>> _______________________________________________
>> DiME mailing list
>> DiME@ietf.org
>> https://www.ietf.org/mailman/listinfo/dime