Hi Adrian, Really thank you for your "sanity review"! It is very helpful for us at this moment. Let me separate my response into two parts to answer your comments.
The following response applies to your comments until Sec 3. I'll send another email to respond to your remaining comments soon. Thanks, Jensen On Mon, Jan 24, 2022 at 7:10 PM Adrian Farrel <adr...@olddog.co.uk> wrote: > Hi authors, > > > > Thanks for starting this work on a YANG model for ALTO servers. I believe > this will be a useful component to support the operational features of ALTO > in real deployments. > > > > I have done a "sanity review" rather than a detailed technical review. > > > > In summary, this looks like a reasonable starting point to address some of > the work in the WG for handling OAM from an operational point of view. > > > > Hope this helps. > > > > Best, > > Adrian > > > > ==observations== > > > > Obviously (!) there is no YANG in this model yet. I think that is OK for > > a work in progress, but I'm not sure about adopting it until at least a > > first attempt has been made. > Very perceptive observation! Some parts of the proposed model had not been tested yet when this document was drafted. We plan to test all the proposed features and add the YANG code before the next IETF meeting. > > > --- > > > > The Title says the document is about OAM for ALTO protocol, but the > > Abstract says "operations and management". I prefer the Abstract's > > choice of words. > Thanks for raising this issue. I double-checked RFC6291 ( https://datatracker.ietf.org/doc/html/rfc6291). And I realize that "OAM" is a broader concept than "operations and management". RFC7285 does only use "operations and management". But this document also wants to provide some maintenance functionality, e.g., allowing upgrade of ALTO services. Maybe "O&M" is better? > > > --- > > > > Extensibility > > > > The document is good to mention that it is a requirement that the model > > be extensible for ALTO protocol extensions. And it is right that it > > focus on the base protocol and the core extensions already published as > > RFCs. > > > > What is needed, however, is a discussion of how that objective has been > > met and how it is expected that extensions will be handled. > > > > I suggest doing this by a new section "Extending this Data Model". You > > won't need to write much. > Very good suggestion! > > > --- > > > > I really like Table 2 and the indication of what requirements are being > > addressed. > > > > Three points: > > > > 1. I did not check back to RFC 7285 to see that all of the requirements > > are listed. I do note that there is a gap (16.2.3) and assume that > > that is deliberate. > Good catch. We notice that 16.2.3 can be merged into 16.2.5. But maybe we can add a sentence to point this out. It can help readers to better understand. > > > 2. The text before the table mentions RFC 7971 but that doesn't appear > > in the table > RFC7971 also follows the guideline by Sec 16 of RFC7285 but adds more metrics. But I agree that listing RFC7971 in the reference is better. > > > 3. There is not traceability of the solution to these requirements > > (e.g., resolved by the cost-metric data object). Section 4.2 maps the > > requirements into more specific requirements, but still lacks the > > traceability of what elements of the model satisfy the requirements. > Good point. We will add such a mapping in the next revision. > > > --- > > > > 4.2 > > > > I'm not sure of the meaning of "should" in the bullet points in this > > section. Since the model has now been written (well, will have been > > written in this document), it is probably safe to say "the model does > > these things". > > > > The section concludes with a note about what may be considered in a > > future version of this document. That is fine *if* it is the intention > > to do that work. Of course, the anxious reader would like that new > > version to be now! > > > > (Note that 5.1 has a similar note. It is probably not necessary to > > have the note present twice.) > > > > --- > > > > 5.1 > > > > The ALTO YANG module defined in this document has all the common > > building blocks for ALTO OAM. > > > > I think this probably has no meaning! "All the common building blocks"? > > > > But also, this is another use of OAM rather than "operations and > > management" > > > > --- > > > > I'm not sure of the benefit of including the full tree at the start of > > 5.1 and then showing each of the subtrees in the following sections. I > > have no objection, but I note that the duplication introduces the chance > > of a conflict. > > > > --- > > > > 5.4 has two paragraphs about additional data sources and retrieval > > mechanisms. > > > > The data-source/source-params node can be augmented for different > > types of data sources. This data model only includes import > > interfaces for a list of predefined data sources. More data sources > > can be supported by future documents and other third-party providers. > > > > Just need to say how. Presumably by augmenting something. > > > > Note: Current source configuration still has limitations. It should > > be revised to support more general southbound and data retrieval > > mechanisms. > > > > Is that revision intended for this document, or (like the previous > > paragraph) for future documents? If intended for this document, you have > > Appendix A for listing the work still to be done and so this sort of > > note could be moved out to there. > > > > --- > > > > 5.4.1 has "current ALTO OAM data model" meaning that there is a general > > issue about naming this work. I still prefer that is the "ALTO > > operations and management data model" > > > > --- > > > > 5.4.2 > > > > It may be obvious to all ALTO people (or maybe all YANG people) what a > > "prometheus data source" is. But I have no clue. At least a reference is > > needed. > > > > --- > > > > 5.6 > > > > Not sure, but I think that maybe the counter32 objects in this section > > should all be of type gauge. > > > > --- > > > > When section 6 is written, I think that some of the server performance > > data in the model will be highly sensitive. It is commercially > > sensitive how hard a server is having to work and how close it is to > > exploding. It is valuable information for an attacker to know that a > > small push will cause a server to be over-committed, or to know which > > servers are doing most work. It may be possible to deduce things about > > the users in the network from how busy a server is. > > > > ==nits== > > > > idnits shows the following warnings that need to be fixed: > > > > ** The document seems to lack an IANA Considerations section. (See > > Section 2.2 of https://www.ietf.org/id-info/checklist for how to > > handle the case when there are no actions for IANA.) > > > > >>> In fact you'll need one of these to get the right codepoints > > assigned for the new YANG module. > > > > == Unused Reference: 'RFC7286' is defined on line 679, but no explicit > > reference was found in the text > > > > == Unused Reference: 'RFC8571' is defined on line 703, but no explicit > > reference was found in the text > > > > == Unused Reference: 'RFC8686' is defined on line 709, but no explicit > > reference was found in the text > > > > --- > > > > Need to capitalise "YANG" throughout. > > > > --- > > > > 5.1 > > > > OLD > > |--rw meta* [meta-key] > > NEW > > +--rw meta* [meta-key] > > END > > > > --- > > > > 5.3 > > > > It can also include an accepted-group node containing a list of user- > > groups that can access this ALTO information resource. > > > > Given the use of "MUST" in the previous and subsequent paragraph, this > > is probably s/can/MAY/ > > > > But then you have > > For each type of ALTO information resource, the creation intent MAY > > also need type-specific parameters. > > > > I don't think "MAY need" is helpful. If it is needed it MUST be present. > > So, probably, > > For each type of ALTO information resource, the creation intent may > > also need type-specific parameters, and if needed they MUST be > > present. >
_______________________________________________ alto mailing list alto@ietf.org https://www.ietf.org/mailman/listinfo/alto
