Re: [sipcore] WGLC: draft-ietf-sipcore-location-conveyance

["Stach, Thomas" <[e-mail]>]

Hi James, Brian, Jon, 

I did some review with focus on section 4. Please find below some comments:

General/Editorial:

The document uses the term "header" and "header field" interchangeably. 
I propose to consistently use "header field".
See e.g. captions of section 4.1, 4.2, 4.4, 8.1, 8.2. 8.5 and several occurrences in the text.

Section2, 
1st para
"... of a Target (Target) to a Location Recipient (LR)."
(Target) does not seem to abbreviate something.

2nd para
"In order to convey location information, this document specifies a new SIP header, the Geolocation header, which ..." 
Since the document defines 3 header fields, what about:
"In order to convey location information, this document specifies three
new SIP header fields, Geolocation, Geolocation-Routing and Geolocation-Error, which carry a reference to
a Location Object, grant permission to route a SIP request based on the loation-value and provide error 
notifications specific to location errors. "
Note, this text proposal still does not mention the new 424 Response code, which could be mentioned too.


ABNF issues in section 4

section 4.1
message-header    /= Geolocation-header ; (message-header from 3261)
Geolocation-header = ...

Related to my comment above I propose to rename Geolocation-header to Geolocation.  
This would also be more inline with the 3261 practice of header field definitions.

locationURI        =  sip-URI / sips-URI / pres-URI
                          / http-URI / HTTPS-URI

What about changing HTTPS-URI to https-URI in order to make consistent use of capitalization or is this too much nitpicking?

"   HTTP-URI and HTTPS-URI are defined according to [RFC2616] and
   [RFC2818], respectively."

a. There is inconsistent capitalization of "HTTP-URI" when compared to the ABNF (another nit).  
b. While 2616 provides ABNF for http-URI, 2818 does not provide ABNF for https-URI.
   Would it makes sense to provide a corresponding ABNF production in this doc?
   e.g.  https_URI = "https:" "//" host [ ":" port ] [ abs_path [ "?" query ]]


"If a
   Geolocation header field is received that contains generic-params,
   each SHOULD be ignored, and SHOULD NOT be removed when forwarding
   ^^^^
   the locationValue."

Does "each" refer to the header field or parameter? 
Though rather obvious that the parameter is meant changing 
"... each SHOULD be ignored..."  to 
"... each parameter SHOULD be ignored..." would make that clearer.

section 4.2

message-header    /= Georouting-header ; (message-header from 3261)
Georouting  = "Geolocation-Routing" HCOLON
                        ( "yes" / "no" / generic-value )

a) Propose to rename Georouting-header to Georouting (or Geolocation-Routing), as per above.
b) I could not find a definition/production/reference for generic-value.
Do you mean gen-value from 3261? I.e gen-value = token / host / quoted-string?
If yes, do you really need this flexibility or would e.g. quoted-string alone be sufficient?

The para after the ABNF definition end with:
" ... Values other than "yes" or "no" are permitted as a
   mechanism for future extensions, and should be treated the same as
   "no". "  
Please add "if not understood" after "no".

Section 4.4

   message-header          /= Geolocation-Error-header ;
                                (message-header from 3261)
   Geolocation-Error        = "Geolocation-Error" HCOLON
                                locationErrorValue
   locationErrorValue       = location-error-code
                               *(SEMI location-error-params)
   location-error-code      = 1*3DIGIT
   location-error-params    = location-error-code-text
                              / generic-param ; from RFC3261
   location-error-code-text = "code" EQUAL quoted-string ; from RFC3261

Geolocation-Error-header is not further expanded. You probably mean Geolocation-Error.

The examples do not conform to this ABNF, e.g. 
Geolocation-Error: 100 "Cannot Process Location" 
does not show the semicolon, the string "code" and the "="
According to the ABNF is would have to read 
Geolocation-Error: 100 ; code="Cannot Process Location" 
                      ^^^^^^^^
- A further question on 
locationErrorValue       = location-error-code *(SEMI location-error-params) 
Does the locationErrorValue really need one or more parameters or would a simple reason phrase be sufficient? 
The current example suggest the latter, whereas the ABNF suggests the former. 
If it is just a reason phrase one could simplify to:
locationErrorValue       = location-error-code [SP location-error-code-text]
location-error-code-text = quoted-string 
However I don't have a strong preference here, other than that the examples should match the ABNF.

- The grammar of RFC 3261 specified explicit ABNF for the response codes. 
Do you want to do the same for location-error-code, which currently only specifies 1*3DIGIT?
The effort of doing so does not seem to be too big for the five already defined codes 100, 200, 300, 201, 202.

- From these specified error codes and their explaining text on page 15/16 it does not become clear 
if the text phrase is a only default string or a fixed value. 
Similar to RFC 3261 I assume the former is meant and not the latter. 
However, if it was the latter this should be nailed down in the ABNF.




Kind Regards

Thomas

> -----Ursprüngliche Nachricht-----
> Von: [e-mail] 
> [mailto:[e-mail]] Im Auftrag von Adam Roach - 
> SIPCORE Chair
> Gesendet: Dienstag, 01. März 2011 00:38
> An: SIPCORE (Session Initiation Protocol Core) WG
> Cc: [e-mail]; 
> Robert Sparks; SIPCORE Chairs
> Betreff: [sipcore] WGLC: draft-ietf-sipcore-location-conveyance
> 
> [as chair]
> 
> The current editor of draft-ietf-sipcore-location-conveyance believes 
> that the document has no remaining technical issues[1], and 
> is ready for 
> evaluation. Today, we are starting a two-week working group last call 
> period. This last call period ends on Tuesday, March 15th.
> 
> The latest version of the document can be retrieved here:
> 
> http://tools.ietf.org/html/draft-ietf-sipcore-location-conveyance
> 
> Any comments on the document should be sent to the SIPCORE 
> mailing list.
> 
> /a
> 
> [1] John Elwell's editorial comments of February 25th have 
> been noted by 
> the authors, and will be treated as WGLC comments.
> _______________________________________________
> sipcore mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/sipcore
>