Subject: [Gen-art] Gen-ART LC Review of draft-ietf-vrrp-unified-mib-09

From: Ben Campbell <[email-masked]>
Date: Tue, 10 May 2011 17:36:20 -0500

[The tools alias for the draft resulted in a bounce from [email-masked]. Resending to include the author's address as listed in the draft.]

I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments you may receive.

Document: draft-ietf-vrrp-unified-mib-09
Reviewer: Ben Campbell
Review Date: 2011-05-10
IETF LC End Date:2011-05-11
IESG Telechat date: (if known)

Summary: This draft is ready for publication as a draft standard. I have a few editorial comments that might be worth considering, but probably should not block publication.

Note: I am inexpert both in MIB definitions and in VRRP. I assume this has (or will be) reviewed by experts in both.

Major issues:

None

Minor issues:

None

Nits/editorial comments:

-- idnits complains about an obsolete normative reference: Obsolete normative reference: RFC 2338 (Obsoleted by RFC 3768)

I see why you have the 2338 reference--but does it need to be normative?

-- General: 

There's some odd formatting--no space between page header and body for pages 2 on. (I reviewed the PDF version, not sure if the TXT version is the same or if this is a PDF rendering issue.)

Does it make sense to use 2119 language in MIB object descriptive text? That will often show up outside the context of the draft, and therefore without the 2119 language definitions. I don't know what the convention is--I just point it out so others more used to MIBs can think about it.

-- section 3:

You've got the 2119 boilerplate twice.

-- section 7:

Please put blank lines between list entries

-- section 9, "vrrpv3OperationsPrimaryIpAddr"

Is "primary" the correct term here? Seems like  addresses would be "master" and backup, or primary and secondary. Master and primary sounds odd.

-- "vrrpv3OperationsUpTime"

It's probably worth describing the time interval unit in the text, as you did for the previous interval in centiseconds.

-- "vrrpv3OperationsRowStatus"

The description talks about how to use the RowStatus variable, but does not describe what it represents in the first place.

--"vrrpv3StatisticsProtoErrReason"

No discontinuity comment?

-- "vrrpv3StatisticsRefreshRate"
s/milli-seconds/milliseconds

Also, You might want to mention the time interval unit in the description.