[pcp] Comments on draft-ietf-pcp-port-set-04.txt

[Dave Thaler <[e-mail]>]

Reviewing -04 and comparing it with the thread below... see inline.

Simon Perreault wrote:
> Thanks a lot for the review! I'll be sure to incorporate your comments into a
> new revision shortly after the gates reopen.
> 
> Le 2013-10-30 18:33, Dave Thaler a écrit :
> > 1) The claim that a port set makes code simpler needs an explanation to
> justify it.
> > E.g., if an app really needs 16 ports, and it fails to get 16 back in
> > its first request, it still has to handle that and say make multiple separate
> requests.
> 
> Two observations:
> 
> - Wouldn't that behaviour be a tiny bit evil? It reminds me of applications that
> create multiple flows in an attempt to trick an SFQ-based QoS into allocating
> them more bandwidth.
> 
> - Often, apps that ask for N ports don't really need N ports. Ideally they
> would like to get N ports, but they can live with <N ports. No need for single-
> port fallback in that case.
> 
> > So if it already has to have
> > code to deal with multiple separate requests, why is it simpler to add
> > port set functionality to and implement both?  This seems counter-intuitive
> to a reader.
> 
> That would only apply to apps that absolutely need N ports to function, and
> don't need them to be contiguous. Those apps need to fall back on single-
> port requests as you describe. But I would argue that these apps are really
> quite rare.
> 
> We can add text describing these concerns in the next revision.

Sure, but the claim in section 2 is that the client side logic *is* simpler.
Not "often" simpler, or "may be simpler".   You can’t assert this when 
sections 6.4 and 6.3 both leave some major questions up to the 
implementation.  So it might be more complicated not simpler.  Suggest 
just deleting the "Client-side simplicity" bullet.

The text added in section 6.4 looks fine to me, except for one editorial
suggestion on this text:
> Suppose a PCP client asks for 16 ports and receives 8.  What should
>   it do?  Should it consider this a final answer?  Should it try a
>   second request, asking for 8 more ports?  Should it fall back to 8
>   individual MAP requests?  This document does not try to answer those
>   questions , but describes issues to be considered when doing so.

Suggest changing last sentence to
"This document leaves the answers to be implementation-specific,
but describes issues to be considered when answering them."

That is, it answers them by saying it's implementation-specific.


> > 2) Text on overlapping ranges needs more detail.  Is it legal for a
> > client to send requests for overlapping ranges in parallel or not?  I
> > ask because this was an attack vector for IP reassembly so can imagine
> > we need the precision here to avoid similar attacks.  This might have an
> impact on the Security Considerations section.
> 
> Hmmm... interesting. I see now that those requests would not be
> idempotent, contrary to single-port MAP requests. The first one to be
> processed by the server would "win". Responses to each request would
> depend on which one has won.
> 
> I don't see an attack here, but the lack of idempotence bugs me. I don't quite
> know what action to take here except describe the situation in the next
> revision.

Yes, Section 6.3 now looks fine to me, except for a typo:
> which port set mapping requests would be arbitrated,  Alternatively,

Change first comma to period.

> > 3) The unstated assumption in section 5.3 seems to be that the client
> > can reuse the same mapping nonce in all independent requests that it
> wants to refresh as a block, is that right?
> 
> Right.
> 
> > Let's say internal port 100 and 110 both have the same mapping nonce,
> > and internal port 105 has a different mapping nonce (e.g., from a different
> PCP client on the same host).
> > If the first PCP client sends a refresh for port 100, Port Set Size =
> > 11, what would the effect be at the PCP server?
> 
> Assuming the request nonce matches the nonce of the mapping at port 100,
> then:
> 
> - The mapping at port 100 would be refreshed.
> 
> - The mapping at port 110 would not be refreshed, but its remaining lifetime
> would be returned by the server, as described in draft-cheshire-pcp-unsupp-
> family. The response's nonce would be copied from the request's nonce.
> 
> If you agree that this is how it should be, I'll add this to the example section.
> However, that would depend on keeping the normative reference to
> unsupp-family (see point 5 below).

We agreed to not have a normative reference to unsupp-family, but I don't
want this to get lost.  Was any change to the port-set document made?
Should there be an example in the unsupp-family draft, or what?


> > 4) Section 4.1 says if the response does not include a PORT_SET, in a
> > response it assumes the server doesn't support it, and sends individual
> MAP requests for the rest of the ports it needs.
> > But section 4.2 says if the server does support PORT_SET but can only
> > return 1 in its first response, It must omit the PORT_SET option.  I think
> that's wrong since it will confuse the client as noted above.
> > If it's included with a Port Set Size of 1, the client knows it can
> > keep trying using PORT_SET for subsequent blocks, which might get more
> than 1 port in each response.
> 
> Agreed. I'll modify the draft accordingly unless others disagree.

I expected the draft to be modified accordingly as I didn't see any disagreement
on the list or in the meeting notes (let me know if I missed something).
However a different change was made to the draft which I think isn't as good.

Specifically, per the "Agreed" above, I expected it to say the server will
return a port set option with size of 1, and the client then knows the server
supports port-sets and can choose to ask for another port-set if it wants.
And if the response has no port-set option, then it knows the server doesn't
support it and can send other requests accordingly.

Instead, -04 changed to keep the statement:
>   If the PCP server ends up mapping only a single port, for any reason,
>   the PORT_SET option MUST NOT be present in the response.

And instead added this text:
> If no PORT_SET option is present in the response, the PCP client
>   cannot conclude that the PCP server does not support the PORT_SET
>   option.  It may just be that the PCP server does support PORT_SET but
>   decided to allocate only a single port, for reasons that are its own.
>   If the client wishes to obtain more ports, it MAY send additional MAP
>   requests (see Section 6.4),

I didn't see it explained why this is better than what I thought we agreed
to on the list.

 
> > 5) If we want this to go to RFC soon, we need to remove normative
> > references to non-RFCs (and this is easy, I suggested text), but I
> > think we should also remove informative references to individual
> > drafts (i.e., drafts that aren't currently WG drafts in any WG), and instead
> only reference in the reverse direction; those other drafts should reference
> this one instead of the other way around.
> 
> AFAIK, informative references don't prevent publishing as an RFC. So I'll
> focus on normative references.
> 
> I'll remove the reference to draft-boucadair-pcp-failure unless others
> disagree.
> 
> About draft-cheshire-pcp-unsupp-family: we could remove the reference
> and let the document stand on its own. The new example showing the effect
> of nonce checking multiple mappings with differing nonces would be added
> to draft-cheshire.
> 
> As an aside: the example in section "5.2 Stateless Mapping Discovery" is not
> specific to draft-tsou-stateless-nat44. It is intended to illustrate how
> discovery is done generically without the client knowing the kind of stateless
> NAT it is speaking to. In fact, if the NAT in that example had been one
> implementing draft-tsou, the numbers would have been different.
> Therefore I suggest keeping the example.

I'm fine with that part, thanks.

-Dave