RE: Review for the EPON-04

Thank you DavidP for the extensive review. 

Lior, Can you please address the issues raised by David and let us know
when will a new draft be available? 

Thanks and Regards,

Dan


 
 

> -----Original Message-----
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of David T. Perkins
> Sent: Saturday, May 27, 2006 2:33 AM
> To: [email protected]
> Cc: Dave Thaler
> Subject: Review for the EPON-04
> 
> HI,
> 
> Below is the review that I did for the EPON MIB document 
> http://www.ietf.org/internet-drafts/draft-ietf-hubmib-efm-epon
> -mib-04.txt
> 
> Repeating from a previous message, this document was 
> difficult for me to review due to several factors. First, the 
> technology is quite unusual, and since it is new, it is not 
> well documented.
> What is available is in almost unreadable IEEE documents.
> Secondly, the interfaces model is unusual.
> The final major issue was that it had quite a bit of grammar 
> problems, which made it difficult to decode.
> 
> The reason that I bring this document to your attention is so 
> that if such a document is developed in the future, that the 
> review can do a much faster turn around than me.
> 
> I have a few questions for the this list:
> 1) Interface documents are special in that they must provide
>    an interpretation of the values of columnar objects from
>    the IF tables. Look at the Ethernet-like MIB document,
>    RFC 3635. If describes the value of each object.
>    Text and example values seem to be a good thing,
>    but is this too much work for the document author?
> 
> 2) The interfaces have many counters, and thus counter
>    stops, resumes, and discontinuity need to be clear.
>    But where and how much should this be addressed.
> 
> 3) The EPON model is a little strange because there
>    is a "provider side" and an "customer side". Some
>    of the events occur only on one side or the other.
>    The approach taken with the EPON tables was to
>    put all counters in a single table (per area)
>    and specify in the DESCRIPTION clause whether
>    the counter was applicable for both, or just
>    the provider or customer. If not applicable,
>    then the value was zero. Instead, should
>    each such table be split into 2 tables
>    (or three tables). 2 tables - one for
>    provider, and one for customer. 3 tables -
>    one for common, one for just provider,
>    and one for customer.
> 
> I'm probalably forgetting a few more questions, but I'm a 
> little burned out from the review.
> 
> Here are my review notes:
> General comments:
> 1) The updated document looks much better than the -03 version. 
> 2) It runs through both SMICng and smilint with no problems.
> 3) I read through the document fairly quickly, and didn't
>    try to verify all the references to other documents.
> 4) There are many grammar problems throughout the document,
>    and, thus, the document would be much improved by
>    a pass from a "copy editor" for technical documents.
> 5) The interface model that the document covers is quite
>    unusual, and thus requires more work in this document 
>    to describe it than other well known and simple interfaces.
> 6) This version resolved many of issues from the previous
>    revision. However, I still have concerns with a 
>    few corner cases. These include:
>      a) discontinuity of counters
>      b) dependencies of stacked interfaces
> 7) I believe that the document needs to be updated and
>    reviewed at least one more time.
> 
> Specific Comments:
> 0) The interface type should be ethernetCsmacd(6) as specified
>    in RFC 3635. 
> 1) The abstract and the overview paragraphs are different.
>    The abstract includes management of P2MP networks,
>    which is not included in the overview.
> 2) The terminology and abbreviations need to be checked
>    for completeness. I noticed the following that need
>    to be added:
>      FEC - forward error correction
>      P2MP - point to multipoint
> 3) In section 1.3, "management I/F" should be written out
>    as "management interface". In the next sentence, I believe
>    that instead of text "The MIB document", it should read
>    "The IEEE MIB document", since IETF MIB documents don't
>    have "packages".
> 4) The most important goal for section 1 is to describe the
>    interface model. This version is so much better than
>    the previous version. However, I'm still somewhat
>    confused. The figure in section 1.2.1 and in 1.2.5
>    show typical devices having OLT and ONU interfaces.
>    I'd like to see two more figures and interface table
>    dumps. These are:
>     An "ONU modem":
>                     --------
>      ONU interface | ONU   | 10megabit interface
>      --------------| Modem |-------------------- 
>                    ---------
>     Does this device have two or three entries in the IF table?
>     And does it have an entry in the IF stack table. I believe
>     that there are 3 IF entries, and one IF stack entries.
>     For example in IF table:
>       ifIndex=1 - interface for 10megabit interface
>       ifIndex=2 - interface for the optical interface
>       ifIndex=200 - interface for the ONU interface
>     And then in IF stack table:
>       ifStackHigherLayer=200, ifStackLowerLayer=2 - map between
>            the physical and the ONU
>            
>     An "headend" with 1 gigibit ethernet interface, and
>     two "OLT" interfaces:
>                        --------
>      1st OLT interface | Head  | gigE interface
>      ------------------| end   |-------------------- 
>                        |       |
>      ------------------|       |
>      2nd OLT interface |       |
>                        ---------
>     With no currently connected ONUs, I believe that there
>     would be the following IF table and IF Stack table entries:
>     For example in IF table:
>       ifIndex=1 - interface for gigE interface
>       ifIndex=2 - interface for 1st optical interface
>       ifIndex=3 - interface for 2nd optical interface
>       ifIndex=200 - interface for the 1st OLT broadcast interface
>       ifIndex=300 - interface for the 2nd OLT broadcast interface
>     And then in IF stack table:
>       ifStackHigherLayer=200, ifStackLowerLayer=2 - map between
>            the 1st physical and its broadcast OLT
>       ifStackHigherLayer=300, ifStackLowerLayer=3 - map between
>            the 2nd physical and its broadcast OLT
>     If two ONUs connected to the first OLT, then the following
>     would be added:
>     For example in the IF table:
>       ifIndex=201 - interface for the 1st ONU of 1st OLT
>       ifIndex=202 - interface for the 2nd ONU of 1st OLT
>     And in the IF stack table:
>       ifStackHigherLayer=201, ifStackLowerLayer=2 - map between
>            the 1st physical and 1st ONU
>       ifStackHigherLayer=202, ifStackLowerLayer=2 - map between
>            the 1st physical and 2nd ONU
>           
>     It seems that for "ONU modems" that there is not a
>     an interface instance for the physical interface,
>     where there is one for "headend" devices?
> 
>     I suggest that you do the following:
>     a) in section 1.3, take the paragraph that starts with
>        "At the OLT", and end it after the second sentence.
>     b) Add the following paragraphs and figures:
>      ------
>        To illustrate the interface modeling, consider
>        two devices. The first device has two
>        physical interfaces, is typically located at
>        a consumer's site, and may be called a "ONU modem".
>        This is shown in figure X below:
>                         --------
>           ONU interface | ONU   | 10megabit interface
>           --------------| modem |-------------------- 
>                         ---------
>                Figure X: ONU modem
> 
>        This device would have 3 entries in the IF table,
>        for example:
>           ifIndex=1 - interface for 10megabit interface
>           ifIndex=2 - interface for the optical interface
>           ifIndex=200 - interface for the ONU interface
> 
>        The second device has three physical interfaces,
>        is typically located at the provider's site, and
>        may be called a "headend". This is shown in
>        figure Y below:
> 
>                             ---------
>           1st OLT interface | Head  | gigE interface
>           ------------------| end   |-------------------- 
>                             |       |
>           ------------------|       |
>           2nd OLT interface |       |
>                             ---------
>                      Figure Y: headend
> 
>        This device would have 5 entries (when no attached ONUs)
>        in the IF table, for example:
>           ifIndex=1 - interface for gigE interface
>           ifIndex=2 - interface for 1st optical interface
>           ifIndex=3 - interface for 2nd optical interface
>           ifIndex=200 - interface for the 1st OLT broadcast interface
>           ifIndex=300 - interface for the 2nd OLT broadcast interface
> 
>        If two ONUs connected to the first OLT interface, then
>        for example, the following entries would be added to
>        the IF table:
>           ifIndex=201 - interface for the 1st ONU of 1st OLT
>           ifIndex=202 - interface for the 2nd ONU of 1st OLT
> 
>      ------
> 
>     c) Continue with a slight change of the sentence that starts
>        with "Therefore the Interface". Rewrite it to be something
>        like:
>      ------
>        For each physical interface, there would be an entry in
>        the tables of the MAU MIB module[RFC3636] and Etherlike
>        MIB module[RFC3635]. And additionally, there would be
>        entries for the virtual links of the ONU and OLT interfaces.
>      ------
>     d) Update the figures and tables through the remainder of the
>        document to match the ifIndex assignments.
>              
> 5) The end of section 1.3, which starts with sentence "As an
>    example provided below are the values for the MPCP" should
>    be moved to section 2 - where the MIB structure is discussed.
>    
> 6) The figures labeled "Table 1" through "Table 4" are useful,
>    but due to their size are problematic. I suggest that
>    only index columns, columns that linkage to other tables,
>    and columns what are different between ONU and OLT entries
>    be included.
> 
> 7) The first sentence after "Table 2" sort of looks like it should
>    be a section head.
>    
> 8) The value for dot3MpcpRemoteMACAddress in "table 3" should
>    be 6 octets of zero (and not one). For example:
>       00:00:00:00:00:00
>       
> 9) There needs to be a footnote, or text that explains the
>    meaning of values OLT_MAC_Address, ONU{1,2,3}_MAC_Address,
>    and BRCT_MAC_Address
>     
> 10) The paragraph after "Table 4" seems to be misplaced,
>     and should come before "Table 2".
> 
> 11) Section 2 ("MIB structure") is weak. Moving the text from
>     section 1.3 should help.
> 
> 12) How the "extended package" tables were related was not
>     well explained in section 2 (nor in their definition).
>     It took me a while to figure out that object
>     dot3ExtPkgObjectReportMaximumNumQueues affected
>     the number of entries in tables dot3ExtPkgQueueTable
>     and dot3ExtPkgQueueSetsTable. And that object
>     dot3ExtPkgObjectReportMaximumNumThreshold affected
>     the number of entries in table dot3ExtPkgQueueSetsTable.
>     In general, I couldn't completely figure out
>     the use of the tables in the "extended package"
>     nor the expected values.
> 
> 13) In section 2, the same terminology should be used. There
>     is used "MIB objects", "MIB module", and "managed object".
> 
> 
> 14) Section 3.1 is quite useful (and required by all interface
>     MIB module documents). (Note "Ether-like" is misspelled
>     as "Ehter-like" in one place that I saw.)
>     Missing is a discussion and example of the IF stack table
>     (and the inverted stack table). Also missing is a discussion
>     of what happens to counters when operation is stopped.
> 
> 15) Section 3.2 mentions the "amended MAU MIB document", but
>     the references specifies the current MAU MIB
>     document. I suggest that you specify the new MAU I-D
>     in the references (if available).
>     
> 16) The abbreviations in the MODULE-IDENTITY specification
>     need to be checked and updated for completeness.
>     
> 17) The MPCP stats table has stats for both OLTs and ONUs,
>     and counters that only have meaning on one of the
>     OLT or ONU. In the later case, the text says the value
>     is always zero. For example, object dot3MpcpDiscoveryWindowsSent
>     always has a value of zero on ONUs, and object
>     dot3MpcpTxRegRequest always has a value of zero
>     on OLTs. This is a valid approach, but somewhat
>     confusing. Another approach would be to split
>     the table (and other similar table) into three
>     tables for both, OLT, and ONU stats. Or to split
>     into two tables with some columns duplicated (but
>     of course with different descriptors). I'd like
>     to hear opinions from others on this.
> 
> 18) In general, there is an inconsistent description specified
>     per object as to whether or not the value is always
>     zero (a place holder). This should be cleaned up,
>     since it is confusing that different words are used.
>     The read questions whether or not the same thing
>     is being said, but just using different words.
>     
>     
> 19) At first, I didn't understand why table dot3OmpEmulationTable
>     is present. It's single object dot3OmpEmulationType appears
>     to me to be the same as object dot3MpcpMode. However, looking
>     at the MODULE-COMPLIANCE definitions, I see the need since
>     for interpreting values in table dot3OmpEmulationStatTable
>     you need to know if the interface is an OLT or ONU.
>     This is not well explained in section 2.
>     Also, maybe table dot3MpcpStatTable should AUGMENT
>     table dot3MpcpControlTable; table dot3OmpEmulationStatTable
>     should AUGMENT table dot3OmpEmulationTable. Table
>     dot3EponFecTable doesn't need another table to
>     indicate if the interface is a OLT or ONU, since
>     the stats apply to both.
> 
> 20) There appear to be some character set problems in the
>     description for object dot3EponFecPCSCodingViolation.
> 
> 21) You should probably say that the value 'unknown(1)' cannot
>     be written to object dot3EponFecMode.
> 
> 22) You should probably rewrite the description of
>     object dot3EponFecBufferHeadCodingViolation so it
>     is clear that the value is meanful only when in
>     1000 Mbps operation, and zero otherwise.
> 
> 23) Object dot3ExtPkgObjectReset is an action object to
>     reset "EPON" interfaces. What does a reset do
>     to counters? Is there an object that counts the
>     number of resets or provides a timestamp of
>     last reset operation? Does a reset of one
>     of the virtual interfaces reset only it,
>     or the physical interface?
> 
> 24) Object dot3ExtPkgObjectPowerDown causes an interface
>     to be powered down or up. How does this work for the
>     virtual interfaces?
>     
> 25) Object dot3ExtPkgObjectNumberOfLLIDs seems silly (useless).
>     Please explain.
> 
> 26) What happens to counters and instances in the FEC table
>     when the value of object dot3ExtPkgObjectFecEnabled is
>     changed?
> 
> 27) Object dot3ExtPkgObjectReportMaximumNumQueues is not
>     well described. Also, it SYNTAX value should
>     probably be Unsigned32(0..7). What happens if
>     it is changed, to counters in the the tables 
>     dot3ExtPkgQueueTable and dot3ExtPkgQueueSetsTable.
> 
> 28) I don't understand the function of object
>     dot3ExtPkgObjectRegisterAction. Does it cause
>     instances to be created or deleted?
> 
> 29) The index dot3QueueIndex is not well explained
>     in table/row for the Queue table.
> 
> 30) I couldn't figure out objects dot3ExtPkgObjectReportNumThreshold
>     and dot3ExtPkgObjectReportMaximumNumThreshold other
>     than one of them controlled the number of 
>     queue sets.
> 
> 31) I believe that the syntax of object
>     dot3ExtPkgObjectReportMaximumNumThreshold should be
>     Unsigned32(0..7).
> 
> 32) It doesn't seem to make sense that there would be
>     entries in table dot3ExtPkgOptIfTable for virtual
>     interfaces. 
> 
> --- that's all
> 
> 
> Regards,
> /david t. perkins
> 
> 
>