Subject: Re: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-ssh-client-server-03

From: "Gary Wu (garywu)" <[email-masked]>
Date: Tue, 1 Aug 2017 19:44:12 +0000

C1:
    
     (M3) transport-params leafrefs
    
     Example of <transport-params> in sec. 4.2 shows ietf-ssh-common
     as the module namespace but this will never happen in real usage.
    
     People will copy the various identityref examples in this section
     and not encode the identityref prefix correctly.

    <KENT> I added prefixes to the example.  
    
    <KENT> This would've been caught earlier (via validation) had the
    transport-params grouping contained some mandatory values, as then
    the identities would have to show up in other examples (e.g., 
    ex-netconf-server.xml), which would then have to encode the 
    identities from another namespace.  I sent email to Gary asking
    him about if any of these values are mandatory.

<GARY> This is something I had debated as well for SSH and TLS transport
parameters.  I settled on modeling what was implemented in popular vendors and
open source, with the ability for the identities to be extended as needed.
    
    <KENT> Also related, I notice that there are a ton of algorithm
    identities here that I'd forgotten about.  Going back to Juergen's
    comment about factoring these out into another module, now it
    seems more prudent than ever! I just filed an issue to track
    this:  https://github.com/netconf-wg/keystore/issues/8

<GARY> IANA has registries for SSH and TLS, if we want to go down that route.
There's no protocol-independent registries for public key algorithms for
keystore that I know of unfortunately.

Thanks,
Gary

On 7/31/17, 1:23 PM, "Kent Watsen" <[email-masked]> wrote:

    Hi Andy, 
    
    Thanks for your review.  Below are my responses to your comments.
    
    Thanks,
    Kent

    --
    
    Reviewer: Andy Bierman
    Review result: Ready with Nits
    
    Review: draft-ietf-netconf-ssh-client-server-03
    
    Modules:
     (M1) [email-masked]
     (M2) [email-masked]
     (M3) [email-masked]

    YANG Usage:
    
     I did not find anything wrong in either module.
     pyang and yangdump-pro do not report any errors or warnings.
    
    Comments:
    
    C1:
    
     (M3) transport-params leafrefs
    
     Example of <transport-params> in sec. 4.2 shows ietf-ssh-common
     as the module namespace but this will never happen in real usage.
    
     People will copy the various identityref examples in this section
     and not encode the identityref prefix correctly.

    <KENT> I added prefixes to the example.  
    
    <KENT> This would've been caught earlier (via validation) had the
    transport-params grouping contained some mandatory values, as then
    the identities would have to show up in other examples (e.g., 
    ex-netconf-server.xml), which would then have to encode the 
    identities from another namespace.  I sent email to Gary asking
    him about if any of these values are mandatory.
    
    <KENT> Also related, I notice that there are a ton of algorithm
    identities here that I'd forgotten about.  Going back to Juergen's
    comment about factoring these out into another module, now it
    seems more prudent than ever! I just filed an issue to track
    this:  https://github.com/netconf-wg/keystore/issues/8

    C2:
    
    Sec 1.2 Tree Diagrams
    
    Old text should be replaced with reference to
    draft-ietf-netmod-yang-tree-diagrams-01
    
    <KENT> Is this safe?  I've resisted doing this so far because I
    wasn't sure if the tree-diagrams draft might break syntax...

    C3:
    
     (M1) IETF copyright says 2014; change to 2017
     (M3) IETF copyright says 2014; change to 2017

    <KENT> done!