Re: [Dime] AD Evaluation of draft-ietf-dime-group-signaling-12

[Marco Liebsch <[e-mail]>]

Hi Ben,

thanks a lot for the review and detailed comments. We'll address editorial items first and converge on the substantive comments.
For the comments per the shepherd’s report, I thought we addressed all of them and confirmed with Jouni at that point in time,
but we'll check again. 

Marco


-----Original Message-----
From: Ben Campbell [mailto:[e-mail]] 
Sent: Dienstag, 19. Februar 2019 07:14
To: [e-mail]
Cc: [e-mail]
Subject: AD Evaluation of draft-ietf-dime-group-signaling-12

Hi,

This is my AD evaluation of draft-ietf-dime-group-signaling-12.

This draft is on the right track, but is not ready for IETF LC. I have a few substantive comments and a number of editorial comments that need to be resolved first.

Thanks!

Ben.

*** Substantive Comments ***
- General: There were comments in the shepherd’s report that need to be addressed.

§4.1: Why is the SHOULD not a MUST? What would be the consequences of not following it?

§4.1.2, last paragraph: How long should a node remember that another node has announced support for groups? Is that memory forever? What happens if that node ceases to support groups in the future?

§4.2:

- "A Diameter node MUST also keep a record about sessions, which have been assigned to a session group by itself.”

Is this the same as saying a node MUST record the owner of each group? Why does it matter which node assigned a specific session to a group?

§4.2 and children:

I’d like to see more text about what combinations are allowed and there order of application. For example, if the presence of a Session-Group-Info AVP with the SESSION_GROUP_ALLOCATION_ACTION flag cleared and Session-Group-Id AVP absent indicates deletion of a session from all groups, what if the message also contains other Session-Group-Info AVPs that assign groups? Is that the same thing as moving a session between groups?

I’d also like to see some earlier text describing the meaning of Session-Id AVPs in group management and group operations. There’s a mention of that in the “format” section, but that seems an odd place to put it.


§4.2.1:

- General: Am I correct to assume that failure of a Diameter command means no groups are assigned? If so, please state that explicitly.

- first paragraph: The “MUST” seems like a statement of fact. That is, there’s no implementation choice to make here.

- "If the Diameter server accepts the client’s request for a group assignment, but the assignment of the session to one or some of the multiple identified session groups fails,”

This seems self contradictory; if the assignment fails then how can the server have accepted it? Is there a practical difference between assignment failure and assignment rejection?

- "If the Diameter server receives a command request from a Diameter client and the command comprises one or multiple Session-Group-Info AVPs and none of them includes a Session-Group-Id AVP, the server MAY decide to assign the session to one or multiple session groups.”

Is there an expectation that the server assigns the session to the same number of groups as the the number of Session-Group-Id AVPs included by the client? If not, why would the client ever include more than one.

Is the client prohibited from sending multiple AVPs, some of which contain session IDs and some of which do not?

- "the Diameter client SHOULD NOT retry to request group assignment for this session, but MAY try to request group assignment for other new sessions.”

Why is the SHOULD NOT not a MUST NOT?

§4.2.3:
- "When a Diameter server enforces an update to the assigned groups midsession, it sends a Re-Authorization Request (RAR) message to the client identifying the session…”

Should that say “or service-specific request”? Or is this operation limited to applications that use RAR?

§4.4.1:

- "If the process of the request
is delay-sensitive, the sender SHOULD NOT set the Group-Response- Action AVP to ALL_GROUPS (1) or PER_GROUP (2).”

Why not MUST NOT?

- "If the answer can be
sent before the complete process of the request for all the sessions or if the request timeout timer is high enough, the sender MAY set the Group-Response-Action AVP to ALL_GROUPS (1) or PER_GROUP (2).”

Can you offer guidance on how to decide if a timeout timer is high enough?

§5:

- It’s worth mentioning that a stateful proxy must advertise support.

- "Session group related AVPs being defined as optional AVP SHOULD be ignored by stateless Proxy Agents and SHOULD NOT be removed from the Diameter commands.”

This seems to put normative requirements on nodes that do not implement this spec. Or is it really a statement of fact?

§7.3: Can you offer guidance on how to ensure sufficient uniqueness? “Eternal” seems like a pretty high bar; is that really the intention?

§10: It doesn’t seem likely that the e2e protection work for Diameter will ever complete, or at least not in the near future. I suggest toning down the hopeful language that talks about it.


*** Editorial Comments and Nits **

- General: The draft needs at least another proofreading and editing pass to improve readability and clarity. In particular, it needs to be proofread for the following:
- plural agreement
- Comma usage (there are many placed incorrectly, and many missing where needed)
- Proper use of “which” vs “that” (and the associated comma use.)
- Convoluted sentence structure

§2: Please use the new boilerplate from RFC 8174.
i
§3.1,
-  first paragraph: “e.g.” requires a comma afterwards: “e.g.,”  (There are several instances of this throughout the draft.)

- "In case of mobile users, the user’s session may get transferred to a new Diameter client during handover and assigned to a different group, which is maintained at the new Diameter client, mid-session.”

This is hard to parse. I suggest moving “mid-session” to earlier in the sentence. For example, “… session may get transferred mid-session to a new Diameter client…”

§3.2: I gather this section is an example. If say, please state that explicitly.

§3.3:
- This section uses confusing language to describe which nodes can do what. In particular, it often says “any” node, where I think it means “either the client or the server”. I assume nodes that are not a party to a session-group can’t modify it, correct? Likewise it says “each” node when I think it means “either node”.

- "Prerequisite for deletion
of a session group is that the Diameter node created the session beforehand, hence the node became the group owner.”

This seems like a complicated way to explain a simple concept. I suggest something to the effect of “A session group may only be deleted by the Diameter node that created it.”

- "The enforcement of more constrained permissions is left to the specification of a particular group signaling enabled Diameter application and compliant implementations of such application MUST enforce the associated permission model.”

This is overly complicated language. I suggest something to the effect of ""Diameter application with explicit support for session groups may define a more constrained permission model.” Also, the MUST is not really appropriate; it’s up to the definitions of those applications to set normative requirements on implementations, not _this_ draft.

- "specification. For example, a more constrained model could require that a client MUST NOT remove a session from a group which is owned by the server.”

Please do not use normative keywords in an example; examples are by definition non-normative.

- Default permissions table: I am confused by the “client” and “server” columns. If I understand this section correctly, the role of “client” and “server” is not relevant. (It is telling that each row has the same value in both columns.) Please consider stating this in terms of group “owner” and “non-owner”.

- Editor’s note: If this draft does not consider overruling a node’s assignment, why talk about it at all?  (Although it seems like it _does_ support that, in the sense that a server can reject or change assignments proposed by a client.)

§4.1: Please avoid normative language in the form of “SHOULD…only…”. That can be ambiguous. It’s better to say “SHOULD NOT … unless. For example, “SHOULD NOT perform group operations with a node unless the node has advertised support”.

§4.1.1:

- It seems to me that §4.1.1 and §4.1.2 have “explicit” and “implicit” reversed, in the sense that, if a Diameter application supports groups, then the announcement of group support is “implied” by the announcement of support for the application. OTOH, the use of Session-Group-Capability-Vector is an “explicit” announcement of group support.

- "New Diameter applications may consider support “

s/consider/specify

- "Such applications provide intrinsic discovery for the support of group commands and announce this capability through the assigned application ID.”

This is confusing. I think you mean support for groups is implied by the announcement of support for the application that explicitly supports it.But it can be read to mean a separate explicit announcement of group support.

§4.2:
- "It is left to the application to
determine the policy of session grouping.”
Does “application” mean Diameter application, or just “implementation”? Also, this is vague; I think this still talks about limits to the number of groups a  session belongs to, which is much more precise than “the policy of session grouping”.

- The second paragraph seems redundant with §3.3.

The phrase “single or multiple” would be easier to read as “one or more”. (This pattern repeats throughout the draft, along with some instances of “one or multiple”)

- “A list of all known session groups should be locally maintained on each node, each group pointing to individual sessions being assigned to the group."

s/should be/is

§4.2.1:

- “… the server MUST assign the new session to each of the one or multiple identified session groups when present in …”

- "If the Diameter server receives a command request from a Diameter client and the command comprises at least one Session-Group-Info AVP”

s/comprises/includes  (“comprises” means “made up of”, which would suggest that the request contains only Session-Group-Info AVPs).  (This occurs several times throughout the draft.)

What does “when” mean in context? Would “...session groups present in…” mean the same thing?

- "In case one or multiple identified session groups are not already stored by the server”

“In case” usually refers to planning for a contingency (For example, “I have a fire extinguisher in case of fire”). I suggest “In the case where…” or even better, “If…”  (Variations of this pattern occur several times.)

- "the server MUST store the new
identified group(s)”
s/new/newly

- "The server sends the response to the client and MAY include as information to the client only those Session-Group- Info AVPs for which the group assignment failed.”

Hard to parse.

- "session. When the Diameter client is
confident that the Diameter server supports session grouping…”

Since this should always be true before the client sends _any_ group operation, why restate it here?

§4.4.1:

- "Either Diameter node (client or server) can request the recipient of a request to process an associated command for all sessions being assigned to one or multiple groups by identifying these groups in the request.”

s/sessions being assigned/sessions assigned

- Please expand “CCF” on first mention.

- sentence starting with "When a server sends, as example, a Re-Authorization Request
(RAR) or a service-specific server-initiated request…”

The sentence is hard to parse.

- "If the sender sends a request including the Group-Response-Action AVP set to ALL_GROUPS (1) or PER_GROUP (2), it MUST expect some delay…”

“expect” is vague for use in a MUST requirement. Please state this in terms of concrete actions or state it descriptively.

§A.1, first paragraph: Please don’t use normative keywords to describe requirements defined in other documents. Doing so introduces a high chance of error if either document is updated in the future.