Subject: [netmod] mbj review of draft-ietf-netmod-syslog-model-06

From: Martin Bjorklund <[email-masked]>
Date: Tue, 08 Mar 2016 16:10:58 +0100 (CET)

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