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

From: "Bert Wijnen (IETF)" <[email-masked]>
Date: Thu, 08 May 2014 15:30:50 +0200

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

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.

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

I have not done a MIB SYNTAX check yet. I assume others have done so.

Bert

---

From: Jeffrey Haas <[email-masked]>
Date: Thu, 8 May 2014 15:31:04 -0400

Bert,

I chatted with Tom briefly about this:

> 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.

> But I do not know the BFD protocol well enough to be sure that I am right, so feel free to correct me

You're correct.  The changes above are reasonable.

> 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

---

From: "Nobo Akiya (nobo)" <[email-masked]>
Date: Fri, 9 May 2014 13:18:39 +0000

Hi Bert, Jeff, Tom,

Thanks for good set of comments Bert!

In summary:
- Bert, I need clarification on the dns matter (see inline below).
- Jeff/Tom, let me better understand what you had in mind for admin/oper values/names.
- Rest of the comments are well understood.

Please see in-line.

> -----Original Message-----
> From: Jeffrey Haas [mailto:[email-masked]]
> Sent: Thursday, May 08, 2014 3:31 PM
> To: Bert Wijnen (IETF)
> Cc: [email-masked]; MIB Doctors
> Subject: Re: MIB DOCTOR review for draft-ietf-bfd-mib-19.txt
> 
> Bert,
> 
> > While OperStatus calls it "adminDown". Possible better named:
> > AdminDisabled, or change the values in AdminStatus. Oh well
> 
> 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.
> 

Let me sync up with you (Jeff, Tom) offline on details of above.

> > 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.

I see, will remove " Only values ... to be supported." from textual description for bfdSessSrcAddrType and bfdSessDstAddrType objects.

> >
> > 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.

Good point, this should be added.

> >
> > 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.

Are you implying that document explicitly mentions that ipv4z(3) and dns(16) are not supported in the MODULE-COMPLIANCE statement?

> >
> > 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?
> > 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.
> 

One type being IPv4 and the other being IPv6 is definitely not valid. But one type being IPv4 or IPv6 and the other being unknown is a valid case. We need to preserve two separate types for source and destination.

> > 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?
> >

Oops, bfdOperStatus and bfdSessOperStatus need to be removed from the Security Considerations section.

-Nobo

---

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

> Are you implying that document explicitly mentions that ipv4z(3) and dns(16) are not supported in the MODULE-COMPLIANCE statement?

I believe he's asking for clarify of exclusive options (only these) vs.
proscriptive (don't use these).  We just need to be clear about that.

While I'd prefer to be proscriptive by saying "don't use dns", so as to
permit other options to be valid if they're ever added (and make sense),
options that are added later that don't make sense are also not excluded.
(This is a fault of the type, IMO.)

-- Jeff

---

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

Inline

> Thanks for good set of comments Bert!
Welcome

> Are you implying that document explicitly mentions that ipv4z(3) and dns(16) are not supported in the MODULE-COMPLIANCE statement?

Well, that is up to you. But if they can be used (and the current module compliance does not
state that they cannot be used), then for dns:

          The DESCRIPTION clause of InetAddress objects that may have
          InetAddressDNS values MUST fully describe how (and when)
          these names are to be resolved to IP addresses.

is what I read in RFC4001 bottom of page 9.
Again, I do not consider this  blocking comment.
But the clearer we can be the better.

So th question I think boils down to:
    when I set an address to a dns name, is it resolved at the point when I issue
    the SET command, or at some later point, maybe when the (re-)connection is tried to be made.
    When I set the row in notRead or notInService, and I activate it agaion later,
    is name re-resolved?

Bert

> One type being IPv4 and the other being IPv6 is definitely not valid. But one type being IPv4 or IPv6 and the other being unknown is a valid case. We need to preserve two separate types for source…
OK, I can live with that. I thought/hoped we could save one object here.

Bert

---

From: "Nobo Akiya (nobo)" <[email-masked]>
Date: Fri, 9 May 2014 15:25:29 +0000

Thanks again Bert, Jeff,

We will change the MODULE-COMPLIANCE statements for all *AddrTypes to:

     DESCRIPTION  "Only unknown(0), ipv4(1), ipv6(2) and ipv6z(4) support are required. dns(16) is not allowed." 

Thanks,
Nobo

> From: Bert Wijnen (IETF) [mailto:[email-masked]]
> Sent: Friday, May 09, 2014 9:42 AM
> To: Nobo Akiya (nobo); Jeffrey Haas
> >> 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.
> >
> >>> 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
> > I see, will remove " Only values ... to be supported." from textual
> description for bfdSessSrcAddrType and bfdSessDstAddrType objects.
> >
> >>>
> >>> clause of an InetAddress object) which InetAddressType objec
> >>> controls the format of the InetAddress. It may be obvious that it is
> >>> the preceding
> >>> 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.
> >
> > Are you implying that document explicitly mentions that ipv4z(3) and
> dns(16) are not supported in the MODULE-COMPLIANCE statement?
> >
> 
> Well, that is up to you. But if they can be used (and the current module
> compliance does not state that they cannot be used), then for dns:
> 
> 
> >>> 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
> > One type being IPv4 and the other being IPv6 is definitely not valid. But
> one type being IPv4 or IPv6 and the other being unknown is a valid case. We
> need to preserve two separate types for source and destination.
> >
> >>> 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

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 9 May 2014 11:27:01 -0400

>      DESCRIPTION  "Only unknown(0), ipv4(1), ipv6(2) and ipv6z(4) support are required. dns(16) is not allowed." 

And ipv4z.  Don't preclude using BFD with zeroconf. :-)

-- Jeff

---

From: "Nobo Akiya (nobo)" <[email-masked]>
Date: Fri, 9 May 2014 16:24:40 +0000

Got it, let me circulate a -20 candidates to authors, prior to posting.

Attached is a diff from -19 to -20 candidate.

Thanks again Bert, jeff.

-Nobo

> Sent: Friday, May 09, 2014 11:27 AM
> To: Nobo Akiya (nobo)
> Cc: Bert Wijnen (IETF); Jeffrey Haas; [email-masked];
> MIB Doctors

---

From: "Nobo Akiya (nobo)" <[email-masked]>
Date: Sun, 11 May 2014 13:38:56 +0000

Hi Bert,

Thank you for running SMICng. I agree that those warning is a non-issue for this MIB, and we can move on.

FYI, I'll be posting -20 in few hours, addressing your comments received this week.

Thanks again!

-Nobo

> Sent: Sunday, May 11, 2014 5:35 AM
> To: Jeffrey Haas
> Subject: SMICng SYNTAX check, was: MIB DOCTOR review for draft-ietf-bfd-
> mib-19.txt
> 
> Finally was able to do a SMICng syntax check.
> Looks good. I only get these 2 warnings:
> 
> W: f(bfd.mi2), (1001,17) Row "bfdSessDiscMapEntry" does not have a
> consistent indexing scheme - index item bfdSessDiscriminator from base
> row bfdSessEntry is not defined as an index item
> W: f(bfd.mi2), (1042,13) Row "bfdSessIpMapEntry" does not have a
> consistent indexing scheme - index item bfdSessInterface from base row
> bfdSessEntry is not defined as an index item
> 
> Probably OK, but you may want to check.
> 
> Bert

---

From: "Nobo Akiya (nobo)" <[email-masked]>
Date: Sun, 11 May 2014 18:38:08 +0000

Hi,

draft-ietf-bfd-mib-20 is now posted.

http://datatracker.ietf.org/doc/draft-ietf-bfd-mib/

Thank you!

-Nobo

> From: Nobo Akiya (nobo)
> Sent: Sunday, May 11, 2014 9:39 AM
> To: 'Bert Wijnen (IETF)'; Jeffrey Haas
> Subject: RE: SMICng SYNTAX check, was: MIB DOCTOR review for draft-ietf-
> bfd-mib-19.txt
> 
> Hi Bert,
> 
> > Subject: SMICng SYNTAX check, was: MIB DOCTOR review for
> > draft-ietf-bfd- mib-19.txt
> >