Subject: [sipcore] Mirja Kühlewind's No Objection on draft-ietf-sipcore-sip-push-21: (with COMMENT)

From: Mirja Kühlewind <[email-masked]>
Date: Thu, 10 Jan 2019 05:01:16 -0800

Mirja Kühlewind has entered the following ballot position for
draft-ietf-sipcore-sip-push-21: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-sipcore-sip-push/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

One question on the new PNS registry:
I don't quite understand why the policy is "Specification Required". I guess
you need some description of the values for pn-provider, pn-param and pn-prid.
However, you could just add these fields to the registry directly (with the
option to just provide a reference in at least the param and prid fields if the
description is rather extensive). However, for e.g. RFC8030 is would easily be
possible to describe everyhing you need within the registry. For apns and fcm
you can simply add a pointer to this new RFC if you think the description is
too long to be included in the registry directly. But this could make
registration for new simple services easier.

And one minor comment on some language:
Should these sentence in 3.1  and 5.2 maybe be normative as well:
"If a SIP UA
   receives a push notification that contains a payload the UA can
   discard the payload, but the UA will still send a binding-refresh
   REGISTER request."
e.g NEW
"If a SIP UA
   receives a push notification that contains a payload the UA can
   discard the payload, but the UA MUST still send a binding-refresh
   REGISTER request."

"A proxy
   will not request push notifications towards a UA that has not
   provided a pn-prid SIP URI parameter."
e.g. NEW
"A proxy
   MUST NOT request push notifications towards a UA that has not
   provided a pn-prid SIP URI parameter."

It seems like these sentences are on purpose not normative (maybe because as
they are part of a note...?), however, I think it would be more clear to state
those requirements normatively as well. Otherwise if you think these
requirement are described normatively somewhere else in the doc, I think a
forward reference would be good (I didn't double-check myself).

nits:
- maybe s/consider the interval too small/consider the interval too short/
- maybe s/response Section 8.1/response (see Section 8.1)/
- s/proxy receives a receives a SIP/proxy receives a SIP/

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 10 Jan 2019 13:48:27 +0000

Hi Mirja,

Thank You for the review! Please see inline.

>   discard the payload, but the UA MUST still send a binding-refresh

I was thinking that since the presence of payload don't have any impact on the normative procedures, a MUST is not needed. But, I am fine to modify as suggested.

>   provided a pn-prid SIP URI parameter."

In this case I'd prefer to keep the existing text, because the normative proxy procedures are described elsewhere.

But, as you suggest below, I can add a reference.

>- s/proxy receives a receives a SIP/proxy receives a SIP/

I will fix as suggested.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 10 Jan 2019 14:39:02 +0000

Hi,

>I was thinking that since the presence of payload don't have any impact on the normative
>procedures, a MUST is not needed. But, I am fine to modify as suggested.

I was looking at the text, and since it's a note I'd like to keep the existing text.

And, in this case, since the normative procedures are defined in the same section, I guess there is no need for a reference.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Thu, 10 Jan 2019 14:49:38 +0000

Hi,

Based on Mirja's COMMENTs, I have updated the pull request used for the review changes.

The commit can be found at:

https://github.com/cdh4u/draft-sip-push/pull/31/commits/09b15c74b76091d0f7e56edd5fa31c90ee4daafe

Note that the following change was also requested by another reviewer, and will be fixed in the commit associated with that review:

"maybe s/response Section 8.1/response (see Section 8.1)/"

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 11 Jan 2019 10:16:06 +0000

Hi,

I realized I forgot to address Mirja's comment regarding the registration earlier. Sorry for that.

>registration for new simple services easier.

I doubt it is enough to figure out how to use a PNS just by looking at the format of the pn- parameter values - and that includes RFC8030. You will need a specification that defines the actual procedures.

Regards,

Christer