Re: [netconf] Shepherd review on draft-ietf-netconf-netconf-client-server-26

[Kent Watsen <[e-mail]>]

Hi Tom,

Thank you for your review of the netconf-client-server draft.
Please see below for my responses to your comments.

Kent



> On Jan 4, 2023, at 7:04 AM, tom petch <[e-mail]> wrote:
> 
> Nothing to do with the shepherd review, just some comments on -28
> 
> -grouping on all the identifiers of groupings I find clumsy and redundant

You're consistent, I'll grant you that ;)


>      downstream modules can augment nodes into it if needed.
> I do not know what downstream is in this context - what is flowing where in this stream?

Fixed - s/downstream/consuming/g



> *  The "transport" choice node enables both the SSH and TLS
>  No, either or, not both; ditto for the server module

Fixed.


> leaf name string
> is used as a key with no restriction on length or on character set - CJK anyone?

CJK?

I saw a discussion about this on "netmod".  IIRC, Juergen proposes setting min-length?  (But not max-length)?

So this?

       type string {
         length "32..max";
       }



> even if due to a reboot
> reboot of what?

Hmm, I wonder... ;)    er, what do you suggest?



> A NETCONF session will be  dropped if it is idle for an interval longer than this
>              number of seconds.  
> netconf-restconf-client-server has a similar statement but about the TCP session- why the difference?
> ditto for server module

I can't find the "similar statement but about the TCP sessions".  What is the issue?



> Sessions that have a notification subscription active are never
>              dropped.";
> sounds like a MUST NOT to me 
> ditto in server module

Fixed.



> the same documentation address is used in all examples.  Were I an implementor I would have chosen different ones for different protocols but YMMV  

Why?  Many NC/RC servers support both protocols at the same time - yes?

> 
>      choice transport {
>         mandatory true;
>         case ssh {
>           if-feature "ssh-listen";
> ....
>         case tls {
>           if-feature "tls-listen";
> suggests to me that it is mandatory to have one or other of the listen features which I do not know if YANG can specify otherwise assuming that that is what is intended

Yes, YANG will assert one "case" is picked.


>               case periodic-connection {
>                 container periodic {
>                   description
>                     "Periodically connect to the NETCONF client.
> 
>                      This connection type decreases resource
>                      utilization, albeit with increased delay in
>                      NETCONF client to NETCONF client interactions.
> right and wrong; depends on the resources.  This needs more explanation perhaps in the body of the I-D rather than in the modules


Sockets, since the connection isn't retained.

Fixed the typo:
	OLD: NETCONF client to NETCONF client interactions
	NEW:NETCONF client to NETCONF server interactions


Kent


> 
> Tom Petch
> 
> 
> 
> 
> 
> 
> 
> [e-mail]> on behalf of maqiufang (A) <maqiufang1=[e-mail]>
> Sent: 06 September 2022 13:16
> To: [e-mail]
> Cc: [e-mail]
> Subject: [netconf] Shepherd review on draft-ietf-netconf-netconf-client-server-26
> 
> Hi, all
> 
> I am glad to be the doc shepherd for this draft as it moves beyond the WG with the other client-server suite of drafts.