[Gen-art] Gen-art review of draft-bberry-pppoe-credit-04.txt

[Elwyn Davies <[e-mail]>]

I was selected as General Area Review Team reviewer for this specification
(for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Document: draft-bberry-pppoe-credit-04.txt
Intended Status: Informational (Individual submission via RFC Editor)
Shepherding AD: Mark Townsley
Review Trigger: IESG Telechat 5 January 2006

Summary:
In my opinion this proposal is technically unsound and should not be 
published in its current form.  At the very least Section 5 has to be 
formulated differently (it is a serious abuse of PPP), the credit 
mechanism would need to be much better specified to demonstrate that it 
is robust and that interoperable implementations could be made, and, in 
my opinion, the value of the complex link quality mechanisms in likely 
deployment scenarios is questionable.

I note that the PPPEXT wg review has also noted some serious concerns 
about the general principles of this proposal.  I haven't repeated their 
views about layer violations and misunderstanding about the purpose of 
PPPoE - there are quite enough other problems.

Even were it to be published I am unclear why this document should be 
Informational.  It is documenting a proposed extension to a standard 
(although, if the authors wished, it could have made use of the existing 
Vendor-specific tag and then it would legitimately have been an 
Informational document on that front - the new message types are another 
matter) and as such ought to be aiming for PS.

Assuming this document is approved, it needs an IANA considerations 
section.  I think that it effectively creates two registries for PPPoE 
discovery message ids and discovery option tags which (AFAICS) weren't 
setup by RFC2516 although the TAG registry is implicit in the wording of 
RFC2516.

Review:
System overview diagram/description: PPPoE is supposed to connect a host 
to an Access Concentrator, not to link two Access Concentrators.  The 
term Host Radio is probably misleading.  I am not clear if this is all 
just an oversize typo or a fundamental misunderstanding.

The scenario in which PPPoE is typically used normally envisages hosts 
tied to an access network via a single link.  It is not clear that the 
routing protocol would have any alternatives other than the PPPoE link 
in most circumstances, so that the only link indication needed is link 
up/link down.  The proposed metric system therfore seems to be overkill 
in most realistic situations.  Also it is not made clear how the radio 
related information is transferred to the PPPoE endpoints or whether the 
'host radio' nodes are intended to intercept the relevant packets and 
fill in the required info (and then fix up the checksum - hmm!). If it 
were used, the interactions between the rate of change of radio 
bandwidth (especially in cellular cases) and the time constants in 
routing protocols needs to be considered.  The authors should take a 
look at draft-iab-link-indications-04.txt.

ss4.3-4.5: If these packets are needed, I think it would make more sense 
to send them as a new PPP sub-protocol on the established session rather 
than overloading the discovery packets.  The addressing of these packets 
needs to be made explicit if not.

s4.5: PPP already has a link quality report sub-protocol (0xc025) - do 
we need another one?  Note if the radio is 'receive only' the link is no 
good for IP!

Section 5: There is a fundamental misunderstanding here.  The example 
packet given in RFC2516 seems to have been taken as meaning that *all* 
session stage packets will be using the LCP PPP protocol type (0xC021).  
This is clearly not the case - for example, plain vanilla user IPv4 
packets will use protocol 0x0021 and there could be any number of 
different protcols going across the link. The proposal to piggyback 
credit tags onto these packets in the manner proposed is therefore 
broken - and a serious abuse of the PPP structure.  A fix defining a new 
PPP protocol number  would be entirely possible, but that is another matter.

Section 6/7: I haven't analysed the credit flow mechanism in s6 in 
detail but it doesn't seem to take into account that Ethernet is not a 
reliable medium.  Section 7 *does* seem to acknowledge this but the 
interaction between the inband and out-of-band credits during a session 
is far from clear - I don't think I could build interoperable 
implementations as it stands.  Also, some idea of default timeout values 
needs to be specified.

The document has no IANA considerations section.  It assigns three new 
(pseudo-)discovery message types and three new 'TAG' numbers that fit 
into the space used for tags defined in RFC2516.  These message ids and 
tags are currently internal to RFC2516 and so have no registries.  Since 
RFC2516 allows new tags to be defined there presumably ought to be an 
IANA registry for these tags but it wasn't setup at the time.  RFC2516 
did not originally envision new discovery messages. I assume that there 
ought be to be expert review of the assignment by analogy with the other 
PPP registries.


Editorial:
The boilerplate is out of date (see idnits output).

The figures need titles and bit position indicators.
Also the rules say that figures are optional add-ons to words - s9 needs 
to specify the contents of the tags in words as well.

The document needs a spell check.

Running idnits:
  Checking nits according to http://www.ietf.org/ID-Checklist.html:
  * The document seems to lack an IANA Considerations section.
   
    Checking conformance with RFC 3978/3979 boilerplate...

  * Found RFC 3978 Section 5.5 boilerplate (on line 30), which is fine, but
    *also* found RFC 2026 Section 10.4C paragraph 4 boilerplate on line 
701. It
    should be removed.
  * The document claims conformance with section 10 of RFC 2026, but uses
    some RFC 3978/3979 boilerplate.  As RFC 3978/3979 replaces section 
10 of RFC
    2026, you should not claim conformance with it if you have changed 
to using
    RFC 3978/3979 boilerplate.
  * The document seems to lack an RFC 3978 Section 5.1 IPR Disclosure
    Acknowledgement -- however, there's a paragraph with a matching 
beginning.
    Boilerplate error?
  * The document seems to lack an RFC 3978 Section 5.4 Copyright Line --
    however, there's a paragraph with a matching beginning. Boilerplate 
error?
  * The document seems to lack an RFC 3978 Section 5.4 Reference to BCP 78.
  * The document seems to lack an RFC 3979 Section 5, para 1 IPR Disclosure
    Acknowledgement -- however, there's a paragraph with a matching 
beginning.
    Boilerplate error?
    ( - It does however have an RFC 2026 Section 10.4(A) Disclaimer.)
  * The document seems to lack an RFC 3979 Section 5, para 2 IPR Disclosure
    Acknowledgement.
  * The document seems to lack an RFC 3979 Section 5, para 3 IPR Disclosure
    Invitation -- however, there's a paragraph with a matching beginning.
    Boilerplate error?
    ( - It does however have an RFC 2026 Section 10.4(B) IPR Disclosure
    Invitation.)
  * The document uses RFC 3667 boilerplate or RFC 3978-like boilerplate
    instead of verbatim RFC 3978 boilerplate.  After 6 May 2005, 
submission of
    drafts without verbatim RFC 3978 boilerplate is not accepted.

    The following non-3978 patterns matched text found in the document.
    That text should be removed or replaced:

    "By submitting this Internet-Draft, I certify that any applicable
    patent or other IPR claims of which I am aware have been disclosed,
    or will be disclosed, and any of which I become aware will be
    disclosed, in accordance with RFC 3668."

  * There are 243 instances of too long lines in the document, the longest
    one being 6 characters in excess of 72.
  * There is 1 instance of lines with non-ascii characters in the document.

  Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt:
  - It seems as if not all pages are separated by form feeds - found 0 form
    feeds but 20 pages

  Miscellaneous warnings:
    None.

_______________________________________________
Gen-art mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/gen-art