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

[<[e-mail]>]

Dear Ben, 

Thank you for the review. 

A new revision to integrate your comments is available online. FWIW, a diff to track the changes: https://www.ietf.org/rfcdiff?url1=draft-ietf-radext-ip-port-radius-ext-12&url2=draft-ietf-radext-ip-port-radius-ext-13 

Please see inline for more details.

Cheers,
Med

> -----Message d'origine-----
> De : Ben Campbell [mailto:[e-mail]]
> Envoyé : mercredi 17 août 2016 03:02
> À : The IESG
> Cc : [e-mail]; [e-mail];
> [e-mail]; MORAND Lionel IMT/OLN; [e-mail]
> Objet : Ben Campbell's No Objection on draft-ietf-radext-ip-port-radius-
> ext-11: (with COMMENT)
> 
> 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.

[Med] NEW text was added in -12. I will skip this point as it is discussed in other threads.

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

[Med] Actually, that sentence is redundant with the previous one which explicitly says: "preferred maximum number". This means that the client may indicate a preference that may be, by definition, consider or ignored by the RADIUS server. The RADIUS server can return that value or another one in the response back to the client. The document does not include any text this as there is nothing to be done at the client side other than enforcing the limit that will be communicated by the RADIUS server.

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

[Med] This NEW text was added: 

	This document focuses on IPv4 address sharing.  IPv6 prefix sharing	
	mechanisms (e.g., NPTv6) are out of scope.

> 
> -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)

[Med] changed to "address sharing device"

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

[Med] We are following the same language as usually done in RADIUS specifications. No change is needed, IMHO.

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

[Med] replaced "session" with "mapping" when appropriate. 

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

[Med] Fixed.

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

[Med] Updated the text to:

OLD:
   2.  IP-Port-Range: This attribute may be carried in RADIUS
       Accounting-Request packet.  The purpose of this attribute is to
       report by an address sharing device (e.g., a CGN) to the RADIUS
       server the range of IP source transport ports that have been
       allocated or deallocated associated with a given IPv4/IPv6
       address for a user.

NEW:
   2.  IP-Port-Range: This attribute may be carried in a RADIUS
       Accounting-Request packet.  The purpose of this attribute is for
       an address sharing device (e.g., a CGN) to report to the RADIUS
       server the range of IP source transport ports that have been
       allocated or deallocated for a user.  The port range is bound to
       an external IPv4 address.

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

[Med] Added this NEW text in the abstract:

    This document defines a mapping between some RADIUS attributes and	
	IPFIX Information Element Identifiers.

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

[Med] Yes, text updated accordingly.

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

[Med] Changed to:

   o  Mapping: denotes a relationship between an internal IP address,
      internal port and the protocol, and an external IP address,
      external port, and the protocol.

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

[Med] Fixed.

> 
> -6, last paragraph:
> The paragraph doesn't parse. Should "deployed" be "deployments"?
> 
[Med] Fixed. Thank you.