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

From: Barry Leiba <[email-masked]>
Date: Mon, 1 Mar 2021 09:50:27 -0500

Thanks, Mark.  I've reviewed your proposed changes and they all look
good.  Thanks for the response and for considering my comments.

Barry

On Tue, Feb 23, 2021 at 1:19 PM Mark Jones <[email-masked]> wrote:
>
> 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