Subject: [CCAMP] AD review of draft-ietf-ccamp-gmpls-ted-mib

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 29 May 2012 23:22:33 +0100

Hi,

I have carried out my normal AD review of your draft. There is no cause
for alarm!

This looks to be a sound document, but as always with MIB modules it is
possible to find a number of nits and questions. I would appreciate a 
revision of the document before we move it forward so that it will get a
smoother ride through the rest of the process. I will put the document
into "Revised ID needed" state in the tracker.

As always, all issues and questions are open for discussion.

Thanks,
Adrian

---

Per idnits, please change the header 
OLD
Intended status: Standards Truck
NEW
Intended status: Proposed Standard
END

---

I am not comfortable with the two definitions
   OAM:   Operations and Management 
   OA&M:  Operations, Administration and Maintenance. 
This is counter to RFC 6291which, although only guidelines, should be
followed unless there is a good reason. That RFC has...
   o  O&M - OAM and Management
   o  OAM - Operations, Administration, and Maintenance
Fortunately neither term is used in your document, and so they can be
deleted.

---

There are some acronyms in 3.3 that are not used in the rest of the
document: FSC, LDP, LSC, LSR, OAM, OA&M, RSVP, 

---

Could you please sort out "MIB" versus "MIB module". There is only one
MIB. There are many MIB modules - this document defines a MIB module.

---

Section 6 is missing a close brace "}" for tedTable

---

The copyright date in the MODULE-IDENTITY is 2011.

---

You need to avoid using citations (e.g. [RFC3630]) within the MIB module
itself. MIB modules are extracted from their documents and stand alone.

---

The DESCRIPTION for the tedTable is
  "This table indicates multiple TED information which has been 
   supported by [RFC3630]." 

This seems to imply this doesn't apply to ISIS. Maybe update the text?

Similarly, a number of objects only give references to OSPF or OSPF-TE.
Not referencing the corresponding IS-IS specs looks like the object only
has meaning for OSPF.

---

   tedEntry OBJECT-TYPE 
       SYNTAX       TedEntry 
       MAX-ACCESS   not-accessible 
       STATUS       current 
       DESCRIPTION 
       "This entry contains TED information commonly utilized in both 
   MPLS and GMPLS." 
      INDEX { tedLocalRouterId, tedRemoteRouterId, 
   tedLinkInformationSource, tedLinkIndex } 
   ::= { tedTable 1 } 
    
   TedEntry ::= SEQUENCE { 
       tedLinkInformationSource     INTEGER, 
       tedLocalRouterId             TedRouterIdTC, 
       tedRemoteRouterId            TedRouterIdTC, 
       tedLinkIndex                 TedLinkIndexTC, 

It is unusual to list the index fields in a different order from that
which they appear in the table entry. Is this intended?

---

Tom's surname is misspelled in several of the references!

---                                                      

I'm having some trouble with the use of 2119 language in the Description
for tedLinkInformationData.

Two examples...

          If tedLinkInformationSource has the value unknown(0), this 
       object SHOULD contain a value of zeroDotZero.  

Under what circumstances can this use of "SHOULD" be varied?

          If tedLinkInformationSource has the value 
       locallyConfigured(1), this object MAY contain the identifier of 
       the corresponding row entry in the teLinkTable of TE-LINK-STD-
       MIB[RFC4220], the identifier of the corresponding row in a local 
       proprietary TE link MIB module, or the value of zeroDotZero 
       otherwise. 

The use of "MAY" here implies that an implementation can choose to fill
in the row pointer if it likes, but this is entirely an implementation 
choice. Is this what you are saying, or do you want to constrain the
behavior more forcefully if TE-LINK-STD-MIB is implemented?

---

   tedRouterIdAddrType OBJECT-TYPE 
       SYNTAX       InetAddressType 
       MAX-ACCESS   read-only 
       STATUS       current 
       DESCRIPTION 
         " This object indicates the TE-Router ID address type. Only 
   values unknown(0), ipv4(1) or ipv6(2) must be supported. " 

Do you mean s/must be/are/  ?

Similarly for tedLinkIdAddrType

---

It's a small point, but I would have preferred you to rename
tedRouterIdAddrType and tedRouterIdAddr as tedTeRouterIdAddrType and
tedTeRouterIdAddr to distinguish them from the normal router ID.

---

I don't understand what values should be returned for objects that 
only apply in some circumstances. For example, in a non-GMPLS system,
what would be returned for tedLinkProtectionType? In either a non-GMPLS
system or one with numbered links, what values would be returned for
tedLocalId and tedRemoteId? What about tedSwCapIndication for non-TDM
links?

Section 6 says in these cases the objects are not supported, but I don't
think it is that simple. Or is it?

Even if it is that simple, I think the Description clauses should not 
the circumstances under which the object is not supported.

(And yes, I do see the compliance statements which are very 
comprehensive, but I think they say what you have to support in 
specific circumstances, not how to behave if you support an object but
have no information to return.)

---    
    
   tedLocalIfAddrEntry OBJECT-TYPE 
       SYNTAX       TedLocalIfAddrEntry 
       MAX-ACCESS   not-accessible 
       STATUS       current 
       DESCRIPTION 
         "This entry contains the IP address information of the local TE 
   link." 
       INDEX { tedLinkIndex, tedLocalIfAddr } 
   ::= { tedLocalIfAddrTable 1 } 
    
   TedLocalIfAddrEntry ::= SEQUENCE { 
       tedLocalIfAddrType    InetAddressType, 
       tedLocalIfAddr        InetAddress 
    
       } 

What would it look like to walk this table by increasing index value?
Would all the shorter v4 addresses show first, or would the numerically
smaller v6 addresses get mixed in with the v4 addresses?

If the latter is possible, you should use the address type as an index 
as well.

---

A broader point about the use of addresses as indexes is found in
RFC 4001 in the Description of the InetAddress TC...

         When this textual convention is used as the syntax of an
         index object, there may be issues with the limit of 128
         sub-identifiers specified in SMIv2, STD 58.  In this case,
         the object definition MUST include a 'SIZE' clause to
         limit the number of potential instance sub-identifiers;
         otherwise the applicable constraints MUST be stated in
         the appropriate conceptual row DESCRIPTION clauses, or
         in the surrounding documentation if there is no single
         DESCRIPTION clause that is appropriate."

---

tedStatusChangeNotificationMaxRate and
tedCreatedDeletedNotificationMaxRate

I am surprised that the Defval for these is 0 (no throttling) since 
the text recognises that this is a risky situation that may cause the
box to blow up. Surely the default should be some safe setting.

But then I wondered what a suitable safe setting would be and realised
it would be "No Notifications". Except there is no way to say this with
these objects.

What do you think?                                 

---

Would the Notifications be more helpful if they indicated the index of
the entry in the tedTable to which they applied?

---

For completeness, we usually get asked to state in the Security section
what the risks are with write access objects. In this case it is really
easy...

   There are only two write-access objects in this MIB module:
   tedStatusChangeNotificationMaxRate and 
   tedCreatedDeletedNotificationMaxRate.  Malicious modification of 
   these objects could cause the management agent, the network, or the
   router to become overloaded with Notifications in cases of high
   churn within the network.

---

From: "Masanori Miyazawa" <[email-masked]>
Date: Wed, 30 May 2012 21:59:03 +0900

Hi, Adrian

Thank you for reviewing the draft.

We will check and answer your comments shortly. 
When we want to modify the draft based on your comments, how should we
update it?  We should post revised document as latest version (version 14)?

With Best Regards,
Masanori

> -----Original Message-----
> From: Adrian Farrel [mailto:[email-masked]]
> Sent: Wednesday, May 30, 2012 7:23 AM
> To: [email-masked]
> Cc: [email-masked]
> Subject: AD review of draft-ietf-ccamp-gmpls-ted-mib
> 
> Hi,
> 
> revision of the document before we move it forward so that it will get
> a smoother ride through the rest of the process. I will put the document
>    o  OAM - Operations, Administration, and Maintenance Fortunately
> neither term is used in your document, and so they can be deleted.
> 
> ---
> 
> I don't understand what values should be returned for objects that only
> apply in some circumstances. For example, in a non-GMPLS system, what
> would be returned for tedLinkProtectionType? In either a non-GMPLS system
> or one with numbered links, what values would be returned for tedLocalId
> and tedRemoteId? What about tedSwCapIndication for non-TDM links?
> 
> (And yes, I do see the compliance statements which are very comprehensive,
> but I think they say what you have to support in specific circumstances,
> not how to behave if you support an object but have no information to
> return.)
> 
> ---
> 
> A broader point about the use of addresses as indexes is found in RFC
> 4001 in the Description of the InetAddress TC...
> 
> I am surprised that the Defval for these is 0 (no throttling) since the
> text recognises that this is a risky situation that may cause the box
> to blow up. Surely the default should be some safe setting.
>

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 30 May 2012 14:07:40 +0100

Yes, please.

If changes are needed after looking at my comments and discussing them, then
please post a new revision.

Cheers,
Adrian

> From: Masanori Miyazawa [mailto:[email-masked]]
> Sent: 30 May 2012 13:59
> To: [email-masked]; [email-masked]
> Cc: [email-masked]
> Subject: RE: AD review of draft-ietf-ccamp-gmpls-ted-mib
> 
> Hi, Adrian
>