Re: [sipcore] Opsdir last call review of draft-ietf-sipcore-locparam-06

[Tim Chown <[e-mail]>]

Hi,

Apologies, it was still in my review queue from the end of January.  Having a quieter week this week I reviewed it today, but didn’t realise it had been approved for publication.

I stand by the review, so I guess it can be published as is or it can be recalled by the AD and the comments addressed.  I do find it a clumsy document to read, and there are some gaps / inconsistencies.

Tim

On 17 Feb 2020, at 16:53, Brian Rosen <[e-mail]<mailto:[e-mail]>> wrote:

What happened here?  This document has just been approved for publication.

Brian

On Feb 17, 2020, at 9:42 AM, Tim Chown via Datatracker <[e-mail]<mailto:[e-mail]>> wrote:

Reviewer: Tim Chown
Review result: Has Issues

Hi,

I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving the operational aspects of
the IETF drafts. Comments that are not addressed in last call may be included
in AD reviews during the IESG review.  Document editors and WG chairs should
treat these comments just like any other last call comments.

The draft describes an extension to RFC6442 to support the inclusion of a
“loc-src” parameter to the SIP Geolocation field such that when multiple
locationValues are present the recipient can make a better informed decision
about which locationValue it wishes to use.

For the context of my comments, I am not that familiar with SIP-related
protocols.

The rationale for the addition of this parameter appears sound, and its
definition seems appropriate. However, the document is written in such a way
that the rationale is split over two sections, and the mechanism for including
the parameter is split across three sections, leaving the document a little
clumsy for readers to follow.

Overall, the document is one that should ultimately be published, but as is it
has issues, and in my view needs some restructuring to make it more concise and
simpler to follow, so is not (yet) ready for publication.

Main comments:

The definition of the parameter itself is fine.

The first issue is around the rationale.  This seems perfectly sound, but the
document splits the reasoning between most of the Introduction and the first
two paragraphs of Section 3.  It would be better to have the rationale in a
Rationale section, without repetition.

The more confusing issue is the way the mechanism is described over Sections 3
(paragraphs 3,4,5), 4 (paragraph 3) and 7 (all paragraphs).  There is
repetition, some clashes, and some ambiguity.  Rewriting the text to have a
single mechanism section would make the text far clearer, in my view.

Here is how I would suggest the text is restructured:

Introduction
Largely paras 1 and 2 of the current intro.

Rationale
Largely para 3 of the intro, and paras 1 and 2 of the existing Rationale
section.

Loc-src parameter specification
Paras 1-2 of the Mechanism section, with the Figure, and para 6 of the
Rationale section.

Mechanism
A rewrite of paras 3-5 of the current Rationale section, para 3 of the current
Mechanism section, and pretty much all of the Security section.

Example
Can stay as is

Privacy Considerations
Fine

Security Considerations
Just one para mentioning the trust issue and the protections defined in the
Mechanism section would be fine.  Don’t split the guidance as it currently is
across three sections - put it all in one pace where - a new Mechanisms section
as above - it can be much more easily followed and implemented.

Other comments:

Section 3
Para 2 says the document makes no comment on the rationale, but the document
explains the rationale.  Perhaps just delete this. Para 5 talks about another
networks with no trust; is the issue messages sent to (as stated) or also
received from?  I think when you pull all the mechanism text into one place you
can make this clearer.

Section 4
Para 3 - what is the difference between a UA and UE from the point of view of
an end node adding (or not adding) a loc-src parameter?  It says a UA MUST NOT
add one, but in Section 5 states that a UE might provide it? The last sentence
talks of removing loc-src for certain case(s) but this is also included in
Sections 3 and 7.  It really needs to be stated clearly in one place.

Section 5
Para 1  - RFC 4483 or RFC 2392?  RFC6442 cites 2392 for cid.

Section 7
Para 1 - What is a “location recipient”?  A receiver which is inspecting a
header with multiple locationValues?  Perhaps define this, or rewrite. Paras 2
and 3 - “strongly recommended” should be “RECOMMENDED” ? I think much of this
section is repeating what has been said before, e.g. in Section 4 it says “MUST
remove” and here it says removing it is (only) strongly recommended.

Bets wishes,
Tim