Dear Adam, all, Thank you for this feedback - indeed we struggled a bit with formally specifying JSON and relied on examples. I believe that with your suggestions we can improve this.
As for IP address prefix notation - yes.. we should follow your suggestion and cite RFC 4632 §3.1 for prefix-length notation (both for IPv4 and IPv6), and RFC 5952 for the syntax of IPv6 addresses. I am so used to doing it this way that it slipped my mind to specify this, but of course it should be unambiguous. As I did most of the JSON text I will take it on me to re-work this text and ask Di to merge it with the changes he is working on. There should be a -08 version coming soon. Tim > On 4 Apr 2018, at 21:22, Adam Roach <a...@nostrum.com> wrote: > > Adam Roach has entered the following ballot position for > draft-ietf-sidr-slurm-07: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > for more information about IESG DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-sidr-slurm/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > Thanks to everyone who worked on this document. The mechanism seems useful. > > I'm concerned that the document doesn't describe the file format itself; > rather, it relies on examples to provide vital, nonsupplemental information > such as the names of JSON object members, expected encodings (e.g., strings > versus numbers), and distinction between arrays and objects. I'm making this a > DISCUSS because I think the ambiguity here -- and, in particular the ambiguity > about IP address prefix notation -- will lead to non-interoperable > implementations. > > Using section §3.2 as an example: > >> A SLURM file consists of: >> >> o A SLURM Version indication that MUST be 1 >> >> o A slurmTarget element (Section 3.3) consisting of: >> >> * Zero or more target elements. In this version of SLURM, there >> are two types of values for the target: ASN or Fully Qualified >> Domain Name(FQDN). If more than one target line is present, >> all targets MUST be acceptable to the RP. >> >> o Validation Output Filters (Section 3.4), consisting of: >> >> * An array of zero or more Prefix Filters, described in >> Section 3.4.1 >> >> * An array of zero or more BGPsec Filters, described in >> Section 3.4.2 >> >> o Locally Added Assertions (Section 3.5), consisting of: >> >> * An array of zero or more Prefix Assertions, described in >> Section 3.5.1 >> >> * An array of zero or more BGPsec Assertions, described in >> Section 3.5.2 >> > > As this is the normative description of the structure, I would have expected > an > indication that the file contains a JSON object (rather than, say, a JSON > array), an indication that the version is to be encoded as a number (rather > than > a string), and clarification of what value members are expected to contain. > > For example, the following JSON object is in compliance with the preceding > normative description (and, as far as I can tell, all other normative text > in the document): > > ["1", > ["65536", "rpki.example.com"], > [ > ["192.0.2.0/255.255.255.0", "All VRPs encompassed by prefix"], > ["64496", "All VPRs maching ASN"], > ["198.51.100.0/255.255.255.0", "64497", "All VRPs encompassed by prefix, > matching ASN"] > ], > [ > ["64496", "All keys for ASN"], > ["Zm9v", "Key matching Router SKI"], > ["64497", "YmFy", "Key for ASN 64497 matching Router SKI"], > ], > [ > ["64496", "198.51.100.0/255.255.255.0", "My other important route"], > ["64496", "2001:DB8::/FFFF:FFFF::", "48", > "My other important de-aggregated routes"], > ], > [ > ["64496", "My known key for my important ASN", > "<some base64 SKI>", "<some base64 public key>"] > ] > ] > > Fixing this should be pretty easy; the document simply needs text added that > describes the JSON structure explicitly, with clear indications of how values > are to be encoded. For example, the preceding text I quote becomes: > > A SLURM file consists of a single JSON object containing the following > members: > > o A "slurmVersion" member that MUST be set to 1, encoded as a number > > o A "slurmTarget" member (Section 3.3) If more than one target line is > present, all targets MUST be acceptable to the RP. The "slurmTarget" > member is encoded as an array of zero or more objects. Each object in the > array contains exactly one member. In this version of SLURM, the member > may be named either: > > * "asn", in which case it contains an ASN, or > > * "hostname", in which case it contains a Fully Qualified Domain > Name (FQDN). > > o A "validationOutputFilters" member (Section 3.4), whose value is an > object. The object MUST contain exactly two members: > > * A "prefixFilters" member, whose value is described in > Section 3.4.1 > > * A "bgpsecFilters" member, whose value is described in > Section 3.4.2 > > o A "locallyAddedAssertions" member (Section 3.5), whose value is an > object. The object MUST contain exactly two members: > > * A "prefixAssertions" member, whose value is described in > Section 3.5.1 > > * A "bgpsecAssertions" member, whose value is described in > Section 3.5.2 > > > Gotchas to watch out for include: > > - If you're using the word "element" to describe something in a JSON object, > you probably need to find a more specific word. This document, for example, > uses "element" instead of "member" in most places. > > - Everywhere you use the word "structure," replace it with either "array" or > "object," as appropriate. > > - When values can be encoded as either a number or a string (e.g., as with > "slurmVersion" above, or with AS numbers), indicate which encoding is > expected. > > - For IP prefixes, be clear about acceptable syntax. For example: is > the RFC 950 syntax ("192.0.2.0/255.255.255.0") acceptable? My suggestion is > to cite RFC 4632 §3.1 for prefix-length notation (both for IPv4 and IPv6), > and RFC 5952 for the syntax of IPv6 addresses. > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > The remaining comments are in document order. > > --------------------------------------------------------------------------- > > Title: > > It seems odd to use the stylized capitalization (e.g., "nUmber") without > following it by the "SLURM" acronym. Consider adding "(SLURM)" to the title. > > --------------------------------------------------------------------------- > > §3.1: > >> This document describes responses in the JSON [RFC8259] format. > > I don't think this means to say "responses," does it? It appears to be > describing a JSON document rather than a request/response protocol. > > > --------------------------------------------------------------------------- > > §3.3: > >> A SLURM file MUST specify a "slurmTarget" element that identifies the >> environment in which the SLURM file is intended to be used. The >> "slurmTarget" element MAY have an empty array as its value, which >> means "applies to all". The meaning of the "slurmTarget" element, if >> present, is determined by the user. If a "slurmTarget" element is >> present, an RP SHOULD verify that the target is an acceptable value, >> and reject this SLURM file if the "slurmTarget" element is not >> acceptable. Each "slurmTarget" element contains merely one "asn" or >> one "hostname". An explanatory "comment" MAY be included in each >> "slurmTarget" element so that it can be shown to users of the RP >> software. > > When reworking this paragraph in particular, please be careful to distinguish > between the "slurmTarget" member and the elements in the array that > constitutes > its value. The preceding text calls both of these things '"slurmTarget" > element,' which is very confusing. > > > _______________________________________________ > sidr mailing list > sidr@ietf.org > https://www.ietf.org/mailman/listinfo/sidr _______________________________________________ sidr mailing list sidr@ietf.org https://www.ietf.org/mailman/listinfo/sidr
