Dear all,

please find attached a file where I kept track of the issues raised
and how we addressed them.

There are some clear open issues:
-- XML file example
-- style issues for the XML schema
that we are going to address soon (hopefully before IETF still), Thomas
offered to help.

As for the other issues raised again by Pasi, please find comments inline
since i have some questions.

> My comment #1 (document would be much easier to read if there 
> was e.g. some appendix with an example XML file matching the 
> schema) still stands.

Right, we know and we will address it in -09.

> About #4: the document now uses the DHCP binary format for 
> coordinate-based location; however, just saying "represented 
> according to [RFC3825]" is not sufficient (e.g., the LCI 
> format in RFC3825 Section 2 includes the DHCP option code -- 
> I guess we don't want to include that here?) Also, it's a bit 
> weird to use the binary DHCP format since XML formats for 
> location information also exist; at least a short rationale 
> (one or two sentences) describing why they were not used 
> would be helpful. (Also, if the binary format is used, it 
> can't be "xs:string" for obvious reasons.)  

I feel confused now, you were suggesting using RFC3825 as a reference
and now you complain about this choice.
Please tell us what you would like ot see there and we will change
accordingly. Also if you wnat to see an XML format for location
information please suggest a reference to use.

> I also suggested that it might be useful to also specify the 
> method that was used to get the location -- if that's not 
> done, perhaps a sentence describing that "the method(s) used 
> to determine the location of the hop are beyond the scope of 
> this specification, and not includedin the measurement 
> result" would be in order.

I will add the suggested text:
"the method(s) used to determine the location of the hop are beyond
the scope of this specification, and not includedin the measurement
result"

> About #5: the text was clarified to say that "[TestName] is 
> locally unique, within the scope of a specific host, 
> initiator of the traceroute measurement". This doesn't really 
> solve the problem, since the XML file doesn't necessarily 
> include the identity of the host (and TestName can occur in 
> requests as well, so there's little chance of actually 
> ensuring it's unique). Also, this complicates mapping of 
> RFC4560 data to this format, since the MIB uses a different 
> definition of scope.

I will adapt the scope and the XML schema to be consistent to
the RFC4560.

> About #6: Section 5.1 now says for 
> InetAddressType/InetAddress "The allowed values are to be 
> intended as imported from [RFC4001] (where the intent was to 
> import only some of the values)". This does not really 
> describe what the allowed values are ("some" is too vague).

I will detail that more.

> About #9: Given that we already know about other types, 
> CtlType really needs to be extensible (as it is in RFC 4560), 
> beyond just "others" (which is useless in e.g. requests).

Can you suggest type? Otherwise in absence of suggestion I will
leave it as it is.
traceRouteCtlType in RFC 4560 says:
"he value of this object may be selected from 
traceRouteImplementationTypeDomains"
Can you please express better what you mean?

> About #10: The information model (Section 5.1) now mentions 
> the "noSpecification" InetAddressType, but its meaning is not 
> described (in particular, it's not obvious what's the 
> difference between "unknown" and "noSpecification").

I will add a sentence.

> About #16: Section 5.2.1 now describes the "request" concept, 
> but it's not obvious what the semantics of some of the 
> information elements are. For example, what does "OSVersion" 
> mean when it's included in a request? (One plausible answer 
> is "it can't be included in request", but that's not what the 
> information model or XML schema currently
> says.)  Other information elements where IMHO possible 
> ambiguity exists are OSName, ToolVersion, ToolName, and CtlIfIndex.

Would you suggest to exclude those from the "request", we can do that.
 
> About #18: _CtlPort has now wrong restriction in XML schema

I will change it to UnsignedInt

> About comments #19...#21: none of these has been addressed in 
> any way. There were to some extent style issues, so it's not 
> absolutely required to address them, but I hope at least some 
> kind of discussion has occurred confirming that what's 
> currently in the document is what we want it to be.
> 
> About #20: The schema still duplicates the address type in 
> two places, and does not enforce that they're consistent. If 
> the duplication is really necessary, the schema should 
> enforce consistency (with appropriate use of 
> xs:choice/xs:group/xs:sequence).
> 
> About #21: There's still an inconsistency between the 
> information model and XML schema about what string is used to 
> describe the case where roundtrip time is not available.

Style comments are going to be addressed in -09
 
> Additional comments for -08:
> 
> 23) CtlBypassRouteTable description now says "Please refer to 
> SO_DONTROUTE for more explanations regarding this." A better 
> reference would be helpful (i.e., "refer to description of 
> SO_DONTROUTE in [something] for more...") 

ok, will do that.

Best regards,
Saverio

============================================================
Dr. Saverio Niccolini
Senior Researcher
NEC Laboratories Europe, Network Research Division      
Kurfuerstenanlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-118
Fax:     +49 (0)6221 4342-155
e-mail:  [EMAIL PROTECTED] <-- !!! NEW ADDRESS !!!
============================================================
NEC Europe Limited Registered Office: NEC House, 1 Victoria
Road, London W3 6BL Registered in England 2832014
 
Security review of draft-ietf-ippm-storetraceroutes
--Paul Hoffman, Director
--VPN Consortium

On a completely non-security plane, I wonder why the authors (and
probably the whole WG) is using XML Schemas instead of RelaxNG to
do data modelling. My first guess is that RelaxNG would make the
model itself much easier to read. The recent discussion in the 
Applications Area showed a strong preference for RelaxNG over
XML Schema because of readability and because there are many
data constructs that can be expressed precisely in RelaxNG that
cannot be expressed unambiguously in XML Schema; the IPPM folks
might want to look at this.
**********************Answer************************************
the WG discussed extensively the use of binary
or textual representation. The XML textual representation was
chosen becasue it has compatibility with the OGF (Open Grid Forum)
approach. Keeping the IETF and the OGF standard was a requirement
of this work.
****************************************************************

Gen-ART review of draft-ietf-ippm-storetraceroutes-07
Document: draft-ietf-ippm-storetraceroutes-07
Reviewer: Pasi Eronen (Nokia)
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.
**********************Answer************************************
!!!pending 
****************************************************************

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.)
**********************Answer************************************
done
****************************************************************

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.)
**********************Answer************************************
done
****************************************************************

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.
**********************Answer************************************
added reference to RFC 3825, I did not find RFC 4476 relevant.
I do not think it is useful to know how the method used to get
the geo 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). 
**********************Answer************************************
added the scope, the TestName element is now locally unique,
within the scope of a specific host, initiator of the traceroute
measurement. 
****************************************************************

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?
**********************Answer************************************
yes, the intent was to import only some of the values, added
text saying it explicitly
****************************************************************

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.
**********************Answer************************************
The text that was used was taken by RFC 4560. Anyway, I
removed "A port that is not in use at the destination (target)
host needs to be specified". The idea was only to specify the 
base port
****************************************************************

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.)
**********************Answer************************************
added the text suggested: "an implementation-dependant printable
string, expected to be useful for a human interpreting the
traceroute results"  
****************************************************************

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.
**********************Answer************************************
added "others" as fourth possibility  
****************************************************************

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.
**********************Answer************************************
added text to inetAddressType data type to say that "noSpecification"
is an additional possibility  
****************************************************************

11) Inconsistency between XML schema and information model:
_ipASNumberMappingType type element (and its semantics) is 
missing from the information model of Section 5.
**********************Answer************************************
added ipASNumberMappingType type and its semantics to the data
types
****************************************************************

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.
**********************Answer************************************
changed CtlByPassRouteTable and CtlDontFragment, they are no
more optional
****************************************************************

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)").
**********************Answer************************************
changed the elements indicated in a way that they are mandatory
to inlcude, this inconsistency was a result of the fact that we
recently removed default values and the XML schema was not updated
consequently
****************************************************************

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).
**********************Answer************************************
right, changed _dateAndTime to be just xs:dateTime
****************************************************************

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.)
**********************Answer************************************
it was arbitrary, now changed to be 255, which is bigger, but
we wanted to keep limited to avoid overflows, if the limitation
needs to be removed I will change it in the next version
****************************************************************

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).
**********************Answer************************************
added text explaining this as suggested
****************************************************************

17) The document uses RFC 2119 key words, but does not include 
RFC 2119 in the references section.
**********************Answer************************************
fixed
****************************************************************

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?
**********************Answer************************************
fixed
****************************************************************


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,

  <ResultsStartDateAndTime>
    <dateAndTime>
      ...something...
    </dateAndTime>
  </ResultsStartDateAndTime>

could be simply

  <ResultsStartDateAndTime>
     ...something....
  </ResultsStartDateAndTime>

I would recommend "flattening" these when possible (at least
_ResultsStartDateAndTime, _ResponseStatus, _Time,
_ResultsEndDateAndTime -- see also the next comment about 
flattening addresses).
**********************Answer************************************
!!!pending -- just style issues
****************************************************************

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:

  <CtlTargetAddressType>
     <targetAddressType>ipv4</targetAddressType>
  </CtlTargetAddressType>
  <CtlTargetAddress>
    <targetAddress>
      <inetAddressIpv4>192.0.1.3</inetAddressIpv4>
    </targetAddress>
  </CtlTargetAddress>

But exactly the same information model could also lead to this
much simpler XML:

  <CtlTargetAddress>
    <inetAddressIpv4>192.0.1.3</inetAddressIpv4>
  </CtlTargetAddress>
**********************Answer************************************
!!!pending -- just style issues
****************************************************************

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:

  <RoundTripTime>
    <probeRoundTripTimeNotAvailable>
      NotAvailable
    </probeRoundTripTimeNotAvailable>
  </RoundTripTime>

could be simply
  
  <RoundTripTime>
    <probeRoundTripTimeNotAvailable />
  </RoundTripTime>
**********************Answer************************************
!!!pending -- just style issues
****************************************************************

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?

  <xs:sequence minOccurs="1">
    <xs:element name="Request"
                type="_Metadata" minOccurs="0" />
    <xs:element name="MeasurementMetadata"
                type="_Metadata" minOccurs="0" />
    <xs:element name="Measurement"
                type="_Measurement" minOccurs="0" />
  </xs:sequence>
**********************Answer************************************
changed
****************************************************************

Last Call: draft-ietf-ippm-storetraceroutes  (Information Model and XML
Data Model for Traceroute  Measurements) to Proposed Standard
Carlos Pignataro (CISCO)

Please find a couple quick comments on draft-ietf-ippm-storetraceroutes.
These are not result of an extensive review, but rather focused checks.
I hope you find them useful:

1. The "MPLSTopLabel" element encoding allows for a single Label Stack
    Element, although traceroute can return a complete stack comprising
    multiple LSEs [RFC 4950]; why limit this to the single top one? See
    [1]

    1. In addition, the name of the element is a misnomer, as the 32-bit
       value encodes the label + Exp + EOS + TTL and not only the label.
**********************Answer************************************
fixed
****************************************************************


2. Why is the AS Number a choice/option of Inet Address? (BTW, ipv4z and
    ipv6z are not included). Shouldn't this be part of the probe result,
    separate from the inet address?
       -A: Report AS# at each hop (from GRR)

    1. Also, why the ASN as a "24 bit number" (and not 32-bit)?
e label.
**********************Answer************************************
the intent was to import only some of the values, added
text saying it explicitly, fixed the ASN to "32 bit number"
****************************************************************

3. There are other common outputs reported by some traceroute
    implementations, such as DNS owner and DS/TOS (or TOS change).
    Should they be included?
       -O: Report owner at each hop (from DNS)
**********************Answer************************************
there was no request to include them nor the RFC 4560 includes
them, it was felt not necessary
****************************************************************


4. Should there be a Toolname (e.g., "TrACESroute", "traceroute-nanog",
    "tcptraceroute", "tracert", in addition to ToolVersion, or is it
    expected that ToolVersion contains the name)?
**********************Answer************************************
ToolName has now been added
****************************************************************

5. CtlType allows for UDP, TCP and ICMP probes (the most common ones),
    but many traceroute implementations allow for any protocol number:
       -I: use this IP protocol instead of UDP
    Is this too constraining, and should any IP protocol # be allowed?
    Are there special config flags needed for the TCP version (e.g., -SAE
    for Syn, Ack, ECN, use of Fin, etc)?

    1. CtlPort seems to apply only to UDP ("units - UDP Port"), how can
       the TCP port be configured?
**********************Answer************************************
Fixed, now an element "others" was added, also removed the dependency
from UDP
****************************************************************


Minor comments:

1. The CtlBypassRouteTable is SO_DONTROUTE, right? Should that be
    clarified? No options for a fill pattern, record route, etc? Or
    options like these?
       -a: Abort after 10 consecutive hops without answer
       -M: Do RFC1191 path MTU discovery
       -Q: Report delay statistics at each hop (min/avg+-stddev/max) (ms)
       -U: Go to next hop on any success


Examples:

[1]
14  customer123-141-193.iplannetworks.net (200.123.141.193)  193 ms  194 ms  
193 ms
     MPLS Label=24 Exp=0 TTL=1 S=0
     MPLS Label=126 Exp=0 TTL=1 S=1
**********************Answer************************************
Added text saying that CtlBypassRouteTable is SO_DONTROUTE
****************************************************************
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to