Hi, I have reviewed this document. In general it is in good shape, but it needs some additional clarifying text.
o The term VRF is defined but not used. UDP is defined, but not TCP. Consider removing the entire section (or replace with terms defined in this draft). o It seems that this model is targeted for syslog originators (also noted in an early review by Rainer Gerhard). I think this should be stated. o The document's intended status is Informational. Should this somehow be reflected in the YANG modules? o There was a long discussion about facilities as identities, starting at: https://mailarchive.ietf.org/arch/msg/netmod/gtGBR7EngO_6KWJhvYDuCVfOas4 The conclusion from that thread is not present in the draft. Specifially, the draft needs to explain why identities are used and how vendors are supposed to derive new identities. Also, it needs to be explained what will happen if the client configures a facility match on e.g. 'daemon', and the system generates a message with facility 'ex:my-daemon', where ex:my-daemon is derived from 'daemon'. The current text just says: description "This case specifies one or more specified facilities will match when comparing the Syslog message facility."; Since the leaf is an identityref, "match" needs to be explained. Does it mean "equality", "derived-from" or "derived-from-or-self"? I think it means the latter. o Section 3: Syslog consists of message producers, a group level suppression filter, and message distributors. These terms are not used in RFC 5424. Maybe say "This document models the syslog system as ..." or something similar. But see below. It is not entirely clear to me how these concepts ("filter" and "message distributor") relate to the YANG model. The YANG model has "log-actions", but that term in not used in the explanation in this section. o Section 3: The text says: The following digram shows syslog messages flowing from a message producer, through the group level suppression filter, and if passed by the group filter to message distributors where further suppression filtering can take place. But the picture does not show and "group level suppression filter". BTW, what does "group level" refer to? o more specific type The YANG model has: leaf name { type inet:uri; description "This leaf specifies the name of the log file which MUST use the uri scheme file:."; } consider using: type inet:uri { pattern 'file:.*' } o The model supports syslog over TCP. Needs a reference to RFC 6587, but should we really support this historic RFC? o The 'udp' transport should have a reference to RFC 5426. o Why doesn't the model support syslog over TLS, RFC 5425? (I think some of the earlier reviews suggested this as well.) o The ietf-syslog.yang module has the "reference" statement wrong: OLD: revision 2015-11-09 { description "Initial Revision"; reference "RFC 5424: The Syslog Protocol RFC 5848: Signed Syslog Messages"; } NEW: reference "RFC 5424: The Syslog Protocol RFC 5848: Signed Syslog Messages"; ... revision 2015-11-09 { description "Initial Revision"; reference "RFC XXXX: SYSLOG YANG Model"; } Ditto for ietf-syslog-types - but that module should not have the reference to RFC 5848. o severity-operator What does it mean to set: severity: all severity-operator: not-equals or severity: none severity-operator: equals ?? I think that the answer is that for 'all' and 'none', the severity-operator isn't applicable. If so, maybe we should add a 'when' expression: leaf severity { ... } leaf severity-operator { when '../severity != "all" and ../severity != "none"'; ... } o severity 'none' I don't understand the description of this value: This enum describes the case where no severities are requested. and: 'none' is a special case which means that no severity selection should occur. I think 'none' means that no message will ever match. Thus, this is an implict way to disable this particular filter. o buffer In your response to my earlier review https://mailarchive.ietf.org/arch/msg/netmod/gtGBR7EngO_6KWJhvYDuCVfOas4 you explained that when a buffer is used, there is supposedly some vendor-specific non-standard method to read the data from the buffers. I think this explanation should go into the document. Also, if the leafs 'buffer-size-bytes' are 'buffer-size-messages' are not supported, I assume that the intention is that an implementation is free to use some local mechanism to limit the buffer? This should be explained. (same for the other limits) o structured-data Why doesn't "buffer" have the leaf "structured-data"; why just "file"? o mandatory log-actions In the current model, all log-actions are mandatory to implement. Should some/all of them have features? What if my device doesn't have a file system? o container terminal choice terminal-scope { "This choice describes the option to specify all terminals or a specific terminal. The all terminals case implies that messages will be sent to all sessions on that terminal"; I cannot parse the last sentence. Somehow 'all terminals' refer to "that terminal"? Which terminal? The "per-terminal" case has a list of "devices" - what is a "device"? Shouldn't it simply be "terminal"? I suggest: OLD: list device-name NEW: list terminal OLD: leaf dname NEW: leaf name Also explain that how terminals are named is implementation-dependant, and how to discover which terminals are available is out of scope for this specification. o container session What is a "session"? NETCONF session? HTTP session? Probably not... I suggest: OLD: list user-name NEW: list user OLD: leaf uname NEW: leaf name we usually don't try to make such leafs (pseudo-)unique o choice user-scope I note that the design with a choice means that it is not possible to configure the system to send e.g. emergency messages to all users at the same time as debug messages to user 'alice'. Is this intentional? If not, it would be trivial to remove the choice. /martin _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
