[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
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: owner-mreview@ops.ietf.org
> [mailto:owner-mreview@ops.ietf.org] On Behalf Of David T. Perkins
> Sent: Saturday, May 27, 2006 2:33 AM
> To: mreview@ops.ietf.org
> 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
>
>
>