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" ; ] } 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".