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

From: Barry Leiba via Datatracker <[email-masked]>
Date: Sun, 31 Jan 2021 11:57:38 -0800

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...”

— 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”.

   Details about
   enforcing a more constraint permission model

I think the word you want is “constrained”.

— 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”?

   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”.

   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.”

— 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?

   A Diameter client, which sent a request for session initiation

Please remove the comma (and change “which” to “that”).

— 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”.

   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.”

— 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”.