Subject: [radext] Ben Campbell's No Objection on draft-ietf-radext-ip-port-radius-ext-11: (with COMMENT)

From: "Ben Campbell" <[email-masked]>
Date: Tue, 16 Aug 2016 18:02:07 -0700

Ben Campbell has entered the following ballot position for
draft-ietf-radext-ip-port-radius-ext-11: 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-radext-ip-port-radius-ext/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I originally intended the two comments under "Major Issues" to be points
for a DISCUSS position. But I see Alissa has already captured the essence
of them in her DISCUSS. Therefore I will ballot NO OBJECTION for now, and
will watch that discussion.

Major Issues:

The guidance for IP-Port-Local-Id TLV contemplates using persistent
client identifiers, such as MAC addresses. Is that necessary? I suspect
the answer will be "yes". But if that  is the case, the draft really
needs a privacy considerations section to discuss the implications and
tradeoffs.

- 3.1.1, paragraph 7: "However, the RADIUS server is not required to
honor such a
   preference."
Please describe what it means to "not honor" such a preference? Does the
server report an error? Does the transaction succeed, but report a
different limit? Does the server just ignore the preference and do its
own thing? (I expect this applies to any number of failure conditions.)

Minor Issues:

- Please clarify the applicability to IPv4 and IPv6. Specifically, I note
that you define TLVs for "internal" IPv4 and IPv6 addresses, but only for
"external" IPv4 address. I assume that to mean that this supports Nat44
and Nat64 use cases, but not anything that uses IPv6 in the "external"
realm? (Whatever the answer, it would be helpful to have a paragraph or
two to make that explicit.)

-2, definition of "port-based device":
Wouldn’t that more properly be called a port-mapping device? (anything
using TCP or UDP would seem to be port-based)

-3.1.1, last paragraph:
This language precludes new extension packet types from using the
attribute without updating this document. Is that really the intent?
Would it make sense to say something like "... MUST NOT appear in any
other RADIUS packet types defined at the time of this writing. If a
future packet type uses the attribute, the specification of that packet
type will need to define it's usage"? (Note that this issue repeats
several times in the document.)

Editorial Comments and Nits:

- The use of the term "session" seems inconsistent. In some places it
seems like an IPFlow or a NAT mapping, but in other places it seems like
a network connection (i.e. NAS attachment.)

- 1, list item 1:
Singular/plural mismatch for "packet". Either "packet" should be
"packets", or you need an article before RADIUS. (e.g. a
RADIUS...packet.) Note that this error repeats many times in similar
patterns throughout the document.

-1, list item 2, last sentence:
Convoluted sentence. Can it be simplified?

-1, last paragraph:
It would be helpful to mention that this draft defines IPFIX elements
near the top of the section, and also in the abstract.

-2, definition of "Internal IP Address": "refers to the IP address that
is used as a
      source IP address"
Used by what device?

-2, definition of "mapping":
I cannot parse the text. (Note that while the last paragraph in the
section says this is taken from 6887 and 6888, neither of them seem to
use this specific language.)

-4.1, second paragraph: " A CGN function in a broadband network would
most likely co-located on a BNG."
There's a missing word in there somewhere. Maybe a "be" before
"co-located"?

-6, last paragraph:
The paragraph doesn't parse. Should "deployed" be "deployments"?