Subject: [CCAMP] AD review of draft-ietf-ccamp-rsvp-te-eth-oam-ext

From: "Adrian Farrel" <[email-masked]>
Date: Mon, 10 Feb 2014 23:07:25 -0000

Hi,

Thanks to the authors for engaging with me on the implementation
status and plans for this document.

The CCAMP OAM configuration framework draft has now progressed far
enough through the system that it is appropriate to move this one
forward, so I have done my usual AD review. The purpose of the 
review is to catch some of the larger issues that might come up in
IETF last call and IESG evaluation. Catching them early helps save
everyone time, and improves the quality of subsequent reviews.

I have a few small editorial issues listed below and a request for
some careful reworking of the IANA section. This is open for
discussion if you have any concerns, but until then I have placed
the document in "Revised I-D Needed" state. When I see a new
revision I will advance the document to IETF last call.

Thanks for the work.

Adrian

===

Section 3.1 says

"procedures are recommended as follows" and then goes on to say "MUST"

That reads oddly.
I think you can just s/are recommended as follows/are as follows/

---

I think you have the Length in the TLVs messed up.
For example, in 3.3 you have
   Length: indicates the total length including sub-TLVs.
"Total length implies "including the T and L fields".
And in 3.3.1 and 3.3.2 you have
   Length: indicates the total length of the TLV including padding.
which has a similar implication.
Yet in 3.3.3 while you still have
   Length: indicates the total length of the TLV including padding.
the figure shoes "Length (4)"

Please update all descriptions of Length to say (depending on your
intention) either:

   Length: indicates the total length of the TLV including padding
           and including the Type and Length fields

or

   Length: indicates the total length of the TLV including padding
           but excluding the Type and Length fields

And then, obviously, check any explicit values in the text.

---

In Section 3.3.4 you have assigned 4 bits for each of the Priority and
CCM I fields.

Yet the text describes how these may be mapped directly to the
equivalent Ethernet fields each of which is described as being 3 bits.

Why is there this discrepancy and how will the top bit be used?

---

The IANA section needs some work.

First I suggest you break it into subsections to clearly show each
different registry being updated.

Then we need some additional text as follows:

- The RSVP-TE OAM Configuration Registry doesn't exist yet so you need
  to provide IANA with a reference using text like "[OAM-CONF-FWK]
  includes a request for IANA to create the RSVP-TE OAM Configuration
  Registry". IANA is requested to assign an OAM Type from this
  registry as follows.

  Similar thing for the sub-TLV type.

- For the sub-TLV you need to tell IANA that the value is to be allocated
  from the technology-specific range

- You need to not state specific values unless you have a strong reason
  for doing so or the values have already been assigned through early
  allocation.

  Replace "1" with TBA1
  Replace "32" with TBA2
  Go through the main body of the text and make these replacements as
  well

  Then add text that says:
    The value of 1 is suggested for TBA1
    The value of 32 is suggested for TBA2

- The term "reserved" has a special meaning in IANA registries. It means
  "do not allocate". So I think you do not want to use that term in your
  new registry!

  Well, 0 may be genuinely reserved, but "5-" should probably read
  "unassigned"

- You need to indicate the maximum legitimate value for the code points
  in the new registry. So replace "5-" with "5-x" where x is the max
  value (governed by the size of the Type field?)

- You need to state the RFC 5226 assignment policy for the new registry.
  Probably "Standards Action", but you may have other ideas. Should any
  of the code points be reserved for Experimentation?

- "OAM Problem"
  - You need to tell IANA which registry you are talking about
  - The return code doesn't exist yet, so you need to include a 
    reference to [OAM-CONF-FWK]