Subject: Re: [sipcore] WGLC: draft-ietf-sipcore-locparam - Christer's review

From: Christer Holmberg <[email-masked]>
Date: Sun, 18 Aug 2019 19:23:15 +0000

Hi,

Below is my WGLC review. 

NUTSHELL:

>From a technical point of view, the document is ready to move forward. However, there are some editorial issues that I'd like the authors to address.

---

GENERAL:

Q_GEN_1:

- Please always use capital 'G' for Geolocation header field.

Q_GEN_2:

- Instead of talking about "The parameter defined in this specification", use the name of the parameter.

Q_GEN_3:

- In some places the text talks about the loc-src parameter. I think it shall talk about the location-source parameter instead, as that is the name in the ABNF. loc-src is just the encoding.

Q_GEN_4:

- While RFC 6442 talks generally about "SIP intermediaries", this draft talks about "proxies". Is there a reason for that?

---

ABSTRACT:

Q_ABS_1:

 The text gives some background about a problem, but there is no words about what the draft does to address that problem. I think there should be a sentence saying: "This document blah blah blah...".

---

SECTION 1:

Q_1_1:

The first sentence says:

   "The SIP geolocation specification [RFC6442] describes the
   "Geolocation" SIP header field which is used to indicate that the SIP
   message is conveying location information.  The specification
   suggests that only one location value should be conveyed."

- The Geolocation SIP header field does not only indicate that the SIP message is used to convey location information, it also contains a reference to the location information.

- Does RFC 6442 really suggest that only one location value should be conveyed? The way I read it is that an intermediary should not add location information to a request that already contains it. Would it be more appropriate to say:

"The specification specifies that SIP intermediaries should not add location values to a SIP request that already contains location value."

Q_1_2:

The text says:

   "This document adds a location-source (loc-src) parameter to the
   location values in [RFC6442] so that the entity adding the location
   value to geolocation header field can identify itself using its
   hostname."

- I think you shall be more specific, and say something like:

"This document extends the Geolocation header field, by allowing an entity adding the location value to identity itself using a hostname. This is done by defining a new geoloc-param header field parameter, location-source."

---

SECTION 3:

Q_3_1:

- s/specific/specification

---

SECTION 4:

Q_4_1:

The syntax says:

location-source = "loc-src=" (hostname )
hostname = <defined in RFC3261>

- I suggest "loc-src" EQUAL (hostname).

- s/(hostname)/hostname

A_4_2:

The text says:

   "Only a fully qualified host name is valid, an IP address MUST NOT be
   added by an entity conforming with this specification.  If a node
   conforming to this specification receives a geolocation header field
   with a loc-src parameter containing an IP address then the parameter
   MUST be removed."

- It is not only about not supporting IP addresses - the syntax does not even allow an IP address. Perhaps something like:

"Only a fully qualified host name is valid. The syntax does not support IP addresses, and if an entity conforming to this..."

Q_4_3:

The text says:

   "Any proxy adding a location value to a geolocation header field
   SHOULD also add its host name using the loc-src parameter so that it
   is clearly identified as the node adding the location."

- I suggest to say "A proxy conformant to this specification adding a location value..."

- s/SHOULD also add its host name using the loc-src parameter/SHOULD also add a location-source header field parameter".

Q_4_4:

The text says:

"A UE MUST NOT provide a loc-src parameter value."

I suggest "A UA MUST NOT insert a location-source header field parameter"

---

SECTION 7:

Q_7_1:

The text says: "the that they". I assume that is a mistake.

Q_7_2:

The text says:

"To avoid problems of wrong interpretation of loc-src the value may be discarded when passed to an other domain."

Is "removed" more appropriate than "discarded"?

Regards,

Christer

-----Alkuperäinen viesti-----
Lähettäjä: sipcore <[email-masked]> Puolesta A. Jean Mahoney
Lähetetty: sunnuntai 18. elokuuta 2019 20.57
Vastaanottaja: [email-masked]
Aihe: Re: [sipcore] WGLC: draft-ietf-sipcore-locparam

Hi all,

The WGLC is being extended to Friday, Sept 13, since many people go on vacation in August and early September.

Thanks!

Jean

On 8/15/19 2:43 PM, A. Jean Mahoney wrote:
> Hi all,
> 
> This begins the Working Group Last Call of draft-ietf-sipcore-locparam 
> (Location Source Parameter for the SIP Geolocation Header Field).
> 
> Please post any feedback to the sipcore mailing list by Friday, August 30.
> 
> https://datatracker.ietf.org/doc/draft-ietf-sipcore-locparam/
> 
> Thanks!
> 
> Jean
> 
> _______________________________________________
> sipcore mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sipcore

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

---

From: James Winterbottom <[email-masked]>
Date: Mon, 19 Aug 2019 06:27:34 +1000

Thanks Christer, these all look pretty reasonable to me.

Thanks for the review.

James

> On 19 Aug 2019, at 5:23 am, Christer Holmberg <[email-masked]> wrote:
> 
> Hi,
> 
> Below is my WGLC review. 
> 
> NUTSHELL:
>

---

From: <[email-masked]>
Date: Tue, 20 Aug 2019 14:02:21 +0000

Hi Christer,
thank you for your review.
I see this also as good changes and have included the proposed changes within the draft.

> - While RFC 6442 talks generally about "SIP intermediaries", this draft talks about "proxies". Is there a reason for that?

I have changed proxies/proxy it to SIP intermediaries/ SIP intermediarity depending in the text where the location-source is added. To be more aligned with RFC6442.

For Q_ABS_1 I have added the following text:
This document defines the location-source parameter so that the entity adding the location value to 
    	Geolocation header field can identify itself using its hostname.

After WGLC will provide the next draft with all changes. 

Best Regards

Roland 

-----Ursprüngliche Nachricht-----
Von: sipcore <[email-masked]> Im Auftrag von James Winterbottom
Gesendet: Sonntag, 18. August 2019 22:28
An: Christer Holmberg <[email-masked]>
Cc: [email-masked]
Betreff: Re: [sipcore] WGLC: draft-ietf-sipcore-locparam - Christer's review

Thanks for the review.

James

> location-source = "loc-src=" (hostname ) hostname = <defined in 
> RFC3261>
> 
>> This begins the Working Group Last Call of 
>> draft-ietf-sipcore-locparam (Location Source Parameter for the SIP Geolocation Header Field).
>>