Re: [pcp] WGLC: draft-ietf-pcp-base-12.txt

["Dan Wing" <[e-mail]>]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of
> Paul Selkirk
> Sent: Monday, May 30, 2011 9:26 PM
> To: [e-mail]
> Subject: Re: [pcp] WGLC: draft-ietf-pcp-base-12.txt
> 
> I'm sorry this review is so long; I didn't have time to write a shorter
> one.
> 
> I also apologize if this touches on issues that are being or have been
> hashed out on the mailing list; I'm way behind on WG mail.
> 
> There are a few places where I have issues with how the protocol
> actually works, a few clear (but easily cleared) errors in the
> current draft (marked with "!!"), and a lot of editorial nits.
> 
> For the errors and nits, I'd be willing to make the necessary changes
> myself, if Dan can give me the document source (since I seem to be
> listed as a co-author now).

Will send you a pointer.

> ----------------
> Policy issues:
> 
> - I'm not happy with the way PEER has evolved, to where it can create
> mappings.  It's heading in the direction of making MAP a special case
> of PEER (with remote port=0, remote peer=0 as wildcards).  I also don't
> think the 7.3 use case is compelling enough to justify the added
> complexity in the protocol (or even the suggested added complexity in
> the client).

The motivation for that change was to eliminate the error condition
if the PCP server received a PEER message for a mapping that didn't
already exist.  The fear was that client code would not be implemented
robustly to deal with that error, because the error would occur
rarely.

I would certainly like to hear from others on this change.  Only a 
few people have provided Yeah/Nay comments on this change.  From
memory, I recall:

   Dan:     yeah
   Simon:   yeah
   Stuart:  yeah
   Paul:    nay
   Francis: nay

> - I don't like the PCP Client's IP Address field.  I don't think it
> adds
> sufficient value to be worth spending 16 bytes in every packet, plus
> additional processing time.  I would support moving it to an Option, or
> at least making it an optional field - e.g. if it's sent as all-zeroes
> in the request, the server MUST process the request, and MAY either
> fill
> it in, or leave it zeroed.  The change log says "This addition was
> consensus at IETF80", but that doesn't match my notes of the meeting -
> it was discussed as a possible approach to a possible problem, but no
> sense of the meeting was ever asked.

The consensus of the meeting is there was a problem that the protocol
could solve.  I admit this solution was not agreed at the meeting --
discussing bits-on-the-wire seemed a poor use of meeting time.

Moving it to an option implies there are cases where the PCP client 
does not want to test for an on-path NAT.  What are those cases?  (I
need to know, in order to write text explaining when the option should
be used and when it shouldn't be used).

> - UNAUTH_THIRD_PARTY_INTERNAL_ADDRESS could be removed, and replaced
> with NOT_AUTHORIZED (more on that inline).
> 
> - There is no discussion about levels of conformance.  There's some
> discussion about particular options being optional (e.g. THIRD_PARTY
> not supported in certain deployment scenarios), but no overall
> statement of what features must be supported by all conformant
> implementations.  e.g. If I had a small embedded server that supported
> MAP opcodes but not PEER, would it be conformant?

Nobody has initiated discussion or provided a straw-man on conformance 
levels.  What do we want?  Myself, if there is any chance of a wireless
devices (which is approximately 100% chance), PEER needs to be supported
because it allows the wireless device to optimize its NAT keepalive
traffic.  But, to me, MAP doesn't need to necessarily be supported.


-d



> ----------------
> General nits:
> 
> Normalize capitalization in all instances of "OpCode" and "Option".
> (8 instances of "opcode", 5 instance of "Opcode", 76 instances of
> "option").
> 
> Normalize capitalization of things like Internal Port and Remote Peer
> Port (didn't count them).
> 
> Change "error code" to "result code" (5 instances).
> 
> Possibly use "NAT" as accepted vernacular throughout, even when
> referring to NAPT, unless there's some clear distinction to be made by
> referring to NAPT.
> 
> There are many places where the text has unnecessary embedded
> definitions of implicit and/or explicit mappings:
> 7.4, 8.6 (twice), 11.1, 12.2, 13, 13.1, 13.2.
> 
> ----------------
> Line-by-line review:
> 
> 1.  Introduction
> 
>    PCP is primarily designed to be implemented in
>    the context of both Carrier-Grade NATs (CGN) and small NATs (e.g.,
>    residential NATs).
> 
> Remove "primarily" - what other kind of NATs are there?
> 
>    PCP allows hosts to operate server for a long
>    time (e.g., a webcam) or a short time (e.g., while playing a game or
>    on a phone call)
> 
> A webcam could be long or short term, ditto a game or phone call.
> Even the client may not be able to predict how long a mapping may be
> needed.  It's immaterial to the protocol.
> 
> 2.1.  Deployment Scenarios
> 
>    PCP can be used in various deployment scenarios, including:
>    ...
> 
> Make this a real bulleted list, rather than a long bulleted sentence.
> Also reorder to group them as current and emerging scenarios, e.g.
> 
>    PCP can be used in various deployment scenarios, including:
> 
>    o  Basic NAT [RFC3022]
> 
>    o  Network Address and Port Translation (NAPT) [RFC3022], such as
>       commonly deployed in residential NAT devices
> 
>    o  Carrier-Grade NAT [I-D.ietf-behave-lsn-requirements]
> 
>    o  Dual-Stack Lite (DS-Lite) [I-D.ietf-softwire-dual-stack-lite]
> 
>    o  Layer-2 aware NAT [I-D.miles-behave-l2nat] and Dual-Stack Extra
>       Lite [I-D.arkko-dual-stack-extra-lite]
> 
>    o  NAT64, both Stateful [RFC6146] and Stateless [RFC6145]
> 
>    o  IPv4 and IPv6 simple firewall control [RFC6092]
> 
> 2.3.  Single-homed Customer Premises Network
> 
>    The PCP machinery assumes a single-homed host model.
> 
> "machinery" is an odd word
> s/The PCP machinery/PCP/
> 
>    This
>    restriction exists because otherwise there would need to be one PCP
>    server for each egress, because the host could not reliably
> determine
>    which egress path packets would take, so the client would need to be
>    able to reliably make the same internal/external mapping in every
> NAT
>    gateway, which in general is not possible (because the other NATs
>    would likely have the necessary port mapped to another host).
> 
> run-on sentence
> 
> 3.  Terminology
> 
> Add "PCP-controlled device" = NAT or firewall
> 
>    PCP Client:
>       A PCP Client can issue PCP request on behalf of a third
> 
> s/request/requests/
> 
>       An interworking
>       function from Universal Plug and Play Internet Gateway Device
>       (UPnP IGD, [IGD]) to PCP is another example of a PCP Client.
> 
> Capitalize "interworking function"
> 
>    Interworking Function:
>       a functional element responsible for interworking another
> protocol
>       with PCP.  For example interworking between UPnP IGD [IGD] with
>       PCP.
> 
> Capitalize "a"
> 
> Also, define "interworking"; maybe something like
>       A functional element responsible for translating another protocol
>       to PCP.
> 
>    subscriber:
>       an entity provided access to the network.  In the case of a
>       commercial ISP, this is typically a single home.
> 
> Capitalize "subscriber", "an"
> 
>    5-tuple  The 5 pieces of information that fully identify a flow,
> from
>       the perspective of a subscriber: source IP address, destination
> IP
> 
> Line break before "The"
> 
> Also, change "subscriber" to "internal host" or "PCP client" for
> consistency.
> 
> 4.  Relationship between PCP Server and its NAT/firewall
> 
>    The PCP server receives PCP requests.  The PCP server might be
>    integrated within the NAT or firewall device (as shown in Figure 1)
>    which is expected to be a common deployment.
> 
>    It is also possible to operate the PCP server in a separate device
>    from the NAT, so long as such operation is indistinguishable from
> the
>    PCP client's perspective.
> 
> This is completely redundant to the Terminology entry.
> In this case, I'd radically trim the terminology section, e.g.
> 
>    PCP Server:
>       A network element which receives and processes PCP requests from
> a
>       PCP client.  Generally this is a PCP-capable NAT gateway or
>       firewall.
> 
> 5.1.  Request Header
> 
>    OpCode:  Opcodes are defined in Section 8 and Section 9.
> 
> This seems like it needs something more.
> 
>    Requested Lifetime:  The Requested Lifetime field is an unsigned 32-
>       bit integer, in seconds, ranging from 0 to 4,294,967,295 seconds.
> 
> s/The Requested Lifetime field is //
> 
>    PCP Client's IP Address:  The IP address of the PCP client, from the
>       PCP client's perspective.  If IPv4, only the first 32 bits are
>       used, the other bits MUST be set to 0.
> 
> This needs a little explanation, e.g. "If the enclosing IP packet is
> IPv4, only the first 32 bits are used; the other bits MUST be sent as
> 0 and MUST be ignore when received."
> 
> This is discussed/explained/rationalized in 6.2, but as "PCP Client IP
> Address" (no apostrophe s).
> 
> It must be optional - if it's all zeroes, the server will not reject
> the packet, and will either send back its idea of the client's IP
> address, or leave it zero'd.
> 
> Also normalize capitalization: "address" is lowercase in both of the
> packet diagrams.
> 
> 5.2.  Response Header
> 
>    Lifetime:  The Lifetime field is an unsigned 32-bit integer, in
>       seconds, ranging from 0 to 4,294,967,295 seconds.
> 
> s/The Lifetime field is //
> 
>    Epoch:  The server's Epoch value.  See Section 6.5 for discussion.
>       This value is set in both success and error responses.
> 
> This needs a concise description here, e.g. "seconds since start of
> epoch".
> 
> 5.3.  Options
> 
>    A PCP OpCode can be extended with an Option.
> 
> PCP packets can be extended with Options. (i.e. one or more)
> 
>    A given Option MAY be included in a request containing a specific
>    OpCode.  The handling of an Option by the PCP client and PCP server
>    MUST be specified in an appropriate document and MUST include
> whether
>    the PCP Option can appear (one or more times) in a request and/or
>    response, and indicate the contents of the Option in the request and
>    in the response.  If several Options are included in a PCP request
> or
>    response, they MAY be encoded in any order by the PCP client and are
>    processed in the order received.
> 
> rewrite as
>    The handling of an Option by the PCP client and PCP server MUST be
>    specified in an appropriate document, which MUST include whether
>    the PCP Option can appear in a request and/or response, whether it
>    can appear more than once, and indicate what sort of Option data it
>    conveys.  If several Options are included in a PCP request, they
>    MAY be encoded in any order by the PCP client, but MUST be
>    processed by the PCP server in the order in which they appear.
> 
> (The original wording of the last sentence was contradicted by the
> next paragraph: "The response MUST encode the Options in the same
> order".)
> 
> Actually, the whole paragraph is redundant to later text describing
> what an Option definition must include.
> 
>    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
> 
> s/may/MAY/
> s/as is necessary //
> 
>    If the "O" bit (high bit) in the OpCode is clear,
> 
>    o  the PCP server MUST only generate a positive PCP response if it
>       can successfully process the PCP request and this Option.
> 
>    o  if the PCP server does not implement this Option, or cannot
>       perform the function indicated by this Option (e.g., due to a
>       parsing error with the option), it MUST generate a failure
>       response with code UNSUPP_OPTION or MALFORMED_OPTION (as
>       appropriate) and include the UNPROCESSED option in the response
>       (Section 6.7.1).
> 
> Remove the first bullet, make one sentence, change "failure response"
> to "error response":
> 
> If the "O" bit (high bit) in the OpCode is clear, the PCP server MUST
> process this Option.  If the PCP server does not implement this
> Option, or cannot perform the function indicated by this Option (e.g.,
> due to a parsing error with the option), it MUST generate an error
> response with result code UNSUPP_OPTION or MALFORMED_OPTION (as
> appropriate), and include the UNPROCESSED option in the response
> (Section 6.7.1).
> 
> PREFER_FAILURE and FILTER do not include "purpose", but include
> non-boilerplate "is included in responses".
> 
> Option boilerplate fields should start with a capital letter, e.g.
>          Name: <mnemonic>
>          Number: <value>
>          Purpose: <textual description>
>          Valid for OpCodes: <list of OpCodes>
>          Length: <rules for length>
>          May appear in: <requests/responses/both>
>          Maximum occurrences: <count>
> 
> 5.4.  Result Codes
> 
>    The following result codes may be returned as a result of any OpCode
>    received by the PCP server.  The only success result code is 0,
> other
>    values indicate an error.  If a PCP server has encountered multiple
>    errors during processing of a request, it SHOULD use the most
>    specific error message.
> 
> s/0,/0;/
> s/has encountered/encounters/
> 
>    4  UNSUPP_OPTION, unsupported Option.  This error only occurs if the
>       Option is in the mandatory-to-process range.
> 
> i.e. the "O" bit is clear
> 
> 6.  General PCP Operation
> 
>    The order of operation is that a PCP client generates
>    and sends a request to the PCP server, which processes the request
>    and generates a response back to the PCP client.
> 
> Is it really necessary to say this?  Or is it saying something
> normative, like this is a lock-step protocol with no request batching
> or pipelining?  In any case, it doesn't belong in the same paragraph
> as the discussion of idempotency.
> 
> 6.1.  General PCP Client: Generating a Request
> 
> I'd still like to see this broken out into separate sections on server
> discovery, retransmission timers, and server failover.
> 
>    The PCP client sends a PCP message the first
>    server in its list of PCP servers.
> 
> s/message/request to/
> 
> It should be stated that there should be only one NAT on each
> interface, and the purpose of having multiple server addresses is to
> have multiple ways to contact *one* server.  e.g. A PCP server
> controlling a DS-Lite AFTR may be reachable by one or more IPv6
> addresses, and may also be reachable by IPv4 encapsulated in IPv6.
> 
> 6.2.  General PCP Server: Processing a Request
> 
>    If the source IP address of the received packet does not match the
>    contents of the PCP Client IP Address field, a response is generated
>    with the ADDRESS_MISMATCH result code.  This is done to detect and
>    prevent accidental use of PCP where a non-PCP-aware NAT or NAPT
>    exists between the PCP client and PCP server.
> 
> s/PCP Client IP Address/PCP Client's IP Address/
> 
> !! ADDRESS_MISMATCH is not defined anywhere (should be in 5.4).
> 
>    Error responses have the same packet layout as success responses,
>    with fields from the request copied into the response, and fields
>    assigned by the PCP server are set as indicated in Figure 3
> 
> period.
> 
> 6.3.  General PCP Client: Processing a Response
> 
>    If the result code is SERVER_OVERLOADED, clients SHOULD NOT send
>    *any* further requests to that PCP server for the indicated error
>    lifetime.
> 
> s/clients/the PCP client/
> 
>    For other error result codes, The PCP client SHOULD NOT
>    resend the same request for the indicated error lifetime.
> 
> s/The/the/
> 
>    If a PCP
>    server indicates an error lifetime in excess of 30 minutes, A PCP
>    client MAY choose to set its retry timer to 30 minutes.
> 
> s/A/a/
> 
>    If the PCP client has discovered a new PCP server (e.g., connected
> to
>    a new network), the PCP client MAY immediately begin communicating
>    with this PCP server, without regard to hold times from
> communicating
>    with a previous PCP server.
> 
> (See section 6.1.)
> 
> 6.5.  Epoch
> 
>    When a client notices that the PCP server reduced its Epoch value,
>    the PCP clients will send PCP requests to refresh their mappings.
> 
> This is redundant to the immediately preceding sentence.
> 
> 6.6.  Version Negotiation
> 
>    When sending a response containing the UNSUPP_VERSION result code,
>    the PCP message MUST be 12 octets long.
> 
> i.e. no PCP Client's IP Address or Opcode section - Is this really
> what you meant, or was this not updated when PCP Client's IP Address
> was added?
> 
> I think this restriction is a mistake, and the response should follow
> normal guidelines, e.g. copy the whole request into the response.
> This restriction was added in -08 with no explanation in the change
> log.
> 
> 6.7.1.  UNPROCESSED Option
> 
>    If
>    a certain Option appeared more than once in the PCP request, that
>    Option value can appear once or as many times as it occurred in the
>    request.
> 
> This was also changed in -08, was "If a certain Option appeared more
> than once in the PCP request, that Option value only appears once in
> the option-code fields."  I'm not sure this change really adds
> anything,
> but it seems harmless.
> 
>          may appear in: responses, and only if the result code is non-
>          zero.
> 
> I would only expect to see it on UNSUPP_OPTION or MALFORMED_OPTION.
> 
> 7.  Introduction to MAP and PEER OpCodes
> 
>    There are four uses for the MAP and PEER OpCodes defined in this
>    document: a host operating a server and wanting an incoming
>    connection; a host operating a client and wanting to optimize the
>    application keepalive traffic or restore lost state in its NAT; and
> a
>    host operating a client and server on the same port.
> 
> That's three (missing 7.3 For Restoring Lost Implicit TCP Dynamic
> Mapping State).
> 
> The pseudocode is complicated, and has unnecessary stuff, but I'm not
> going to fight it.
> 
> The pseudocode for PEER (figure 7) doesn't process the PCP response.
> 
> 7.3.  For Restoring Lost Implicit TCP Dynamic Mapping State
> 
> This use case is problematical and contentious, because it requires
> the PCP server to create an implicit dynamic mapping, which may not be
> possible on that NAT (e.g. it may use different port ranges for
> implicit and PCP-controlled mappings).  It's also at odds with the
> description of PEER in 9.1, which still says "The PEER OpCodes provide
> a single function: the ability for the PCP client to query and
> (possibly) extend the lifetime of an existing mapping."  OTOH, it's in
> line with later text in 9.3, but that needs to be tightened up and
> made normative.
> 
> 7.4.  For Operating a Symmetric Client/Server
> 
> Organizationally, I'd like to see this as 7.2 (after Operating a
> Server).  a) It's a closely related topic, and b) this would put the
> two MAP scenarios together.
> 
>       PCP does not attempt to change or
>       dictate how a NAT creates its mappings (endpoint independent
>       mapping, or otherwise)
> 
> s/mappings/implicit mappings/
> After all, explicit dynamic mappings are EIM by definition.
> 
> 8.  MAP OpCodes
> 
>     MAP6=2:   create a mapping between an internal target address and
>               external IPv6 address (e.g., NAT46, or firewall)
> 
> s/target //
> 
>    Note also that all NAT mappings (created by PCP or otherwise) are by
>    necessity bidirectional and symmetrical.  For any packet going in
> one
>    direction (in or out) that is translated by the NAT, a reply going
> in
>    the opposite direction needs to have the corresponding opposite
>    translation done so that the reply arrives at the right endpoint.
>    This means that if a client creates a MAP mapping, and then later
>    sends an outgoing packet using the mapping's internal source port,
>    the NAT should translate that packet's Internal Address and Port to
>    the mapping's External Address and Port, so that replies addressed
> to
>    the External Address and Port are correctly translated to the
>    mapping's Internal Address and Port.
> 
> Interesting but irrelevant.
> 
> 8.1.  OpCode Packet Formats
> 
>    For both of the MAP OpCodes, if the assigned
>    external IP address and assigned external port match the request's
>    Internal IP address and port, the functionality is purely a
> firewall;
>    otherwise it pertains to a network address translator which might
>    also perform firewall-like functions.
> 
> Again, interesting but irrelevant.
> 
> 8.2.  OpCode-Specific Result Codes
> 
> PEER should mention that it uses the same result codes as MAP.
> 
>    In addition to the general PCP result codes (Section 5.4), the
>    following additional result codes may be returned as a result of the
>    four MAP OpCodes received by the PCP server.
> 
> !! s/four/two/
> 
>    26 EXCESSIVE_REMOTE_PEERS, indicates the PCP server was not able to
>       create the filters in this request.  This result code MUST only
> be
>       returned if the MAP request contained the REMOTE_FILTER Option.
>       See Section 10.3 for processing information.  This is a long
>       lifetime error.
> 
> !! s/REMOTE_FILTER/FILTER/
> 
> 8.3.  OpCode-Specific Client: Generating a Request
> 
>    The request MAY contain values in the suggested-external-ip-address
>    and suggested-external-port fields.
> 
> rewrite as
>    The request MAY contain values in the Suggested External Port and
>    Suggested External IP Address fields.
> 
> 8.4.  OpCode-Specific Server: Processing a Request
> 
>    If the server is overloaded by requests (from a particular client or
>    from all clients), it MAY simply discard requests, as the requests
>    will be retried by PCP clients, or MAY generate the
> SERVER_OVERLOADED
>    error response, or both.
> 
> Remove "or both" - "discard" implies "without responding".
> 
> Also, this is generic, and should be moved to section 6.2.
> 
>    If the request contains internal-port=0 and the lifetime is non-
> zero,
>    the server MUST generate a MALFORMED_REQUEST error.
> rewrite as
>    If the lifetime is non-zero, and the request contains internal-
> port=0
>    or protocol=0, the server MUST generate a MALFORMED_REQUEST error.
> 
>    If the requested lifetime is not zero, it indicates a request to
>    create a mapping or extend the lifetime of an existing mapping.
> add
>    If the requested lifetime is zero, it indicates a request to delete
>    an existing mapping or set of mappings.
> 
>    If the PCP server can allocate the suggested external port, and the
>    request did not contain the PREFER_FAILURE Option, it SHOULD do so.
> rewrite as
>    If the PCP server can allocate the suggested external port, it
> SHOULD
>    do so.
> 
> Again, don't pretend a bulleted list is really a long sentence.  And
> even if it's structured as an overly long sentence, the proper
> connector is not ", or;" but "; or"
> 
>    By default, a PCP-controlled device MUST NOT create mappings for a
>    protocol not indicated in the request.  For example, if the request
>    was for a TCP mapping, a UDP mapping MUST NOT be created.
> 
> As opposed to NAT-PMP. Say it.
> 
>    If all of the proceeding operations were successful (did not
> generate
>    an error response), then the requested mappings are created or
>    refreshed
> 
> s/mappings/mapping/
> only one per request
> 
> 8.5.  OpCode-Specific Client: Processing a Response
> 
>    After performing common PCP response processing,
> add "(see section 6.3)"
> 
>     the response is
>    further matched with an outstanding request by comparing the
>    protocol, internal IP address, internal port.
> add "and" before "internal port".
> 
>    On error responses,
> change to "an error response"
> 
>    the assigned external address and assigned external port can also be
>    used to match the responses (which is useful if several requests
> with
>    the PREFER_FAILURE option are outstanding).
> 
> That makes no sense.  On an error response, the "assigned" external
> addr/port are copied from the request.  The client should not have
> multiple requests in flight to map the same internal addr/port to
> different external addr/port.
> 
>    If a successful response,
> 
> This clause no verb.  "On a successful response" or "If the response
> indicates success".
> 
>    the PCP client can use the external IP
>    address and port(s) as desired.  Typically the PCP client will
>    communicate the external IP address and port(s) to another host on
>    the Internet using an application-specific rendezvous mechanism such
>    as DNS SRV records.
> 
> It's only "port(s)" if there are multiple requests.  Each request
> creates a mapping for exactly one port - unless the NAT happens to be
> NAT-only, and the request is just to learn to external addr.  Still, I
> think it would be clearer to treat each request as creating a single
> addr/port mapping.
> 
>    On an error response, clients SHOULD NOT repeat the same request to
>    the same PCP server within the lifetime returned in the response.
> 
> Except the client is supposed to initiate renewal at 1/2 the lifetime.
> 
> 8.6.  Mapping Lifetime and Deletion
> 
>    It is RECOMMENDED that the
>    server be configured to restrict lifetimes to less than 24 hours,
>    because they will consume ports
> 
> s/configured/configurable/
> s/they/mappings/
> 
>    Mappings created by PCP MAP requests are not
>    special or different to mappings created other ways.
> 
> This is British usage.  Since the spelling and grammar are mostly
> American in this document, you want to say "different from".
> 
>    it is implementation-dependent if outgoing traffic extends the
>    lifetime of such mappings
> 
> add "beyond the PCP-assigned lifetime"
> 
>    o  If the internal port is non-zero (port!=0) and protocol is non-
>       zero (protocol!=0), it indicates a request to delete the
> indicated
>       mapping immediately.
> 
> maybe "the single indicated mapping"
> 
>    o  If the internal port is zero (port==0) and the protocol is non-
>       zero (protocol!=0), it indicates a request to delete all mappings
>       for this Internal Address for the given internal port for all
>       transport protocols.
> 
> No, that's backwards.  It indicates a request to delete all mappings
> for this Internal Address on this protocol.
> 
> The effect you describe would be for port!=0 && protocol==0, which is
> not specified in this document, because it's not actually useful.
> 
>    The suggested external address and port fields are ignored in
>    requests where the requested lifetime is 0.
> 
> The Suggested External Port and Suggested External IP Address fields...
> 
>    If the PCP client attempts to delete an implicit dynamic
>    mapping (e.g., created by a TCP SYN), the PCP server deletes the
>    mapping and responds with the SUCCESS result code.
> 
> Not all NATs may support this.  And shouldn't it be a PEER opcode with
> lifetime==0?
> 
>    If
>    the PCP MAP request was for port=0 (indicating 'all ports'), the PCP
>    server deletes all of the explicit dynamic mappings it can (but not
>    any implicit mappings)
> 
> Compare and contrast with deleting a single implicit mapping.
> 
>    An explicit dynamic mapping MUST NOT have its lifetime
>    reduced by transport protocol messages (e.g., TCP RST, TCP FIN).
> 
> This should be a separate paragraph, perhaps with some justification.
> 
>    To reduce unwanted traffic and data corruption, UDP and TCP ports
>    should not be immediately re-used for an interval (TIME_WAIT
> interval
>    as discussed in [RFC0793]).  However, the PCP server MUST allow the
>    same subscriber and same internal address to re-acquire the same
> port
>    during that interval.
> 
> As I mentioned in a previous review, these two requirements are
> incompatible.  The MUST in the second sentence changes the "should not"
> in the first sentence to a MUST NOT; or the "should not" in the first
> sentence changes the MUST in the second sentence to a SHOULD.  It's
> more
> realistic in the SHOULD form, so I'd go with that.
> 
> Also remove the word "immediately".  Possibly refer to this explicitly
> as a hold-down timer.
> 
>    *  all ports, all protocols, all Internal Addresses for which the
>    client is authorized:  internal address=0, via the THIRD_PARTY
> option
> 
> I would still recommend setting protocol==0, port==0.
> 
> 8.7.  Subscriber Renumbering and Address Change Events
> 
>    PCP defined in this document
>    does not provide machinery to reduce the subscriber renumbering
>    problem.
> 
> Well, duh.
> 
> 9.1.  OpCode Packet Formats
> 
>    The PEER OpCodes provide a single function: the ability for the PCP
>    client to query and (possibly) extend the lifetime of an existing
>    mapping.
> 
> No longer true.
> 
>    Requested Lifetime (in common header):  Requested lifetime of this
>       mapping, in seconds.  Note that, depending on the implementation
>       of the PCP-controlled device, it may not be possible to reduce
> the
>       lifetime of a mapping (or delete it, with requested lifetime=0)
>       using PEER.
> 
> There is no discussion about deleting mappings with PEER.
> 
> External_AF is the only packet field with an underscore.  I would
> expect it to be "External AF".
> 
>    External_AF:  For success responses, this contains the address
> family
>       of the external IP address associated with this peer connection,
>       to properly decode the External IP Address.  This field is
>       necessary because the Remote Peer's IP Address is from the PCP
>       client's perspective, whereas the External_AF and External IP
>       Address are from the PCP-controlled device's perspective.  As an
>       example, if the PCP-controlled device is a NAT64, the PCP client
>       only knows the remote peer's IPv6 address, whereas the NAT64
> knows
>       the remote peer's IPv4 address.  Values are from IANA's address
>       family numbers (IPv4 is 1, IPv6 is 2).  For error responses, the
>       value is copied from the request.
> 
> So this actually creates a difference between MAP and PEER in NAT64:
> with MAP, the client would have to know that it's behind a NAT64, and
> send a MAP4 request over v6.  Or possibly it would send MAP6, and the
> NAT64 would return a v4-mapped v6 address, which the client would then
> figure out is mapped, and advertise the corresponding v4 address to
> its peers?  Whereas on a PEER6, it has this peer address that's really
> v4-mapped v6, and the NAT64 returns an external addr that's explicitly
> v4.  It all sounds complicated, and I'd like Simon to explain just how
> he sees this all working in NAT64.
> 
>    External Port:  For success responses, this is the external port
>       number, assigned by the NAT (or firewall) to this mapping.  If
>       firewall or 1:1 NAT, this will match the internal port.  For
> error
>       responses, this MUST be 0.
> 
> The last restriction doesn't make sense, and doesn't match the usage
> in the MAP opcodes ("On error responses, the value from Suggested
> External Port is used.").  On an error response, the external addr and
> port should be ignored - they could be copied from the request, they
> could be zeroed, but they don't contain any meaningful data in either
> case.
> 
>    External IP Address:  For success responses, this contains the
>       external IP address, assigned by the NAT (or firewall) to this
>       mapping.  This field allows the PCP client and its remote peer to
>       determine if there is another NAT between the PCP-controlled NAT
>       and remote peer.
> 
> And what will they do with that information?
> 
> 9.2.  OpCode-Specific Client: Generating a Request
> 
>    The PEER4 or PEER6 OpCodes MAY be sent before or after establishing
>    bi-directional communication with the remote peer.  If sent before,
>    PEER4 or PEER6 OpCodes will create a mapping in the PCP-controlled
>    device (for the purpose described in Section 7.3), and the client
>    SHOULD set the External_AF and Suggested External IP Address to the
>    values of the previous mapping.
> 
> and Suggested External Port
> 
>    If sent after, the PEER4 or PEER6
>    OpCodes query (and control) the implicit dynamic mapping (for the
>    purpose described in Section 7.2).
> 
> and the Suggested External Port and Suggested External IP Address
> fields SHOULD be set to zero.
> 
>    If the described mapping does not yet exist yet, it
>    is created, honoring and the Suggested External Port and Suggested
>    External IP Address are honored
> 
> s/and //
> s/are honored //
> 
>    (if possible; if not possible, a
>    mapping on a different IP address or different port is created).
> 
> This implies that the PREFER_FAILURE Option is also valid for PEER
> opcodes.
> 
>    By
>    having PEER create such a mapping, we avoid a race condition between
>    the PEER request or the initial outgoing packet arriving at the NAT
>    gateway first
> 
> If the race condition is really a concern, we just mandate a delay
> before sending the PEER request.  The PEER response would also tell
> whether the implicit mapping had been successfully created.
> 
> It should be possible to delete mappings with PEER, just as for MAP.
> 
> Mention THIRD_PARTY with PEER.  Also USER_EX_QUOTA.
> 
> MAP and PEER processing should follow pretty much the same path on the
> server; the spec should explicitly say what's the same and what's
> different.
> 
> 9.4.  OpCode-Specific Client: Processing a Response
> 
>    If a successful response, the application can use the assigned
> 
> This clause no verb.  "On a successful response" or "If the response
> indicates success".
> 
>    If the PCP client wishes to keep this mapping alive beyond the
>    indicated lifetime, it SHOULD issue a new PCP request prior to the
>    expiration.  That is, inside->outside traffic is not sufficient to
>    ensure the mapping will continue to exist.  It is RECOMMENDED to
> send
>    a single renewal request packet when a mapping is halfway to
>    expiration time, then, if no SUCCESS response is received, another
>    single renewal request 3/4 of the way to expiration time, and then
>    another at 7/8 of the way to expiration time, and so on, subject to
>    the constraint that renewal requests MUST NOT be sent less than four
>    seconds apart (a PCP client MUST NOT ever-closer-together requests
> in
>    the last few seconds before a mapping expires).
> 
> For MAP, the language about renewing mappings is in the Generating a
> Request section, not the Processing a Response section.
> 
> 10.1.  THIRD_PARTY Option for MAP and PEER OpCodes
> 
>    A THIRD_PARTY Option MUST NOT contain the same address as the source
>    address of the packet.  A PCP server receiving a THIRD_PARTY Option
>    specifying the same address as the source address of the packet MUST
>    return a MALFORMED_REQUEST result code.  This is because many PCP
>    servers may not implement the THIRD_PARTY Option at all, and a
> client
>    using the THIRD_PARTY Option to specify the same address as the
>    source address of the packet will cause mapping requests to fail
>    where they would otherwise have succeeded.
> 
> I would expect the operation to succeed if the server supports
> THIRD_PARTY, because the client is making a valid request, albeit a
> bit oddly.  And if the server doesn't support THIRD_PARTY, it will
> return UNSUPP_OPTION, regardless of the contents of the option.
> 
> I would prefer to have to option header appear in all option layout
> diagrams, for clarity, e.g.
> 
>     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
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    | Option Code=4 |  Reserved     |   Option-Length=4 or 16       |
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>    :                                                               :
>    :    Internal IP Address (32 bits of 128 bits, depending        :
>    :                                       on Option length)       :
>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>    The following additional result codes may be returned as a result of
>    using this Option.
> 
> s/codes/code/
> 
>    51 UNAUTH_THIRD_PARTY_INTERNAL_ADDRESS
> 
> How about using the existing NOT_AUTHORIZED code?  In fact, 8.4 says
> the server MUST return NOT_AUTHORIZED in this specific case, and 9.3
> doesn't mention THIRD_PARTY, so UNAUTH_THIRD_PARTY_INTERNAL_ADDRESS is
> only mentioned in this section.  Seriously, this scenario would seem
> to be covered under the following clause from NOT_AUTHORIZED: "the PCP
> client requested a mapping that cannot be fulfilled by the PCP
> server's security policy."
> 
> Paragraph break before "Determining which PCP clients are authorized
> to use the THIRD_PARTY option depends on the deployment scenario."
> 
>    For home deployments (where the PCP server is
>    embedded in the NAT device), this option MUST NOT be processed.
> 
> I would say "where the PCP server and the NAT are embedded in a home
> gateway device".  Also, THIRD_PARTY would typically not be implemented
> on a home gateway, so would generate UNSUPP_OPTION.
> 
>    It is RECOMMENDED that PCP servers embedded into customer premise
>    equipment be configured to refuse third party mappings by default.
>    With this default, if a user wants to create a third party mapping,
>    the user needs to interact out-of-band with their customer premise
>    router (e.g., using its HTTP administrative interface).
> 
> This is largely redundant to the previous paragraph.
> 
>    It is RECOMMENDED that PCP servers embedded into service provider
> NAT
>    and firewall devices be configured to permit the THIRD_PARTY
> option,...
> 
> This whole paragraph is confusing, and it's hard to tell what's
> actually being recommended or dis-recommended here.  The simple fact
> of the matter is that THIRD_PARTY is most useful in DS-Lite, where the
> NAT can send packets "directly" to internal hosts that don't embed a
> PCP client; i.e. the B4 embeds a UPnP IWF and/or a NAT-PMP IWF and/or
> a PCP-PCP proxy.
> 
> In a NAT444 scenario, the CGN can't "see" the internal hosts, so
> THIRD_PARTY is pointless.  In NAT64 and NAT46, there isn't an obvious
> place for an IWF or proxy, so it probably isn't useful there, although
> I haven't spent a lot of time thinking about it.
> 
>    If the packet arrived from a
>    different address, the PCP server MUST generate an
>    UNAUTH_THIRD_PARTY_INTERNAL_ADDRESS error.
> 
> s/arrived/arrives/
> 
> 10.2.  PREFER_FAILURE Option for MAP OpCodes
> 
>    This option is only used with the MAP4 and MAP6 OpCodes.
> 
> Since we redefined PEER to create or re-create mappings, we should
> also support PREFER_FAILURE, and should mention it here and in 11.3.1.
> 
> If UNAUTH_THIRD_PARTY_INTERNAL_ADDRESS is defined under the
> THIRD_PARTY Option (and I've argued that it shouldn't be defined at
> all), then CANNOT_PROVIDE_EXTERNAL_PORT should be defined under the
> PREFER_FAILURE Option.  Likewise EXCESSIVE_REMOTE_PEERS should be
> defined under the FILTER Option.  But really, I'd prefer to keep all
> the result codes in 8.2.
> 
> 10.3.  FILTER Option for MAP OpCodes
> 
>    This Option indicates filtering incoming packets is desired.  The
>    remote peer port and remote peer IP Address indicate the permitted
>    remote peer's source IP address and port for packets from the
>    Internet.
> 
> s/remote peer port/Remote Peer Port/
> s/remote peer IP Address/Remote Peer IP Address/
> 
> Change the "prefix-length" field name to "Prefix Length".
> 
> I've never been able to figure out why "Reserved" is the first field,
> not the second.
> 
> Note that FILTER is unidirectional - it doen't prevent an internal
> host from sending packets to any remote host, it just prevents remote
> hosts from replying.  I'm not saying this needs to be changed.
> 
>       is included in responses: MUST, if it appeared in the request
> 
> As previously noted, this field is not in the 5.3 boilerplate.
> In 5.3, we should say that, if a request includes an Option that is
> marked mandatory-to-process, the response (whether success or error)
> MUST include that Option.
> 
>       length: 2 if used with MAP4, 5 if used with MAP6
> 
> !! Wrong; this is the old length (measured in 4-byte words).
> Should be "8 if used with MAP4, 20 if used with MAP6".
> 
>    If a MAP request has a lifetime of 0 and contains the
>    FILTER option, the error MALFORMED_OPTION is returned.
> 
> This is an example of the nonsensical-option clause of 8.4.
> 
> 11.1.  Implementing MAP with non-EIM port-mapping NAPT
> 
>    For implicit dynamic mappings, some existing NAT devices have
>    endpoint-independent mapping (EIM) behavior while other NAPT devices
>    have non-endpoint-independent mapping (non-EIM) behavior.
> 
> Here you use NAT and NAPT in the same sentence.  As I said before, I
> would refer to all such devices by the vernacular "NAT", unless
> there's some clear distinction to be made by referring to NAPT.
> 
>    1.  have implicit dynamic mappings (e.g., TCP SYN) use a different
>    2.  on arrival of a packet (from the Internet or from an internal
> 
> Capitalize
> 
> BTW, I prefer the second approach, which uses the most specific match
> (somewhat analogously to kernel packet routing).  OTOH, I know Francis
> prefers the first approach.
> 
> As I see it, the conflict would only arise if there was an existing
> implicit mapping, and then an explicit mapping was created.  If the
> explicit mapping exists, the NAT will use it for all new connections
> from that internal addr/port/proto, rather than creating a new
> implicit mapping.
> 
> 11.2.  Interaction of MAP with Implicit Dynamic Mappings
> 
> I'm a bit confused by this one.  If the NAT creates an implicit
> mapping that conflicts with an explicit mapping, it's a different
> (more specific) mapping in the NAT's mapping table.  Even if the NAT
> deletes that mapping on termination of the connection, that won't
> affect the (more general) explicit mapping - it would still allow
> incoming connections.
> 
> 13.  Security Considerations
> 
>    This document defines Port Control Protocol and two types of
> OpCodes,
>    PEER and MAP.  The PEER OpCode allows querying and extending (if
>    permitted) the lifetime of an existing implicit dynamic mapping, so
> a
>    host can reduce its keepalive messages.  The MAP OpCode allows
>    creating a mapping so a host can receive incoming unsolicited
>    connections from the Internet in order to run a server.
> 
> This whole paragraph can be deleted, because it doesn't say anything
> new, and it doesn't say anything about security.
> 
> 14.1.  Port Number
> 
>    PCP will use port 5351 (currently assigned by IANA to NAT-PMP).  We
>    request that IANA re-assign that same port number to PCP, and
>    relinquish UDP port 44323.
> 
> The bit about relinquishing port 44323 should be marked for removal by
> the RFC Editor.
> 
> 14.2.  OpCodes
> 
>    Additional OpCodes in the range 4-95 can be created via Standards
>    Action [RFC5226], and the range 96-126 is for Private Use [RFC5226].
> 
> !! s/4/5/ (4 is PEER6)
> How/why was the number 95 chosen?
> 
> 14.3.  Result Codes
> 
>    IANA shall create a new registry for PCP result codes, numbered
>    0-255, initially populated with the result codes from Section 5.4,
>    Section 8.2, Section 10.3, and Section 10.1.  The values 0 and 255
>    are reserved.
> 
> 10.3 doesn't define a result code.
> 
>    Additional Result Codes can be defined via Specification Required
>    [RFC5226].
> 
> There needs to be a Private range for result codes, to support Private
> OpCodes and Options.
> 
> 14.4.  Options
> 
>    IANA shall create a new registry for PCP Options, numbered 0-255
> with
>    an associated mnemonic.  The values 0-127 are mandatory-to-process,
>    and 128-255 are optional-to-process.  The initial registry contains
>    the options described in Section 10 and Section 10.1.  The option
>    values 127 and 255 are reserved.
> 
> and sections 10.2 and 10.3 - why don't we just say section 10 (as a
> whole), and drop the reference to 10.1?
> _______________________________________________
> pcp mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pcp