Subject: [forces] AD review of draft-ietf-forces-protoextension

From: "Adrian Farrel" <[email-masked]>
Date: Sun, 20 Jul 2014 03:34:22 +0100

Hi,

I have done my usual AD review having received a publication request
for this document. There are a few nits, and some larger concerns with
the IANA section.

I'll put the document into "Revised I-D Needed" state and wait to see a
new revision.

Thanks,
Adrian

===

Abstract
Typo
s/1. s/1 s/

---

As usual, the RFC Editor will want the Introduction to be the first
section in the document. Could you please shuffle the sections to help
with that, and take care of any acronym expansions in the Introduction
that will result.

---

I am not too happy about the restatement of definitions from other
sources instead of referencing them. This can cause discrepancies and
be difficult to handle if there are updates or changes. I have not 
checked the definitions for consistency with the "various documents".

I suggest that you only keep the definitions that are absolutely 
fundamental for this document, and even then you should point at the
normative definitions.

Note that the currently included terminology creates some issues. For
example:

      ForCES Protocol - Protocol that runs in the Fp reference points in
      the ForCES Framework [RFC3746].

...what is an "Fp reference point"?

---

4.1.  Table Ranges

   We propose to add a Table-range TLV (type ID 0x117) that will be
   associated with the PATH-DATA TLV in the same manner the KEYINFO-TLV
   is.

Proposal or definition?
Similar in other sections.

---

Section 6 needs work

   This document registers two new top Level TLVs and two new path flags
   and updates an IANA registered FE Protocol object Logical Functional
   Block (LFB).

   XXX: when this document is undergoing IANA review we should update
   https://www.iana.org/assignments/forces/forces.xml section on FEPO to
   have the new version reflected.

I don't understand that paragraph. I don't see any mention of FEPO on
that page and have no understanding of what version you're talking about.

   The following new TLVs are defined:

   o  TABLERANGE-TLV (type ID 0x117)

   o  EXTENDEDRESULT-TLV (type ID 0x118)

Presume these come from the "TLV Types" sub-registry. Please note that.

   The following new path flags are defined:

   o  F_SELTABRANGE (value 0x2 i.e bit 1)

Trying to work out which registry this is from. I don't see Path Flags
mentioned anywhere. Is this from the "Header Flags" sub-registry?

If so, why 0x2 and not 0x1?

   The Defined Result Values are changed:

   o  codes 0x21-0xFE are reserved.

   o  codes 0x18-0x20 are defined by this document.

   o  codes 0x100-0x200 are reserved for vendor use.

Name that registry: "RESULT-TLV Result Values sub-registry"?
Or are you doing something more sophisticated and combining the old
registry with a new registry for the EXTENDEDRESULT-TLV Result Value?

And...
- Do you mean "reserved" or "unassigned"?
- Would you like to clarify for IANA what assignments are being made
  for 0x18-0x20
- You are not changing the allocation rules for the range 0x18 to
  0xfe so, notwithstanding this being a standards track document, we
  need to get DE approval for the allocations 0x18-0x20. Since Jamal
  is an author, that means Joel.
- If you're doing the 32 bit thing then what about the values larger 
  than 0x200?
- What does "for vendor use" mean? Which 5226 policy do you intend to
  be applied? The text in 4.2.2 suggests FCFS.

I suspect what you want here is
- allocate 0x18-0x20 from the RESULT-TLV Result Values sub-registry
- define a new sub-registry for EXTENDEDRESULT-TLV Result Values
  - state that 0x00-0xff is mirrored from the RESULT-TLV Result Values
    sub-registry and must not be allocated
  - define new allocation policies for 0x100-0xffff

---

I think RFC 2119 is a normative reference.

---

From: Jamal Hadi Salim <[email-masked]>
Date: Sun, 20 Jul 2014 07:26:38 -0400

Thanks for your review Adrian. Appreciated as always.
I will update the document this week sometime.

cheers,
jamal

On Sat, Jul 19, 2014 at 10:34 PM, Adrian Farrel <[email-masked]> wrote:
> _______________________________________________
> forces mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/forces