[Pppext] AD review of draft-ietf-pppext-trill-protocol

[Jari Arkko <[e-mail]>]

I have reviewed this draft. I think it is basically in good shape, but 
needs to make a few things more explicit & correct small issues. For 
what it is worth, I'm happy with the resolution of the system ID issue; 
it really needs to be out of scope and dealt in ISIS WG, if they think 
its an issue.

I'm expecting a new draft version. Then I can start an IETF Last Call.

Also, for the record, as James is both the WG chair and the author, I 
have reviewed the discussion in the mailing list and believe that the 
Working Group has consensus to move the document forward.

Section 2.1:

> Data
>
>   This field contains data in the same format as for the
>   corresponding LCP Code numbers.
>
>   

Can you clarify what this actually means. It was not clear (to this 
reader, at least). Is this where the configuration options would be, if 
some were defined? But if so, what does LCP code numbers have to do with it?

Section 2.2:

> When TNCP is in Opened state, TNP packets MAY be sent by setting the
> PPP Protocol field to hex TBD-00XX (TNP) and placing the TRILL-
> encapsulated data in the PPP Information field.
>
> A summary of this format is provided below:
>
>     0                   1                   2                   3
>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    | V | R |M|Op-Length| Hop Count | Egress (RB2) Nickname         |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    | Ingress (RB1) Nickname        | Inner Destination MAC ...
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
>
> This is identical to the TRILL Ethernet format except that the Outer
> MAC header and Ethertype are replaced by the PPP headers and Protocol
> Field, and the Ethernet FCS is not present.  Both user data and ESADI
> packets are encoded in this format.  

Please add a reference to the document and Section where these fields 
are defined.

> When TNCP is in Opened state, TLSP packets MAY be sent by setting the
> PPP Protocol field to hex TBD-40XX (TLSP) and placing the IS-IS
> Payload in the PPP Information field.  

TRILL version of IS-IS, I presume. Please provide a reference to the RFC 
that specifies the IS-IS payload used in this context.

> 1. On a PPP link, TRILL always uses P2P Hellos.  There is no need
>    for TRILL-Hello frames, nor is per-port configuration necessary.
>    P2P Hello messages, per section 9.3 of [6 <http://tools.ietf.org/html/draft-ietf-pppext-trill-protocol-05#ref-6>], do not use Neighbor
>    IDs.
>   

Section 9.3 of [6] seems to talk about something else. If you meant [1] 
it has no Section 9.3. Please clarify.

> If the peer is not an RBridge, then TRILL is not
> possible.
>   

I think you mean that if the peer is not an RBridge then the negotiation 
in this specification fails and no TRILL is used for the PPP link.

> The encapsulated network layer data, carried in TNP packets, and
> topology information, carried in TLSP packets, MUST NOT be sent
> unless TNCP is in Opened state.  If a TNP or TLSP packet is received
> when TNCP is not in Opened state and LCP is Opened, an implementation
> SHOULD respond using LCP Protocol-Reject.
>
>   
> 3. TRILL PPP Behavior
I did not find a specification for the state machine (open/closed) in 
the draft. Please specify.

> 4. MTU-probe and MTU-ack messages are not needed on a PPP link.
>    Implementations MUST NOT send MTU-probe and SHOULD NOT reply to
>    these messages.  The MTU computed by LCP SHOULD be used instead.
>    Negotiating an LCP MTU of at least 1524, to allow for an inner
>    Ethernet payload of 1500 octets, is RECOMMENDED.
>   

I think you mean TRILL MTU-probe. Please point to the appropriate 
section of [1] so that the reader knows for sure which messages you are 
referring to.

> OUI

Expand the acronym

> Resolving that issue is outside the
> scope of this document, but see [8 <http://tools.ietf.org/html/draft-ietf-pppext-trill-protocol-05#ref-8>] for one mechanism that should
> be considered in this situation.
>   

I would make this even clearer -- there's no official status yet for 
[8]. I'd use this:

 Resolving that issue is outside the
 scope of this document. Solutions
 to this issue may be defined elsewhere in the future, see
 [8] for an example.

Jari