From: Ben Campbell <ben@nostrum.com>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_3946243B-46B6-4335-AD95-5485281D1B79";
 protocol="application/pgp-signature"; micalg=pgp-sha512
Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\))
Message-Id: <555EE6E7-E817-4D12-9DF1-4AAE4CF5BA41@nostrum.com>
Date: Tue, 19 Feb 2019 00:13:57 -0600
Cc: dime@ietf.org
To: draft-ietf-dime-group-signaling.all@ietf.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/dime/sFWNTcILIY93eEN8kcBRmQq-vw0>
Subject: [Dime] AD Evaluation of draft-ietf-dime-group-signaling-12
Precedence: list


--Apple-Mail=_3946243B-46B6-4335-AD95-5485281D1B79
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

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=E2=80=99s report that =
need to be addressed.

=C2=A74.1: Why is the SHOULD not a MUST? What would be the consequences =
of not following it?

=C2=A74.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?

=C2=A74.2:

- "A Diameter node MUST also keep a record about sessions,
which have been assigned to a session group by itself.=E2=80=9D

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?

=C2=A74.2 and children:

I=E2=80=99d 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=E2=80=99d also like to see some earlier text describing the meaning of =
Session-Id AVPs in group management and group operations. There=E2=80=99s =
a mention of that in the =E2=80=9Cformat=E2=80=9D section, but that =
seems an odd place to put it.


=C2=A74.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 =E2=80=9CMUST=E2=80=9D seems like a statement of =
fact. That is, there=E2=80=99s no implementation choice to make here.

- "If the Diameter server accepts the client=E2=80=99s request for a =
group
assignment, but the assignment of the session to one or some of the
multiple identified session groups fails,=E2=80=9D

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.=E2=80=9D

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.=E2=80=9D

Why is the SHOULD NOT not a MUST NOT?

=C2=A74.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=E2=80=A6=E2=80=9D

Should that say =E2=80=9Cor service-specific request=E2=80=9D? Or is =
this operation limited to applications that use RAR?

=C2=A74.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).=E2=80=9D

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).=E2=80=9D=


Can you offer guidance on how to decide if a timeout timer is high =
enough?

=C2=A75:

- It=E2=80=99s 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.=E2=80=9D

This seems to put normative requirements on nodes that do not implement =
this spec. Or is it really a statement of fact?

=C2=A77.3: Can you offer guidance on how to ensure sufficient =
uniqueness? =E2=80=9CEternal=E2=80=9D seems like a pretty high bar; is =
that really the intention?

=C2=A710: It doesn=E2=80=99t 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 =E2=80=9Cwhich=E2=80=9D vs =E2=80=9Cthat=E2=80=9D (and =
the associated comma use.)
- Convoluted sentence structure

=C2=A72: Please use the new boilerplate from RFC 8174.
i
=C2=A73.1,
-  first paragraph: =E2=80=9Ce.g.=E2=80=9D requires a comma afterwards: =
=E2=80=9Ce.g.,=E2=80=9D  (There are several instances of this throughout =
the draft.)

- "In case of mobile users, the user=E2=80=99s 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.=E2=80=9D=


This is hard to parse. I suggest moving =E2=80=9Cmid-session=E2=80=9D to =
earlier in the sentence. For example, =E2=80=9C=E2=80=A6 session may get =
transferred mid-session to a new Diameter client=E2=80=A6=E2=80=9D

=C2=A73.2: I gather this section is an example. If say, please state =
that explicitly.

=C2=A73.3:
- This section uses confusing language to describe which nodes can do =
what. In particular, it often says =E2=80=9Cany=E2=80=9D node, where I =
think it means =E2=80=9Ceither the client or the server=E2=80=9D. I =
assume nodes that are not a party to a session-group can=E2=80=99t =
modify it, correct? Likewise it says =E2=80=9Ceach=E2=80=9D node when I =
think it means =E2=80=9Ceither node=E2=80=9D.

- "Prerequisite for deletion
of a session group is that the Diameter node created the session
beforehand, hence the node became the group owner.=E2=80=9D

This seems like a complicated way to explain a simple concept. I suggest =
something to the effect of =E2=80=9CA session group may only be deleted =
by the Diameter node that created it.=E2=80=9D

- "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.=E2=80=9D

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.=E2=80=9D Also, the MUST is =
not really appropriate; it=E2=80=99s 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.=E2=80=9D

Please do not use normative keywords in an example; examples are by =
definition non-normative.

- Default permissions table: I am confused by the =E2=80=9Cclient=E2=80=9D=
 and =E2=80=9Cserver=E2=80=9D columns. If I understand this section =
correctly, the role of =E2=80=9Cclient=E2=80=9D and =E2=80=9Cserver=E2=80=9D=
 is not relevant. (It is telling that each row has the same value in =
both columns.) Please consider stating this in terms of group =
=E2=80=9Cowner=E2=80=9D and =E2=80=9Cnon-owner=E2=80=9D.

- Editor=E2=80=99s note: If this draft does not consider overruling a =
node=E2=80=99s 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.)

=C2=A74.1: Please avoid normative language in the form of =
=E2=80=9CSHOULD=E2=80=A6only=E2=80=A6=E2=80=9D. That can be ambiguous. =
It=E2=80=99s better to say =E2=80=9CSHOULD NOT =E2=80=A6 unless. For =
example, =E2=80=9CSHOULD NOT perform group operations with a node unless =
the node has advertised support=E2=80=9D.

=C2=A74.1.1:

- It seems to me that =C2=A74.1.1 and =C2=A74.1.2 have =E2=80=9Cexplicit=E2=
=80=9D and =E2=80=9Cimplicit=E2=80=9D reversed, in the sense that, if a =
Diameter application supports groups, then the announcement of group =
support is =E2=80=9Cimplied=E2=80=9D by the announcement of support for =
the application. OTOH, the use of Session-Group-Capability-Vector is an =
=E2=80=9Cexplicit=E2=80=9D announcement of group support.

- "New Diameter applications may consider support =E2=80=9C

s/consider/specify

- "Such applications provide intrinsic discovery for the
support of group commands and announce this capability through the
assigned application ID.=E2=80=9D

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.

=C2=A74.2:
- "It is left to the application to
determine the policy of session grouping.=E2=80=9D
Does =E2=80=9Capplication=E2=80=9D mean Diameter application, or just =
=E2=80=9Cimplementation=E2=80=9D? 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 =E2=80=9Cthe policy of session =
grouping=E2=80=9D.

- The second paragraph seems redundant with =C2=A73.3.

The phrase =E2=80=9Csingle or multiple=E2=80=9D would be easier to read =
as =E2=80=9Cone or more=E2=80=9D. (This pattern repeats throughout the =
draft, along with some instances of =E2=80=9Cone or multiple=E2=80=9D)

- =E2=80=9CA 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

=C2=A74.2.1:

- =E2=80=9C=E2=80=A6 the server MUST assign the new session to each of =
the one
or multiple identified session groups when present in =E2=80=A6=E2=80=9D

- "If the Diameter server receives a command request from a Diameter
client and the command comprises at least one Session-Group-Info AVP=E2=80=
=9D

s/comprises/includes  (=E2=80=9Ccomprises=E2=80=9D means =E2=80=9Cmade =
up of=E2=80=9D, which would suggest that the request contains only =
Session-Group-Info AVPs).  (This occurs several times throughout the =
draft.)

What does =E2=80=9Cwhen=E2=80=9D mean in context? Would =E2=80=9C...sessio=
n groups present in=E2=80=A6=E2=80=9D mean the same thing?

- "In case one or multiple identified session groups
are not already stored by the server=E2=80=9D

=E2=80=9CIn case=E2=80=9D usually refers to planning for a contingency =
(For example, =E2=80=9CI have a fire extinguisher in case of fire=E2=80=9D=
). I suggest =E2=80=9CIn the case where=E2=80=A6=E2=80=9D or even =
better, =E2=80=9CIf=E2=80=A6=E2=80=9D  (Variations of this pattern occur =
several times.)

- "the server MUST store the new
identified group(s)=E2=80=9D
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.=E2=80=9D

Hard to parse.

- "session. When the Diameter client is
confident that the Diameter server supports session grouping=E2=80=A6=E2=80=
=9D

Since this should always be true before the client sends _any_ group =
operation, why restate it here?

=C2=A74.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.=E2=80=9D

s/sessions being assigned/sessions assigned

- Please expand =E2=80=9CCCF=E2=80=9D on first mention.

- sentence starting with "When a server sends, as example, a =
Re-Authorization Request
(RAR) or a service-specific server-initiated request=E2=80=A6=E2=80=9D

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=E2=80=A6=
=E2=80=9D

=E2=80=9Cexpect=E2=80=9D is vague for use in a MUST requirement. Please =
state this in terms of concrete actions or state it descriptively.

=C2=A7A.1, first paragraph: Please don=E2=80=99t 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.









--Apple-Mail=_3946243B-46B6-4335-AD95-5485281D1B79
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org

iQIzBAEBCgAdFiEExW9rpd7ez4DexOFOgFZKbJXz1A0FAlxrnqUACgkQgFZKbJXz
1A2hWRAAxKZKpJWYqRPts0kCGM9qFIzO+nl5NJaZ6vqADSYw+vC7I+WHUbZ0xmmz
N7P6VSw4jsNGt386piwv0hUvN6AM5Kyi8t1SJIicgzh5/rwMOm9kxGzxQbSPTZOl
MlHJWWGVfoXz88JCKIea3LuETrpqAJCxT4toPOUKZsZVwGT9vRyShQMH5BM4Ttgn
J0oqwtAAGO3rmMuTmYODwuMxXdDx2TMYEA4xGduKg3g6BK0+wpHAcBFqL/PM1Nwz
JR6Qfb1mxdVJfI/J/K8vca82hR/Ofug6oPbXka91Ci6cwfXq4IZdbIBn5R2zqTVq
MGyzD6c2ZqhDMraHsrHvi0PR6UMTI9QEzLsa0TxdKBGY3MdhYH8jNvB3HDcyC0x4
SfCCUbro3Wa8mSzVALxkbjNRdu34gtfNBAnbcRCZnE8oWuG9GUYi9PdvTtKOe52Q
VZ7trlRtesMZxOHEkobPs3xFBD0eue3Roxks7sAUP1MKF7Gn9NdGCTU0HKYlntn9
2YseLMbDinMonGxcq1us0MRNc68syhzpSUAn3D34etYl2YA+Vv4mc9PKuxwMBL4I
oEWbUhnoBKK7RfFAMGZt21IR2M0WqH75h+79YNeffFVvk7DxcQE9d18qgMf/6/Hf
vMJmLYboJsKSCbsl0UCWNsrxlfuRhAJn+q6lgXAUMHAKoLoSG30=
=gDTX
-----END PGP SIGNATURE-----

--Apple-Mail=_3946243B-46B6-4335-AD95-5485281D1B79--