Subject: Re: [sipcore] Review of sip-push-10 - Martin's comments - Pull request

From: Christer Holmberg <[email-masked]>
Date: Sun, 1 Apr 2018 12:31:03 +0000

Hi,

Based on Martin's comments, I have created a pull request:

https://github.com/cdh4u/draft-sip-push/pull/17

Regards,

Christer

-----Original Message-----
From: sipcore [mailto:[email-masked]] On Behalf Of Christer Holmberg
Sent: 29 March 2018 09:42
To: Martin Thomson <[email-masked]>; SIPCORE <[email-masked]>
Subject: Re: [sipcore] Review of sip-push-10 - Martin's comments

Hi Martin,

Thank You for the review! Please see inline.

>I also found the prose a little hard to follow.  Some restructuring 
>might go a long way to improving clarity.
>
>The description of RFC 8292 integration isn't particularly helpful.
>Does the UA register once, discover that the Proxy/Registrar supports 
>vapid, then re-register immediately?  None of that is well-described.

I think this is related to your comment on Section 4. See below.

---

>The document talks in passing about its assumptions about the push 
>message, but never actually states the requirements on that message.
>Please add a clear description of what the message is.  For instance, 
>is any message sufficient?  Or does the message have to be empty?
>What would it mean to receive a message with a payload?

The text does say that the proxy MUST NOT include any payload. Only the push notification itself is used.

>An important limitation of APNS and FCM here is that a single app can't 
>hold multiple contexts, therefore any message it receives will cause 
>re-registration even if the message was for other purposes ("Scarves, 
>5% off until Monday!", bam).  You should document that.

Ok, I will look into that.

---

>Section 4:
>Can a UA include Feature-Caps on its own?  Maybe it doesn't want to do 
>all the push service stuff until it knows that the server supports the 
>features it needs (vapid for example).

A UA cannot include a Feature-Caps.

But, we could define a sip.pns media feature tag, that a UA can include to indicate generic support of SIP Push, but it would not be used to request usage of SIP Push.

Then, when the proxy receives it, it would return the normal information about what it supports, but it would not yet start requesting push notifications.

---

>Section 5.3.1:
>I don't understand the relevance of all the 423-related text here.
>Maybe it's a lack of clarity about the expected processing model.
>FWIW, the only thing of value in this very long paragraph is that 
>sip.pns is not to be included with a 423.
>
>However, I would argue the opposite and that sip.pns is most valuable 
>in a 423.

The text does not say that sip.pns is not included in 423.

But, the text does not say that sip.pns IS included either, so I can add text about that.

>For example, a UA knows that it only stays alive for a short time, so 
>it conservatively creates registrations with a short duration.  That's 
>not very useful, but the UA might expect to only be available while the 
>app is in the foreground where it is able to refresh frequently.  
>However, if it receives a signal from the proxy/registrar that 
>indicates push support (in a 423 perhaps), it might be inspired to 
>extend the registration and rely on push.
>
>How would a proxy decide to send a 555 response or ignore the push 
>information in the REGISTER request?

Local configuration.

>Why would a proxy add Feature-Caps instead of just stripping out the 
>push parameters?  (That's probably a consequence of my ignorance of 
>REGISTER procedures, but I am curious.)  Why does the Feature-Caps need 
>a value?  The downstream proxy only needs to know that push is taken 
>care of, not which push is in use.  Is it just for symmetry with the 
>downstream notification?

There is no technical reson for including a value - it is enough to inform the downstream proxy that push is taken care of.

It¹s just for information. I can add a note indicating that.

>What value does the parameter take? The pn-provider label?

Yes.

---

>Section 5.3.2:
>Why include so much text here?  If the proxy considers the UA to be 
>registered - and only challenged with respect to active connectivity - 
>then normal SIP processing applies with only one tweak: send a push 
>message and wait until the registration is restored.
>
>The idea that the proxy might be able to correlate a REGISTER request 
>that includes a different contact URI with this new transaction/dialog 
>is a little baffling - either the registration is restored with the 
>right contact, or the registration doesn't happen and message delivery 
>will ultimately time out and fail.

The text covers the ³race condition² case where the UA changes its contact as the same time as the proxy receives a request towards the UA, using the old contact. The request should not be forwarded towards the UA using the old contact.

>In this regard, the only text you need is the timeout text you have 
>near the end of this section.

In this case I disagree. I think the text is needed. In addition to the ³race condition² and timeout cases, it also specifies that the proxy needs to wait for the REGISTER response before forwarding the request, and what the proxy does in case of a non-2xx REGISTER response.

---

>Section 6 contains no information.  If anything, you can use NAT as 
>further justification for use of a push notification service, but it 
>doesn't warrant an entire section.

I suggest to move the first paragraph to Section 4, and second paragraph to Section 5.3.2.

---

>Section 7.5:
>I don't understand the purpose behind sip.pnsreg is defined here.  How 
>is this different than, say, the usual registration expiration logic?
>Maybe this goes back to the basic registration logic.

It is used to indicate that a UA can use e.g., internal timers to send periodic re-registrations, but still want to use SIP Push when there are incoming requests.

---

>Nits:
>Section 7.6: typo: indciates

Will fix.

---

>Section 11: "When Firebase..." seems to be copy-pasta

Will fix.

³When Generic Event Delivery Using HTTP Push is used,Š"

>s/push URI/push subscription URI/

"push URI² terminology is more common in RFC 8030, so that¹s why I used it. But I can replace it with ³push subscription URI².

>I don't think that you need to include a URI for RFC 8030.

I¹ll remove it.

Regards,

Christer

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 3 Apr 2018 15:52:39 +1000

I still have a question regarding the model we are using for the
registration:  though it might be impossible to send a packet from
proxy to UA, is the registration still considered active?  That is,
does the push only cause a refresh of addressing information?  Or,
does the registration end when connectivity fails and the push is used
to induce the creation of a new registration?  I think that it's the
former, but it would be nice to have that written down clearly.

> The text does say that the proxy MUST NOT include any payload. Only the push notification itself is used.

You also have to say what the UA needs to do when a payload arrives.
That's not clear in the PR.

>>What value does the parameter take? The pn-provider label?

"identifying the PNS" might make it clear to you, but you would be
better saying "identifying the type of PNS using a valid value for
pn-provider".

> The text covers the ³race condition² case where the UA changes its contact as the same time as the proxy receives a request towards the UA, using the old contact. The request should not be forwarde…

If that is the case, then it really isn't clear.  You should say that
it's possible for a registration change to be in progress when a new
request/dialog is created, and that the proxy/registrar needs to use
the most recent (i.e., accurate) information it has.  But that isn't
something that is new or special, so why expend so many words on it?

> In this case I disagree. I think the text is needed. In addition to the ³race condition² and timeout cases, it also specifies that the proxy needs to wait for the REGISTER response before forwardin…

How is any of that special?  I mean, prior to push, a message could
come in during the handling of a non-push REGISTER.  So how is this
not already handled by other, older RFCs?

> I suggest to move the first paragraph to Section 4, and second paragraph to Section 5.3.2.

I meant that the text added no new information.  I didn't mean to
imply that moving it was needed.

> It is used to indicate that a UA can use e.g., internal timers to send periodic re-registrations, but still want to use SIP Push when there are incoming requests.

Oh, that is not very clear.  You now have two reasons to send a push
(though both are the same).  The first is to force a registration
refresh so that incoming requests can reach the UA, the second is for
registration maintenance.  This could be made clearer without changing
emphasis on the former with text to the effect "A registrar can use
the same mechanism for refreshing registrations.  The registrar can
use a push message to awaken a UA that is otherwise unable to create a
REGISTER request prior to the expiration of a registration.  This is
the assumed behaviour unless a UA uses the +sip.pnsreg media feature
tag (see Section XXX)." That's introductory material.

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 3 Apr 2018 06:45:52 +0000

Hi,

Some of your issues are discussed in the PR, so I will remove them from
this thread.

>> The text covers the ³race condition² case where the UA changes its
>>contact as the same time as the proxy receives a request towards the UA,
>>using the old contact. The request should not be forwarded towards the
>>UA using the old contact.
>
>> In this case I disagree. I think the text is needed. In addition to the
>>³race condition² and timeout cases, it also specifies that the proxy
>>needs to wait for the REGISTER response before forwarding the request,
>>and what the proxy does in case of a non-2xx REGISTER response.
>

In older RFCs a non-registrar proxy will simply forward a request using
normal SIP routing procedures, without caring about registrations etc.

The proxy doing push does care about registrations, so it is not handled
by other RFCs.

>imply that moving it was needed.

Ok, so I can remove it all together.

>> It is used to indicate that a UA can use e.g., internal timers to send
>>periodic re-registrations, but still want to use SIP Push when there are
>>incoming requests.
>
>registration maintenance.
>This could be made clearer without changing

The two reasons (incoming request and registration maintenance) to use
push have been there from day one.

The text your refer covers the case when one does not need to use push for
registration maintenance (because the UA is able to awake using other
means in order to perform registration maintenance), but only for incoming
requests.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 5 Apr 2018 16:54:24 +0000

Hi,

I think I forgot to reply to the following.

>registration:  though it might be impossible to send a packet from proxy to UA, 
>is the registration still considered active?  That is, does the push only cause a refresh 
>of addressing information?  Or, does the registration end when connectivity fails and 
>the push is used to induce the creation of a new registration?  I think that it's the former, 
>but it would be nice to have that written down clearly.

The registration ends when the registration timer expires, using normal RFC 3261 procedures. 

Push itself does not directly impact the registration timer: push triggers the UA to send a re-registration, and that re-registration will then reset the registration expiration timer (again, using normal RFC 3261 procedures).

So, to answer your question: the push only causes a refresh of the registration.

Regards,

Christer

>>REGISTER procedures, but I am curious.)  Why does the Feature-Caps 
>>need a value?  The downstream proxy only needs to know that push is 
>>taken care of, not which push is in use.  Is it just for symmetry with 
>>the downstream notification?
>

"identifying the PNS" might make it clear to you, but you would be better saying "identifying the type of PNS using a valid value for pn-provider".

> The text covers the ³race condition² case where the UA changes its contact as the same time as the proxy receives a request towards the UA, using the old contact. The request should not be forwarde…

If that is the case, then it really isn't clear.  You should say that it's possible for a registration change to be in progress when a new request/dialog is created, and that the proxy/registrar needs to use the most recent (i.e., accurate) information it has.  But that isn't something that is new or special, so why expend so many words on it?

> In this case I disagree. I think the text is needed. In addition to the ³race condition² and timeout cases, it also specifies that the proxy needs to wait for the REGISTER response before forwardin…

How is any of that special?  I mean, prior to push, a message could come in during the handling of a non-push REGISTER.  So how is this not already handled by other, older RFCs?

> I suggest to move the first paragraph to Section 4, and second paragraph to Section 5.3.2.

I meant that the text added no new information.  I didn't mean to imply that moving it was needed.

> It is used to indicate that a UA can use e.g., internal timers to send periodic re-registrations, but still want to use SIP Push when there are incoming requests.

Oh, that is not very clear.  You now have two reasons to send a push (though both are the same).  The first is to force a registration refresh so that incoming requests can reach the UA, the second is for registration maintenance.  This could be made clearer without changing emphasis on the former with text to the effect "A registrar can use the same mechanism for refreshing registrations.  The registrar can use a push message to awaken a UA that is otherwise unable to create a REGISTER request prior to the expiration of a registration.  This is the assumed behaviour unless a UA uses the +sip.pnsreg media feature tag (see Section XXX)." That's introductory material.

---

From: Martin Thomson <[email-masked]>
Date: Fri, 6 Apr 2018 11:13:16 +1000

It would help to mention that.  In particular, that while the
registrar/proxy might not be able to send messages to the UA, it can
use push to restore that capability.

---

From: [email-masked] (Dale R. Worley)
Date: Thu, 05 Apr 2018 22:03:05 -0400

Martin Thomson <[email-masked]> writes:
> does the push only cause a refresh of addressing information?

Yes, this is the model.  It's the standard SIP model.  And really, we
couldn't use any other model, because no element of the system (other
than the UA) *knows* that it is impossible to send a packet from the
proxy to the UA.

Dale

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 6 Apr 2018 07:04:51 +0000

Hi,

> It would help to mention that.  In particular, that while the registrar/proxy might not be able to 
> send messages to the UA, it can use push to restore that capability.

One reason the why proxy requests the push notification towards the UA is to be able to send message to the UA.

But, if the registration has expired, there will be no push notifications. The registrar won't even forwards any requests towards the UA to begin with.

Regards,

Christer

>>to induce the creation of a new registration?  I think that it's the former, but it would be nice to have that written down clearly.
>
>>>taken care of, not which push is in use.  Is it just for symmetry 
>>>with the downstream notification?
>>
>>>that includes a different contact URI with this new 
>>>transaction/dialog is a little baffling - either the registration is 
>>>restored with the right contact, or the registration doesn't happen 
>>>and message delivery will ultimately time out and fail.
>>
>>>I don't understand the purpose behind sip.pnsreg is defined here.  
>>>How is this different than, say, the usual registration expiration logic?