Subject: [MIB-DOCTORS] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

From: "Joan Cucchiara" <[email-masked]>
Date: Fri, 5 Mar 2010 14:14:01 -0500

Hello Kalyan,

The examples in the document are very helpful and
there was a great deal of work that went into this document.
Thank you for that!

MIB compiler output is first, followed by comments.

Thanks,
  -Joan

MIB Compiler: SMICng
--------------------

W: f(VRRP-MIB.my), (437,8) Row "vrrpAssociatedIpAddrEntry" has indexing
   that may create variables with more than 128 sub-ids
W: f(VRRP-MIB.my), (158,23) Row "vrrpOperationsEntry" does not have a
   consistent indexing scheme - index items from current
   table must come after index items from other tables
W: f(VRRP-MIB.my), (453,23) Row "vrrpAssociatedIpAddrEntry" does not have
   a consistent indexing scheme - cannot specify an index item from
   additional "base row" vrrpOperationsEntry, since can have
   only one "base row" which is ifEntry

WRT the INDEX To Large error.  I think this was addressed by Bert Wijnen in 
prior
versions but I don't see any wording in the DESCRIPTION clause or any SIZE 
limit
on the Index vrrpAssociatedIpAddr.  So something like this (and some text 
describing
why the size limits in the DESCRIPTION clause would be appropriate. 
Additionally,
the vrrpOperationsInetAddrType should be restricted.

       vrrpAssociatedIpAddr OBJECT-TYPE
           SYNTAX       InetAddress (SIZE(0|4|16))
           MAX-ACCESS   not-accessible
           STATUS       current
           DESCRIPTION

Please discuss the other 2 warnings wrt Indexing.  Specifically, why are
tables using AddrType, VrId and ifIndex, rather than ifIndex, VrId, 
AddrType?
(Please see comment below regarding section 8.)

MIB Compiler: SmiLint
------------------------
mibs/VRRP-MIB.my:437: [5] {index-exceeds-too-large}
   warning: index of row `vrrpAssociatedIpAddrEntry'
   can exceed OID size limit by 142 subidentifier(s)

Comments
--------

1)NIT:  Does not strip cleanly with SmicngPRO mstrip
(This doesn't need to be fixed for MIB Dr. review, just
as an fyi).

2) Please Discuss:
My understanding (based on an old email exchange (fall 2008) with the MIB 
author
and the AD of the WG at that time) was that the VRRP unified MIB
was not going to be an update of the original VRRP-MIB (RFC2787), but
rather a new MIB based on the new unified spec that was being
created.  There are a couple issues that I'd like clarified wrt to an
updated MIB (this draft) vs. a new MIB:

A) draft-ietf-vrrp-unified-spec-05.txt, section 8.4 states "that VRRPv2 and 
VRRPv3
interoperation is optional" and the
recommendation is that it should only be done for transitioning from
VRRPv2 to VRRPv3.  A new MIB (rather than an updated
MIB) would likely re-enforce the guidelines from the specification.
Could you discuss why an updated MIB is better in this regard?

B)  Additionally, this new MIB is based on a MIB that is 10 years old 
(RFC2787),
while not specifically an issue, I do see that this updated MIB is
using older MIB conventions.  Not a show-stopper but certainly a
consideration.  Was this considered?

C) Comment: Most of the MIB objects contained in RFC2787 are deprecated
by this MIB. This indicates that creating a new MIB would be
fairly straightforward.

3) The terminology of this document does not consistently utilize the 
Definitions
outlined in the draft-ietf-vrrp-unified-spec-05.txt.  So for example,
VRRP is used in places to mean VRRP Router.  The use of IPvX is not
done.  Would be better to be consistent between the spec and the MIB.

4)  DEPRECATED OBJECT DESCRIPTIONS:

Way back when, Dave Perkins suggested starting the DESCRIPTION clause
for a deprecated object with:

"This object is DEPRECATED.  This is done because..."
followed by the original description.

While you have done a great job including the reasons why an object was
deprecated, could you move this to the beginning of the DESCRIPTION clause
(rather than end)?

5) Title: Please mention that this is Version 3 somewhere in the title
(as does the specification)

6) Section 4. Relationship to RFC2787

The following sentence is troublesome because the MIB Module "deprecates"
objects and does not obsolete them.  While you may be trying to indicate
that this document will obsolete RFC2787, that is not exactly clear.

(perhaps, this is another reason to create a new MIB module vs. an updated
one?)

   "RFC2787 [RFC2787] defines managed objects for VRRP over IPv4 and is
   now obsoleted by this memo."

7) Section 5. Relation to Interface Group (IF-MIB)

A) Please remove the word "physical" in front of physical interface.

B) Should specify a reference such as:  The VRRP-MIB Module imports
ifIndex from the IF-MIB.  At this time, the latest version
of IF-MIB is from [RFC2863].

C) A reference to [RFC2863] needs to be provided in the
Normative References.

8) Section 7. VRRP MIB Structure

This section should discuss the relationship between the tables.

The groups listed are part of the conformance, and while okay
to mention these, that really isn't the MIB's structure.

NIT: Should have a (3) before "The vrrpAssociatedIpAddrTable"

9) Section 8. VRRP MIB Table Design

The MIB design should focus on indexing the tables such that
the table lookups are more advantageous for the device (e.g. router),
 not the NM app.  The NM app should be able to
handle rearranging data for display purposes.

I would like to see a discussion of the indexing wrt the device.
The warnings from the MIB compiler indicate that there may be a
better indexing (table structure) available.  That doesn't mean
that you need to resolve the warnings, but I would like to see
some text included which shows why the chosen indexes are
advantageous for the VRRP router, and the Virtual Routers.

IMHO, if an interface goes down,
then would be probably more advantageous
to know the Virtual router(s) associated with that interface.
(and subsequently the IP addresses on those Virtual Routers).

Please combine Section 7 and Section 8.

10) Section 9.

Thank you for these examples.

11) Section 10.  Please specify that this is
the VRRP MIB Module Definition

* DATE and REVISION dates need to be updated.

* VrId (Textual Convention)

There is already a VrId in RFC2787. While it looks as if the
only difference is the DESCRIPTION clause, need to caution against
doing redefining this.  I would suggest creating a VRID TC and
making the DESCRIPTION simple, for example:  This value uniquely identifies 
a
Virtual Router on a VRRP router.

While the remaining text is informative, it is not really necessary.
Please also give a REFERENCE clause for this.

* vrrpNodeVersion

This scalar has correctly been deprecated, but I don't see any
version object replacement in a table.  Why not?  (There is
field in the protocol for this.)

* vrrpNotificationCntl, vrrpTrapNewMasterCntl, vrrpTrapProtoErrorCntl

Could notifications be generated or not, using
RFC3413?

*StorageType objects:

If an operator configures the VRRP Router with Virtual Router(s)
and IPvX addresses then I suspect that this information would be
saved to nonVolatile storage.  Do you agree?

In other words, I'm trying to think of a scenario when an operator
wouldn't want to save this information (once he/she is satisfied with
the configuration) in the sake of simplification, would there be
benefit to just state that the configuration should be saved
to NV storage?  If so, then
the use of StorageType objects in this MIB could be removed and
replaced with text in the Table Entry DESCRIPTION.

(RFC4181, Section 4.6.4 specifies that StorageType objects can be
used OR, the Table Entry DESCRIPTION clause specify what happens
to dynamically-created rows after an agent restart.)

* RowStatus Objects

There is some discussion about having to have the corresponding
Virtual Router in vrrpOperationsState have the value of
'initialize' before changing a value in the Row, and then the
RowStatus object is supposed to not be "active(1)", but I don't
understand what values are acceptable for modifying a read-creat
object in a row?  Also, how does this impact vrrpOperationsState?

* NIT: AdminState and OperationsState, could these be renamed to use
use AdminStatus and OperStatus?

* vrrpRouterVrIdErrors

Please verify that the DESCRIPTION is still valid for this MIB?
Should a value be added to the Statistics Table instead (i.e. on
a per Virtual Router basis?

* Please refrain from using the term "trap" in the current 
objects/notifications.
Please use the term "notification" as appropriate for the
current objects/notifications.  (Deprecated items are fine to leave the term
trap.)

* vrrpNewMasterReason, vrrpTrapProtoErrReason

Where are these Enum values from?  Could you add a reference?
Also, please explain the values.

* Conformance looks fine, but will review again once above MIB
Module comments are addressed.  Same with security section.

Thanks,
  -Joan

---

From: Adrian Farrel <[email-masked]>
Date: Fri, 05 Mar 2010 19:19:47 +0000

Joan,

Very many thanks.

Cheers,
Adrian
----- Original Message ----- 
From: "Joan Cucchiara" <[email-masked]>
To: "Mukesh Gupta" <[email-masked]>; "Romascanu, Dan (Dan)" 
<[email-masked]>; "Adrian Farrel" <[email-masked]>; "Ronald 
Bonica" <[email-masked]>; <[email-masked]>; <[email-masked]>
Cc: "MIB Doctors (E-mail)" <[email-masked]>; 
<[email-masked]>; <[email-masked]>; 
"Radia Perlman" <[email-masked]>
Sent: Friday, March 05, 2010 7:14 PM
Subject: draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

> WRT the INDEX To Large error.  I think this was addressed by Bert Wijnen 
> in prior
> A) draft-ietf-vrrp-unified-spec-05.txt, section 8.4 states "that VRRPv2 
> and VRRPv3
> making the DESCRIPTION simple, for example:  This value uniquely 
> identifies a

---

From: "Kalyan (Srinivas)Tata" <[email-masked]>
Date: Mon, 8 Mar 2010 12:17:01 -0800

Hi Joan,
Thanks for the comments. I will reply back in a day with explanations of very valid issues you raised. 

Thanks
Kalyan

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Joan Cucchiara
Sent: Friday, March 05, 2010 11:14 AM
To: Mukesh Gupta; Romascanu, Dan (Dan); Adrian Farrel; Ronald Bonica; [email-masked]; [email-masked]
Cc: MIB Doctors (E-mail); Radia Perlman; [email-masked]; [email-masked]
Subject: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Hello Kalyan,

Thanks,
  -Joan

MIB Compiler: SMICng
--------------------

MIB Compiler: SmiLint
------------------------

Comments
--------

2) Please Discuss:

10) Section 9.

* vrrpNodeVersion

*StorageType objects:

* RowStatus Objects

* vrrpRouterVrIdErrors

Thanks,
  -Joan

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

Scanned by Check Point Total Security Gateway.

---

From: "Kalyan (Srinivas)Tata" <[email-masked]>
Date: Mon, 8 Mar 2010 23:27:57 -0800

HI Joan,

My comments inline.

MIB Compiler: SMICng
--------------------

[Kalyan>] Will do. 

[Kalyan>] The table indexing order was suggested by Bert during very initial review of the draft. The comment was that it would be easier for the administrator to sort based on the address type. Though I am not sure if I changed from ifIndex, VrId, AddrType. I probably had it wrong at that time too. I will change this ifIndex, VrId, AddrType 

MIB Compiler: SmiLint
------------------------

Comments
--------

[Kalyan>] Will fix it.

2) Please Discuss:

[Kalyan>] We had this discussion during the first draft. At that time updating made more sense but now with a separate unified spec, probably creating a new MIB would be better. Also my reasoning was that the previous draft has gone through multiple reviews and revisions, I wanted to just make the required changes to quickly get this done with less issues (and less work from me :) ).  But, I see your point, a new draft would certainly be much cleaner and probably the way to go.

[Kalyan>] Will be more aware of this in the next revision.

[Kalyan>] Will do.

[Kalyan>] I remember asking Mukesh if we should change the title and I think he mentioned that we keep the same one. We can change the title if we are going with a new MIB.

[Kalyan>] 

[Kalyan>] I was meaning to say that this document will obsolete RFC2787. I get it :), going to a new MIB probably is the right thing.

[Kalyan>] Will do.

[Kalyan>] Will do

[Kalyan>] Will do.

[Kalyan>] I do not have any strong reasons for the indexing, As I mentioned, It was suggested in the earlier meeting and it remained that way during all these revisions. I will change this to ifIndex, VrId, AddrType

10) Section 9.

[Kalyan>] Agreed.

* vrrpNodeVersion

[Kalyan>] This MIB would support only version 3 of the protocol and hence I removed the version from the table.

[Kalyan>] Notifications could be filtered/generated using RFC3413. I added these to conveniently control notifications within this MIB (Based on request from some one on the mailing list) You suggest we getrid of these?

*StorageType objects:

[Kalyan>] I am OK with either way of doing this - Will add the text to description and remove the StorageType.
[Kalyan>] 
* RowStatus Objects

[Kalyan>] Will do.

* vrrpRouterVrIdErrors

[Kalyan>] vrId itself is invalid, so we probably cant index into the table? (as one of the indexes for the table is vrId)? May be I am not understanding the problem correctly.

[Kalyan>] Will do.

[Kalyan>] These are not defined in the spec. We just came up with possible reasons for a VR to transmission to Master state. 

[Kalyan>] Thanks again for the detailed review. I probably is a good idea to start with a new MIB and address all the issues you raised. 

Thanks,
  -Joan

---

From: "Joan Cucchiara" <[email-masked]>
Date: Wed, 10 Mar 2010 13:54:43 -0500

Hi Kalyan,

Thank you for your quick response.  Please see
comments inline.

Thanks,
  -Joan

> From: "Kalyan (Srinivas)Tata" <[email-masked]>
> To: "Joan Cucchiara" <[email-masked]>; "Mukesh Gupta"
> <[email-masked]>; "Romascanu, Dan (Dan)" <[email-masked]>; "Adrian
> Farrel" <[email-masked]>; "Ronald Bonica" <[email-masked]>;
> <[email-masked]>; <[email-masked]>
> Cc: "MIB Doctors (E-mail)" <[email-masked]>; "Radia Perlman"
> <[email-masked]>; <[email-masked]>;
> <[email-masked]>
> Sent: Tuesday, March 09, 2010 2:27 AM
> Subject: RE: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review
>
>
> HI Joan,
>
> My comments inline.
>
>>
>> MIB Compiler: SMICng
>> --------------------
>>
>> in
> [Kalyan>] The table indexing order was suggested by Bert during very
> initial review of the draft. The comment was that it would be easier for
> the administrator to sort based on the address type. Though I am not sure
> if I changed from ifIndex, VrId, AddrType. I probably had it wrong at that
> time too. I will change this ifIndex, VrId, AddrType
>

Okay, I can see his point that accessing by IP addresses has some
advantages.   Unfortunately,  the way the tables are currently, you'd
need to have "AddrType, Addr"   together in order to do this.

Let's leave this an open issue for the moment and close on the other issues 
first.

>> and
> [Kalyan>] We had this discussion during the first draft. At that time
> updating made more sense but now with a separate unified spec, probably
> creating a new MIB would be better. Also my reasoning was that the
> previous draft has gone through multiple reviews and revisions, I wanted
> to just make the required changes to quickly get this done with less
> issues (and less work from me :) ).  But, I see your point, a new draft
> would certainly be much cleaner and probably the way to go.

Would be good for the WG to give an opinion on this issue since LC
is going on now.

> [Kalyan>] I remember asking Mukesh if we should change the title and I
> think he mentioned that we keep the same one. We can change the title if
> we are going with a new MIB.
>

Agreed.

> [Kalyan>] I was meaning to say that this document will obsolete RFC2787. I
> get it :), going to a new MIB probably is the right thing.
>
> [Kalyan>] I do not have any strong reasons for the indexing, As I
> mentioned, It was suggested in the earlier meeting and it remained that
> way during all these revisions. I will change this to ifIndex, VrId,
> AddrType
>

So will revisit this in the next email or two.

>> identifies

Okay.

> [Kalyan>] Notifications could be filtered/generated using RFC3413. I added
> these to conveniently control notifications within this MIB (Based on
> request from some one on the mailing list) You suggest we getrid of these?
>

Yes, I think that most operators would likely use rfc3413.  Having this in 2 
places
may create some confusion...

> description and remove the StorageType.

Thanks!

> [Kalyan>] vrId itself is invalid, so we probably cant index into the
> table? (as one of the indexes for the table is vrId)? May be I am not
> understanding the problem correctly.
>

Sorry, I meant to ask if this scalar is valid wrt the DESCRIPTION?
Should an Errors object be added to the Statistics Table to record errors
on a per VRRP Router basis?

> [Kalyan>] These are not defined in the spec. We just came up with possible
> reasons for a VR to transmission to Master state.
>

Okay, please add more description as to how each reason is determined.

> [Kalyan>] Thanks again for the detailed review. I probably is a good idea
> to start with a new MIB and address all the issues you raised.
>

Thanks,
  -Joan

---

From: "Joan Cucchiara" <[email-masked]>
Date: Tue, 16 Mar 2010 15:04:55 -0500

Just to be clear,  "starting over" is not as drastic as it may sound.
Most (all?) of the objects already defined in this draft for VRRPv3 with a
STATUS current, would remain.  Although, the title and object names
would likely change to indicate Version 3 (v3), e.g. VRRP-MIB would
likely change to VRRPv3-MIB.

The deprecated objects (in this case, most of the objects in VRRP-MIB 
(RFC2787))
would not appear in this new MIB.

The advantage is that a device would support the VRRP-MIB or the
VRRPv3-MIB, as opposed to a VRRP-MIB with v3 objects and a "phase-out" of
v2 objects.  The v3 specification states that supporting both v2 and
v3 is not supported, a separate v3 MIB would help to drive this point home.

Aside from the LC comments which need to be addressed regardless of whether 
or
not a specific VRRPv3-MIB is created, what is involved would largely be 
removal
of v2 specifics (which are already documented by RFC 2797) and name changes 
to add a v3
as appropriate.  So most of this is editorial in nature, do you agree 
Kalyan?

Thanks,
  -Joan

----- Original Message ----- 
From: "Kalyan (Srinivas)Tata" <[email-masked]>
To: "Joan Cucchiara" <[email-masked]>; "Romascanu, Dan (Dan)" 
<[email-masked]>; "Adrian Farrel" <[email-masked]>; "Ronald 
Bonica" <[email-masked]>; <[email-masked]>; <[email-masked]>
Cc: "MIB Doctors (E-mail)" <[email-masked]>
Sent: Tuesday, March 16, 2010 1:51 PM
Subject: RE: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Hi Radia and Mukesh,
        I think we need WG to express opinion on whether to start with a new 
MIB or continue with this one. This will determine how to go about with 
changes suggested by Joan.

Thanks,
Kalyan

-----Original Message-----
From: Joan Cucchiara [mailto:[email-masked]]
Sent: Wednesday, March 10, 2010 10:55 AM
To: Kalyan (Srinivas)Tata; Romascanu, Dan (Dan); Adrian Farrel; Ronald 
Bonica; [email-masked]; [email-masked]
Cc: MIB Doctors (E-mail)
Subject: Re: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Hi Kalyan,

Thanks,
  -Joan

Agreed.

Okay.

Thanks!

Thanks,
  -Joan

---

From: "Kalyan (Srinivas)Tata" <[email-masked]>
Date: Tue, 16 Mar 2010 11:51:01 -0700

Hi Radia and Mukesh,
        I think we need WG to express opinion on whether to start with a new MIB or continue with this one. This will determine how to go about with changes suggested by Joan.

Thanks,
Kalyan

-----Original Message-----
From: Joan Cucchiara [mailto:[email-masked]]
Sent: Wednesday, March 10, 2010 10:55 AM
To: Kalyan (Srinivas)Tata; Romascanu, Dan (Dan); Adrian Farrel; Ronald Bonica; [email-masked]; [email-masked]
Cc: MIB Doctors (E-mail)
Subject: Re: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Hi Kalyan,

Thanks,
  -Joan

Agreed.

Okay.

Thanks!

Thanks,
  -Joan

---

From: Mukesh Gupta <[email-masked]>
Date: Tue, 16 Mar 2010 14:09:07 -0700

WG Members,

Please voice your opinions about this so that we can move forward with the MIB draft.

Kalyan/Joan, historically, we have not heard a lot of opinions on the MIB draft on the WG mailing list, probably because we do not have enough MIB experts in the WG.  So, we have relied on the feedback/opinion of the MIB doctors.

[speaking as a WG member]
I am not a MIB expert myself but, after reading through your discussion, it does seem to me that a new clean MIB would be a way to go.

Regards
Mukesh

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Kalyan (Srinivas)Tata
Sent: Tuesday, March 16, 2010 11:51 AM
To: Joan Cucchiara; Romascanu, Dan (Dan); Adrian Farrel; Ronald Bonica; [email-masked]; [email-masked]
Cc: MIB Doctors (E-mail)
Subject: Re: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Hi Radia and Mukesh,

Thanks,
Kalyan

-----Original Message-----
From: Joan Cucchiara [mailto:[email-masked]]
Sent: Wednesday, March 10, 2010 10:55 AM
To: Kalyan (Srinivas)Tata; Romascanu, Dan (Dan); Adrian Farrel; Ronald Bonica; [email-masked]; [email-masked]
Cc: MIB Doctors (E-mail)
Subject: Re: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Hi Kalyan,

Thanks,
  -Joan

Agreed.

Okay.

Thanks!

Thanks,
  -Joan

---

From: "Kalyan (Srinivas)Tata" <[email-masked]>
Date: Tue, 16 Mar 2010 16:33:45 -0700

Hi Joan,
        Yes, I agree it is mostly editing work and all the structure would remain the same.

Thanks
Kalyan

-----Original Message-----
From: Joan Cucchiara [mailto:[email-masked]]
Sent: Tuesday, March 16, 2010 1:05 PM
To: Kalyan (Srinivas)Tata; Romascanu, Dan (Dan); Adrian Farrel; Ronald Bonica; [email-masked]; [email-masked]
Cc: MIB Doctors (E-mail)
Subject: Re: [VRRP] draft-ietf-vrrp-unified-mib-07.txt MIB Dr. Review

Thanks,
  -Joan

----- Original Message -----
From: "Kalyan (Srinivas)Tata" <[email-masked]>
To: "Joan Cucchiara" <[email-masked]>; "Romascanu, Dan (Dan)"

Hi Radia and Mukesh,

Thanks,
Kalyan

-----Original Message-----
From: Joan Cucchiara [mailto:[email-masked]]
Sent: Wednesday, March 10, 2010 10:55 AM
To: Kalyan (Srinivas)Tata; Romascanu, Dan (Dan); Adrian Farrel; Ronald

Hi Kalyan,

Thanks,
  -Joan

Agreed.

Okay.

Thanks!

Thanks,
  -Joan