Re: [netconf] More comments was Shepherd's comments on draft-ietf-netconf-crypto-types

[tom petch <[e-mail]>]

Some additional comments unrelated to those of the Shepherd

RFC6960 is in the module so needs adding to I-D references

X.690 in the module lacks a date which makes the reference ambiguous

X.690 in the I-D Refs is -2015.  This is not the latest version - I do not know what has changed.

RFC6125 has a bis in WGLC

The IETF has abolished the page number which makes s.2.2.1 10 pages of stream of consciousness.  I suggest introducing subsections.  I see some obvious places for them

In the same vein I suggest including the subsections of  s.2.1 in the ToC. 

I find the use of -grouping in the identifier of a grouping prolix.

2.1.3.  Typedefs
   The following diagram illustrates the relationship amongst the
   "typedef" statements defined in the "ietf-crypto-types" module:
Well no it does not illustrate for me without a crib to tell me what is going on

s.2.2.1 
OLD
   The diagram above uses syntax that is similar to but not defined in

suggest here and elsewhere
   The diagram above uses syntax that is similar to but not the same as that in 

    feature p10-based-csrs {
       description
         "Indicates that the erver implements support
/erver/server/

OLD
        How the associated algorithm is known is outside the
          scope of this module.  This statement also applies when
          the octet string has been encrypted.";
Seems clumsy; I suggest
NEW
         The identity of the associated algorithm is outside the
          scope of this specification.  This is also true when 
   the octet string has been encrypted.";
 
     grouping asymmetric-key-pair-with-cert-grouping {
     grouping asymmetric-key-pair-with-certs-grouping {
These are both described as
'         Implementations SHOULD assert that certificates contain '
ie plural in both cases.  I think a more comprehensive description is needed of what the difference is between the two.  Also, here is a case where that terminal -grouping is not just redundant but is likely to cause errors.
     grouping asymmetric-key-pair-with-cert {
     grouping asymmetric-key-pair-with-certs {
would make the difference slightly more obvious but depending on what the difference is meant to be, which I do not see in the description clause, I think one or both identifiers need changing

Tom Petch

________________________________________
From: netconf <[e-mail]> on behalf of Kent Watsen <[e-mail]>
Sent: 12 December 2022 16:07

Thank you Mahesh for your comments.

See below for responses.

K.



On Dec 4, 2022, at 10:40 PM, Mahesh Jethanandani <[e-mail]<mailto:[e-mail]>> wrote:

Hi Kent,

The YANG Data Types and Grouping for Crypto Types draft is short, well written, and easy to understand. Thanks for including plenty of examples on how to use the model.

There are however a few minor comments that would nice to address before forwarding the document for AD review.

Thanks again.


Section 1.1 - Relation to other RFCs

Now that the set of modules and drafts in the “client-server” suite of drafts is known, can the language in this section be updated to reflect it. Specifically, can the second paragraph be removed or replaced because you list the drafts (if not the modules themselves) in the dependency graph.

Removed, and cleaned-up the language in the next paragraph.   Now reads:

    This document presents one or more YANG modules [RFC7950]
    that are part of a collection of RFCs that work together
    to, ultimately, enable the configuration of both the clients
    and servers of both the NETCONF [RFC6241] and
    RESTCONF [RFC8040] protocols.

    The normative dependency relationship between the various RFCs
    in this collection is presented in the below diagram. The labels
    in the diagram represent the primary purpose provided by each
    RFC.  Hyperlinks to each RFC are provided below the diagram.

This change will effect all in the suite of drafts.


Section 2.1.1 Features

This is more of a nit, and would not mind if the comment is ignored. Can we remove the leading | (pipe symbol) in front of the sentence - “The diagram above uses syntax …”. The same is true for other “tree diagrams” that follow RFC 8340 syntax. I do understand that other documents in the cluster follow that format, but it is not clear what the pipe symbols mean or why the text needs to be formatted differently from the rest of the text in the document.

This is how the <aside> element, in the xml2rfc file, renders.  I wish that it rendered as a box but, alas, such is not the case.  In any case, Juergen made the same comment before, so it seems pretty annoying, so I removed all of them, in the entire suite of drafts.



Section 2.1.1 Identities

s/format that key data/format for key data/

Fixed.


Section 2.1.4.* Groupings

Most of the grouping display an abridged and a full tree diagram. But the difference between them are a few extra lines. If the diagrams were truly different in size, i.e., the extra number of lines in the full tree diagram was more the size of the abridged tree diagram, I could have understood displaying both of them. Are the two diagrams really necessary in every section?

Not necessary.  I just removed the "expanded" diagrams.  Now this draft is more like others in the suite of drafts


Thanks again,
Kent // author