Telechat Review of draft-ietf-softwire-map-radius-22
review-ietf-softwire-map-radius-22-intdir-telechat-volz-2019-05-13-00

Request Review of draft-ietf-softwire-map-radius
Requested rev. no specific revision (document currently at 26)
Type Telechat Review
Team Internet Area Directorate (intdir)
Deadline 2019-05-15
Requested 2019-04-23
Requested by Éric Vyncke
Authors Sheng Jiang, Yu Fu, Chongfeng Xie, Tianxiang Li, Mohamed Boucadair
Draft last updated 2019-05-13
Completed reviews Intdir Telechat review of -22 by Bernie Volz (diff)
Opsdir Telechat review of -22 by Al Morton (diff)
Genart Last Call review of -23 by Joel Halpern (diff)
Opsdir Last Call review of -23 by Al Morton (diff)
Secdir Last Call review of -23 by Donald Eastlake (diff)
Genart Telechat review of -24 by Joel Halpern (diff)
Assignment Reviewer Bernie Volz
State Completed
Review review-ietf-softwire-map-radius-22-intdir-telechat-volz-2019-05-13
Posted at https://mailarchive.ietf.org/arch/msg/int-dir/eOKvrVJe9bWZE1bdFyRgq3VElfc
Reviewed rev. 22 (document currently at 26)
Review result Ready with Issues
Review completed: 2019-05-13

Review
review-ietf-softwire-map-radius-22-intdir-telechat-volz-2019-05-13

I am an assigned INT directorate reviewer for draft-ietf-softwire-map-radius-22. These comments were written primarily for the benefit of the Internet Area Directors. Document editors and shepherd(s) should treat these comments just like they would treat comments from any other IETF contributors and resolve them along with any other Last Call comments that have been received. For more details on the INT Directorate, see https://datatracker.ietf.org/group/intdir/about/.

This draft looks pretty good but there are a few quickly fixed issues and a bunch of minor nits. But, otherwise the draft looks ready to move forward.

Issues:

Section 3.1.3.1

I think the following text is in error:
   Defining multiple TLV-types achieves the same design goals as the
   "Softwire46 Rule Flags" defined in Section 4.1 of [RFC7598].  Using
   TLV-type set to 4 is equivalent to setting the F-flag in the
   OPTION_S46_RULE S46 Rule Flags field.
It should say (s/ 4 / 5 /):
   Defining multiple TLV-types achieves the same design goals as the
   "Softwire46 Rule Flags" defined in Section 4.1 of [RFC7598].  Using
   TLV-type set to 5 is equivalent to setting the F-flag in the
   OPTION_S46_RULE S46 Rule Flags field.
(I assume that "setting the F-flag" means setting it to 1.)

I'm also not sure what the following means:
	     5 Forwarding Permitted Mapping Rule (may be used for
	        forwarding. Can also be a Basic Mapping Rule)
Shouldn't this just be:
	     5 Forwarding Permitted Mapping Rule

FYI - The text in RFC7598 is:
   o  F-flag: 1-bit field that specifies whether the rule is to be used
      for forwarding (FMR).  If set, this rule is used as an FMR; if not
      set, this rule is a BMR only and MUST NOT be used for forwarding.
      Note: A BMR can also be used as an FMR for forwarding if the
      F-flag is set.  The BMR is determined by a longest-prefix match of
      the Rule IPv6 prefix against the End-user IPv6 prefix(es).

Section 5:
The "CoA-Request" message is not mentioned in this table, but was mentioned in 3.1:
      The Softwire46-Configuration Attribute MAY appear in a CoA-Request
      packet.
It may also be appropriate to include a table number/title?


Minor Nits:

Section 3.1:
	s/ efer / refer /

Section 3.1.2:
	Remove the 0+ definition under Table 2 as it is not used and therefore not needed.

Section 3.2:
	s/ orderd / ordered /
	s/ attribute include one or / attributes includes one or /		(use includes)

Section 3.3: Suggestion
	It may be more consistent and shorter to combine "MAY appear", "MAY contain" rules? For example:

      The Softwire46-Multicast Attribute MAY appear in an Access-Request,
      Access-Accept, CoA-Request, and Accounting-Request packet.

      The Softwire46-Multicast Attribute MAY contain ASM-Prefix64 (see
      Section 3.3.1), SSM-Prefix64 (see Section 3.3.2), and U-Prefix64 (see
      Section 3.3.3) attributes.

Section 4:
	In 4, s/Theses are/These are/
	In 5, s/CE send a/CE sends a/

Appendix A.7:
	The "TLV Field" column is a bit odd since these are really subfields from RFC8044.
	So, rename "TLV Subfield"? And, the fields are "Prefix-Length" and "Prefix" from
	the TLV attribute.

- Bernie

-----Original Message-----
From: Éric Vyncke via Datatracker <noreply@ietf.org> 
Sent: Tuesday, April 23, 2019 1:20 PM
To: Bernie Volz (volz) <volz@cisco.com>; Carlos Bernardos <cjbc@it.uc3m.es>
Subject: intdir Telechat Review requested: draft-ietf-softwire-map-radius


Telechat review of: draft-ietf-softwire-map-radius (no specific version)
Deadline: 2019-05-15
Requested by: Éric Vyncke

https://datatracker.ietf.org/doc/draft-ietf-softwire-map-radius/reviewrequest/11924/login/

intdir Telechat Review requested: draft-ietf-softwire-map-radius