Re: [sipcore] Review of draft-ietf-sipcore-keep-05

"WORLEY, Dale R (Dale)" <dworley@avaya.com> Tue, 10 August 2010 01:22 UTC

Return-Path: <dworley@avaya.com>
X-Original-To: sipcore@core3.amsl.com
Delivered-To: sipcore@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 20BCC3A6894 for <sipcore@core3.amsl.com>; Mon, 9 Aug 2010 18:22:31 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102.376
X-Spam-Level:
X-Spam-Status: No, score=-102.376 tagged_above=-999 required=5 tests=[AWL=0.223, BAYES_00=-2.599, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bXC9kWgwFkHh for <sipcore@core3.amsl.com>; Mon, 9 Aug 2010 18:22:29 -0700 (PDT)
Received: from p-us1-iereast-outbound-tmp.us1.avaya.com (p-us1-iereast-outbound-tmp.us1.avaya.com [135.11.29.16]) by core3.amsl.com (Postfix) with ESMTP id 601CF3A68E6 for <sipcore@ietf.org>; Mon, 9 Aug 2010 18:22:29 -0700 (PDT)
X-IronPort-AV: E=Sophos;i="4.55,345,1278302400"; d="scan'208";a="29023060"
Received: from unknown (HELO p-us1-erheast.us1.avaya.com) ([135.11.50.53]) by p-us1-iereast-outbound-tmp.us1.avaya.com with ESMTP; 09 Aug 2010 21:23:03 -0400
X-IronPort-AV: E=Sophos;i="4.55,345,1278302400"; d="scan'208";a="491554537"
Received: from dc-us1hcex2.us1.avaya.com (HELO DC-US1HCEX2.global.avaya.com) ([135.11.52.21]) by p-us1-erheast-out.us1.avaya.com with ESMTP; 09 Aug 2010 21:22:28 -0400
Received: from DC-US1MBEX4.global.avaya.com ([169.254.1.161]) by DC-US1HCEX2.global.avaya.com ([::1]) with mapi; Mon, 9 Aug 2010 21:22:28 -0400
From: "WORLEY, Dale R (Dale)" <dworley@avaya.com>
To: Christer Holmberg <christer.holmberg@ericsson.com>, "sipcore@ietf.org" <sipcore@ietf.org>
Date: Mon, 09 Aug 2010 21:22:27 -0400
Thread-Topic: Review of draft-ietf-sipcore-keep-05
Thread-Index: AQHLNxQfbKRBGdvoDEWm/nBr9CW465LX0d7FgAIKXi0=
Message-ID: <CD5674C3CD99574EBA7432465FC13C1B21FE98EF90@DC-US1MBEX4.global.avaya.com>
References: <CD5674C3CD99574EBA7432465FC13C1B21FE98EF86@DC-US1MBEX4.global.avaya.com>, <7F2072F1E0DE894DA4B517B93C6A05850CB1D6@ESESSCMS0356.eemea.ericsson.se>
In-Reply-To: <7F2072F1E0DE894DA4B517B93C6A05850CB1D6@ESESSCMS0356.eemea.ericsson.se>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [sipcore] Review of draft-ietf-sipcore-keep-05
X-BeenThere: sipcore@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: SIP Core Working Group <sipcore.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/sipcore>, <mailto:sipcore-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/sipcore>
List-Post: <mailto:sipcore@ietf.org>
List-Help: <mailto:sipcore-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/sipcore>, <mailto:sipcore-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 10 Aug 2010 01:22:31 -0000

Only the items for which I have further comment:

> From: Christer Holmberg [christer.holmberg@ericsson.com]

One general point that might be worth adding to section 1 is, "This
specification is an extension of SIP Outbound and is to be read based
on [RFC5626]."  A considerable number of my comments and confusions
would have been eliminated had I understood that this draft can only
be effectively read given an understanding of 5626.

> >Section 3
> >
> >"Edge proxy"
> >
> >It's not clear that "edge proxy" has any normative significance in
> >this I-D, as this mechanism can be used between any two entities.
> >It seems that this definition and the accompanying Note could be
> >removed at no loss.  (The definition needs revision, also.)
> 
> Section 5 talks about the edge proxy, where it describes that the
> usage of Flow-Timer is only defined between a UA and an edge proxy,
> so I think it is useful to have the definition.

OK, I can see the usefulness of that.

   Edge proxy: As defined in [RFC5626], a SIP proxy that is located
   topologically between the registering User Agent (UA) and the
   Authoritative Proxy.

One change that I think would have helped me avoid confusion would be
to change "a SIP proxy" to "any SIP proxy".  Because naively I think
of "edge proxy" as meaning only the one proxy closest to the UA; but
in Outbound, it refers to any proxy between the Authoritative Proxy
and the UA, there can be many of them for a single UA.  The phrase
"any SIP proxy" warns me that there might be more than one in a
particular UA/Registrar connection.  OTOH, if we warn the reader to
read RFC 5626 first, that probably doesn't matter.

> I guess I could also add the following sentence to section 1:
> 
>         "In addition, [RFC5626] only allows keep-alives to be
>         negotiated between a UA and an edge proxy, and not between other
>         SIP entities."

That certainly is a significant restriction on Outbound.

> >'"keep" parameter'
> >
> >Strictly speaking, this entry is a description not a definition; the
> >phrase '"keep" parameter' identifies the parameter unambiguously.  It
> >would be better to remove this entry and add its information to
> >section 4.1, which needs to be a bit more explicit about the
> >mechanism.  (See below.)
> 
> The definition text used to be even longer, but parts were
> removed. However, the terminology is used in the documents, so I
> think it is good to have it in the definitions, instead of writing
> "the Via header field "keep" parameter" everywhere.

OK, now I see what the point of the definition is.  In that case, you
can say:

   "keep" parameter: The "keep" header field parameter of the topmost
   Via header of a SIP message.

That defines the phrase exactly; it's shorter (which people seem to
want); and it removes the text which describes how the parameter is
used and leaves only the definition itself.

> >   A SIP entity indicates willingness to send keep-alives towards the
> >   adjacent downstream SIP entity using SIP requests by applying a
> >   "keep" header parameter to the topmost Via (which is the Via that
> >   it inserts).  The associated response is used by the adjacent
> >   downstream SIP entity to indicate willingness to receive
> >  keep-alives by applying to that topmost Via a "keep" header
> >   parameter with an integer value.  The integer value, if not zero,
> >   indicates the keep-alive frequency that is recommended by the
> >   adjacent downstream SIP entity.
> >
> >That gives the outline of the mechanism in one paragraph.
> 
> During WGLC, there were comments/discussions about removing text,
> and making it shorter and easier to read. I don't want to start
> adding text again at this point, unless something is unclear or
> wrong.
> 
> Also, in this case, section 4.1 is a general overview section. The
> details on how to indicate willingness to send and receive
> keep-alives are described in sections 4.3. and 4.4, so there is
> no need to repeat the same text in 4.1.

The current text was unclear to me in that while it said that
keepalives were negotiated via requests and responses it gave no hint
of *how*.  If brevity is important, I would mention the specific use
of "keep" in the request and response, and leave the discussion of
frequency to the more detailed sections.

> I would not like to start adding normative text on how and when
> entities shall use the mechanism, because it might depend on the
> entity types, and the environments where they are deployed.
> 
> However, I guess we could add some general guideline text:
> 
>         "In general, it can be useful for SIP entities to indicate
>         willingness to send keep-alives, even if they are not aware
>         of any necessity for them to send keep-alives, since the
>         adjacent downstream SIP entity might have knowledge about
>         the necessity. Similarly, it can be useful for SIP entities
>         to indicate willingness to receive keep-alives, even if they
>         are not aware of any necessity for the adjacent upstream SIP
>         entity to send them."

I would add a final clause to the second sentence like the final
clause of the first sentence, "... since the adjacent upstream SIP
entity might have knowledge about the necessity."  I agree that this
should not be normative, but it seems to me to be something that the
reader should be warned about, since it's not at all obvious at first
reading that both devices should negotiate the possibility of
keep-alives even if nether knows that they are needed.

> The text refers to RFC 5626 regarding the STUN and CRLF keep-alive
> messages, and RFC 5626 defines when each is used.
>
> Also, remember that the purpose of the daft is not to change the
> keep-alive mechanism in RFC 5626, only to provide a mechanism to
> explicitly negotiate the usage of it, so we should not repeat
> mechanism text that is already defined in RFC 5626.
> 
> BUT, in order to try to make everyone happy, I could add the following sentence to the first section of 4.3:
> 
>         "[RFC5626] defines when to use STUN, respectively CRLF, for keep-alives."

This is an example of where this draft is really an extension of RFC
5626, which is something you understand implicitly and I didn't know.
I think that confusion can be cured by the note in section 1 that I
mention above.

> Maybe we could simply say:
> 
>         "This specification does not specify actions to take if
>         negotiated keep-alives are not received. As defined in
>         [RFC5626], the receiving SIP entity may consider a
>         connection to be dead in such situations."

Again, this draft is just a way of activating the mechanism defined in 5626.

> My understanding of 5626 is that only an edge proxy can add the
> Flow-Timer header field, since Outbound only defines the usage of
> keep-alives between the UA and edge proxy.
> 
> Outbound itself wouldn't work if multiple entities were allowed to
> insert Flow-Timer.

The critical thing is that since only an edge proxy, and only a
*first* edge proxy, can add a Flow-Timer header, you will never have
an entity adding "keep=n" to a response which *already* has a
Flow-Timer header, although it might add both "keep=n" and Flow-Timer
to a response.  If one hasn't read 5626, this isn't obvious.

> The mechanism allows proxy-to-proxy keep-alives, and there also used
> to be an example (or, at least a use-case description) about
> proxy-to-proxy. However, it was removed, because people confused it
> with proxy-to-proxy heartbeats, which is a different thing than
> keep-alives.

Yes, it probably isn't worth the confusion to describe the
proxy-to-proxy case, which will probably be uncommon.

Dale