Re: [MIB-DOCTORS] MIB doctor review for draft-ietf-bfd-tc-mib-07.txt

[Benoit Claise <[e-mail]>]

Jeff, authors,

> Bert,
>
> Thanks for the review.  Commenting as shepherd rather than as one of the
> authors:
>
> On Thu, May 08, 2014 at 02:26:32PM +0200, Bert Wijnen (IETF) wrote:
>> Sorry for my late review. Vacation and blabla-excuses.
>> Hope this is still helpful.
>>
>> 1.I can live with (i.e. this is just a comment, not a blocking issue) TCs for
>>        BfdCtrlDestPortNumberTC
>>        BfdCtrlSourcePortNumberTC
>>    But I fail to see why InetPortNumber TC would not suffice.
> Mostly because they add suggestions on the values involved and those
> suggestions may evolve over time as the protocol continues to also evolve.
>
> Prior to refactoring to the TC MIB, the constraints/recommendations  were in
> the base MIB, meaning that the base MIB would have to be reissued rather
> than the somewhat easier to update TC MIB.  (I.e. structure doesn't change,
> merely constraints.)
>
> While I suspect this is mostly preference rather than a real technical
> objection, I don't think anyone is eager to ever re-open work on the base
> MIB. :-)
>
>> 2.The same is more or less true for
>>
>>      BfdSessIndexTC ::= TEXTUAL-CONVENTION
>>      DISPLAY-HINT   "d"
>>      STATUS         current
>>      DESCRIPTION
>>          "An index used to uniquely identify BFD sessions."
>>      SYNTAX Unsigned32 (1..4294967295)
>>
>>    If you see it used in, for example:
>>      bfdSessDiscMapIndex OBJECT-TYPE
>>          SYNTAX     BfdSessIndexTC
>>          MAX-ACCESS read-only
>>          STATUS     current
>>          DESCRIPTION
>>              "This object specifies a mapping between a
>>               local discriminator and a BFD Session in
>>               the BfdSessTable."
>>          ::= { bfdSessDiscMapEntry 1 }
>>
>>    Then I wonder what value the TC adds?
> 0 is special in this case.
>
>> 3.Same for:
>>      BfdIntervalTC ::= TEXTUAL-CONVENTION
>>      DISPLAY-HINT  "d"
>>      STATUS        current
>>      DESCRIPTION
>>          "The BFD interval in microseconds."
>>      SYNTAX Unsigned32 (0..4294967295)
>>
>>
>>    If you see it used in, for example
>>
>>      bfdSessDesiredMinTxInterval OBJECT-TYPE
>>          SYNTAX     BfdIntervalTC
>>          MAX-ACCESS read-create
>>          STATUS     current
>>          DESCRIPTION
>>              "This object specifies the minimum interval, in
>>               microseconds, that the local system would like to use
>>               when transmitting BFD Control packets. The value of
>>               zero(0) is reserved in this case, and should not be
>>               used."
>>          REFERENCE
>>              "Section 4.1 from Katz, D. and D. Ward, Bidirectional
>>               Forwarding Detection (BFD), RFC 5880, June 2012."
>>          ::= { bfdSessEntry 25 }
>>
>>     Then what value is added by using a TC. In fact you can even quetion if it
>>     is not conflicting, because according to the TC description clause I would
>>     expect zero to be a valid interval value, where as here it describes
>>     that zero is a special value and should not be used. So it is only special
>>     Since it should NOT BE used, or does zero mean something special?
>>     Assuming zero SHOULD NOT be used. I personally would just do:
>>
>>      bfdSessDesiredMinTxInterval OBJECT-TYPE
>>          SYNTAX Unsigned32 (1..4294967295)
>>          MAX-ACCESS read-create
>>          STATUS     current
>>          UNITS      microseconds
>>          DESCRIPTION
>>              "This object specifies the minimum interval that the local
>>               system would like to use when transmitting BFD Control packets.
>>          REFERENCE
>>              "Section 4.1 from Katz, D. and D. Ward, Bidirectional
>>               Forwarding Detection (BFD), RFC 5880, June 2012."
>>          ::= { bfdSessEntry 25 }
>>
>>    In case zero has special meaning I would do:
>>      bfdSessDesiredMinTxInterval OBJECT-TYPE
>>          SYNTAX Unsigned32 (0 | 1..4294967295)
>>          MAX-ACCESS read-create
>>          STATUS     current
>>          UNITS      microseconds
>>          DESCRIPTION
>>              "This object specifies the minimum interval that the local
>>               system would like to use when transmitting BFD Control packets.
>>               The value zero has been reserved for a special maning:
>>               <describe what zero means>
>>          REFERENCE
>>              "Section 4.1 from Katz, D. and D. Ward, Bidirectional
>>               Forwarding Detection (BFD), RFC 5880, June 2012."
>>          ::= { bfdSessEntry 25 }
> The primary benefit of having it in the object is that we get to specify
> units.  (I never understood why that was missing from TC.)  Beyond that,
> it's just a refactoring of constraints in case they're further refined.
Fine with the units explanation. However, you don't answer the 
discrepancy of the value 0.
I'll file this in my AD review.

Regards, Benoit

>
> [...]
>
>> It boils down to the question: does the BFD-TC-STD-MIB make sense at all or is it
>> just extra/superfluous text? was the concept of TC invented for that?
> What's funny is when I was chasing similar issues in much earlier days of
> the BGP MIB work I was pushed heavily to move things to TC.  Again, seems to
> be preference. :-)
>
>> The IANA-BFD-TC-STD-MIB module makes sense I think, because it allows to add
>> definitions by the IANA function.
>>
>> I have not yet done a MIB SYNTAX checker run on it. But I assume others have done so.
> It would be appreciated if you'd run available checks on it.  I'm aware that
> it's passed smilint in the past but I'm somewhat paranoid that a typo
> introduces an error at some point.  None of us had smicng.
>
> -- Jeff
>
> _______________________________________________
> MIB-DOCTORS mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/mib-doctors
> .
>