wip: add a new trait to notify application of some events. For now only support MTU updates.

[stormshield-damiend]

This is a draft PR to discuss about a possible new API to allow the application that uses quinn or quinn-proto to do some actions in reaction to some events happening on endpoints or connections.

This demonstrates (using the shortest possible code path) how it could be done to notify about MTU updates.

A possible use case for this would be for the application to lower the size of transported data. For example a VPN like client using quinn could use this to lower the MTU of the physical interfaces used to capture the packets to tunnel.

[Ralith]

Thanks for working on this!

I've hesitated to pursue this feature in the past because every application I can imagine for it is better served by polling the max datagram size immediately before preparing a datagram, rather getting notified of changes by a side channel. Do you have a concrete case in mind for which an event is better?

That aside, I'd prefer to avoid callbacks in the API as much as possible. At the quinn-proto layer, we handle events in general by polling any time they might have occurred, and the existing API is already sufficient for that. At the quinn layer, we can poll in the connection driver after e.g. processing a UDP datagram or a timeout and e.g. trigger a Notify future to broadcast the event to async consumers.

[stormshield-damiend]

Thanks for working on this!

Thanks for your feedback

I've hesitated to pursue this feature in the past because every application I can imagine for it is better served by polling the max datagram size immediately before preparing a datagram, rather getting notified of changes by a side channel. Do you have a concrete case in mind for which an event is better?

At first I have implemented it like that, I have sampled the max_datagram_size every X packet in the future(s) that sends them. Problem is how to find a proper sampling value ? Another point is that sampling the max_datagram takes a lock, this means taking one more connection lock per sends during sampling. I did not measure the impact in performance but I can do it if there is some interest.

As we are using Path MTU discovery, there is already an event based algorithm that detects the MTU of the path for a connection. We find it smarter to use this to be notified as early as possible to prevent the need to drop some packets due to a MTU lowering on one connection. We also think we will need to plug our code to react on black hole detection to prevent using a too much high MTU.

That aside, I'd prefer to avoid callbacks in the API as much as possible. At the quinn-proto layer, we handle events in general by polling any time they might have occurred, and the existing API is already sufficient for that. At the quinn layer, we can poll in the connection driver after e.g. processing a UDP datagram or a timeout and e.g. trigger a Notify future to broadcast the event to async consumers.

Fair enough, as stated this is a poc and I have used the most simple code path available, i can look at Notify or mpsc approach once we are settled for the previously discuss point.

[Ralith]

Problem is how to find a proper sampling value

My recommendation is to sample it every single time you want to send a datagram, and use that information to construct a datagram of a suitable size.

Another point is that sampling the max_datagram takes a lock, this means taking one more connection lock per sends during sampling. I did not measure the impact in performance but I can do it if there is some interest.

We could introduce an atomic path in quinn for this information to avoid the lock if it presents a problem in practice. Long-term I want to move most synchronization into the quinn-proto layer, which will make that even more natural.

We find it smarter to use this to be notified as early as possible to prevent the need to drop some packets due to a MTU lowering on one connection.

If you query the max datagram size before creating each datagram, and only create datagrams immediately before sending them, you should almost never find yourself with a too-large datagram that must be dropped/rebuilt. This is the same behavior you'd get with an event mechanism.

We also think we will need to plug our code to react on black hole detection to prevent using a too much high MTU.

Application code shouldn't need to be aware of this. Quinn will never adopt an MTU that hasn't been successfully probed.

[stormshield-damiend]

Feel free to close this draft as we stopped working on this on our side as the problem is far more complex than this API when you have multiple interfaces. We may propose a new approach when we restart working on this.