Subject: RE: Comments on draft-ietf-bfd-mib-09

From: "Nobo Akiya (nobo)" <[email-masked]>
Date: Mon, 26 Apr 2010 02:00:03 -0700

Hello Jeffrey,

Thanx for comments & my apologies for delay.

Please see inline for reply to each item.

> -----Original Message-----
> From: Jeffrey Haas [mailto:[email-masked]] 
> Sent: Sunday, March 21, 2010 2:47 AM
> To: [email-masked]; Zafar Ali (zali); Nobo Akiya (nobo)
> Cc: [email-masked]
> Subject: Comments on draft-ietf-bfd-mib-09
> 
> Authors:
> 
> These comments are as an individual contributor rather than 
> in my role of BFD co-chair.
> 
> I have not run the mib through snmplint.
> 
> ---
> 
> Regarding bfdSessType, I realize this has been in the MIB for 
> a while in the context of the type of multi-hop session.  
> However, I'd like to suggest that this is heading for a 
> collision with likely future working group tasks.  In this 
> case, it would be draft-katz-ward-bfd-multipoint.  That draft 
> defines a new BFD variable meant to cover the BFD session 
> type.  Further, that type covers semantics that don't overlap 
> well with those currently covered by bfdSessType.
> 
> The current object describes the session in terms of its 
> mult-hop properties.  While this is potentially useful for 
> helping a MIB consumer understand some of the session 
> properties and why some of the various other objects have 
> their current values, it's somewhat problematic in terms of 
> configuring a session from the MIB.  I can see why OOB would 
> only occur for BFD sessions created by other applications 
> (e.g. OSPF, LSP-PING), however it's unclear how the MIB 
> implementation should handle setting this variable for other 
> MIB-created sessions.
> 
> This object could use some additional clarification text to 
> cover the use cases.  I'd also suggest factoring it out into 
> a textual convention.

Most implementations explicitly state whether the session session is
singlehop or multihop, and this object goes gand-in-hand with that. It
probably doesn't hurt to be in there. ACK on TC.

> 
> Similarly, the bfdSessMultiHopUniLinkMode object could use 
> clarification for its use cases - in particular why it's 
> read-only instead of read-create.
> This object, name aside, seems potentially useful for setting 
> a BFD session to be active instead of passive.  If that's the 
> intention I'd suggest making it read-create and deleting the 
> MultiHopUniLink portion of the name.  This would require 
> clarification about acceptable values for certain types of sessions.

This object was added with BFD-MPLS in mind, but immediate goal for this
draft should be to complete this draft for BFD-IP. Best to take out this
object for now. Please see (ref#1) below for more explanations.

> 
> bfdSessSourceUdpPort is constrained by the BFD specifciations 
> to be limited to values 49152..65535.  I don't recall if 
> ranges can be applied to a textual convention - I suspect you 
> would need to to create a new one to cover these 
> restrictions.  Also, given the above, the DEFVAL should be 
> adjusted or clarified.  For example, 0 would permit the SNMP 
> agent to choose its own source port.

ACK.

> 
> The DEFVAL for bfdSessEchoSourceUdpPort when 0 should be 
> similarly clarified.

ACK.

> 
> I would suggest that the SYNTAX of the bfdSessState object be 
> put into a TEXTUAL-CONVENTION.  

ACK.

> 
> bfdSessDiag is accessible-for-notify rather than read-only.  
> Is there a reason why the user wouldn't want to see this 
> status in a MIB poll?

ACK to change to read-only.

> 
> bfdSessOperMode has semantics that may end up being 
> incompatible with draft-katz-war-bfd-multipoint.  That 
> document is not yet a working group item, but I'd suggest 
> considering how such a document would impact this object.  
> Perhaps factoring this into a TEXTUAL-CONVENTION would make sense.
> 
> Additionally, bfdSessOperMode is read-only.  Why wouldn't 
> this be read-create?

Demand mode is orthogonal to the operational status, thus probably good
that this configuration be available. ACK to create TC and change to
read-create.

> 
> bfdSessMultiPointFlag is included in this MIB as a read-only 
> object.  Given that the semantics and impact of this flag are 
> covered by draft-katz-ward-bfd-multipoint, would you suggest:
> 1. Keeping the object in here, read-only?
> 2. Updating it to read-create and plan on updating the base 
> BFD MIB to accomodate the draft if it is adopted as a WG item?
> 3. Remove the object and plan on making the multipoint 
> changes as AUGMENTation tables to the base MIB?

Good point. 2 sounds like the way to go.

> 
> For bfdSessGTSMTTL, what are the semantics you expect a 0 TTL 
> to have (the DEFVAL)?  Should this be 0 only if GTSM is 
> disabled in bfdSessGTSM?

Good point, 0 should indicate GTSM disabled. Description will be
updated.

>  Also, you should consider updating 
> the DESCRIPTION to accommodate requirements of bfd-mpls where 
> the TTL of the bottom label MUST be 1.

There's been split opinions about whether this draft should support
BFD-IP only or should include BFD-MPLS as well. This is something that
we need WG to help decide. Until it is decided, we will focus on
completing this draft to be BFD-IP only. (ref#1)

> 
> In bfdSessDesiredMinTxInterval, consider adding text to the 
> DESCRIPTION to note that a value of 0 disables transmitting 
> periodic control packets from BFD.

For bfdSessDesiredMinTxInterval, value of 0 is reserved according to the
base spec. We can specify that instead. As for bfdSessReqMinRxInterval,
value of 0 will indicate the remote to not send any control packets,
again according to base spec. We can specify that.

> 
> Similarly in bfdSessReqMinEchoRxInterval, consider adding 
> text to the DESCRIPTION to note that a value of 0 indicates 
> that no echo traffic is desired and this value MUST be 0 for 
> multi-hop sessions.

ACK.

> 
> bfdSessAuthPresFlag is read-only.  Since it's in the 
> read-write conformance group, I suspect this is an error.  If 
> that's not the case, you should add text to the DESCRIPTION 
> to clarify its value in the presence of the configuration of 
> bfdSessAuthentication(Type|KeyId|Key).

ACK to change to read-create.

> 
> bfdSessAuthentiacationType probably should call out value 0 
> as being used when no authentication is in use.  I would also 
> suggest factoring the SYNTAX into a TEXTUAL-CONVENTION.

ACK. Since 0 is reserved, probably best to define -1 as
no-authentication.

> 
> bfdSessAuthenticationKey carries a *lot* of semantics that 
> should vary depending on the authentication type.  While the 
> valid range of the field could be 0..252 based on the PDU 
> description, the values are constrained based on the type:
> - Password: 1..16
> - MD5: 16
> - SHA-1: 20
> 
> I would suggest creating a TEXTUAL-CONVENTION that describes 
> these semantics.

Let me check with other folks on this one.

> 
> bfdSessPerfEntry: s/bfdCounterDiscontinuityTime/bfdSessPerfDiscTime/

ACK.

> 
> In bfdSessDiscMapIndex and bfdSessIpMapIndex, I think you 
> mean BfdSessIndexTC instead of BfdIndex, correct?

Yes, ACK.

> 
> In bfdSessIpMapTable, shouldn't the table apply to multi-hop 
> types as well?

That's a good point. Leaving aside virtualization of MIB modules, we are
missing the IP source address. We will add this in bfdSessTable as well
as bfdSessIpMapTable .

> 
> In the Security Considerations, where bfdSessPerfTable is 
> mentioned, it should also mention bfdSessPerfHCTable.

We are ok here, as there is no bfdSessPerfHCTable, high capacity
counters are part of bfdSessPerfTable.

> 
> ---
> 
> Conformance Group issues:
> 
> bfdSessDiscMapTable is not in a conformance group 
> bfdsessIpMapTable is not in a conformance group

Table names shouldn't appear in the conformance statement.

> 
> bfdSessionPerfHCGroup and bfdNotificationGroup should not be 
> in both a GROUP clause and in MANDATORY-GROUPS.  
> 
> bfdNotificationGroup sounds like it is meant to be optional 
> and probably should be excluded from MANDATORY-GROUPS.

ACK to remove bfdSessionPerfHCGroup from MANDATORY-GROUPS.
bfdNotificationGroup should belong in  MANDATORY-GROUPS, will make
changes accordingly.

> 
> ---
> 
> Editorial nits:
> In section 5.3:
> "   The session performance table is used for collecting BFD 
> performance
>    counts on a per session basis."
> 
> s/counts/counters/

ACK.

> 
> In section 5.5:
> 
> "   BfdSessIndexTC used in the bfdSessionTable.  This table SHOULD
>    contains those BFD sessions are of IP type."
> 
> "that are of IP type".

ACK.

> 
>     BfdInterval ::= TEXTUAL-CONVENTION
>     DISPLAY-HINT   "d"
>     STATUS         current
>     DESCRIPTION
>         "The BFD interval delay in microseconds."
> 
> I'd suggest deleting "delay".

ACK.

> 
> In bfdSessAddr:
> "             It can also be used to enabled BFD on a specific"
> 
> s/enabled/enable/

ACK.

Thanx,
Nobo