msquic icon indicating copy to clipboard operation
msquic copied to clipboard

Listener->LocalAddress is empty after ListenerStart()

Open ami-GS opened this issue 2 years ago • 5 comments

Describe the bug

Issue is found when I test with my hacky Linux XDP. ParameterValidation/WithValidateConnectionEventArgs.ValidateConnectionEvents/2 is disabled at build time. https://github.com/microsoft/msquic/blob/290e634880f24440b288b21db7342b1d6f0f52c9/src/test/bin/quic_gtest.h#LL700C5-L700C5 By running it forcibly with --duoNic, test crash. However the cause seems to be common. See thread for more detail

Affected OS

  • [X] Windows
  • [X] Linux
  • [ ] macOS
  • [ ] Other (specify below)

Additional OS information

MsQuic version

main

Steps taken to reproduce bug

Run the test ./artifacts/bin/linux/x64_Debug_openssl3/msquictest --gtest_filter="ParameterValidation/WithValidateConnectionEventArgs.ValidateConnectionEvents/2" --duoNic

Expected behavior

Pass

Actual outcome

Crash

Additional details

ami-GS avatar May 17 '23 06:05 ami-GS

Found the cause, https://github.com/microsoft/msquic/blob/290e634880f24440b288b21db7342b1d6f0f52c9/src/test/lib/EventTest.cpp#LL534C1-L537C65 Listener.Start set address to Binding->LocalAddress, but Listener->LocalAddress is empty yet. This should happen not only for XDP, but also conventional datapath if they call Listener.GetLocalAddr. Listener.GetLocalAddr get the empty address which cause QuicAddrFamily == 0, which assert CXPLAT_DBG_ASSERT(QuicAddrGetFamily(Addr) == QUIC_ADDRESS_FAMILY_INET6); https://github.com/microsoft/msquic/blob/290e634880f24440b288b21db7342b1d6f0f52c9/src/test/lib/HandshakeTest.cpp#LL79C1-L80C55

How to synchronize Binding->LocalAddress and Listener->LocalAddress during (or immediately after) ListenerStart?

ami-GS avatar May 17 '23 06:05 ami-GS

It is common, if not generally the default case, that the server will use this kind of bind, which indicates a "wildcard" bind, or that it wants to listen on all IP (v6 and v4) addresses. At the XDP layer, this means it needs to interpret this as a bind to all IP addresses that are available.

If you look here you will see the logic to determine which type of addresses are used and then it creates a socket here to "play nice" with the existing networking stack.

nibanks avatar May 17 '23 12:05 nibanks

Maybe we are talking different topic? Problem is Listener->LocalAddress is not set appropriately (or is this expected?) even though Binding->LocalAddress is set. This makes GetParam for LocalAddress get empty address.

ami-GS avatar May 18 '23 05:05 ami-GS

@ami-GS can you answer the following questions for me please?

  1. When do you query the local address?
  2. What exactly is the value at that time?
  3. What do you expect it to be?

nibanks avatar Jun 13 '23 12:06 nibanks

The call stack is

$6 0x00005555556ba239 in QuicAddrSetToDuoNic (Addr=0x7fffffffcfe0) at /home/daiki/workspace/msquic/src/test/lib/TestHelpers.h:36 $7 QuicTestPrimeResumption (QuicAddrFamily=, Registration=..., ServerConfiguration=..., ClientConfiguration=..., ResumptionTicket=ResumptionTicket@entry=0x7fffffffd2d8) at /home/daiki/workspace/msquic/src/test/lib/HandshakeTest.cpp:80 $8 0x00005555556aab99 in QuicTestValidateConnectionEvents3 (Registration=..., Listener=0x555555d36100, ServerLocalAddr=...) at /home/daiki/workspace/msquic/src/inc/msquic_posix.h:279 $9 0x00005555556b8707 in QuicTestValidateConnectionEvents (Test=2) at /home/daiki/workspace/msquic/src/inc/msquic.hpp:770

And, in the $7 (QuicTestPrimeResumption), Listener.Start set address to Binding->LocalAddress, but Listener->LocalAddress is not. However the Listener.GetLocalAddr() tries to read address from Listener->LocalAddress which is empty yet. https://github.com/microsoft/msquic/blob/290e634880f24440b288b21db7342b1d6f0f52c9/src/test/lib/EventTest.cpp#LL534C1-L537C65

So,

  1. query GetLocalAddress immediately after Listener.Start
  2. empty (e.g. sa_family == 0)
  3. Address need to be filled appropriately. It should be same as Binding->LocalAddress

ami-GS avatar Jun 14 '23 18:06 ami-GS