Document: draft-ietf-ippm-storetraceroutes-07 Reviewer: Pasi Eronen Review Date: 2008-01-10 IETF LC End Date: 2008-01-15 IESG Telechat date: (not known yet) Summary: This draft is on the right track but has open issues, described in the review. Comments: 1) A general comment: The document would be much easier to read if Section 4 included a sample traceroute output, and some appendix included an example XML file matching the schema. 2) The information model doesn't describe the relationships between the elements: e.g. the information elements in 5.2.2 are just a list of information elements, without good specification on how they're grouped, how many times they can occur, etc. (This information is obviously present in the XML schema)." (See e.g. RFC 5070 for an example of an information model that describes not just the individual elements, but their relationships. I'm not necessarily looking for anything that fancy, but something along the lines "The Measurement class contains one ResultsStartDateAndTime element, and one or more ResultsProbe elements. Each ResultsProbe element contains..." Also, information about which elements are always present and which may be omitted is clearly part of the information model.) 3) The information model contains elements OSName, OSVersion, and ToolVersion, but no ToolName element. While clearly many operating systems ship with a single well-defined traceroute-like tool, additional tools (beyond those shipping with the OS) are available. Thus, I'd recommend adding a ToolName element. (This would also help in interpreting implementation-dependent information elements, such as CtlMiscOptions.) 4) Section 5.2.2.9 specifies the HopGeoLocation element, but not its contents, semantics, or methods used to determine it. We do have IETF standards for representing both geospatial and civil locations (e.g. RFC 3825 and 4476); presumably they could be used. Like was done for the AS number, it might be useful to also specify the method that was used to get the location. 5) Section 5.2.3.1 states that the TestName element is "locally unique"; without some definition of "locally", this is meaningless. In RFC 4560, the scope is clear: it's unique "within the scope of a traceRouteCtlOwnerIndex" (of a specific Management Information Base, running on particular node -- this part of the scope being communicated by SNMP). 6) Section 5.1, InetAddressType: "The allowed values are to be intended as imported from [RFC4001]; an additional allowed value is "asnumber". At least the XML schema later doesn't seem to allow all values from RFC 4001 (in particular, ipv4z and ipv6z); perhaps the intent was to import only some of the values? 7) Section 5.2.1.7 defines the CtlPort as "Specifies the base UDP port used by the traceroute measurement. A port that is not in use at the destination (target) host needs to be specified." This description needs some clarification, as different traceroute tools may function slightly differently here. At least some common implementations use a range of port numbers (selecting different port number for each probe), and thus it's not sufficient to specify "a port that is not in use"; several other ports need to be not in use as well. Other implementations may (at least optionally) use the same port number for all probes (e.g. "-c" option in OpenBSD traceroute). And for TCP measurements, you may want to use a port that is in use. 8) Section 5.2.2.14: The ResultsHopRawOutputData should contain some description of what's expected to be contained (beyond just saying "raw output data"). Appendix C.2 suggests that this element is similar to traceRouteProbeHistoryLastRC in RFC 4560, but at least that contained an integer code. For example, is this the line printed by traceroute tool (expected to be printable ASCII useful for humans); the raw ICMP packet received (in which case it isn't UTF8, and needs to use xs:base64Binary data type in XML schema), or something else? (Even "an implementation-dependant printable string, expected to be useful for a human interpreting the traceroute results" would be much more detailed specification.) 9) Section 5.2.1.18 defines just three fixed values for CtlType (UDP, TCP, and ICMP); these are also used in the XML schema, with no possibility for extension. This is in contrast with RFC 4560, where CtlType is extensible. The most common cases (UDP packet to unused port, and ICMP Echo Request) are pretty clear, but other alternatives are possible. E.g. as described in Appendix A.1, for TCP, you might use SYN packets or FIN packets -- which would lead to different results in the presence of stateful firewalls (pretty common). Also, for UDP, you could use an UDP port which is in use (and try to get the application to respond, instead of getting ICMP Port Unreachable); even RFC 4560 supports UDP pings with "echo" protocol and SNMP query (and I have a vague recollection of once seeing a tool that used DNS queries, but can't really recall anything certain). Given this, I'd recommend at least documenting that other possibilities (beyond the ones described) here exist, and having some sort of idea how they would fit in the XML. 10) Inconsistency between XML schema and information model: the schema's inetAddressTypeWithoutDns type contains a "noSpecification" element, which is not present in inetAddressType type, or in information model. 11) Inconsistency between XML schema and information model: _ipASNumberMappingType type element (and its semantics) is missing from the information model of Section 5. 12) Inconsistency between XML schema and information model: information model describes CtlByPassRoute and CtlDontFragment as truth value (true/false), but the XML schema allows three different options (true, false, and omit the whole XML tag). The semantics of the last option are not defined in the information model. 13) Inconsistency between XML schema and information model: similar concerns apply to several other data elements that have minOccurs="0": the information model should specify what omitting the element means. For some elements (e.g. HopGeoLocation) this is pretty obvious; for others (at least CtlProbeDataSize, CtlTimeout, CtlProbesPerHop, CtlPort, CtlMaxTtl, CtlDSField, CtlInitialTtl, and CtlType), some kind of semantics should be defined (e.g. for MaxTtl, possible semantics could be "30" (this is the definition used in RFC 4560 -- but in that case, making the field mandatory to include would be simpler), "implementation dependent default", "not known", or "no maximum (except 255)"). 14) XML schema, _dateAndTime type: this type seems unnecessary, as the basic xs:dateTime type (which is used as a component here) can represent time with millisecond resolution (and if milliseconds were included twice, that would be confusing). 15) XML schema: Many of the strings (TestName, OSName, OSVersion, ToolVersion, CtlMiscOptions, CtlDescr, HopGeoLocation) have maxLength restrictions which seem rather arbitary and small. For example, for OSVersion, one obvious choice on Linux would be the output of "uname -rv" (possibly combined with something like contents of /etc/redhat-release), but that's easily over 32 characters. Also, the maximum lengths don't match the length restrictions in RFC 4560 (e.g. CtlMiscOptions is max. 255 octets in RFC 4560, but 100 characters here). (I took a quick look at recent RFCs which contain XML schemas, and a common practice seems to be omitting maxLength restrictions for fields that can contain arbitrary text.) 16) The XML schema contains the concept if "request", in addition to storing measurements of traceroutes that have already happened. The information model in Section 5 should probably mention that the Configuration Information Elements can describe not just traceroute measurements that have already happened, but also configuration to be used when requesting a measurement to be made (quite different semantically, even if the individual information elements are the same). 17) The document uses RFC 2119 key words, but does not include RFC 2119 in the references section. 18) Minor nits 5.1, typo in "Unsigned8 - The type "Unsigned32" represents a value" 5.2.1.7, CtlPort should be Unsigned16 instead of Unsigned32 5.2.1.8, CtlMaxTtl should be Unsigned8 instead of Unsigned32 5.2.1.9: CtlDSField should be Unsigned8 instead of Unsigned32 5.2.1.14, CtlMaxFailures should be Unsigned8 instead of Unsigned32 5.2.1.16, CtlInitialTtl should be Unsigned8 instead of Unsigned32 5.2.2.5, HopIndex should be Unsigned8 instead of Unsigned32 5.2.2.6, IndexPerHop should be Unsigned8 instead of Unsigned32 XML schema, _inetAddressASNumber type "as a 24 bit number": old BGP used 16-bit AS numbers, now extended to 32 bits. Typo? The remaining comments about schema details are, to some extent, style issues, and thus just my opinion as to what would be "nicer" way to do this: 19) XML schema: there are several places where it uses a combination of xs:complexType+xs:sequence+xs:element, where a single xs:simpleType would have been sufficient (and would lead to simpler XML). For example, ...something... could be simply ...something.... I would recommend "flattening" these when possible (at least _ResultsStartDateAndTime, _ResponseStatus, _Time, _ResultsEndDateAndTime -- see also the next comment about flattening addresses). 20) XML schema: The representation of InetAddresses is particularly complex (with several levels of nesting), and also encodes the address type in two different places (giving opportunities for mismatches, which can't be easily constrained with the schema). If I'm reading the schema right, the XML would look like this: ipv4 192.0.1.3 But exactly the same information model could also lead to this much simpler XML: 192.0.1.3 21) XML schema: _probeRoundTripTimeNotAvailable type uses the string "NotAvailable", while 5.2.2.11 uses the string "RoundTripTimeNotAvailable". Also, the encoding is a bit redundant: NotAvailable could be simply 22) XML schema: The definition of _traceRoute type is pretty complex and long; it seems that something along this lines would have the same effect?