Subject: Re: [MIB-DOCTORS] MIB DOCTOR review for draft-ietf-bfd-mib-19.txt

From: Randy Presuhn <[email-masked]>
Date: Thu, 8 May 2014 12:59:16 -0700 (GMT-07:00)

Hi -

Are they trying to bring the model into
alignment with the model described in pages 5-13 of
http://www.itu.int/rec/T-REC-X.731-199201-I/en ?

Randy

-----Original Message-----
>From: Jeffrey Haas <[email-masked]>
>Sent: May 8, 2014 12:31 PM
>To: "Bert Wijnen (IETF)" <[email-masked]>
>Cc: MIB Doctors <[email-masked]>, [email-masked]
>Subject: Re: [MIB-DOCTORS] MIB DOCTOR review for draft-ietf-bfd-mib-19.txt
>
>Bert,
>
>I chatted with Tom briefly about this:
>
>On Thu, May 08, 2014 at 03:30:50PM +0200, Bert Wijnen (IETF) wrote:
>> Sorry for late review. Hope it is still useful.
>> 
>> Bert
>> 
>> I see:
>> 
>>   bfdAdminStatus OBJECT-TYPE
>>         SYNTAX     INTEGER {
>>             enabled(1),
>>             disabled(2)
>>         }
>>         MAX-ACCESS read-write
>>         STATUS     current
>>         DESCRIPTION
>>             "The desired global administrative status of the BFD
>>              system in this device."
>>         ::= { bfdScalarObjects 1 }
>> 
>>     bfdOperStatus OBJECT-TYPE
>>         SYNTAX     INTEGER {
>>             up(1),
>>             down(2),
>>             adminDown(3)
>>         }
>> 
>> And I find it weird that the AdminStatus has only 2 values: enabled/disabled.
>> While OperStatus calls it "adminDown". Possible better named: AdminDisabled,
>> or change the values in AdminStatus. Oh well
>
>A likely better mapping would be:
>enabled (1)
>disabled (2) (preserve the existing code points)
>adminDown(3)
>down(4)
>
>Transitions from anything to enabled kick the state machine into normal
>mode.  Transitions to disabled will transition the state machine to
>adminDown and then stop the BFD session.  Transitions to adminDown leave BFD
>running, but in the adminDown state.  Transitions to down similarly leave
>BFD running but transition to the Down state.
>
>I believe if the above is done that bfdOperStatus as a read-only variable is
>correct.
>
>> Similar comment for bfdSessAdminStatus and bfdSessOperStatus
>> 
>>     bfdSessSrcAddrType OBJECT-TYPE
>>         SYNTAX     InetAddressType
>>         MAX-ACCESS read-create
>>         STATUS     current
>>         DESCRIPTION
>>             "This object specifies IP address type of the source IP
>>              address of this BFD session. Only values unknown(0),
>>              ipv4(1), ipv6(2), or ipv6z(4) have to be supported.
>>              The value of unknown(0) is allowed only when the session
>>              is singleHop(1) and the source IP address of this BFD
>>              session is derived from the outgoing interface, or when
>>              the BFD session is not associated with a specific
>>              interface. If any other unsupported values are attempted
>>              in a set operation, the agent MUST return an
>>              inconsistentValue error."
>>       ::= { bfdSessEntry 19 }
>> 
>>     bfdSessSrcAddr OBJECT-TYPE
>>         SYNTAX     InetAddress
>>         MAX-ACCESS read-create
>>         STATUS     current
>>         DESCRIPTION
>>             "This object specifies the source IP address of this BFD
>>              session."
>> 
>> First of all, the"allowed values" in the AddrType should be specified in a MODULE-COMPLIANCE
>> statement (as you did), NOT in the textual description. The authoritative place for such specification
>> is in the MODULE-COMPLIANCE such that it is machine readable. Adding it here in the description clauase
>> has the risk of conflicts with the authoritative place.
>> 
>> Futher, according to RFC4001, you MUST specify (in the description clause of an InetAddress object)
>> which InetAddressType objec controls the format of the InetAddress. It may be obvious that it is the
>> preceding InetAddressType object, but it is better to be specific.
>> 
>> Although you state that dns does not have to be supported, it does not say that dns is not
>> allowed. In the case of a dns InetAddressType, you must (according to rfc4001) specify
>> when such a dns name gets resolved.
>> 
>> You use bfdSessDstAddrType and bfdSessSourceAddrType. I would in fact think that for one BFD session
>> they are both of the same type. You're not gonna connect an IPv4 to an IPv6 address, are you?
>> So maybe one InetAddressType object can serv for both InetAddresses?
>> But I do not know the BFD protocol well enough to be sure that I am right, so feel free to correct me
>> on this one.
>
>You're correct.  The changes above are reasonable.
>
>> In the security considerations section I see:
>> 
>>  o  bfdOperStatus - Improper change of bfdOperStatus, from up(1) to
>>       down(2) or up(1) to adminDown(3), can cause significant disruption
>>       of the connectivity to those portions of the Internet reached via
>>       all the applicable remote BFD peers.
>> 
>> As far as I can tell from the MIB module, this is a READ-ONLY object, so it cannot (when the
>> spec is properly implemented) be changed. It is the operational status that the system determines
>> itself based on the actual status of the system, or possiblu based on AdminStatus, no?
>> 
>> Same for bfdSessOperStatus
>
>This section should be rewritten to bfdAdminStatus (and bfdSess equivalent).
>Addition of bfdAdmin* was very late in the process.
>
>It's up to the authors to get some new text in to cover the comments.
>Thankfully I don't think they're show-stoppers.
>
>-- Jeff
>
>_______________________________________________
>MIB-DOCTORS mailing list
>[email-masked]
>https://www.ietf.org/mailman/listinfo/mib-doctors

---

From: Jeffrey Haas <[email-masked]>
Date: Thu, 8 May 2014 16:06:02 -0400

Randy,

> http://www.itu.int/rec/T-REC-X.731-199201-I/en ?

I believe Adrian made the recommendation to separate Admin vs. Oper state as
part of AD review.  I can't speak to his motivation, but certainly this
provides one possible reason.  Even without that motivation, it's certainly
a common design pattern in IETF writable MIBs and makes sense for BFD.

-- Jeff

---

From: Thomas Nadeau <[email-masked]>
Date: Thu, 8 May 2014 16:14:05 -0400

> a common design pattern in IETF writable MIBs and makes sense for BFD.

	I agree. I'd also point out that the MIBs here are based on two very widely deployed implementations (Cisco and Juniper), and so we don't want to wander too far from those.

	--Tom

---

From: "Bert Wijnen (IETF)" <[email-masked]>
Date: Fri, 09 May 2014 09:13:23 +0200

> 	I agree. I'd also point out that the MIBs here are based on two very widely deployed implementations (Cisco and Juniper), and so we don't want to wander too far from those.

TO be 100% clear:
I did/do not object to using a AdminStatus and OperStatus. That is
common practice in IETF indeed.
I was/am concerned about the naming inconsistencies of the enumerations.
And even those are not showstoppers.

Bert

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 9 May 2014 09:19:02 -0400

Bert,

> And even those are not showstoppers.

Even so, the naming discrepancy you noticed did suggest we didn't take
sufficient care to cover common BFD operational procedures.  The fix will
remain backward compatible with existing deployments and let us fill in that
operational gap.

Thanks for catching it.

Listening to the chatter of the authors, it seems the likely text to address
your comments may take a few days to come out as everyone is time
constrained right now.

-- Jeff