Subject: AD review of draft-ietf-bfd-mib

From: "Adrian Farrel" <[email-masked]>
Date: Mon, 30 Dec 2013 13:02:58 -0000

Hi authors of draft-ietf-bfd-mib,

I have done my usual AD review after receiving a publication request.
The purpose of the review is to catch issues that would show up during
IETF last call or IESG review and so save the reviewers some time and
get the quality up to a level that is acceptable.

This document represents a lot of work: thank you. The issues I have 
found are relatively small. Some of them take the form of questions and
you can answer these with email rather than feeling compelled to make 
documentation changes. Furthermore, all of the issues are open for
discussion.

I will put the document into "Revised I-D" state and wait to hear back
from you.

Thanks,
Adrian

===

You'll obviously want to make updates to accommodate changes to 
draft-ietf-bfd-tc-mib

---

Tom may want to change his coordinates.

---

Could you swap sections 1 and 2 so that the document starts with the 
Introduction.

---

Section 2

s/an portion/a portion/

---

Section 4.4

I don't think the table maps a discriminator to a TC. Maybe it maps the
discriminator to the session or the session index or something.

The same thing shows up in some of the Description clauses, for example,
bfdSessDiscMapTable

---

Similarly 4.5

---

It is nice if you name the RFCs in comments next to the import clauses.

---

If I am creating an entry in bfdSessTable (and it seems that I can since
most of the objects are read-create) how do I know what value to pick 
for bfdSessIndex?

A way to handle this is through a "next available index" global object.

---

bfdAdminStatus is ambiguous!
Does setting bfdSessAdminStatus to "start" mean that the session is up
or only that the implementation will attempt to bring it up?
The normal approach is to have adminStatus described as "the desired
operational status" and a separate object reports the actual operStatus.
Since you have bfdSessOperMode, I think this is your intention.

---

bfdSessState and bfdSessRemoteHeardFlag are read-only. What does it mean
to have default values?

---

Doesn't bfdSessMultipointFlag need a reference to the document that 
defines multipoint BFD? If so, doesn't that create a normative reference
gate for this MIB module? That means you either take multipoint out or
you will have to wait until it completes the process.

---

You appear to have two mechanisms to turn off GTSM

    bfdSessGTSM OBJECT-TYPE
        SYNTAX  TruthValue
        DESCRIPTION
           "Setting the value of this object to true(1) will enable GTSM
            protection of the BFD session. 
... So presumably setting it to false will disable GTSM. But...

    bfdSessGTSMTTL OBJECT-TYPE
        SYNTAX Unsigned32 (0..255)
        DESCRIPTION
             The value of zero(0) indicates that
             bfdSessGTSM is disabled."

---

bfdSessAuthPresFlag has a default of false indicating that no 
authentication is to be used. And bfdSessAuthenticationType has a 
default of -1 meaning no authentication is in use. Doesn't that mean
that the default for bfdSessGTSM should be true and the default for
bfdSessGTSMTTL should be non-zero?

---

Why don't objects like bfdSessDesiredMinTxInterval have default values?

---

Shouldn't the default for bfdSessAuthenticationType be noAuthentication
not -1?

---

I am unsure about how the three objects for authentication work.

bfdSessAuthenticationType and bfdSessAuthenticationKeyID say that they
must be -1 if bfdSessAuthPresFlag is false.

Now, suppose I want to turn on authentication. I want to set
bfdSessAuthPresFlag to true, but if I do, the settings of 
bfdSessAuthenticationType and bfdSessAuthenticationKeyID are bogus.
But if I want to set bfdSessAuthenticationType and
bfdSessAuthenticationKeyID to real values in preparation to setting 
bfdSessAuthPresFlag to true then I will have broken the rule.

The only option appears to set all three objects at once (which is
possible only depending on implementation).

---

I'm surprised that entries in bfdSessDiscMapTable are writeable.
That is to say, that bfdSessDiscMapStorageType and 
bfdSessDiscMapRowStatus exist and are writeable. Surely this table
is automatic and entries an artifice of entries in bfdSessTable.
                                          
Why would you want an entry in this table to have a different 
storage type or row status from those in bfdSessTable?

What does it mean that those two objects are creatable? Does it mean
that you can create an entry in this table without knowing the value
of bfdSessDiscMapIndex which is automatic?

Same questions about bfdSessIpMapTable

---

The Security section says...

   The bfdSessAuthenticationType, bfdSessAuthenticationKeyID, and
   bfdSessAuthenticationKey objects hold security methods and associated
   security keys of BFD sessions.  These objects SHOULD be considered
   highly sensitive objects.  In order for these sensitive information
   from being improperly accessed, implementers MAY wish to disallow
   read and create access to these objects.

You don't want to prevent write access as well?
                                                                  
But note that preventing read access is pretty much not having the
object. So why *do* you have the objects and then suggest not providing
any access to them?