Subject: [netmod] review of draft-ietf-netmod-interfaces-cfg-04

From: Juergen Schoenwaelder <[email-masked]>
Date: Sun, 10 Jun 2012 15:48:25 +0200

I have reviewed draft-ietf-netmod-interfaces-cfg-04 and here are my
comments/questions.

a) p9: s/2011/2012/

b) p15: s/destined to it/it should receive and process/   (twice)

   interfaces/interface also allows to modify the MTU - so perhaps
   this needs to be reflected as well, i.e., setting the MTU to a very
   small value can be used to slow down interfaces.

c) p17: Are the references RFC2579, RFC6241, RFC6242 really normative?
        The routing document I think has RFC6241 and RFC 6242 as
        informative. Perhaps we should also explicitely refer to
        RFC6536 or even encourage its implementation.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 11 Jun 2012 15:48:46 +0200 (CEST)

> I have reviewed draft-ietf-netmod-interfaces-cfg-04 and here are my

Ok.

>    small value can be used to slow down interfaces.

Ok.

>         informative.

No, fixed.

> Perhaps we should also explicitely refer to

This seems like a good idea.  But we should probably update the
Security Sections Template with this reference.  RFC 6087 has a link
to the template, but apparently this link is now broken :(
http://www.ops.ietf.org/netconf/yang-security-considerations.txt

/martin

---

From: "Alexander Clemm (alex)" <[email-masked]>
Date: Mon, 11 Jun 2012 16:59:26 -0700

Hello Martin,

I have reviewed the draft as well, here are my comments

p5 - location leaf is optional but mandatory in case the type represents
a physical interface.  This is not reflected in the YANG module.  For
consistency reasons, it seems that if it is supposed to be mandatory,
this needs to be added in the YANG module, which may however become
overly complex in turn.  Alternatively, remove this condition.  

p7 - ifPromiscuousode not mapped because not a configuration object.
Agree with this in spirit, but neither is ifIndex, yet it is included.
It might be good to add a comment why to keep ifIndex (important for
mappings, want it persisted between reboots, etc etc) 

p10 - leaf name "A device MAY restrict the allowed values for this
leaf...."  It would be good to provide a section that provides clear
instructions/ recommendations how to provide such restrictions.  (An
implementation will have to be clear on what those restrictions are, as
ultimately they will need to be validated.)  Section 3.1 could be
expanded for this purpose, which already talks about how to augment this
for interface type specific data.  

p10 - leaf description "This leaf MAY be mapped to ifAlias..."   Similar
comment - if an implementation restricts the values, where is this
specified.  Since this is more a binary choice - either it maps to
ifAlias or it doesn't - it seems this should be supported in the model
itself, perhaps using the feature construct.  

p10 - leaf type:  If the value of this leaf can be derived from the
name, does this need to be mandatory?  Or rather, does this not
introduce another integrity constraint or depend on device feature (the
ability to derive the type from the interface name)?   

p11 - leaf enabled:  Why not simply call the leaf adminStatus?  Seems a
little clearer.  Of course, then it can no longer be simply be a
Boolean; however, the term "enabled" appears more like a concept of an
operational state to me.  
As a side note, if the purpose of this is to turn on or off an
interface, have you considered defining this as an RPC?  This might come
in handy also in light of the Netconf-light discussions.  If all that a
device supports is a copy-config in its entirety, you would need to
replace the entire configuration whenever you want to enable or disable
a single interface.  Arguably, this could simply be defined as its own
operation instead.

--- Alex

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf
Of Martin Bjorklund
Sent: Monday, June 11, 2012 6:49 AM
To: [email-masked]
Cc: [email-masked]
Subject: Re: [netmod] review of draft-ietf-netmod-interfaces-cfg-04

Ok.

Ok.

No, fixed.

/martin
_______________________________________________
netmod mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netmod

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 12 Jun 2012 13:36:31 +0200 (CEST)

Hi Alex,

Thanks for your reviews!  Comments inline.

> overly complex in turn.  Alternatively, remove this condition.  

It doesn't make much sense to make 'location' mandatory in the data
model.  However, you are right that the text and YANG module are not
consistent.  I will clarify that this in the YANG module.

> mappings, want it persisted between reboots, etc etc) 

Actually, ifIndex is not included in the configuration model - it is
included as a non-config object, in order to facilitate the mapping.
This document does not assume anything about if/how ifIndex is
persistent between reboots.

> for interface type specific data.  

The current text says:

           Such an implementation MAY restrict the allowed values
           for this leaf so that it matches the restrictions of
           ifName.";
        reference
          "RFC 2863: The Interfaces Group MIB - ifName";

This really just refers the reader to the ifName definition (which
leads to RFC 2579 DisplayString, which in turn references RFC 854).  I
would prefer to not duplicate this text, but keep the reference.

> itself, perhaps using the feature construct.  

Whether this leaf is mapped to ifAlias or not doesn't affect this data
model, so defining a feature is probably not useful.  The text was
included to guide implementors that implement both this data model and
IF-MIB.

> name, does this need to be mandatory?

In the general case, the value of this leaf cannot necessarily be
dervied from the name.

The basic assumption is that all interfaces MUST have a type.  Thus,
this leaf is mandatory (it must exist in a valid config).  Note that
the name is an arbitrary string.  However, many devices do not allow
arbitrary strings; instead they encode the type (and location) in the
name.  Different systems have different such naming schemes, of
course.

> Or rather, does this not

> Boolean; however, the term "enabled" appears more like a concept of an

In a previous version it was actually called if-admin-status.  I think
we changed it b/c it does not have the exact same semantics as
ifAdminStatus, and also the term "enabled" is used in other data
models (routing, ip).

> interface, have you considered defining this as an RPC?

No, this is a persistently stored configuration object, so it should
not be modelled as an RPC.

> This might come
> a single interface.

This is OT, but if the device only supports copy-config then you have
to live with that...  any tiny little change requires you to send
everything.  OTOH, such a constrained device probably doesn't have too
many config params.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 12 Jun 2012 13:52:49 +0200

> included to guide implementors that implement both this data model and

I am personally fine with the references. One thing we could do,
however, is to specify how a server reacts that enforces ifName /
ifAlias restrictions and you send it some fancy UTF-8 characters.

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 12 Jun 2012 14:08:23 +0200 (CEST)

> ifAlias restrictions and you send it some fancy UTF-8 characters.

Ok.  Something like this maybe:

           If a NETCONF server that implements this restriction is
           sent a value that doesn't match the restriction, it MUST
           reply with an rpc-error with the error-tag
           'invalid-value'.";

/martin

---

From: "Alexander Clemm (alex)" <[email-masked]>
Date: Tue, 12 Jun 2012 15:01:26 -0700

Hello Martin,

thank you for your response - some replies below, <ALEX> delineated

Thanks
--- Alex

...

<ALEX> 

My comment pertained to the reason that is given for not including
ifPromiscousMode.  I agree with the fact that it is not included, and I
agree also with having ifIndex there to facilitate mappings.  However,
since neither ifPromiscuousMode nor ifIndex are part of the
configuration model, yet ifIndex is included, not being part of the
configuration model is by itself not enough reason for an object to be
excluded.  Hence the comment that it might be good to include a brief
explanation what it is that makes ifIndex special.  

</ALEX>

The current text says:

<ALEX> 

The text given in the leaf description makes sense and clearly is all
that can be specified here.  My comment pertains to the product that now
wants to implement that.  This implementation may very well (as
specified here) restrict the allowed values.  How and where are these
restrictions to be expressed?  One way is to not express these at all.
In that case, some validation above and beyond what is specified in the
YANG module needs to occur, and the rules for that validation are not
obvious from the YANG module itself.  An alternative would consist in
introducing an augmentation that defines these additional constraints.
I believe the usefulness of the document would be improved if it could
provide some guidance on how to do that instead of leaving it up to
readers of the document to figure it out for themselves; it would make
the spec easier to use and show implementers how the value of YANG-based
validation can still be derived.  

</ALEX>

.....

<ALEX>

I understand this is MODELED as a persistently stored configuration
object, but it doesn't have to be that.  You can also interpret this
simply as an action.  Just as a consideration, with regards to the
Netconf-light discussion, if you force people to replace the entire
configuration each time they plan to enable or disable a single
interface will seem awkward - just my 2 cents.

</ALEX>

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 13 Jun 2012 04:49:04 +0200

> interface will seem awkward - just my 2 cents.

Its awkward but introducing separate RPCs for every possible
configuration change is hopeless and a high price for supporting one
specific class of devices. NETCONF Light was originally designed for
really small devices where replacing the entire (but small) structured
config is already an improvement over replacing the whole firmware to
turn something on/off.

I think we should not let considerations for such devices impact the
design of data models that are primarily targeting devices that can
easily support NETCONF.

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 13 Jun 2012 09:24:08 +0200 (CEST)

Hi,

"Alexander Clemm (alex)" <[email-masked]> wrote:
> ifPromiscousMode.

It is writable in the MIB, but does not represent configuration.
Hence it is not included.

ifIndex is not writable in the MIB, so the reason for not including
ifPromiscousMode does not apply to ifIndex.

Do you have a suggestion for some text to make this more clear?

> YANG module needs to occur

Yes, this is the current situation.

> and the rules for that validation are not
> obvious from the YANG module itself.

Correct.  The YANG module doesn't have any formal restriction, but it
allows implementations to restrict values according to the rules of a
DisplayString.

> An alternative would consist in

Unfortunately, this isn't really possible with standard YANG.  You
cannot add constraints via augmentations.

> interface will seem awkward - just my 2 cents.

I agree with Juergens comment on this one.  If we start to add RPCs to
change some config params, where do we stop?  NETCONF is by design
"document-oriented" (see Juergen's expired
http://tools.ietf.org/id/draft-schoenw-sming-lessons-01.txt) and we
should not change that.

/martin

---

From: "Alexander Clemm (alex)" <[email-masked]>
Date: Wed, 13 Jun 2012 14:34:19 -0700

Hi Martin,

inline replies w/<ALEX>

Thanks
--- Alex

...

<ALEX> 
How about adding something like the following just after the original
sentence (modify as appropriate:

Original sentence:
   The IF-MIB also defines the writable object ifPromiscuousMode.  Since
   this object typically is not a configuration object, it is not mapped
   to the "ietf-interfaces" module.

Possible addition:  
The object ifIndex, while it is not a configuration object either, is
fairly ubiquitous and hence mapped to the "ietf-interfaces" module in
order to facilitate mappings between interface configuration information
and information in MIBs.   

</ALEX

....

> validation can still be derived.  

<ALEX>
So, is there another way of address this issue?  What are implementers
to do if they have constraints - are there any recommendations that can
be made?  
- Should they treat this outside Netconf/YANG?   This is not very
satisfactory because it detriments the value proposition and usefulness
of this object as validation has to occur in different places anyway,
but being clear about it doesn't leave people second-guessing.  
- Introduce augmentation with a second "shadow object" which does have
the appropriate constraints, including a constraint that its value must
mirror/be set to the same value of the other object?  (That would
presumably be legal, correct?, but ugly & I hope not)
- Make use of deviation?  
- Other options?  
</ALEX>

....

<ALEX>
I understand the argument about config params, but there is a grey line
between this and RPCs, and RPCs are also supported by Netconf despite
its document-orientation.  (Any RPC could be addressed also through
parameters - think Ping MIB - I agree this would be awkward which is why
I consider this a feature.)  To me, configuration is something that
guides the behavior of a system, versus an RPC which define actions to
be taken at a particular instant in time.  This particular case is a
borderline case which can be argued either way, that I can agree with.
(I don't think whether an interface is enabled or disabled is just "any"
config parameter, it is of a different nature than e.g. a config
parameter that defines an interface name.)  Do we have guidelines when
to use which?  
</ALEX>

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 13 Jun 2012 23:52:36 +0200 (CEST)

Hi,

> presumably be legal, correct?, but ugly & I hope not)

This might be possible, although it requires that the restriction can
be formally expressed in YANG - for a string it means that you define
a regexp that enforces the restriction.  Sometimes this is not
possible, and then you have to resort to descriptions in plain text,
and enforment in special code.

> parameter that defines an interface name.)  Do we have guidelines when

If you want the value to be part of a config backup, it should be
modelled as data.

/martin

---

From: "Alexander Clemm (alex)" <[email-masked]>
Date: Wed, 13 Jun 2012 15:03:09 -0700

Hi Martin,

thank you for your response to the 2nd item.

Regarding the 1st, in short what you are saying is that we are running
into the limitations of YANG here (which implementers in this case will
likely encounter) but it should not necessary to give guidance.  I
understand that you can add enforcements in separate code and describe
them separate from the YANG module itself.  I had been wondering if it
is possible to do a little better than that but I can live with it.  

Regards
--- Alex

-----Original Message-----
From: Martin Bjorklund [mailto:[email-masked]] 
Sent: Wednesday, June 13, 2012 2:53 PM
To: Alexander Clemm (alex)
Cc: [email-masked]; [email-masked]
Subject: Re: [netmod] review of draft-ietf-netmod-interfaces-cfg-04

Hi,

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 14 Jun 2012 08:54:15 +0200

> presumably be legal, correct?, but ugly & I hope not)

The simple answer is that a value that is not supported causes a
meaningful error to be returned. As a matter of fact, it was never
possible to capture all constraints in machine readable format. And
while we talk here about constraints that originate from another
standard, we also face reality constraints that are device or
operating system specific. This grey area always existed and it will
continue to exit. We deal with that by returning a meaningful error
and textual descriptions.

The reality is that my OS has certain restrictions. The SNMP MIB
module has certain restrictions. The YANG module has certain
restrictions. We do not know what the OS restrictions are so we can't
do much (other than providing an error indication). We know what the
SNMP restrictions are and we know that YANG supports a superset.  This
is what we deal with. The rest is outside. (In fact, there might be
implementations that decide to support more flexible YANG names and
choose to tweak the SNMP code to render stuff into what the IF-MIB
allows - perhaps they needed to do this anyway because the underlying
OS was never limited to IF-MIB contraints and perhaps not even to
ietf-interfaces constraints as they do not even enforce a character
set.)

Bottom line: I believe what is in the I-D is the best we can do.

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 14 Jun 2012 10:19:55 +0200 (CEST)

> is possible to do a little better than that but I can live with it.  

I think Juergen's reply describes the problem well.  As for the
guidance part; I think that the current text is sufficient when it
says:

           This leaf MAY be mapped to ifName by an implementation.
           Such an implementation MAY restrict the allowed values for
           this leaf so that it matches the restrictions of ifName.
           If a NETCONF server that implements this restriction is
           sent a value that doesn't match the restriction, it MUST
           reply with an rpc-error with the error-tag
           'invalid-value'.";
        reference
          "RFC 2863: The Interfaces Group MIB - ifName";

(the last sentence is new)

The text mentions that ifName has some restrictions, and it gives a
reference to its defintion.

But in general I absolutely agree with you.  A formal definition of a
restriction is better than one described in plain text.  However,
sometimes it is not possible to define a restriction formally
(depending on the formal language available of course).

/martin