Last Call Review of draft-ietf-alto-protocol-25
review-ietf-alto-protocol-25-genart-lc-thomson-2014-01-26-2-00

Request Review of draft-ietf-alto-protocol
Requested rev. no specific revision (document currently at 27)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2014-02-04
Requested 2014-01-31
Other Reviews Genart Last Call review of -25 by Martin Thomson (diff)
Secdir Last Call review of -25 by Dan Harkins (diff)
Opsdir Last Call review of -25 by Jürgen Schönwälder (diff)
Review State Completed
Reviewer Martin Thomson
Review review-ietf-alto-protocol-25-genart-lc-thomson-2014-01-26-2
Posted at http://www.ietf.org/mail-archive/web/gen-art/current/msg09644.html
Reviewed rev. 25 (document currently at 27)
Review result Not Ready
Draft last updated 2014-01-26
Review closed: 2014-01-26

Review
review-ietf-alto-protocol-25-genart-lc-thomson-2014-01-26-2

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-alto-protocol-25
Reviewer: Martin Thomson
Review Date: 2014-01-25
IETF LC End Date: 2014-02-04
IESG Telechat date: ?

Summary:  This is a pretty damned good document and an impressive
achievement.  Don't let the number of issues I raise lead you to
conclude that it isn't.  It's huge and complex and does a decent job
of containing that complexity, even though at times it requires a deal
of back-and-forth to comprehend properly.  I don't think it should be
published until at least the major issues I raise have been addressed,
but those issues don't represent fundamental flaws.

Major Issues:

You can't use US ASCII encoding as you do; simply stating that the
value is a string would be better.  JSON is required to be
UTF-{8|16|32}, so mandating an alternative encoding could be a real
problem.  Same for most of the string-based types.  In most cases you
can simply s/US ASCII string/string/ safely.  This extends to the way
that specific characters are identified; this should use the U+
notation, rather than the 0x notation used.

Section 10.2 defines a resource ID.  Why do this when you could use
URIs?  After all, that's what URIs are for.  I realize that this would
be something of a major change to the protocol, but I think that the
question is particularly important.

Use of URIs has particularly benefits too.  For example, the "uses"
attribute of the IRD would be made far more easily navigable if URIs
were used.

It's unclear to me what value VersionTag (Section 10.3) has over URI +
ETag.  Section 10.3 doesn't really explain this at all, and the bulk
of the explanation is ad hoc at best.  I'll note that the way of
describing a dependency is quite roundabout and potentially
problematic.  Each resource that has a dependency has a name+tag for
each dependency.

The name is a name from an IRD, but this creates a uniqueness problem
if multiple IRDs reference the same IR.  This seems likely for cases
where adjunct services like the endpoint cost service rely on a common
network map.

On the other hand, if this were changed to URI + ETag, consistent with
my earlier recommendation, there would be no need for the extra
indirection and HTTP headers could carry version information for each
resource.  All that would be required is to identify dependencies
correctly.  Even better, a link relation.

I note also that vtag is used in filtered documents.  I think that
this is problematic because two differently filtered maps will have
the same ID, and - according to 11.3.1.6 - the same vtag.  That makes
it impossible to identify a filtered map.  Better to instead identify
each with their own URI (use Content-Location) and to separately
identify the complete map from the filtered one so that it can be made
clear that identifiers in each of the filtered maps is from the same
namespace.

Minor Issues:

I think that the document needs to be clearer with respect to how the
source and destination in a cost map are identified.  For example,
Section 6.1.2 specially calls out IP addresses as the basis for costs.
 However, the introduction to Section 6 states (though I may be
inferring this) that PID is the only basis.

Section 8.2 leaves quite a few questions unanswered.  For instance, is
'object' and 'object-map' the only reserved words?  How is Type2
actually defined as a string?  How might I define Type4 as an array?
I'm not expecting formal grammar or a rigorous definition, but the
definition is a wee bit sparse.

The MUST in Section 8.3.3 isn't really necessary.  Particularly when
it is so immediately contradicted.

Section 8.3.4.3 isn't exhaustive, obviously, but it does highlight
just two of the major fallback techniques:
 * ask another server
 * manage without the information (since it's basically advisory in
most use cases anyway)
The third option is to find an alternative way to obtain the desired
information, since the protocol provides redundancy at more than the
resource level.  For example, a complete cost map could be requested
if the endpoint cost service fails.  This would be suboptimal, but
probably better than managing without.

Section 8.3.7 seems odd.  At the server, it would be easier to require
that cookies not be set.  A client is likely going to use a general
purpose client for making requests, so cookies set by the server are
probably going to be returned as per spec.  In either case, forcing
cookies to be ignored is potentially restrictive on the server choices
(authentication springs to mind here) or requiring of special work on
the client.  Either requires greater justification than this document
provides.

In Section 8.4.1, why not define ResponseMeta as an object with an
optional "code" attribute and all of the other optional "meta"
occupants you define?  Otherwise, this object is a non-definition of
sorts.

Section 10.6 and 10.8.2 define a prefixing rule for cost metric type
and endpoint properties.  This is contradictory to the received wisdom
of RFC 6648 and I encourage you not to define these special rules.
(Note: "priv:" =~ "x-", "exp:" == "x=")

With that, I think that it would be appropriate to lower the
registration requirements to "Expert Review".  "IETF Review" is a high
bar to set.

I am not convinced that a namespace system based on a period ('.') is
necessary when the CostType object can be expanded with additional
attributes.  Removing the significance of ':' and '.' (and either
allowing as regular characters or forbidding entirely) would be both
sufficient and considerably simpler.

I found it quite difficult to work out what an endpoint property was.
The explanation is a little lacking here.  I also think that the
structure could be greatly improved.

Furthermore, I find the construction of identifiers to be brittle.
The following would simpler in my opinion:

Request:
  {
    "properties" : [ "pid",
                     "my-example-prop" ],
    "endpoints"  : [ "ipv4:192.0.2.34",
                     "ipv4:203.0.113.129" ],
    "network-maps": [ "

http://example.com/my-default-network-map&quot

; ]
  }

Response:
    {
    "meta" : {
      "depends" : { "

http://example.com/my-default-network-map":

 "xyzzy" }
    },
    "endpoint-properties": {
      "ipv4:192.0.2.34"    : { "pid": {
"

http://example.com/my-default-network-map":

 "PID1" }
                               "my-example-prop": "1" },
      "ipv4:203.0.113.129" : { "pid": {
"

http://example.com/my-default-network-map":

 "PID3" } }
    }
  }

I'm tempted to suggest that there be a little consistency with respect
to network locations as well.  Sometimes type and value are in
separate fields, other times not.  (I know that this was a point of
long discussion in the WG though, so I'm less concerned here.)

Section 15.4.1 should probably mention the tracking of clients using
cookies or fingerprinting techniques.  (Is this where the Cookie
prohibition in Section 8.3.7 came from?  Because if it is, then it
should be MUST NOT send for clients as opposed to MUST NOT use for
servers.)


For Section 15.5.2, a server could also limit the size of M and N that
it is happy to accept a request for.

Is Appendix C normative?  It looks like it, but it's insufficiently
specified, particularly with respect to the formatting of "field" to
be normative.

Nits:

There's a bunch of lowercase "may" and "should" throughout.  I know
that opinions differ on the use of these in combination with 2119
lanauge, but I'll note that some of the statements appear to be
normative.

Later in p3, the SHOULD here doesn't seem particularly testable, as
phrased, though a rewording might make it so.

Section 8.2 is a little cramped.  Don't be scared of using more whitespace.

In Section 8.2, why 'JSONString' and not simply 'string'?  After all,
you use object rather than JSONObject.

Section 8.3.4.2 s/should normally be/is/

In Section 8.4.2 check the spelling of Info[r]ResourceDirectory.

Section 9.1.4, consider quoting "accepts" in "The associated accepts
attribute of an Information Resource [...]"

Last character of Section 9.2.5.1, delete ".

In Section 10.1, the definition of PID seems like an exemplar
candidate for ABNF.

In Section 11.2.1, the idea of optimizing for compactness in a
text-based format is somewhat laughable.  Compactness is best achieved
in these cases through the use of compression.  Readability is
laudable, as is accessibility to software.

Section 15.3.2, p2 s/need/needs/

Section 15.5.1 p1, missing space(s), consider capitalizing "m" and "n".