On Fri, Dec 30, 2016 at 10:16 AM, Clyde Wildes (cwildes) <cwil...@cisco.com> wrote:
> Hi Andy, > > > > Thanks for taking the time to review the model. > > > > My comments are inline as [clyde]… > > > > *From: *netmod <netmod-boun...@ietf.org> on behalf of Andy Bierman < > a...@yumaworks.com> > *Date: *Tuesday, December 27, 2016 at 3:04 PM > *To: *Alex Campbell <alex.campb...@aviatnet.com> > *Cc: *"netmod@ietf.org" <netmod@ietf.org> > *Subject: *Re: [netmod] WG Last Call for draft-ietf-netmod-syslog-model-11 > > > > Hi, > > > > I am also considering an implementation. > > I share the same concerns that Alex has brought up. > > > > Some detailed comments: > > > > 1) /syslog/actions: seems like everything is in this container. > > Why is it needed? Seems like it could be removed as it serves no purpose > > > > [clyde] Although this model is currently designated as config only, we > could add operational data and rpc leaves in the future. The actions > container is to future-proof the model. > > > > 2) 8 features: the granularity seems wrong. The main container for each > section > > should have its own if-feature > > /console > > /buffer > > /file > > /remote > > > > [clyde] We have gone back and forth on this…some have complained that > there are too many features. I will be happy to add a feature for each > action. Note that we studied the implementation of each action by six > vendors including Linux and opted to not add features for actions > implemented by at least 3 vendors. Vendors not implementing an action could > create a deviation. > I prefer 1 mandatory-to-implement and a minimal number of additional options. /console /file /remote These are all mandatory-to-implement.. IMO only /file should be mandatory-to-implement. > > > 3) What is the 'buffer' container for? > > How is the internal memory accessed by the client? > > > > [clyde] buffer is implemented by vendors typically for routers capable of > generating many syslog messages in event-storm bursts. Logging to memory > (aka buffer) allows the preservation of syslog messages which might > otherwise be lost. > > > IMO it should be removed from the draft. We certainly have changed the IETF NM focus. In SNMP-land we routinely left the configuration out of scope and standardized the monitoring. Now we are standardizing the configuration and leaving the monitoring out of scope? I prefer complete standard solutions only. There is no standard way to access the /console either. Since the console provides "show log" I really do not see a need for /buffer at all. A “show log” command is used to access the buffers. As this model is > current designed as a configuration only model, there is no operational > leaves for show log, or rpc leaves for clear log. > > > > 4) selector-facility: Seems like no-facilities servers the same purpose > > as an empty facility-list. The choice is not needed; just use the > facility-list > > > > [clyde] This was changed as a result of Alex’s feedback – please see my > response to him. The model will be changed to the following: > > > > 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."; > > } > > > > > > 5) pattern-match: > > > > 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."; > > } > > > > The field SYSLOG-MSG is referenced but never defined or listed in > the terminology section. > > > > [clyde] This will be fixed in the next draft. > > > > 6) how are the syslog-facility identities mapped to SYSLOG messages? > > 6a) how to distinguish acme:foo-facility from example:foo-facility in a > SYSLOG message? > > > > [clyde] I do not understand your question. The current implementation of > facilities was designed with the help of several Yang Doctors. The > requirement is to support the facilities as called out in RFC 5424 as well > as vendor specific facilities that can be added through augmentation. > Vendor specific facilities are not meant to be used across multiple vendor > implementations. > > > The filter is based on an identityref, which is a module-qualified name, e.g., acme:foo-facility and example:foo-facility are different entities. In the syslog message, only the string foo-facility will be present. The draft claims to provide extensible facilities,(see A.1) but it only seems to work if the identities do not contain any duplicates. > 7) source-interface: what if the server does not let a source interface be > used and instead > > normal routing determines the source interface (this leaf is very > router-centric) > > > > [clyde] source-interface is optional. If not specified normal routing flow > would be used. > > > > 8) signing-options: are these widely deployed on all routers and Linux > hosts? > > > > [clyde] Alex Clemm asked that we include syslog signing-options. This is > implemented by at least Linux rsyslog. > > > > 9) logrotate: there are several features related to log file cleanup > allowing lots of > > server variability and forces the client to support all the options. > Can't this be simplified > > and all the micro-behavior YANG features removed? > > > > [clyde] This was designed by merging the requirements from several > vendors. All of the variants specified are with if-feature so that the > client does not have to support all options. > > > There seems to be some procedures implied by these YANG objects, but it is not specified. The 4 different methods (each with its own feature), are in a container. Since container 'file-rotation' is in list 'log-file', the rotation variant can be different for every file. Is this really how implementations work? Also, the different parameters in this container can interact if the server supports more than 1 feature. The draft does not say anything about combining them. E.g.: leaf number-of-files { if-feature file-limit-size; type uint32; description "This leaf specifies the maximum number of log files retained. Specify 1 for implementations that only support one log file."; } How does the client know if the server only supports 1 file or not? This should really be revisions, since these files are per log-file list entry. If only 1 revision of the log-file is retained, then the meaning of the other leafs is unclear. If there is only 1 log-file revision, then what happens if the max-file-size # of megabytes, rollover # of minutes, or retention # of hours is reached? Does syslog monitoring stop for the log-file entry? How does the client access different revisions of the log file? Or even list them? How does the client know the current size of lifetime of the log-file They do not have names. Is it assumed they will be the log-file/name field appended with ".1", ".2", etc.? > 10) numeric limits: there is some odd usage of YANG types; some limits are > uint64, some uint32, > > some uint16. Seems like uint32 is sufficient > > > > [clyde] The signing-options counts are as per the syslog-sign spec (RFC > 5848) which is uint16. I will make all others uint32 except for the buffer > size limit which I will leave at unit64. > > > > Result: > > <seven signing-options counters> uint16 > > buffer-limit-bytes uint64 > > buffer-limit-messages uint32 (was formally uint64) > > number-of-files uint32 > > max-file-size uint32 (was formally uint64) > > rollover unit32 > > retention unit32 (was formally uint16) > > > > > > Thanks, > > > > Clyde > > > > > Andy > Andy > > > > > > On Tue, Dec 13, 2016 at 8:16 PM, Alex Campbell <alex.campb...@aviatnet.com> > 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. > > * 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. > > * What is the behaviour of a selector if neither "no-facilities" nor > "facility-list" is present? > * 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? > * Not all servers have a console; there should be a feature to indicate > whether logging to the console is supported. > * Likewise, not all servers may support logging to user sessions. > * Likewise, not all servers may support a user-accessible filesystem. > * RFC 5424 states that the severity and protocol values are not normative. > * 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? > * "log-severity" defines a severity filter, not a severity, so its name is > misleading. > * Perhaps the "severity" type and the facility identities should have > "reference" statements referring to RFC 5424, rather than referring to it > in the description. > * In section "8.2", "admisintrator" is a typo. > > I assume that the means of accessing the memory buffer and log files are > out of scope of this data model. > > Alex > > ________________________________________ > From: netmod <netmod-boun...@ietf.org> on behalf of Kent Watsen < > kwat...@juniper.net> > Sent: Wednesday, 14 December 2016 2:01 p.m. > To: netmod@ietf.org > 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 > netmod@ietf.org > https://www.ietf.org/mailman/listinfo/netmod > > _______________________________________________ > netmod mailing list > netmod@ietf.org > https://www.ietf.org/mailman/listinfo/netmod > > >
_______________________________________________ netmod mailing list netmod@ietf.org https://www.ietf.org/mailman/listinfo/netmod