Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: [QUIC] QuicListener #67560

Closed
ManickaP opened this issue Apr 4, 2022 · 11 comments · Fixed by #71579
Closed

[API Proposal]: [QUIC] QuicListener #67560

ManickaP opened this issue Apr 4, 2022 · 11 comments · Fixed by #71579
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Quic blocking Marks issues that we want to fast track in order to unblock other important work
Milestone

Comments

@ManickaP
Copy link
Member

ManickaP commented Apr 4, 2022

Background and motivation

API design for exposing QuicListener and related classes to the public.

There are 2 major design approaches. The first one requires options for incoming connection (to finish the handshake) during listener creation. The alternative doesn't expect these options until Accept is called. Both solutions take into account #49587.

Options Upfront

This design was discussed in our team and is based on what we have now. Thus it is safer since we have at least some experience with this shape.

  • It does not allow for meaningful state object for ServerOptionsSelectionCallback since it'd need to be provided upfront (one object per listener).
  • It'll finish handshake for all incoming connections regardless whether there's anyone ready to accept them.

Related issues:

API Proposal

// Change from the current class is removal of all the constructors which are replaced with static Create method.
// All the Quic classes will have static Create so that they can became abstract in the future.
// Also all the overloads accepting implementation provider (temporary) are removed.
public class QuicListener : IAsyncDisposable
{
    public IPEndPoint ListenEndPoint { get; }

    public async ValueTask<QuicConnection> AcceptConnectionAsync(CancellationToken cancellationToken = default);

    public void DisposeAsync();
}

// QuicServerConnectionOptions in https://github.com/dotnet/runtime/issues/68902
public delegate ValueTask<QuicServerConnectionOptions> ServerOptionsSelectionCallback(QuicConnection connection, SslClientHelloInfo clientHelloInfo, CancellationToken cancellationToken);

// Listener options necessary for starting a listener.
public class QuicListenerOptions
{
    public IPEndPoint ListenEndPoint { get; set; }

    public List<SslApplicationProtocol> ApplicationProtocols { get; set; }

    public int ListenBacklog { get; set; } = 512;

    public ServerOptionsSelectionCallback ConnectionOptionsCallback { get; set; }
}

API Usage

var connectionOptions = new QuicServerConnectionOptions() 
{
    IdleTimeout = TimeSpan.FromMinutes(5),
    MaxBidirectionalStreams = 1000,
    MaxUnidirectionalStreams = 10,
    ServerSslOptions = new SslServerAuthenticationOptions()
    {
        ApplicationProtocols = new List<SslApplicationProtocol>(){ SslApplicationProtocol.Http3 },
        ServerCertificate = TestCertificateExtensions.ServerCertificate,
    }
};

await using var listener = await QuicProvider.CreateListenerAsync(new QuicListenerOptions()
{
    ListenEndPoint = new IPEndPoint(IPAddress.Loopback, 5000),
    ApplicationProtocols = { SslApplicationProtocol.Http3 },
    ListenBacklog = 1024,
    ConnectionOptionsCallback = (connection, clientHelloInfo, cancellationToken) => 
    {
        // ...
        return new ValueTask<ConnectionOptions>(connectionOptions);
    },
}, cancellationToken);

while (running)
{
    await using var connection = await listener.AcceptConnectionAsync(cancellationToken);
    // More work with connection
}

Risks

As I'll state with all QUIC APIs. We might consider making all of these PreviewFeature. Not to deter user from using it, but to give us flexibility to tune the API shape based on customer feedback.
We don't have many users now and we're mostly making these APIs based on what Kestrel needs, our limited experience with System.Net.Quic and my meager experiments with other QUIC implementations.

cc: @JamesNK @Tratcher @wfurt @CarnaViire @rzikm

EDIT: Hid Options in Accept since Options Upfront are the preferred design. Also removed stateObject for the callback.

Alternative Designs

Static QuicServerConnectionOptions

Apart from ServerOptionsSelectionCallback, QuicListenerOptions could also have direct property for QuicServerConnectionOptions, bypassing the callback. Since it can always be provided via the callback, there's no functional need for it at the moment. Also ASP.NET Core needs the callback so we don't even have a use for it now.

QuicServerConnectionOptions in Accept

This design came out from my experiments with listener shape. It's been prototyped but it's unproven design.

  • It allows meaningful state object for ServerOptionsSelectionCallback callback.
  • It makes accept method cluttered with parameters.
  • It allows for different options per connection
  • Unless there's an accept pending, the connection handshake might timeout (default is 1/3 of idle timeout) while the connection waits for someone to accept it. As a result, we might discard connections which we would have otherwise accepted.
  • On the other hand, we wouldn't waste cycles on finishing handshake for connections which we don't accept in the end.

API Proposal

/// <summary>Represents server side of QUIC transport. Listens for incoming QuicConnections.</summary>
public sealed class QuicListener : IAsyncDisposable
{
    // Static create method instead of ctor so that the class can became abstract in the future.
    public static QuicListener Create(QuicListenerOptions options);

    public IPEndPoint ListenEndPoint;

    // Accept methods now have SSL and connection options as extra arguments.
    // Those options are connection specific and there's no need to provide them at the time of listener creation.
    // This also allows us to pass sslOptionsCallbackState
    public ValueTask<QuicConnection> AcceptConnectionAsync(ServerSslOptionsSelectionCallback sslOptionsCallback, object? sslOptionsCallbackState, QuicConnectionOptions connectionOptions, CancellationToken cancellationToken = default);
    public ValueTask<QuicConnection> AcceptConnectionAsync(SslServerAuthenticationOptions sslOptions, QuicConnectionOptions connectionOptions, CancellationToken cancellationToken = default);

    public ValueTask DisposeAsync();
}


public delegate ValueTask<SslServerAuthenticationOptions> ServerSslOptionsSelectionCallback(QuicConnection connection, object? state, SslClientHelloInfo clientHelloInfo, CancellationToken cancellationToken);

public class QuicListenerOptions
{
    /// <summary>The local endpoint to listen on.</summary>
    [Required]
    public IPEndPoint ListenEndPoint { get; init; } = null!;

    /// <summary>The application protocols.</summary>
    [Required]
    public SslApplicationProtocol[] ApplicationProtocols { get; init; } = Array.Empty<SslApplicationProtocol>();

    /// <summary>Number of connections to be held without accepting the connection.</summary>
    public int ListenBacklog { get; init; } = 512;
}

/// <summary>Options for a new connection, the same options are used for incoming and outgoing connections.</summary>
public class QuicConnectionOptions
{
    /// <summary>Limit on the number of bidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxBidirectionalStreams { get; init; } = 100;

    /// <summary>Limit on the number of unidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxUnidirectionalStreams { get; init; } = 100;

    /// <summary>Idle timeout for connections, after which the connection will be closed.</summary>
    public TimeSpan IdleTimeout { get; init; } = TimeSpan.FromMinutes(2);

    // This class will potentially expand with other connection options which we'll deem interesting for user to set.
}

API Usage

await using var listener = QuicListener.Create(new QuicListenerOptions() {
    ListenEndPoint = new IPEndPoint(IPAddress.Loopback, 5000),
    ApplicationProtocols = { SslApplicationProtocol.Http3 },
    ListenBacklog = 1024,
});

var connectionOptions = new QuicConnectionOptions() {
    IdleTimeout = TimeSpan.FromMinutes(5),
    MaxBidirectionalStreams = 1000,
    MaxUnidirectionalStreams = 10
},

var serverSslOptions = new SslServerAuthenticationOptions() {
    ApplicationProtocols = new List<SslApplicationProtocol>(){ SslApplicationProtocol.Http3 },
    ServerCertificate = TestCertificateExtensions.ServerCertificate
};
var serverSslOptionsSelectionCallback = (connection, clientHelloInfo, state, cancellationToken) => {
    // ...
    return new ValueTask<SslServerAuthenticationOptions>(serverSslOptions);
};
while (running) {
    // Either this way
    await using var connection = await listener.AcceptConnectionAsync(serverSslOptions, connectionOptions, cancellationToken);
    // Or that way
    await using var connection = await listener.AcceptConnectionAsync(serverSslOptionsSelectionCallback, null, connectionOptions, cancellationToken);
    // More work with connection
}
@ManickaP ManickaP added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 4, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Quic untriaged New issue has not been triaged by the area owner labels Apr 4, 2022
@ghost
Copy link

ghost commented Apr 4, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

API design for exposing QuicListener and related classes to the public.

There are 2 major design approaches. The first one requires options for incoming connection (to finish the handshake) during listener creation. The alternative expects these options not until Accept is called. Both solutions take into account #49587.

Options Upfront

This design was discussed in our team and is based on what we have now. Thus, it is safer since we have at least some experience with this shape.

  • It does not allow for meaningful state object for ServerOptionsSelectionCallback since it'd need to be provided upfront (one object per listener).
  • It'll finish handshake for all incoming connections regardless whether there's anyone ready to accept them.

Options in Accept

This design came out from my experiments with listener shape. It's been prototyped but it's unproven design.

  • It allows meaningful state object for ServerOptionsSelectionCallback callback.
  • It makes accept method cluttered with parameters.
  • It allows for different options per connection
  • Unless there's an accept pending, the connection handshake might timeout (default is 1/3 of idle timeout) while the connection waits for someone to accept it. As a result, we might discard connections which we would have otherwise accepted.
  • On the other hand, we wouldn't waste cycles on finishing handshake for connections which we don't accept in the end.

Related issues:

API Proposal

// Change from the current class is removal of all the constructors which are replaced with static Create method.
// All the Quic classes will have static Create so that they can became abstract in the future.
// Also all the overloads accepting implementation provider (temporary) are removed.
public class QuicListener : IAsyncDisposable
{
    // Static create method instead of ctor so that the class can became abstract in the future.
    public static QuicListener Create(QuicListenerOptions options);

    public IPEndPoint ListenEndPoint { get; }

    public async ValueTask<QuicConnection> AcceptConnectionAsync(CancellationToken cancellationToken = default);

    public void DisposeAsync();
}

// The state object is questionable here since there's no way to provide one per accepted connection in this design.
public delegate ValueTask<SslServerAuthenticationOptions> ServerOptionsSelectionCallback(QuicConnection connection, object? state, SslClientHelloInfo clientHelloInfo, CancellationToken cancellationToken);

public class QuicListenerOptions
{
    // Listener options necessary for starting a listener.
    [Required]
    public IPEndPoint ListenEndPoint { get; init; }
    [Required]
    public List<SslApplicationProtocol> ApplicationProtocols { get; init; }
    public int ListenBacklog { get; init; } = 512;

    // Options for incoming connections.
    [Required]
    public QuicConnectionOptions ConnectionOptions { get; init; }

    // SSL options for incoming connections, either one of the following must be defined.
    // Direct options, no callback will be invoked if provided.
    public SslServerAuthenticationOptions? ServerSslOptions { get; init; }
    // Callback instead of direct options, invoked only if ServerSslOptions not specified.
    public ServerOptionsSelectionCallback? ServerSslOptionsSelectionCallback { get; init; }
    // Optional state for ServerSslOptionsSelectionCallback. Might not make sense if it's just one object per listener.
    public object? ServerSslOptionsSelectionCallbackState { get; init; }
}

/// <summary>Options for a new connection, the same options are used for incoming and outgoing connections.</summary>
public class QuicConnectionOptions
{
    /// <summary>Limit on the number of bidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxBidirectionalStreams { get; init; } = 100;

    /// <summary>Limit on the number of unidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxUnidirectionalStreams { get; init; } = 100;

    /// <summary>Idle timeout for connections, after which the connection will be closed.</summary>
    public TimeSpan IdleTimeout { get; init; } = TimeSpan.FromMinutes(2);

    // This class will potentially expand with other connection options which we'll deem interesting for user to set.
}

API Usage

Options Upfront

var serverSslOptions = new SslServerAuthenticationOptions() {
    ApplicationProtocols = new List<SslApplicationProtocol>(){ SslApplicationProtocol.Http3 },
    ServerCertificate = TestCertificateExtensions.ServerCertificate
};

await using var listener = MsQuicListener.Create(new QuicListenerOptions() {
    ListenEndPoint = new IPEndPoint(IPAddress.Loopback, 5000),
    ApplicationProtocols = { SslApplicationProtocol.Http3 },
    ListenBacklog = 1024,

    ConnectionOptions = new QuicConnectionOptions() {
        IdleTimeout = TimeSpan.FromMinutes(5),
        MaxBidirectionalStreams = 1000,
        MaxUnidirectionalStreams = 10
    },

    // Either this
    ServerSslOptions = serverSslOptions,
    // Or these 2
    ServerSslOptionsSelectionCallback = (connection, clientHelloInfo, state, cancellationToken) => {
        // ...
        return new ValueTask<SslServerAuthenticationOptions>(serverSslOptions);
    },
    ServerSslOptionsSelectionCallbackState = null,
});

while (running) {
    await using var connection = await listener.AcceptConnectionAsync(cancellationToken);
    // More work with connection
}

Options in Accept

await using var listener = MsQuicListener.Create(new QuicListenerOptions() {
    ListenEndPoint = new IPEndPoint(IPAddress.Loopback, 5000),
    ApplicationProtocols = { SslApplicationProtocol.Http3 },
    ListenBacklog = 1024,
});

var connectionOptions = new QuicConnectionOptions() {
    IdleTimeout = TimeSpan.FromMinutes(5),
    MaxBidirectionalStreams = 1000,
    MaxUnidirectionalStreams = 10
},

var serverSslOptions = new SslServerAuthenticationOptions() {
    ApplicationProtocols = new List<SslApplicationProtocol>(){ SslApplicationProtocol.Http3 },
    ServerCertificate = TestCertificateExtensions.ServerCertificate
};
var serverSslOptionsSelectionCallback = (connection, clientHelloInfo, state, cancellationToken) => {
    // ...
    return new ValueTask<SslServerAuthenticationOptions>(serverSslOptions);
};
while (running) {
    // Either this way
    await using var connection = await listener.AcceptConnectionAsync(serverSslOptions, connectionOptions, cancellationToken);
    // Or that way
    await using var connection = await listener.AcceptConnectionAsync(serverSslOptionsSelectionCallback, null, connectionOptions, cancellationToken);
    // More work with connection
}

Alternative Designs

/// <summary>Represents server side of QUIC transport. Listens for incoming QuicConnections.</summary>
public sealed class QuicListener : IAsyncDisposable
{
    // Static create method instead of ctor so that the class can became abstract in the future.
    public static QuicListener Create(QuicListenerOptions options);

    public IPEndPoint ListenEndPoint;

    // Accept methods now have SSL and connection options as extra arguments.
    // Those options are connection specific and there's no need to provide them at the time of listener creation.
    // This also allows us to pass sslOptionsCallbackState
    public ValueTask<QuicConnection> AcceptConnectionAsync(ServerSslOptionsSelectionCallback sslOptionsCallback, object? sslOptionsCallbackState, QuicConnectionOptions connectionOptions, CancellationToken cancellationToken = default);
    public ValueTask<QuicConnection> AcceptConnectionAsync(SslServerAuthenticationOptions sslOptions, QuicConnectionOptions connectionOptions, CancellationToken cancellationToken = default);

    public ValueTask DisposeAsync();
}


public delegate ValueTask<SslServerAuthenticationOptions> ServerSslOptionsSelectionCallback(QuicConnection connection, object? state, SslClientHelloInfo clientHelloInfo, CancellationToken cancellationToken);

public class QuicListenerOptions
{
    /// <summary>The local endpoint to listen on.</summary>
    [Required]
    public IPEndPoint ListenEndPoint { get; init; } = null!;

    /// <summary>The application protocols.</summary>
    [Required]
    public SslApplicationProtocol[] ApplicationProtocols { get; init; } = Array.Empty<SslApplicationProtocol>();

    /// <summary>Number of connections to be held without accepting the connection.</summary>
    public int ListenBacklog { get; init; } = 512;
}

/// <summary>Options for a new connection, the same options are used for incoming and outgoing connections.</summary>
public class QuicConnectionOptions
{
    /// <summary>Limit on the number of bidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxBidirectionalStreams { get; init; } = 100;

    /// <summary>Limit on the number of unidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxUnidirectionalStreams { get; init; } = 100;

    /// <summary>Idle timeout for connections, after which the connection will be closed.</summary>
    public TimeSpan IdleTimeout { get; init; } = TimeSpan.FromMinutes(2);

    // This class will potentially expand with other connection options which we'll deem interesting for user to set.
}

Risks

As I'll state with all QUIC APIs. We might consider making all of these PreviewFeature. Not to deter user from using it, but to give us flexibility to tune the API shape based on customer feedback.
We don't have many users now and we're mostly making these APIs based on what Kestrel needs, our limited experience with System.Net.Quic and my meager experiments with other QUIC implementations.

Author: ManickaP
Assignees: -
Labels:

api-suggestion, untriaged, area-System.Net.Quic

Milestone: -

@hez2010
Copy link
Contributor

hez2010 commented Apr 5, 2022

Why not integrate it into the existing HttpListener?

@ManickaP
Copy link
Member Author

ManickaP commented Apr 5, 2022

QUIC is transport protocol on it's own. HTTP can use it (and it does in HTTP/3) but that's just one way to use it, there are others (e.g.: SMB over QUIC), including fully custom protocols based on QUIC. You can look at this more like sockets with some more features.

@rzikm
Copy link
Member

rzikm commented Apr 5, 2022

I am conflicted about the duplication of List<SslApplicationProtocol> in both QuicListenerOptions and SslServerAuthenticationOptions. It will be confusing to the user that the one in the latter is silently ignored, have we tracked down if it is feasible to change MsQuic to not require ALPN upfront, @wfurt @nibanks?

More to the topic: I personally like the "specify SslServerAuthenticationOptions/SslServerAuthenticationOptionsCallback upfront" version more, it makes simple scenarios much cleaner while being flexible enough to cover more complex cases. It is also similar to the certificate selection machinery that is already in the client-side SslStream and which users may be familiar with. However, one thing that may be missing is the ability to specify QuicConnectionOptions per connection (which the second design supports).

@nibanks
Copy link

nibanks commented Apr 5, 2022

In order to support multiple independent listeners (for different protocols, such as SMB and HTTP) on the same port, the listeners must preregister the ALPN(s) they are interested in upfront so the stack and multiplex between them.

Think of this as the same as the local UDP port. You don't open a socket and get all local UDP ports on the system and then decide if you want to take a specific port or ports. You register it up front and then selectively get only what you need/want.

@ManickaP
Copy link
Member Author

ManickaP commented Apr 5, 2022

Yeah the duplicate ALPN specification has been discussed with msquic and ASP.NET and we have reached a working compromise (microsoft/msquic#2418), this API takes that into account.

@Tratcher
Copy link
Member

Tratcher commented Apr 5, 2022

MsQuicListener.Create(new QuicListenerOptions() {?
Shouldn't that say QuickListener?

@ManickaP
Copy link
Member Author

ManickaP commented Apr 6, 2022

MsQuicListener.Create(new QuicListenerOptions() {? Shouldn't that say QuickListener?

Fixed, good catch.

@JamesNK
Copy link
Member

JamesNK commented Apr 6, 2022

ASP.NET Core feedback:

  • We prefer the first design of specifying everything upfront. We don't see any benefit of providing options in AcceptConnectionAsync. We don't know who is making the connection when AcceptConnectionAsync is called so the same options would be passed every time. Simpler to specify them once when the listener is created.

  • It doesn't seem like ServerSslOptionsSelectionCallbackState is worth it since the callback is specified once on the listener. The callback accessing the state directly via a closure allows the same thing, at the small cost of an allocation. However, we don't have a strong opinion on this.

  • What happens if ListenBacklog is hit? Does the server send a result to the client that indicates the connection wasn't processed and the client can retry?

  • We discussed whether ListenEndPoint is necessary. Yes, it is because the endpoint in the creation options could have an ephemeral port and ListenEndPoint is required so we can figure out the port number resolved by the OS.

  • ServerOptionsSelectionCallback is currently returning SslServerAuthenticationOptions. That allows customization of TLS options using SNI. We discussed whether it makes sense for non-TLS connection configuration to also be configurable. i.e. the values on QuicConnectionOptions. We don't have a requirement for this at the moment, but we think it is likely that some advanced customers might want to customize idle timeout, stream limits, and other connection configuration based on the caller. A question is whether msquic supports this.

Basically, the callback would start returning QuicConnectionOptions. API design for this:

public class QuicConnectionOptions
{
    public int MaxBidirectionalStreams { get; init; } = 100;
    public int MaxUnidirectionalStreams { get; init; } = 100;
    public TimeSpan IdleTimeout { get; init; } = TimeSpan.FromMinutes(2);

    // Add TLS options to QuicConnectionOptions
    public SslServerAuthenticationOptions ServerSslOptions { get; init; }
}

// Callback returns QuicConnectionOptions
public delegate ValueTask<QuicConnectionOptions> ServerOptionsSelectionCallback(QuicConnection connection, object? state, SslClientHelloInfo clientHelloInfo, CancellationToken cancellationToken);

public class QuicListenerOptions
{
    // Either specify options once, or via a callback
    public QuicConnectionOptions ConnectionOptions { get; init; }
    public ServerOptionsSelectionCallback? ConnectionOptionsSelectionCallback { get; init; }

    public object? ConnectionOptionsSelectionCallbackState { get; init; }
}

I think this is actually a bit cleaner. The TLS options for a connection are no longer their own special thing and are associated with the rest of the QUIC connection options.

@karelz karelz added this to the 7.0.0 milestone Apr 7, 2022
@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Apr 7, 2022
@ManickaP
Copy link
Member Author

ManickaP commented Apr 7, 2022

We prefer the first design of specifying everything upfront.

👍


It doesn't seem like ServerSslOptionsSelectionCallbackState is worth it since the callback is specified once on the listener.

👍


What happens if ListenBacklog is hit? Does the server send a result to the client that indicates the connection wasn't processed and the client can retry?

Currently we don't send anything to the client and the client waits on connection idle timeout. We even finish the handshake before we decide to not to accept the connection. This is the result of cumulative changes/fixes in 6.0 without proper design and I'm not fond of the way it works now. Options to improve:

  • Get rid of this limit. It's possible, but accept queue might grow uncontrollably.
  • Improve the behavior to reject the connection before we finish the handshake, which would send CONNECTION_REFUSED. I think it's doable but might not be 100% exact in enforcing the limit.

We discussed whether it makes sense for non-TLS connection configuration to also be configurable. i.e. the values on QuicConnectionOptions.

We cannot straightforwardly combine SslServerAuthenticationOptions with QuicConnectionOptions since QuicConnectionOptions are used for incoming as well as outgoing connection. I.e. they get paired up with SslClientAuthenticationOptions as well.
Obviously, there are solutions to this: inheritance, composition, generics, anonymous tuples... Either way, we certainly can design the callback to return both TLS and non-TLS options, technically it's doable.

@ManickaP ManickaP self-assigned this May 23, 2022
@ManickaP ManickaP added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jun 13, 2022
@terrajobst terrajobst added the blocking Marks issues that we want to fast track in order to unblock other important work label Jun 13, 2022
@terrajobst
Copy link
Member

terrajobst commented Jun 14, 2022

Video

  • QuicListenerOptions.ListenBacklog. We shouldn't introduce an opinion on the buffer boundary, e.g. 512
    • We shouldn't have unbounded buffers/queues on our end (due to OOMs) but equally we shouldn't add have a fixed opinion on the upper bound across all platforms
    • Ideally, we'd just map to the underlying implementation (e.g. MSQUIC) but that's difficult as MSQUIC for example doesn't throttle based on number of connection but on CPU load.
  • QuicListenerOptions.ListenEndPoint
    • We should align this with TcpListener and Socket and call it LocalEndPoint
  • QuicListenerOptions
    • Consider having a constructor that forces the required properties
    • Also sync with @333fred to see whether we can use C# 11's required keyword; not sure how this works for collections
    • Also, if you go with properties and default constructor, think about nullable annotations
  • We should seal the types
    • This means we don't have to implement the Dispose pattern
    • Also, we'd get away with not implementing IDisposable
  • ServerOptionsSelectionCallback
    • Could just be a func, given that the types are sufficiently descriptive
namespace System.Net.Quic;

public sealed class QuicListener : IAsyncDisposable
{
    public static bool IsSupported { get; }
    public static ValueTask<QuicListener> ListenAsync(QuicListenerOptions options,
                                                      CancellationToken cancellationToken = default);

    public IPEndPoint ListenEndPoint { get; }
    public ValueTask<QuicConnection> AcceptConnectionAsync(CancellationToken cancellationToken = default);
    public ValueTask DisposeAsync();
}

public sealed class QuicListenerOptions
{
    public QuicListenerOptions();
    public IPEndPoint ListenEndPoint { get; set; }
    public List<SslApplicationProtocol> ApplicationProtocols { get; }
    public int ListenBacklog { get; set; }
    public Func<QuicConnection, SslClientHelloInfo, CancellationToken, ValueTask<QuicServerConnectionOptions>> ConnectionOptionsCallback { get; set; }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 14, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 2, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Quic blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants