quicwg / multipath

In-progress version of draft-ietf-quic-multipath
Other
51 stars 18 forks source link

Split path identifiers as odd-even for client and server initiated #329

Closed huitema closed 4 months ago

huitema commented 6 months ago

During the IETF 119 meeting, we decided to split the path identifier space into even numbers for client initiated paths and odd numbers for server initiated paths. This PR implement the requested changes.

Yanmei-Liu commented 5 months ago

Thanks a lot for the PR! I have a suggestion on simplify the transport parameters: We could define the the new transport parameter "initial_max_path_ids" as "the max initial Path IDs each endpoint is willing to accept". Then we only need one new transport parameter(0x0f739bbc1b666d08):

  1. client send the "initial_max_path_ids":"X" to inform the server that it would only accept server-initialized paths with Path IDs between [0, X]; (even)
  2. server send the "initial_max_path_ids":"Y" to inform the client that it would only accept client-initialized paths with Path IDs between [1, Y]; (odd)
  3. After handshake is finished, client and server could decide by their own to create how many paths to use. Note that endpoints could always choose the Minimum value between the local strict concurrent paths and the remote strict number.
  4. Endpoints SHOULD issue at least one unused Path ID inside the available Path ID range, but endpoints could always choose to limit the number of Path IDs which is indeed issued inside the connection, as part of the strategy to limit the concurrent paths created by the remote.
huitema commented 5 months ago

@Yanmei-Liu sorry, but having each side just announcing a number of path that it is willing to accept does not work.

This multipath proposal requires nodes to maintain three resources:

Take the example of the client originated path number 2. It can only be created if the client has sent MP_NEW_CONNECTION_ID with Path-ID=2 to the peer, and has also received MP_NEW_CONNECTION_ID with Path-ID=2 from the peer. Each peer will publish both even and odds CID. Each peer will need to accept number-of-path*number-of-CID-per-path CID, and will need to store them in memory. To avoid risks of DOS attacks, we need to control the max number of path in each direction, and that requires two variables: the max number of even paths and the max number of odd paths.

huitema commented 5 months ago

I did the merge with draft 7. I think this PR is now ready, modulo one big question asked by Quentin in his review: "I don't know if we already need to define the server-initiated messages now, or if we only stick to the client-initiated ones for now (base draft), and let people define them in a future extension to multipath."

We could absolutely simplify the PR by not defining the server initiated messages. In practice, the only change from draft 07 would be, "the Path IDs are even numbers between 0 and 2^32 - 2." We would not need the second transport parameter, or the server variant of MAX PATH. The impact on implementations would be much smaller: multiply path-id by 2, change the name and ID of the "enable multipath" TP, change the name and ID of the "max paths" frame. This is very tempting, but I will not do that unless I get further feedback.

Yanmei-Liu commented 5 months ago

I did the merge with draft 7. I think this PR is now ready, modulo one big question asked by Quentin in his review: "I don't know if we already need to define the server-initiated messages now, or if we only stick to the client-initiated ones for now (base draft), and let people define them in a future extension to multipath."

We could absolutely simplify the PR by not defining the server initiated messages. In practice, the only change from draft 07 would be, "the Path IDs are even numbers between 0 and 2^32 - 2." We would not need the second transport parameter, or the server variant of MAX PATH. The impact on implementations would be much smaller: multiply path-id by 2, change the name and ID of the "enable multipath" TP, change the name and ID of the "max paths" frame. This is very tempting, but I will not do that unless I get further feedback.

After these days discussion, I'd support that we only add "the Path IDs initialized by client are even numbers between 0 and 2^32 - 2" at this time, keep the possibility that we can always add the mechanism for server-initialize paths when real scenarios appear at any time. We can always do that whenever people need, and even in a separate extension would work well with the original draft.

huitema commented 5 months ago

@Yanmei-Liu OK. I will probably do a separate PR for the "trimmed" version, that will be cleaner than writing it on top of this one.

huitema commented 5 months ago

DO NOT MERGE THIS PLEASE!

mirjak commented 5 months ago

If we anyway need a second transport parameter to realise the server initiation, there is actually no difference if we do it now or later. So I guess we could keep it simple for now.

huitema commented 5 months ago

Check PR #331 as a much simpler way to reserve even path identifiers for future extension.

mirjak commented 5 months ago

Just double-checking: this PR proposed two transport parameters. If that is needed it doesn't make a technical difference if they are defined in the same spec or as an extension later.

However, it is not fully clear to me why two parameters are needed. Each endpoint needs to announce the max-path value (for one half of the path id space) but they could use the same transport parameter in each direction. Or why not?

mirjak commented 5 months ago

Also, maybe I now manager to fully confuse myself, but why do we need the max path limit at all. I thought everything endpoint can implicitly control the max number of paths but not providing CID for more paths...?

mirjak commented 5 months ago

one more addition: My comments assume that the same path ID is used in both direction and always opens a bidirectional path and therefore both endpoint need to provide the respective CIDs first before you can open the paths. Note that this is connected to PR #315 and issue #294.

huitema commented 5 months ago

@mirjak I agree with the general comment that "if we need two parameters, we should just define one now, and leave the other for an extension. " That why I wrote a simpler proposal in PR #331. Can you review that too?

See previous discussion for why we need two MAX PATHS.

mirjak commented 5 months ago

@huitema I read the previous discussion, however, it's still not clear to me. If the same path ID has to be used for a path in both directions, that means for me that you need to get CIDs for a path ID from both endpoints before you can start using that path ID. That means both sides have a way to control the maximum number of paths by only providing a certain number of path IDs even without the MAX_PATHS limit. Or what do I miss?

huitema commented 5 months ago

@mirjak Suppose the client publishes "max server initiated paths = 1,000,000,000", and proceeds to send MP_NEW_CONNECTION_IDs for all these paths. How is the server able to limit the number of CIDs that it will accept?

mirjak commented 4 months ago

Ah I didn't realize that we changed the active_connection_id_limit to be now per path. Is that really necessary or the correct thing to do?

If we can keep active_connection_id_limit to limit the total numbers of CID an endpoint is willing to maintain (because maintaining with or without a path ID associated to the CID is not really a difference), each endpoint can still control the maximum number of paths but announcing CIDs respectively.

huitema commented 4 months ago

@mirjak Why don't you open an issue about reversing the decision to make max-CID a path property, versus a global property?

In the short term, given the uncertainties, my proposal would be to close this PR, and focus on PR #331 instead?

mirjak commented 4 months ago

I created issue #332.

My plan was merge #331 but still keep this PR open, so we can continue the discussion.

For me, if separate transport parameter are needed, it doesn't make a difference if this is one extension or two. However, if we don't need a separate transport parameter maybe it is easier to define the server-initiated paths right away.

huitema commented 4 months ago

Closing this PR after the 6/3/2024 interim.