Hi Russ, Thanks for the very nice review. Please see our responses inline.
Thanks, Jensen On Thu, Aug 19, 2021 at 10:40 PM Russ Housley via Datatracker < [email protected]> wrote: > Reviewer: Russ Housley > Review result: Almost Ready > > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed > by the IESG for the IETF Chair. Please wait for direction from your > document shepherd or AD before posting a new version of the draft. > > For more information, please see the FAQ at > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > Document: draft-ietf-alto-cdni-request-routing-alto-16 > Reviewer: Russ Housley > Review Date: 2021-08-19 > IETF LC End Date: 2021-08-30 > IESG Telechat date: unknown > > Summary: Almost Ready > > > Major Concerns: > > Section 2.2, "Security" bullet: it says: > > o Security: The identification between uCDNs and dCDNs is an > important requirement. ALTO maps can be signed and hence provide > inherent integrity protection. Please see Section 8. > > Section 8 does not talk about digital signatures. Please add this > discussion to Section 8. In addition, if the digital signature is done > well, it would provide both authentication and integrity protection. > Thanks for pointing out it. Although Sec 8 does not explicitly describe the protection strategy, it cites Sec 15 of RFC7285 where the detailed strategy is discussed. We will update this bullet in Sec 2.2 as follows: o Security: The identification between uCDNs and dCDNs is an important requirement (see Section 8). ALTO maps can signed and hence provide inherent integrity protection. Please see Section 15.1.2 of RFC7285 for detailed protection strategies. > > Section 5.6, 3rd paragraph after bullets: I do not understand the > second MUST statement in this paragraph. The sentence seems to contain > a mix of defining the superset and a MUST statement. I cannot suggest > a rewording. > Yes, the original sentence mixed a definition and a MUST statement. To make it easy to read, we would like to propose the following change to separate the definition and the MUST statement: OLD: The returned CDNI Advertisement resource MUST contain only BaseAdvertisementObject objects whose CDNI capability object is the superset of one of CDNI capability object in "cdni-fci-capabilities". Specifically, that a CDNI capability object A is the superset of another CDNI capability object B means that these two CDNI capability objects have the same capability type and mandatory properties in capability value of A MUST include mandatory properties in capability value of B semantically. See Section 5.7.2 for a concrete example. NEW: The returned filtered CDNI Advertisement resource MUST contain all the BaseAdvertisementObject objects satisfying the following condition: The CDNI capability object of each included BaseAdvertisementObject object MUST follow two constraints: o The "cdni-capabilities" field of the input includes a CDNI capability object X having the same capability type as it. o All the mandatory properties in its capability value is a superset of mandatory properties in capability value of X semantically. See Section 5.7.2 for a concrete example. > > Minor Concerns: > > Section 1: I think that the Introduction would be improved by > stating very early that this document specifies an extension of the > base ALTO protocol. > Thanks for the suggestion, we will explicitly make this statement. > > Section 4.2.4 includes: > > data: "/cdni-advertisement/capabilities-with-footprints > /0/footprints/0/footprint-value/-", > data: "value": "germany" > > Since Section 6.1.2.2 says that a countrycode domain is encoded > as an ISO 3166-1 alpha-2 code in lowercase, I was surprised to see > "germany" in this example. > If you check the example in Sec 4.2.3, you will find "germany" here is not a country code but an ALTO PID name. If the name is confusing, we can change it to make it more like a PID name. > > > Nits: > > General: Sometimes this document says "ALTO" and other times it says > "The ALTO protocol". Please be consistent. > Fixed. > > Abstract: I think the Abstract can be improved. I suggest: > > The Content Delivery Networks Interconnection (CDNI) framework in > RFC 6707 defines a set of protocols to interconnect CDNs to achieve > multiple goals, including extending the reach of a given CDN. A CDNI > Request Routing Footprint & Capabilities Advertisement interface > (FCI) is needed to achieve the goals of a CDNI. RFC 8008 defines > precisely the semantics of FCI and provides guidelines on the FCI > protocol, but the exact protocol is specified. This document > specifies a FCI protocol as an extension to the Application-Layer > Traffic Optimization (ALTO) protocol, and it follows the guidelines > in RFC 8008. Thanks for the suggestion. The new text looks good. We will update it. > > > Section 2.1, 4th bullet: please remove "(" and ")" in this text: > "... prefix set (or ASN, respectively)." > > Section 2.1, last bullet: s/prior agreed/previously agreed/ > > Section 2.1, last bullet: s/uCDN (upstream CDN)/uCDN/ > > Section 2.2, bullets: Please pick one style and use it for all of the > bullets. Some end with "(see Section X).", and others end with > "Please see Section X.". Please be consistent. > Fixed > > Section 2.2, 1st bullet: please make two bullets, one for > Application Layer-oriented, and another for CDNI. > This bullet explains that ALTO is can provide application layer-oriented information and therefore is a good match for CDNI. I am not quite sure what you mean by separating this bullet. Could you explain more? Thanks. > > Section 7.1, Table 1: Please adjust the table so that the media subtype > is not split across two lines. Without changing the column widths, I > suggest: > > +----------------+-------------------------+------------------------+ > | Type | Subtype | Specification | > +----------------+-------------------------+------------------------+ > | application | alto-cdni+json | Section 3 of RFCthis | > | | | | > | application | alto-cdnifilter+json | Section 5 of RFCthis | > | | | | > +----------------+-------------------------+------------------------+ > > [RFC Editor: Please replace RFCthis with the published RFC number for > this document.] > Thanks for catching it. This is a format issue. We have fixed it. > > > > _______________________________________________ > alto mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/alto >
_______________________________________________ alto mailing list [email protected] https://www.ietf.org/mailman/listinfo/alto
