Juergen, Thanks for your detailed review!
I addressed all of your comments in the latest draft. Regards, Clyde On 6/15/16, 8:29 AM, "netmod on behalf of Juergen Schoenwaelder" <[email protected] on behalf of [email protected]> wrote: >Hi, > >Dan Romascanu encouraged me to look at this I-D as part of his efforts >to organize YANG document reviews. Since the YANG definitions are not >consistent with the tree diagram, I have not really looked at the YANG >definitions yet. So the comments below are essentially about the >surrounding document structure, terminology, etc. > >- Abstract: The 'which is used to convey event notification messages' > may relate to 'the Syslog protocol' or 'a data model' and hence the > sentence is I think potentially confusing. Perhaps simply remove the > phrase altogether? Readers who do not know what SYSLOG is should > stop reading here anyway. Or break the sentence into two to make the > structure clearer. Perhaps add one more sentence describing what the > scope of the data model is. > >- The text uses Syslog, syslog, and SYSLOG. It is not clear what the > difference is (if any). If there is no semantic difference, I > suggest to pick one writing style (and 5424 seems to prefer all > lowercase except in cases where a sentence starts etc). > >- Change title of 4.1 to 'The ietf-syslog-types Module' and 4.2 > to 'The ietf-syslog Module' or something similar and consider > changing the title of section 4 to "Syslog YANG Modules" since > it contains the definitions of the modules themselves. > >- In section 2, should 'monitor and control' be 'configure and > monitor' in section 2? Are there actually definitions that support > monitoring? Perhaps the scope is just configuration? Otherwise, I > would have expected to see a bunch of basic counters (messages > received, forwarded, dropped, the usual monitoring stuff). > >- In section 2, s/network management agents such as NETCONF/network > management protocols such as NETCONF/ > >- In section 2, the phrase 'This module' is a hanging reference. There > is no prior text talking about a module. Perhaps replace with 'The > data model' > >- I did not find the term 'message distributor' in RFC 5424. The > figure first made me assume that only a console, log buffers, or log > files are message distributors but later it was stated that relays > and collectors (RFC 5424 defined terms) are also 'message > distributors'. Perhaps it helps to clearly spell out terms imported > from RFC 5424 and to clearly define any additional terms that go > beyond what is defined in RFC 5424. > >- Is it possible to shorten 'log-input-transports' to simply > 'log-inputs' (en par with log-actions)? And since both containers > are under syslog, perhaps even the 'log-' prefix is not needed and > all we have are /syslog/inputs and /syslog/actions? (I generally > find it useful to look at the resulting paths and whether they are > 'engineer friendly'. > >- I guess I have to define multiple /syslog/input/receiver in order to > listen on multiple UDP ports, e.g. to support both 0.0.0.0:514 and > [::]:514? This may be just find - just checking that this is the > idea. > >- I actually do not find the definition of log-input-transports in the > YANG model. It seems the tree diagram is not consistent with the > YANG model. So I can't judge what the structured-data boolean flag > is doing or what the syslog-sign presence container would do for > inputs. > >- The security considerations are quite generic; I think the template > asks for a more specific discussion. > >- I am not sure why RFC 6020 is an informative reference and why all > the normative references are actually normative. It might be useful > to go over the list to make sure we get the normative / informative > distinction right. > >- My understanding is that RFC 5424 numbers facilities and there is a > hard limit on the number of facilities that the protocol can > distinguish. RFC 5424 says: > > Facility values MUST be in the range of 0 to 23 inclusive. > > Given this, the 'extending facilities' appendix does not make much > sense to me. And given the fact that the number of facilities is > fixed (which is due to the way things are encoded in RFC 5424), I am > not even sure that using an identity instead of an enumeration is > useful (except if we expect a future version of SYSLOG to use a very > different encoding of facilities). I mean, to stay within the bounds > of RFC 5424, all one can do is to add an identity that maps to an > already allocated code point. > >- Do not use 1.1.1.1 as an example IP address, consider using an IPv6 > address from the IPv6 example address space. > >- Section 4.3 should not be in Section 4 and the title 'A Syslog > Example' is kind of misleading since the text shows NETCONF messages > operating on the SYSLOG YANG data model. I suggest to move 4.3 to > a new top-level section 5 "Usage Examples". Does it make sense to > show some complete example configs for typical use cases? > >- Before the YANG model definitions, it is a common idea to describe > what is imported from which RFCs. For example, one of the YANG > modules imports ietf-interfaces but there is no (normative) > reference to the relevant RFC. See the first sentence in section 4 > of RFC 7277 for an example how to do this. > >- I suggest to remove the subsection title 3.1. or to change the title > to "SYSLOG Data Model" and lifting it up to the top-level so that it > becomes section 4. > >As said above, I have not reviewed the YANG definitions yet since >apparently it is not in sync with the rest of the document. But I >thought I send these comments now anyway so that they can already be >taken into account. > >/js > >-- >Juergen Schoenwaelder Jacobs University Bremen gGmbH >Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany >Fax: +49 421 200 3103 <http://www.jacobs-university.de/> > >_______________________________________________ >netmod mailing list >[email protected] >https://www.ietf.org/mailman/listinfo/netmod _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
