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

From: "WORLEY, Dale R (Dale)" <[email-masked]>
Date: Sun, 8 Aug 2010 12:09:47 -0400

I've not been in on the technical discussion, so I can't comment on
the mechanism design.  But the exposition in the draft could be
improved in a few places.

The running title is "STUN-keep", which is surely not what is intended.

Section 1.1

Uses "session" twice, but I think "dialog" is more correct.  The
session (media) contains the RTP, which will probably keep the NAT
open; the problem is keeping the NAT open for the signaling (dialog).

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.)

"Keep-alives"

It would help if this definition mentioned that both CRLF and STUN
keepalives were intended.  Also, the phrase "refers to" should be
removed.  E.g.:

    Keep-alives:  The keep-alive messages defined in SIP Outbound
    [RFC5626], including both "CRLF" and STUN keep-alive messages.

'"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.)

Section 4.1, 2nd paragraph is:

   SIP entities indicate willingness to send keep-alives towards the
   adjacent downstream SIP entity using SIP requests.  The associated
   responses are used by SIP entities to indicate willingness to receive
   keep-alives.  SIP entities that indicate willingness to receive keep-
   alives can provide a recommended keep-alive frequency.

This should be made more explicit.  (And can be clarified using the
singular.)

   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.

The 3rd paragraph should probably be expanded to make explicit a
somewhat subtle point:

   The procedures to negotiate usage of keep-alives are identical for
   SIP UAs and proxies.  However, it is expected that the sender of
   keep-alives is the sender of an initial request (REGISTER or
   INVITE), which in normal usage is a UA, and so a proxy will not
   initiate negotiation for sending keep-alives.  Thus, a UA which is
   willing to send keep-alives SHOULD always indicate that it is
   willing to send keep-alives even if it is not aware of any
   necessity of doing so, allowing the adjacent downstream entity to
   indicate if it knows that keep-alives are necessary (by responding
   with a non-zero value).  Similarly, an entity which is willing to
   receive keep-alives SHOULD always respond with a keep value of zero
   when it is not aware of any necessity for keep-alives, thus leaving
   the sender to send keep-alives if the sender is aware of any
   necessity to do so.

Urgh.  I suspect that these additions should instead be added to 4.3
and 4.4 as appropriate.  But the mechanism needs to be used correctly
by both entities in order to ensure that keep-alives are sent when
either entity knows that they are needed, and the draft doesn't make
explicit how to do that.

Section 4.3

It's not clear to me what keep-alives the upstream entity must send
once keep-alives are negotiated.  The first paragraph says that the
entity must be able to send both STUN and CRLF keep-alives, but that
seems to be not entirely accurate, as the sender seems to have the
choice of what type of keep-alive to send, and so can choose to send
only one type.  I suspect that there are rules elsewhere regarding
what sort of keep-alive is to be sent in what circumstances; if so,
there should be a reference to them.  Otherwise, a note should be
added to indicate that the sender has the choice of what sort of
keep-alive to add, and thus does not need to implement both.  (The
recipient is required to implement both, of course.)

Section 4.4

In every place where "frequency" is used, it would be more correct to
use "period".  (A frequency is events-per-second, whereas a period is
seconds-per-event.)

In some place it must be stated that the period is specified in
seconds, which is not done anywhere in the normative part of this
draft.

Section 5

In the first paragraph, a few places could be phrased more clearly:

   If a SIP entity receives a SIP response >>whose topmost<< Via
   header field contains a "keep" parameter with a non-zero value
   >>(which indicates a recommended keep-alive period)<<, it MUST use
   the procedures defined >>in [RFC5626], using the value as if it was
   the value of the Flow-Timer header field<<.  According to >>those<<
   procedures, the SIP entity must send keep-alives at least as often
   as the indicated recommended keep-alive >>period<<, >>and the SIP
   entity should<< send its keep-alives so that the interval between
   >>keep-alives<< is randomly distributed between 80% and 100% of the
   recommended keep-alive >>period<<.

After this paragraph should be a note saying when the receiving SIP
entity is allowed to assume the connection is dead.  Copying from RFC
5626, we could say:

   Following [RFC5626], if a recommended keep-alive period has been
   negotiated, the receiving SIP entity may, after not receiving a
   keep-alive for that period, consider the flow to be dead.  Note
   that the entity should wait for a time larger than the period in
   order to have a grace period to account for transport delay.

3rd paragraph

It makes sense that if a single entity inserts both a keep=n parameter
and a Flow-Timer: n header that they should contain the same value.
But that is of no use to (and is not enforceable by) an upstream SIP
entity, since from the upstream entity's point of view, the Flow-Timer
header may have been inserted by an entity further downstream, not its
adjacent downstream entity.  That is, an entity expects to see
responses with Flow-Timer and keep=n which disagree, and (I think)
must use the keep value instead of the Flow-Timer value.

Actually, I think the situation is probably more complex than I
realize.  The Flow-Timer value seems to apply only between the edge
proxy and the UA, and it it seems that only the UAC should pay
attention to its value.  But it's not clear who can add the Flow-Timer
header.  It appears that it can only appear in REGISTER responses.  It
can be added by the UAS (obviously), but it appears from RFC 5626
section 5.4 that it can also be added to the response by intermediate
proxies.

This section needs revision to make clear the interaction between
Flow-Timer and keep=n.  I suspect the only workable answer is that an
entity must examine both values (if it supports both mechanisms),
determine whether Flow-Timer applies to this hop, and if so, send
keep-alives according to the minimum of the two specified periods.
Since the two periods might be specified by different entities, there
doesn't seem to be any workable alternative rule.

Section 6

All examples use STUN keep-alives.  It would probably be beneficial to
change one example to use CRLF, or to add a note explaining whether
and when CRLF might be used in the example situations.

It is possible for two proxies to use this mechanism on a hop between
themselves.  Would it be useful to add an example of this?

Dale

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 9 Aug 2010 09:21:20 +0200

Hi Dale,

Thanks for your comments. See inline.

>improved in a few places.

Some of your comments ARE related to issues that were discussed during WGLC, and I will indicate it when it occurs.

--------------

>The running title is "STUN-keep", which is surely not what is intended.

I can change that to "keep-alive".

--------------

>open; the problem is keeping the NAT open for the signaling (dialog).

I can change "session" to "dialog".

--------------

>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.

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."

--------------

>    [RFC5626], including both "CRLF" and STUN keep-alive messages.

People have asked me to remove text, and make it shorter and easier to read, so I would not like to start adding text unless something is broken or very unclear. In this case I think that sections 4.3 and 4.4 makes it clear that both CRLF and STUN are included.

But, I DO agree that "refers to" can be removed:

    	"Keep-alives:  The keep-alive messages defined in SIP Outbound [RFC5626]."

--------------

>section 4.1, which needs to be a bit more explicit about the

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.

--------------

>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.

--------------

>   initiate negotiation for sending keep-alives. 

The mechanism allows any SIP entity to negotiate sending of keep-alives with its adjacent downstream entity.

>   Thus, a UA which is

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 willingess to 
	receive keep-alives, even if they are not aware of any necessity for the adjacent upstream SIP entity to 
	send them."

--------------

>recipient is required to implement both, of course.)

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."

--------------

>seconds, which is not done anywhere in the normative part of this

"frequency" is used in Outbound, and I've been asked to use the same terminology. 

I DO agree, however, that it should be indictated that the frequency is provided in seconds.

I suggest to change the last sentence of the second paragraph to:

	"The parameter can contain a recommended keep-alive frequency, given in seconds, or a zero value."

I suggest to add "given in seconds" also to the first paragraph of section 5:

	"If a SIP entity receives a SIP response, where its Via header field
   	contains a "keep" parameter with a non-zero value that indicates a
   	recommended keep-alive frequency, given in seconds, it MUST use the 
	procedures defined for the Flow-Timer header field [RFC5626]."

--------------

>   order to have a grace period to account for transport delay.

I don't think we can use "flow", since that is Outbound specific.

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."

--------------

>must use the keep value instead of the Flow-Timer value.

The text previously said that one MUST NOT insert Flow-Timer if it inserts keep=n.

However, based on WGLC comments from Paul we agreed to remove that restriction, in order for this draft to not make restrictions to Outbound (in cases where both Outbound and this draft is used).

--------------

>doesn't seem to be any workable alternative rule.

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.

--------------

>and when CRLF might be used in the example situations.

I can do something like that.

>themselves.  Would it be useful to add an example of this?

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.

Regards,

Christer

---

From: Paul Kyzivat <[email-masked]>
Date: Mon, 09 Aug 2010 10:08:17 -0400

General comment here:

While it is good to remove unnecessary text to simplify the document,
if adding a bit of text makes the document clearer, then that should be 
considered a good thing to do.

	Thanks,
	Paul

Christer Holmberg wrote:
> _______________________________________________
> sipcore mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sipcore
>

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 9 Aug 2010 19:08:24 +0200

Hi,

>considered a good thing to do.

Of course. 

But, in this case I think it is much about duplicating text that already exists elsewhere in the document, which I don't think is needed.

And, from my experience with IESG, they are very likely going to ask for such duplications to be removed anyway.

Regards,

Christer

---

From: "WORLEY, Dale R (Dale)" <[email-masked]>
Date: Mon, 9 Aug 2010 21:22:27 -0400

Only the items for which I have further comment:

> From: Christer Holmberg [[email-masked]]

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 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.

>         "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.

> 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.

> 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.
> 
>         "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.
> 

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.

>         "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.
> 

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

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 10 Aug 2010 07:19:20 +0200

Hi Dale, 

>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.

I think *extension* is the wrong wording.

The draft defines a negotiation mechanism for the keep-alive mechanism, which is defined in the Outbound spec (at some point I think we said that the keep-alive mechanism should have been in a separate spec from the rest of Outbound in the first place, but it was too late to make that change at that point). 

Would it be more clear if I added the following sentence to section 1?

"The details of the keep-alive mechanisms are described in [RFC 5626]."

Regards,

Christer

> > usage of Flow-Timer is only defined between a UA and an 
> edge proxy, so 
> > I think it is useful to have the definition.
> 
> 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.
> 
> > 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:
> 
> 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.
> 
> > 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.
> > 
> keep-alives 
> > are described in sections 4.3. and 4.4, so there is no need 
> to repeat 
> > the same text in 4.1.
> 
> 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 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 
> > BUT, in order to try to make everyone happy, I could add 
> the following sentence to the first section of 4.3:
> > 
> 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.
> 
> *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.
>