Subject: Re: [MIB-DOCTORS] [mpls] MIB Doctor Review of draft-ietf-mpls-tp-oam-id-mib-02.txt

From: Venkatesan Mahalingam <[email-masked]>
Date: Thu, 20 Jun 2013 20:15:18 -0700

Joan,

We have addressed the below comments in the 03 version. We would be happy
to address any of your further comments.

Please let us know if any of the comments are not addressed properly.

http://tools.ietf.org/html/draft-ietf-mpls-tp-oam-id-mib-03

-Venkat.

On Sat, Apr 20, 2013 at 2:10 PM, venkatesan mahalingam <[email-masked]
> wrote:

> Joan,
>
> Please find below my comments with the tag VM>. We will publish the new
> version of this draft soon.
>
> -Venkat.
>
>
> On Mon, Mar 11, 2013 at 8:34 PM, Joan Cucchiara <[email-masked]
> > wrote:
>
>>
>> Authors,
>>
>> Most of the comments during the LC have been addressed.
>> Thank you for that.   Please see some follow-up comments below.
>>
>> Thanks,
>> -Joan
>>
>>
>>
>>
>> * MIB compiles cleanly with smicng and smilint
>>
>>
>> Specific Comments:
>> ====================
>>
>> Section 3.3 Acronyms
>>
>>
>> * MIP is specified slightly differently in the referenced docs.
>> Please be consistant.
>>
> VM> OK.
>
>>
>>
>> Section 6.
>>
>> This example, specifies the mplsOamIdMeMpEntry as a MEP, but why
>> isn't the SourceMepIndex or SinkMepIndex == mplsOamIdMeMpIndex?
>>
>> Also, there are at least 2 MEPs in an ME, and at least one ME
>> in a MEG and these relationships are not completely evolved
>> in this example.  I think the example should be expanded
>> to agree with what is stated in the first paragraph.
>>
>> VM> OK, the example will be expanded to represent MEG->ME->Source/Sink
> MEPs clearly.
> As per this MIB module,
> MEG->ME->MPIndex([Source and Sink MEP] or MIP index)
> For example,
>
> If we have multiple MEPs in a single ME within an MEG node.
> MEG index - 1
> ME index - 1
> MP index - 1 (Source and Sink MEPs1)
> MP index - 2 (Source and Sink MEPs2)
>
> *Entry-1:*
> ME Table - 1,1,1
> mplsOamIdMeSourceMepIndex - 2
> mplsOamIdMeSinkMepIndex - 3
>
> *Entry-2:*
> ME Table - 1,1,2
> mplsOamIdMeSourceMepIndex - 4
> mplsOamIdMeSinkMepIndex - 5
>
> *MIP entry:*
>
> MEG index - 1
> ME index - 1
> MP index - 3 (MIP index)
> ME Table - 1,1,3
> mplsOamIdMeMpIfIndex - MPLS incoming/outgoing interface.
>
>
>
>> MIB Module comments
>> -------------------
>>
>> * TC:  MplsOamPhbTCValue
>>
>>
>>         MplsOamPhbTCValue ::= TEXTUAL-CONVENTION
>>            STATUS              current
>>            DESCRIPTION
>>                "This is the Per-hop Behavior (PHB) traffic class values
>>                 for the MPLS OAM operations."
>>            SYNTAX        INTEGER {
>>                            be (1),
>>                            af1 (2),
>>                            af2 (3),
>>                            af3 (4),
>>                            af4 (5),
>>                            ef (6),
>>                            cs6 (7),
>>                            cs7 (8)
>>                          }
>>
>> VM> As MPLS header traffic class (TC) field has only 3 bits, we derived
> the above TC values (8 possible values to represent it in 3 bits) from the
> below TC values.
>
>
>> Rfc3270, "Multi-Protocol Label Switching (MPLS) Support of
>> Differentiated Services", specifies that MPLS TP will use DSCP as per
>> rfc2474 and other specs.   Is that the intent wrt this TC?
>>
>> If not, please explain where these values are defined, otherwise,
>> if these values are as per rfc3270, then please be consistant with the
>> labels.
>>
>> TC labels should correspond more closely to DiffServ BHB traffic class
>> values.
>> In other words,
>>
>> http://www.iana.org/**assignments/dscp-registry/**dscp-registry.xml<http://www.iana.org/assignments/dscp-registry/dscp-registry.xml>
>>
>>   Name     Space  Reference
>>   CS0         000000 [RFC2474]
>>   CS1         001000 [RFC2474]
>>   CS2         010000 [RFC2474]
>>   CS3         011000 [RFC2474]
>>   CS4         100000 [RFC2474]
>>   CS5         101000 [RFC2474]
>>   CS6         110000 [RFC2474]
>>   CS7         111000 [RFC2474]
>>   AF11        001010 [RFC2597]
>>   AF12        001100 [RFC2597]
>>   AF13        001110 [RFC2597]
>>   AF21        010010 [RFC2597]
>>   AF22        010100 [RFC2597]
>>   AF23        010110 [RFC2597]
>>   AF31        011010 [RFC2597]
>>   AF32        011100 [RFC2597]
>>   AF33        011110 [RFC2597]
>>   AF41        100010 [RFC2597]
>>   AF42        100100 [RFC2597]
>>   AF43        100110 [RFC2597]
>>   EF PHB      101110 [RFC3246]
>>   VOICE-ADMIT 101100 [RFC5865]
>>
>>
>> Continuing with that thought: I believe this TC could (and should) be
>> formalized into an IANA-Maintained MIB if these values are the same
>> as the above IANA-Maintained assignments for DFCPs.
>> (NOTE: this was mentioned also in the LC comments.)  Please discuss.
>>
>> Also, this TC should have a REFERENCE clause.
>>
>>
>>
>> * mplsOamIdMegIndex
>> There is no information about how to employ mplsOamIdMegIndexNext to
>> obtain a value for this index.   Please update the DESCRIPTION
>> accordingly.
>>
>>
>> VM> Edited.
>
>>
>> * mplsOamIdMegOperatorType
>> Why does this say "should have valid values...", isn't this a MUST?
>> Also, s/while making/when/
>>
>> * mplsOamIdMegIdCc
>>
>> s/contains non-null ICC/MUST contain a/
>>
>> s/otherwise null ICC value/otherwise a null ICC value/
>>
>> s/should be assigned/MUST be assigned/
>>
>> * mplsOamIdMegIdIcc
>>
>> Same comments as above.   Please use MUST.
>>
>> * mplsOamIdMegIdUmc
>> Same comments as above.  Please use MUST.
>>
>>
>> VM> Edited.
>
>> * mplsOamIdMegServiceType
>> Could you please specify the service pointer by the object's name?
>>
>> Also, the references are within the DESCRIPTION which is fine, but
>> they should also be in a REFERENCE clause.
>>
>>
>> VM> Edited.
>
>> * mplsOamIdMeIndexNext and mplsOamIdMpIndexNext
>> These objects are not referred to by mplsOamIdMeIndex or
>> mplsOamIdMeMpIndex.
>> There is not enough description to understand how the IndexNext objects
>> are to be used.
>>
>> VM> Edited.
>
>>
>> * MplsOamIdMeTable
>>
>> The mplsOamIdMeEntry states "An entry in this table
>> represents MPLS-TP maintenance entity."   Yet, looking at the
>>              INDEX { mplsOamIdMegIndex,
>>                      mplsOamIdMeIndex,
>>                      mplsOamIdMeMpIndex
>>                    }
>>
>> This is not an ME because an ME by definition has 2 (source/sink)MEPs.
>> An entry in this table represents either a MEP or MIP, not an ME.
>>
>> VM> Yes, this is not an ME but it contains ME information (MEPs/MIP).
> Would it make more sense if we change it to mplsOamIdMeInfoEntry?
>
>>
>> *) What is the benefit of combining MEP and MIP (i.e. the objects
>> which contain "Mp" as part of their object name)?
>> Many other objects in this table, need to figure out if the entry
>> is describing a MEP or MIP before the value can be interpreted correctly.
>> Additionally, there is duplicate info in the form of having a Source and
>> Sink specified for each Mp. Could you elaborate on what the
>> benefit is of having listing MEPs and MIPs in this way?
>>
>> It seems like the original intent may have been to specify an ME
>> as being an entry in this table.  However, that would mean the table
>> should probably be indexed by MEG index, a ME index, a source MEP index
>> and a sink MEP index.
>>
>> This would  greatly simplify many of the object descriptions.
>>
>> Have you considered specifying MIPs in a 3rd table, such that each
>> ME would have 2 MEPs and zero or more MIPs?
>>
>> Please discuss.
>>
>> VM> We thought through all the possible options, it looks like lot of
> informations are duplicated unnecessarily if we have the seperate tables
> for MEP/MIP configurations. For up/down MEPs configurations, all the
> informations in the ME table will be used but for MIP, except the
> Source/Sink MEPs objects, all other objects will be used. So, we wanted to
> combine MEP/MIP in a single table for better management.
>
> So, we will certainly improvise the existing MEG&ME tables example to
> provide all possible configuration options.
>
>>
>>
>> *) mplsOamIdMeMpIfIndex
>>
>> Rfc6370, Section 4.discusses an IF_NUM and an IF_ID and states
>> "Note that IF_Num had no relation with the ifNum object defined in
>> RFC2863.  Further, no mapping is mandated between IF_Num and ifIndex in
>> RFC 2863."
>>
>> I don't see any mention of ifIndex in RFC 6371, so could you tell me what
>> Section?   Is this object supposed to represent IF_NUM in rfc6370?
>>
> VM> Reference should be replaced with rfc-6370 section4.
> and RFC2863 should be removed. Good catch. Thanks.
>
>>
>> *) mplsOamIdMeServicePointer
>>
>> The DESCRIPTION contains wording which is very loose.  Could you
>> please use wording which specifies a "SHOULD" or "MUST"?
>> Under what circumstances should this be 0.0?
>>
>> VM> Edited.
>
>>
>> Compliance Statement of the MIB
>>
>> *) Compliance (This has been asked before and I have not seen any
>> discussion about it.)
>>
>> There is no read-only compliance. Has it been made clear
>> to the WGs (MPLS and PWE3) that SNMP sets will need to
>> be supported in order to be compliant with the MIB?
>>
>> VM> Not yet, sorry for the delay. We will make read-only compliance clear
> to WG after this draft submission.
>
>>
>> *) question above, about whether the intention is to support
>> ifIndex as per rfc2863 or IF_ID (or IF_NUM) as per rfc6370 may
>> affect this.
>>
>>      "MODULE IF-MIB -- The Interfaces Group MIB, RFC 2863.
>>      MANDATORY-GROUPS {
>>         ifGeneralInformationGroup,
>>         ifCounterDiscontinuityGroup"
>>
>>
>> *)    mplsOamIdNotificationObjectsGr**oup  OBJECT-GROUP
>>
>> I don't see a need to make a specific group for
>> these objects.  They are already specified by mplsOamIdGroups.
>>
>> VM> Edited.
>
>>
>> Section 8. Security Section
>>
>> Need to reference specific read-create objects and also read-only which
>> could impact the network.
>>
>> Additionally, the incomplete sentence:
>> "These are the tables and objects and their sensitivity/vulnerability: "
>> needs to be completed.
>>
>> VM> Edited.
>
>>
>> Section 9.  IANA Considerations
>>
>> s/specified this document/specified in this document/
>> missing the word "in"
>>
>>
>> VM> Edited.
>
>> Section 11.
>> Thank you for the ack!
>>
>> ______________________________**_________________
>> mpls mailing list
>> [email-masked]
>> https://www.ietf.org/mailman/**listinfo/mpls<https://www.ietf.org/mailman/listinfo/mpls>
>>
>
>