Subject: [pcp] -06 review: options

From: Paul Selkirk <[email-masked]>
Date: Mon, 07 Mar 2011 22:04:00 +0000

For clarity, Option format diagrams should include the general TLV
Option header, with the option number filled in, and the length
(if fixed).

Is option value 0 reserved, since it's not assigned here?
1 UNPROCESSED
2 REMOTE_PEER_FILTER
3 PREFER_FAILURE
4 THIRD_PARTY

Option boilerplate fields should start with a capital letter, e.g.
         Name: <mnemonic>
         Number: <value>
         Purpose:
         Valid for OpCodes: <list of OpCodes>
         Length: <rules for length>
         May appear in: <requests/responses/both>
         Maximum occurrences: <count>

REMOTE_PEER_FILTER and PREFER_FAILURE are missing the required
"purpose" field.  They also both have an additional "is included in
responses" field, which is unnecessary, because they're in the
mandatory-to-process range (therefore mandatory to report success or
failure).  [5.3: "If a given Option was included in a request, and
understood and processed by the PCP server, it MUST be included in the
response."]

Instead of "may appear in", perhaps we want something like "may be
unsolicited".  Or better yet, drop it from the boilerplate, and just
note in the text that UNPROCESSED is a response-only option.

PREFER_FAILURE has "maximum occurrences: no", which implies that it
can never occur, which is nonsense.  Instead of "maximum occurrences",
maybe a binary flag "may occur more than once" (which only
REMOTE_PEER_FILTER would set to true).

5.3.  Options
   The response MUST encode the Options in the same order, but may omit
   some PCP Options in the response, as is necessary to indicate the PCP
   server does not understand that Option or that Option is not
   permitted to be included in responses by the definition of the Option
   itself.  Additional Options included in the response (if any) MUST be
   included at the end.

The last sentence says that a) the response can include unrequested
options, and b) only the requested options are ordered numerically.
Should the client validate the ordering of requested options?

6.7.1.  UNPROCESSED
   For simplicity, no more than 4
   options can be encoded.  This option MUST NOT appear more than once
   in a PCP response, no matter how many PCP options appeared in the
   request and were unprocessed by the PCP server.

And the boilerplate says "length: 1", but Figure 5 shows 8 options.

   The order of the Options in the PCP request has
   no relationship with the order of the Option values in this
   UNPROCESSED Option.

This seems a bit odd in view of the strict ordering requirement.

8.4.  OpCode-Specific Server: Processing a Request
   If an Option with value greater than 128 exists but that option does
   not make sense (e.g., the PREFER_FAILURE option is included in a
   request with lifetime=0), the request is invalid and generates a
   MALFORMED_OPTION error.

s/greater/less/

8.8.1.  REMOTE_PEER_FILTER
   Because of interactions with dynamic ports this Option MUST only be
   used by a client that is operating a server (that is, using the MAP
   OpCode), as this ensures that no other application will be assigned
   the same ephemeral port for its outgoing connection.

The parenthetical clause is kind of puzzling, since the boilerplate
already declared that this option is only to be used with MAP opcodes.

8.8.2.  PREFER_FAILURE
   The option is provided only because the semantics of IGD version 1
   offer no viable alternative way to implement an IGD interworking
   function.

This sentence is redundant to previous text, and can be removed.

REMOTE_PEER_FILTER and PREFER_FAILURE might want to mention that they
can't be used in delete operations, and will generate a
MALFORMED_OPTION response.  (Or is this implicitly covered under the
"option does not make sense" clause quoted above?)

				paul