Re: [Dime] Barry Leiba's No Objection on draft-ietf-dime-group-signaling-13: (with COMMENT)

[Mark Jones <[e-mail]>]

Appreciate the thorough review, Barry. Comments inline below.

On 2021-01-31 2:57 p.m., Barry Leiba via Datatracker wrote:
> Barry Leiba has entered the following ballot position for
> draft-ietf-dime-group-signaling-13: 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-dime-group-signaling/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> I have to say that I found this to be challenging to read: much of it is very
> dense, with paragraphs giving sometimes-subtly-different conditions, and I
> would be reading it thinking, “Does this paragraph apply to my situation?  I
> don’t think so, but what about the next one?  Wait, maybe it was the other one
> after all.”  I just have to assume that someon actually implementing Diameter
> would have an easier time following it.
> 
> I have some editorial comments that won’t help with that aspect, but that I
> hope will help with clarity in general:
> 
> — Section 3.1 —
> 
>    A session group, which has sessions assigned, can be deleted, e.g.,
>    due to a change in multiple users' subscription profile so that the
>    group's assigned sessions do not share certain characteristics
> 
> It appears that “has sessions assigned” is restrictive (it’s a required
> description, not just extra information).  If so, it should say, “A session
> group that has sessions assigned can be deleted, e.g., due to...” without the
> internal commas.
> 
> Another way to put it might be, “A session group can be deleted even if it has
> sessions assigned, e.g., due to...”
> 

We intended to say that a session group can be deleted and provide an example.
The concept of "group" is clearly introduced in the paragraph just above and the
details are provided in the subsequent sections. We propose the following
rephrasing:

"It may be required to delete a session group, e.g. at the expiry of a
promotional period that applied to multiple subscriber profiles".


> — Section 3.3 —
> 
>    This specification follows the most flexible model where both, a
>    Diameter client and a Diameter server can create a new group and
> 
> Nit: no comma here
> 
>    Either the client and the server
>    can assign a session to a group.
> 
> Nit: “Either the client or the server can”, or “Both the client and the server
> can”.
> 

Suggested wording:
"This specification allows a permissive model where either Diameter client or
Diameter server can create a new group. The creator of the group becomes the
group owner and assigns a new identifier to the group according to the rules in
Section 7.3."


>    Details about
>    enforcing a more constraint permission model
> 
> I think the word you want is “constrained”.
> 

Agreed. Addressed in the next revision.

> — Section 4.2 —
> 
>    Diameter AAA
>    applications typically assign client and server roles to the Diameter
>    nodes, which are referred to as relevant Diameter nodes to utilize
>    session grouping and issue group commands.
> 
> I’m having trouble parsing this sentence and determining what you’re trying to
> tell me.  Can you please rephrase or repunctuate it to make it clearer?  What
> are referred to as “relevant Diameter nodes”?  What goes with “to utilize
> session grouping”?
> 

We agree that it needs rephrasing and propose:
"Although Diameter is a peer-to-peer protocol, Diameter AAA applications
typically assign the role of "Diameter client" to the Diameter node initiating
the Diameter session and the role of "Diameter server" to the Diameter
node authorizing the Diameter session. This specification does not restrict
group creation, assignment or deletion actions to a specific role. In the
following sections, Diameter node is used to refer to either role."


>    Diameter nodes, which are group-aware, MUST store and maintain an
>    entry about the group assignment together with a session's state.
> 
> Similar to earlier: is “are group aware” meant to be restrictive (there are
> Diameter nodes that are group aware and those that are not, and you’re talking
> about the former), or non-restrictive (all Diameter nodes are group aware, and
> you’re just pointing that fact out).  It is currently written as
> non-restrictive, but I think you mean it to be restrictive.  If that is
> correct, remove both commas and change “which” to “that”.
> 

We propose rephrasing to:
"Any Diameter node that has advertised support of session grouping and group
operations MUST store and maintain the group assignment as part of the session's
state."


>    A Diameter node MUST also keep a record about sessions, which
>    have been assigned to a session group by itself.
> 
> The comma shouldn’t be there, and “which” should be “that”.  But it’s a bit
> awkward anyway: may I suggest rephrasing in active voice as, “Each Diameter
> node MUST also keep a record about sessions that it has assigned to a session
> group.”
> 

Agree with suggested rephrasing. Will implement in the next revision.

> — Section 4.2.1 —
> 
>    If the
>    response from the server indicates success in the result code but
>    solely the assignment of the session to a session group has been
>    rejected by the server, the client treats the session as single
>    session without group assignment.
> 
> What does “solely” mean in this sentence?
> 

It should have been "only". However, we believe this sentence has been made
redundant by previous edits and can now be removed. The previous sentence
already states that the client MUST remove the session from the group if the
server rejects the proposed assignment. The session then proceeds as if it were
not assigned to the group.

>    A Diameter client, which sent a request for session initiation
> 
> Please remove the comma (and change “which” to “that”).
> 

We propose the following rephrasing:

"If a Diameter client sends a request for session initiation containing one or
more Session-Group-Info AVPs but the response from the Diameter server does not
contain a Session-Group-Info AVP, the Diameter client MUST proceed as if the
request was processed without group assignments."


> — Section 4.2.2 —
> 
>    The session, which is to be removed from a group, is
> 
> Make it, “The session that is to be removed from the group is”
> 
>    When a Diameter client decides to remove a session from all session
>    groups, to which the session has been previously assigned,
> 
> Remove the comma after “groups”.
> 

Agreed. Fixed in next revision.


>    The session, which is to be removed from all groups, to
>    which the session has been previously assigned, is identified in the
>    Session-Id AVP of the command request.
> 
> Make it, “The session that is to be removed from all groups to which it had
> been previously assigned is identified in the Session-Id AVP of the command
> request.”
> 

We came up with the following rephrasing to improve the readability:
"The Session-Id AVP in the re-authorization request identifies the session that
is to be removed from all groups to which it had been previously assigned."

> — Section 7.3 —
> 
>    The
>    <DiameterIdentity> portion of the Session-Group-Id MUST identify the
>    Diameter node, which owns the session group.
> 
> Please remove the comma and change “which” to “that”.
> 

Agreed. Will be fixed in next revision.


./Mark