Re: [netmod] AD review: draft-ietf-netmod-snmp-cfg-05

[Juergen Schoenwaelder <[e-mail]>]

On Tue, Jun 03, 2014 at 06:15:08PM +0200, Benoit Claise wrote:
> Dear all,
> 
> This is my AD review.
> Good work on a complete YANG model. It depends heavily on the knowledge 
> of the different SNMP MIB modules: SNMP-TARGET-MIB, 
> SNMP-NOTIFICATION-MIB, SNMP-PROXY-MIB, SNMP-COMMUNITY-MIB, etc. This is 
> good as there is no specification redefinition. And I'm glad to find the 
> examples in the appendix for the operators wanted "standard" SNMP 
> configurations.
> 
> My feedback:
> -
> 
>    The configuration data model in particular_targets_SNMP deployments
>    where SNMP runs in read-only mode and NETCONF is used to configure
>    the SNMP agent.  Nevertheless, the data model has been_designed_to
>    allow implementations that support write access both via SNMP and
>    NETCONF in order to interwork with SNMP-managed management
>    applications manipulating SNMP agent configuration using SNMP.
> 
>    The YANG data model focuses on configuration.
> 
> I don't understand what you mean by "targets" or "designed to"
> So you made some tradeoffs in the design? Is this the way we should 
> understand this?
> If which one?

I think "in particular targets" should be read as in "will likely be
primarily used in". Or to say it in other words: If a deployment uses
SNMP to configure SNMP agents, then this data model may be of limited
value.

Concerning "designed to": The data model has been designed to cover
everything that the SNMP configuration models describe. It allows an
implementation that supports both configuration via NETCONF and
configuration via SNMP. The details are discussed in section 3.2.

> - At the beginning of VACM and SNMP, we faced one issue. Someone with a 
> read-only community string could query the read-write community string.
> 
> 
> Router#sh run | i snmp
> 
> snmp-server engineID local 000000090200009092827820
> 
> snmp-server group v1group v1 read includeeverything
> 
> snmp-server view includeeverything internet included
> 
> snmp-server community _claise _RW
> 
> snmp-server user _public _v1group v1
> 
> ...
> 
> "snmpwalk -v 1 <Router> _public _internet.6.3.16 | grep _claise_"... you 
> would be surprised
> 
> Basically, the trick is that we need a default view on VACM.
> I see http://tools.ietf.org/html/rfc3415#section-7.4
> Do we need something specific in this draft to stress that issue?

I do not think that this document is the place to discuss how default
VACM rules should look like. This should go into a separate document
because this is not specific to the YANG data model but rather
specific to VACM.

> Editorial:
> 
> -
> 
> One small alignment issue below: see the port line
> 
>     +--rw snmp
>          +--rw engine
>             +--rw enabled?               boolean
>             +--rw listen* [name]
>             |  +--rw name    snmp:identifier
>             |  +--rw (transport)
>             |     +--:(udp)
>             |        +--rw udp
>             |           +--rw ip      inet:ip-address
>            |           +--rw port?   inet:port-number
>             +--rw version
>             |  +--rw v1?    empty
>             |  +--rw v2c?   empty
>             |  +--rw v3?    empty
>             +--rw engine-id?             snmp:engine-id
>             +--rw enable-authen-traps?   boolean
 
fixed 
 
> -
> 
>    The "/snmp/engine/version" container can be used to enable/disable
>    the different message processing models.
> 
> "message processing models": that's a specific SNMP term, well not well 
> known to SNMP users.
> I would add a reference to RFC 3412
>
> -
> Similarly to
>    This submodule defines the feature "tsm".  A server implements this
>    feature if it supports the Transport Security Model (tsm) [RFC5591  
>    <http://tools.ietf.org/html/rfc5591>].
> 

OK. Although I think this should be a reference to RFC 3411 since RFC
3411 defines the components (called models in SNMP speak) and RFC 3412
is just one specific instance.

>    ...
> 
>      feature tsm {
>        description
>          "A server implements this feature if it supports the
>          Transport Security Model for SNMP.";
>        reference
>          "RFC5591  <http://tools.ietf.org/html/rfc5591>: Transport Security 
>          Model for the
>                    Simple Network Management Protocol (SNMP)";
>      }
> 
> It would be great to have references for:
> 
>    This submodule defines the feature "notification-filter".  A server
>    implements this feature if it supports SNMP notification filtering.

Added reference to RFC 3413.
 
>    ...
> 
>      feature notification-filter {
>        description
>          "A server implements this feature if it supports SNMP
>          notification filtering.";
>      }

Added reference	to RFC 3413.

> And for
> 
>    This submodule defines the feature "proxy".  A server implements this
>    feature if it can act as an SNMP Proxy.

Added reference to RFC 3413.

>    ...
> 
>      feature proxy {
>        description
>          "A server implements this feature if it can act as an
>          SNMP Proxy";
>      }
> 

Added reference to RFC 3413.

> 
>      augment /snmp:snmp/snmp:target {
>        when "snmp:v1 or snmp:v2c";
>        leaf mms {
>          type union {
>            type enumeration {
>              enum "unknown" { value 0; }
>            }
>            type int32 {
>              range "484..max";
>            }
>          }
>          default "484";
>          reference
>            "SNMP-COMMUNITY-MIB.snmpTargetAddrMMS";
>        }
>      }
> 
> 
> mms: I thought it was a mistake: nms versus mms.
> I had to lookup snmpTargetAddrMMS to understand "maximum packet size"
> Proposal: either add a description or have a better name

I prefer to keep the name since the SNMPv3 specifications use
this name. I added a description clause (and I surprised that
a leaf without a description clause slipped through).

Do you want us to post a new revision?

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1, 28759 Bremen, Germany
Fax:   [phone]         <http://www.jacobs-university.de/>