Hi Alex, Thanks for your review. My comments are inline as [clyde]…
On 12/13/16, 8:16 PM, "netmod on behalf of Alex Campbell" <[email protected] on behalf of [email protected]> wrote: I am considering to implement the data model in this draft. I have reviewed this draft and found the following issues. In approximately decreasing order of severity: * In the "selector-facility" choice statement the cases have misleading names - the case where no facility is matched is named "facility", and the case where specific facilities are matched is named "name". I suggest "no-facilities" and "specified-facilities", or similar. [clyde] I understand your concern and agree. Please see the next answer where I have removed the no-facilities case from the model. * I disagree with the premise of the "no-facilities" case, which is that it can be used to disable a log action, according to the description: description "This case specifies no facilities will match when comparing the syslog message facility. This is a method that can be used to effectively disable a particular log-action (buffer, file, etc)."; If an administrator wants to disable a log action they should do it by either removing it from the configuration, or by setting an "enabled" leaf to false. With that in mind, there is no reason for the "no-facilities" case to exist. [clyde] I agree with your suggestion to simplify by removing the no-facilities case. Please see the revised selector grouping which will appear in the next draft: grouping selector { description "This grouping defines a syslog selector which is used to select log messages for the log-action (console, file, remote, etc.). Choose one or both of the following: facility [<facility> <severity>...] pattern-match regular-expression-match-string If both facility and pattern-match are specified, both must match in order for a log message to be selected."; container selector { description "This container describes the log selector parameters for syslog."; list facility-list { key facility; description "This list describes a collection of syslog facilities and severities."; leaf facility { type union { type identityref { base syslogtypes:syslog-facility; } type enumeration { enum all { description "This enum describes the case where all facilities are requested."; } } } description "The leaf uniquely identifies a syslog facility."; } uses log-severity; } leaf pattern-match { if-feature select-match; type string; description "This leaf desribes a Posix 1003.2 regular expression string that can be used to select a syslog message for logging. The match is performed on the RFC 5424 SYSLOG-MSG field."; } } } * What is the behaviour of a selector if neither "no-facilities" nor "facility-list" is present? [clyde] At least one or both of the following must be specified: facility; pattern-match (if supported). I have updated the description at the beginning of the selector – please see above. * In the "selector" grouping it is not clear how the facility and pattern conditions are combined to decide whether a message is selected. Must they both match the message, or is it sufficient for either one to match the message? [clyde] If both are specified they must both match the message. This is consistent with the syslog implementations by Cisco and Juniper. * Not all servers have a console; there should be a feature to indicate whether logging to the console is supported. [clyde] I have received comments in earlier reviews that we should implement the minimum set of functionality that is contained in at least three vendor implementations. Console is supported by Alcatel/Lucent, Brocade, Cisco (XE, XR, and NXOS), Juniper, and Linux-rsyslog. Removal of the console action for your case could be done through a vendor specific deviation statement. * Likewise, not all servers may support logging to user sessions. [clyde] User sessions are supported by Alcatel/Lucent, and Juniper. I will make this a feature in the next revision of the draft since only two vendors implement it. * Likewise, not all servers may support a user-accessible filesystem. [clyde] Files are supported by Alcatel/Lucent, Cisco (XE, XR, and NXOS), Juniper, and Linux-rsyslog. Removal of the file action for your case could be done through a vendor specific deviation statement. * RFC 5424 states that the severity and protocol values are not normative. [clyde] Is it that you are asking for RFC 5424 to be removed from the Normative Reference section of the draft and added to the Informative section? * It's not clear to me why this needs to be split into two modules. Is it so that other modules can define logging parameters but still be usable on a device without syslog? [clyde] We were guided by an early Yang Dr.’s advice in the choice of two modules – one for types and one for the model. I will defer to our mentor Jürgen for his advice. * "log-severity" defines a severity filter, not a severity, so its name is misleading. [clyde] Please suggest a better name. * Perhaps the "severity" type and the facility identities should have "reference" statements referring to RFC 5424, rather than referring to it in the description. [clyde] I will defer to our mentor Jürgen for his advice. We previously followed his advice here. * In section "8.2", "admisintrator" is a typo. [clyde] This will be fixed in the next draft. I assume that the means of accessing the memory buffer and log files are out of scope of this data model. [clyde] This draft covers syslog configuration only. Thanks, Clyde Alex ________________________________________ From: netmod <[email protected]> on behalf of Kent Watsen <[email protected]> Sent: Wednesday, 14 December 2016 2:01 p.m. To: [email protected] Subject: [netmod] WG Last Call for draft-ietf-netmod-syslog-model-11 This is a notice to start a two-week NETMOD WG last call for the document: A YANG Data Model for Syslog Configuration https://tools.ietf.org/html/draft-ietf-netmod-syslog-model-11 Please indicate your support or concerns by Tuesday, December 27, 2016. We are particularly interested in statements of the form: * I have reviewed this draft and found no issues. * I have reviewed this draft and found the following issues: ... As well as: * I have implemented the data model in this draft. * I am implementing the data model in this draft. * I am considering to implement the data model in this draft. * I am not considering to implement the data model in this draft. Thank you, NETMOD WG Chairs _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
