# tsvart review of draft-ietf-netconf-netconf-client-server-37
CC @larseggert
This document has been reviewed as part of the transport area review
team's ongoing effort to review key IETF documents. These comments were
written primarily for the transport area directors, but are copied to the
document's authors and WG to allow them to address any issues raised and
also to the IETF discussion list for information.
When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@xxxxxxxx if you reply to or forward this review.
## TL;DR
Am not a YANG exports. LGTM overall, some minor comments below.
## Comments
### Section 2.3, paragraph 35
```
In the case that the previous connection is still
active, establishing a new connection is NOT
RECOMMENDED.";
```
Can this YANG model really enforce that requirement? This seems to be one for the actual implementation.
### Section 2.3, paragraph 36
```
leaf max-wait {
type uint16 {
range "1..max";
}
units "seconds";
default "5";
description
"Specifies the amount of time in seconds after which,
if the connection is not established, an endpoint
connection attempt is considered unsuccessful.";
}
```
5 sec feels short.
### Section 3.3, paragraph 34
```
description
"Specifies the maximum number of seconds that a NETCONF
session may remain idle. A NETCONF session will be
dropped if it is idle for an interval longer than this
number of seconds. If set to zero, then the server
will never drop a session because it is idle.";
```
I wonder if this text should make it clearer that a server may of course still drop a connection for arbitrary other reasons at any time, and that this "never drop" statement is hence less binding than it might first seem.
### Section 3.3, paragraph 33
```
"Maintain a persistent connection to the NETCONF
client. If the connection goes down, immediately
start trying to reconnect to the NETCONF client,
using the reconnection strategy.
```
Is there a deployment assumption here that no firewalls and NATs exist on the path? Because typically it is the client that needs to reinitiate a connection to the server to make said middleboxes happy.
### Section 3.3, paragraph 37
```
In the case that the previous connection is still
active (i.e., the NETCONF client has not closed
it yet), establishing a new connection is NOT
RECOMMENDED.";
```
See above.
### Section 3.3, paragraph 39
```
"The reconnection strategy directs how a NETCONF server
reconnects to a NETCONF client, after discovering its
connection to the client has dropped, even if due to a
reboot. The NETCONF server starts with the specified
endpoint and tries to connect to it max-attempts times
before trying the next endpoint in the list (round
robin).";
```
See above, in the presence of NATs and firewalls, server-to-client (re)connections are not typically going to be successful.
### Section 3.3, paragraph 38
```
description
"Specifies the amount of time in seconds after which,
if the connection is not established, an endpoint
connection attempt is considered unsuccessful.";
```
Short, see above.
## Nits
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.
### Typos
#### Section 2.3, paragraph 36
```
- this configuraton is applied (e.g., on boot).
+ this configuration is applied (e.g., on boot).
+ +
```
#### Section 3.3, paragraph 41
```
- this configuraton is applied (e.g., on boot).
+ this configuration is applied (e.g., on boot).
+ +
```
### Outdated references
Document references `draft-ietf-netconf-restconf-client-server-37`, but `-38`
is the latest available revision.
Document references `draft-ietf-netconf-netconf-client-server-36`, but `-37` is
the latest available revision.
Document references `draft-ietf-netconf-http-client-server-21`, but `-23` is
the latest available revision.
### Grammar/style
#### "Editorial Note", paragraph 10
```
The "Relation to other RFCs" section Section 1.1 contains the text "one or
^^^^^^^^^^^^^^^
```
Possible typo: you repeated a word.
#### "Editorial Note", paragraph 12
```
The "Relation to other RFCs" section Section 1.1 contains a self-reference
^^^^^^^^^^^^^^^
```
Possible typo: you repeated a word.
#### "Editorial Note", paragraph 13
```
olding mode is used. The AD suggested suggested putting a request here for t
^^^^^^^^^^^^^^^^^^^
```
Possible typo: you repeated a word.
#### Section 2.3, paragraph 27
```
ll occur 15 minutes past midnight everyday."; } leaf idle-timeout { type uin
^^^^^^^^
```
"Everyday" is an adjective. Did you mean "every day"?
#### Section 2.3, paragraph 29
```
rom certificate fields to NETCONF user names. * For the referenced grouping
^^^^^^^^^^
```
It's more common nowadays to write this noun as one word.
#### Section 3.2, paragraph 6
```
ll occur 15 minutes past midnight everyday."; } leaf idle-timeout { type uin
^^^^^^^^
```
"Everyday" is an adjective. Did you mean "every day"?
#### "I", paragraph 1
```
urity Considerations text to also look a SC-section from imported modules. *
^
```
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".
## Notes
This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].
[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool
Attachment:
signature.asc
Description: Message signed with OpenPGP
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
