Skip to content

Conversation

@rzikm
Copy link
Member

@rzikm rzikm commented Aug 29, 2022

Fixes #73045, closes #74755.

This PR adds a new member to the QuicError enum to allow us to clearly communicate to the user that a QuicListener cannot be started with specified ALPN set because another QuicListener with conflicting ALPNs is already listening on the desired port.

@ghost
Copy link

ghost commented Aug 29, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost ghost assigned rzikm Aug 29, 2022
@ghost
Copy link

ghost commented Aug 29, 2022

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

Issue Details

Fixes #73045.

This PR adds a new member to the QuicError enum to allow us to clearly communicate to the user that a QuicListener cannot be started with specified ALPN set because another QuicListener with conflicting ALPNs is already listening on the desired port.

Author: rzikm
Assignees: -
Labels:

new-api-needs-documentation, area-System.Net.Quic

Milestone: -

@rzikm
Copy link
Member Author

rzikm commented Aug 29, 2022

Shelving until #74755 is approved

@rzikm rzikm marked this pull request as ready for review September 12, 2022 08:25
@rzikm rzikm requested a review from a team September 12, 2022 08:26
@rzikm rzikm closed this Sep 12, 2022
@rzikm rzikm reopened this Sep 12, 2022
@karelz karelz added this to the 8.0.0 milestone Sep 12, 2022
@rzikm rzikm force-pushed the 73045-QUIC-QuicListener-not-reporting-address-in-use-correctly branch from 79793e6 to 575704b Compare September 15, 2022 13:47
<value>A QUIC protocol error was encountered</value>
</data>
<data name="net_quic_alpn_in_use" xml:space="preserve">
<value>Another QUIC listener is already listening on one of the requested application protocols on the same port.</value>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused about the wording of the error. If we ask for multiple ALPNs and one is in use but the second is available -- do we still fail?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the two ALPN lists must be fully disjoint.

- ${{ if eq(parameters.platform, 'Linux_musl_x64') }}:
- ${{ if or(ne(parameters.jobParameters.isExtraPlatforms, true), eq(parameters.jobParameters.includeAllPlatforms, true)) }}:
- (Alpine.314.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.14-helix-amd64-20220803180115-99b3286
- (Alpine.314.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.14-helix-amd64-20220912172439-e7e8d1c
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous image had an older version of MsQuic which returned INVALID_STATE instead of ALPN_IN_USE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that in 2.1+ or just in msquic main @rzikm?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is in 2.1

@rzikm rzikm requested a review from CarnaViire September 27, 2022 11:49
Copy link
Member

@CarnaViire CarnaViire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@rzikm rzikm merged commit b20eeb0 into dotnet:main Sep 30, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal] QuicError.AlpnInUse QUIC: QuicListener not reporting address in use correctly

4 participants