Subject: Re: [netmod] draft-ietf-netmod-interfaces-cfg-11.txt

From: Martin Bjorklund <[email-masked]>
Date: Thu, 16 May 2013 08:49:57 +0200 (CEST)

[adding netmod to cc]

Hi David,

Thank you for the fast review and reply!

"ietfdbh" <[email-masked]> wrote:
> Hi,
> 
> A few comments on the new draft.
> It looks much better on many of the issues.
> Still a bit of work to do.
> 
> 1) The ifAlias-related text places a new compliance requirement on SNMP
> implementations.
>              Since ifAlias is defined to be stored in non-volatile  
>                   storage, the SNMP implementation MUST map ifAlias to the  
>                   value of 'description' in the persistently stored  
>                   datastore.
> (This really should be specified as the "MIB implementation", not the "SNMP
> implementation", since the MIB should be able to be used by non-SNMP
> protocols, per RFC1052).

Ok.

> To add such a compliance requirement would require stating that this
> document UPDATES RFC2863, something I don't believe this WG is authorized to
> do.

Hmm, that was not the intention.  This document does not change any
semantics of RFC 2863.  On the contrary - it specifies what
implementations that want to have the same underlying instrumentation
for ifAlias and description have to do in order to stay compliant with
both RFC 2863 and NETCONF/YANG requirements.

> I'm not sure whether we can specify a YANG implementation requirement that
> handles this appropriately, Or whether we just need to have an "operational
> considerations" note for admins warning about the different persistence
> models, And advising admins to sync ifAlias (or the underlying shared
> instrumentation) with their persistent NETCONF datastore (:running or
> :startup).
> (since it is designed to be persistent across reboots, this may only need to
> be done once.)

This cannot be controlled by admins; the note is directed to
implementations.  If an implementation chooses to map description to
ifAlias, and follows this requirement, it means that the admin will
see changes done to ifAlias in the 'description', and vice versa.
(As noted, this is how it works today in some vendors implementations
of ifAlias and 'description' in the CLI).

> 2) interfaces-state/name
> s/ This leaf MAY be mapped to ifName by an implementation./ This leaf MAY be
> mapped from ifName by an implementation./
>  interfaces-state/name contains a "MUST restrict the values" clause. Isn't
> this only necessary on config=true objects?
> interfaces-state/name simply must be able to read the ifName (whose value is
> write-restricted by IF-MIB and interfaces/name, and maybe the CLI).
> I suspect this was just a cut-n-paste error.

I will remove this text.

> 3) interfaces-state
> It's been a while since I was actively involved with YANG.
> I assume that config=false applies to all objects in the interfaces list
> within interfaces-state?

Yes, this is correct.  The config property is inherited by the
sub-nodes, recursively.

> I see the objects/leafs are not each stated as
> being config-false.

> 4) "This leaf has the same semantics as ifAdminStatus"
> ifAdminStatus is read-write, interfaces-state/admin-status is config-false,
> so this isn't quite accurate.
> s/ This leaf has the same semantics as ifAdminStatus/ This leaf has the same
> read semantics as ifAdminStatus/

Yes, makes sense.  I will make this change.

> 5) Does section 4 need to be updated to reflect the config versus status
> split?
> The table doesn't separate config versus state. Should it?

I suggest we simply make it clear which one we mean when there's a
risk for confusion.  So I suggest:

| YANG data node                   | IF-MIB object          |
|----------------------------------+------------------------|
| /interfaces-state/interface      | ifEntry                |
| /interfaces-state/name           | ifName                 |

> 6) Section 4 says ifPromiscuousMode is not included because it is not a
> configuration object.
> now that we have interfaces-state, should ifPromiscuousMode be included?

Do you think it should be included?

/martin