go-libp2p
go-libp2p copied to clipboard
feat: add `DisableReuseport` option to quic-transport
Moved from https://github.com/libp2p/go-libp2p-quic-transport/pull/272
Fixes #1428
Hey, I just started to add the DisableReuseport options to quic transport. It seems to be working as expected with some cellular operators that couldn't maintain the connection (I tried with this script https://github.com/gfanton/libp2p-reuseport-test)
Just some few things:
- It seems that there is no option/configuration mechanism, so I added one. Let me know if you are ok with this or if you want to use another method to pass the argument.
- I wrapped all the
conn_testswith a simple table test case to test everything conn related with the reuseport_on/off - I have to use
net.ListenUDPinstead ofnet.DialUDPfor the Dial method to work, otherwise the call to theWriteTomethod fails.
Also there are two tests that currently fail with the reuseport disabled:
TestHolePunching
=== RUN TestHolePunching/reuseport_off
conn_test.go:59: using a Secp256k1 key: 16Uiu2HAmPxEseg6Hti88hpHTyUB3tQk9bw7paj9yS3rPkBgPnWgM
conn_test.go:59: using a Ed25519 key: 12D3KooWAaU8sc9j783H9hH3eG3yyJDnvz9Le1LdhCQFbYe5gFyu
conn_test.go:581:
Error Trace: conn_test.go:581
asm_arm64.s:1133
Error: An error is expected but got nil.
Test: TestHolePunching/reuseport_off
Messages: didn't expect to accept any connections
conn_test.go:602:
Error Trace: conn_test.go:602
Error: Condition never satisfied
Test: TestHolePunching/reuseport_off
TestStatelessReset
=== RUN TestStatelessReset/reuseport_off
conn_test.go:59: using a ECDSA key: QmRoF9AeMPnxYMrErq91y2xCvjiULp9n9cJuVwDHu9Kdao
conn_test.go:59: using a RSA key: QmNvwzR7VDFy1wFdHE3PgV8mCztJbFWz1d5NXVL3wXYVgX
conn_test.go:534:
Error Trace: conn_test.go:66
conn_test.go:534
Error: Received unexpected error:
listen udp4 127.0.0.1:55213: bind: address already in use
Test: TestStatelessReset/reuseport_off
Do these tests make sense when used with resuseport disabled?
The QUIC tests were very flaky, and we've cleaned them up since. Could you rebase this PR on the current master?
I just rebased the branch on master, it seems that all the concerned tests pass now!
I've made the requested changes, but I've missed some DisableReuseport option in some test and still have the same issues described above with TestHolePunching & TestStatelessReset:
- for
TestStatelessReset, I managed to fix it in my last commit by closing the connection withlistener.Closereleasing the used port. - for
TestHolePunchingI'm still facing the same issue, but I'm not really sure that this test has any interest with reuseport disabled. Do you have any clue about this ?
thanks !
Hole punching is only expected to work with reuseport enabled. You can disable that test for the non-reuseport version (please add a comment why!).
done 👍
One test is failing, but it doesn't seem to be related to my PR.