Revert "freebsd,linux: add recvmmsg() + sendmmsg() udp implementation" #2792
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 3d71366. This reverts commit d9cd7d4. The first reverted commit is the sendmmsg/recvmmsg support, the second one a fix-up to deliver datagrams in order. The second commit has been implicated in causing bind9 to crash on freebsd. A quick review of the code suggests that downstream code written against pre-v1.35.0 libuv can't safely deal with multi-datagram support because they are unaware of the `UV_UDP_MMSG_CHUNK` flag and what that implies for buffer management, hence I'm moving to revert it. The `UV_UDP_MMSG_CHUNK` flag remains part of `uv_udp_flags` for API/ABI backwards compatibility reasons but it is no longer used. Fixes: libuv#2791 Refs: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=245653
|
As mentioned in #2791 (comment) how about we turn it off with a flag instead? That would retain the previous behavior and ada new way to turn it on, instead of completely scraping the feature. I can work on such patch. |
uqs
pushed a commit
to freebsd/freebsd-ports
that referenced
this pull request
Apr 16, 2020
…ns/bind916). See <libuv/libuv#2791> and <libuv/libuv#2792> PR: 245653 Reported by: lysfjord.daniel(at)smokepit.net and many git-svn-id: svn+ssh://svn.freebsd.org/ports/head@531835 35697150-7ecd-e111-bb59-0022644237b5
uqs
pushed a commit
to freebsd/freebsd-ports
that referenced
this pull request
Apr 16, 2020
…ns/bind916). See <libuv/libuv#2791> and <libuv/libuv#2792> PR: 245653 Reported by: lysfjord.daniel(at)smokepit.net and many
Jehops
pushed a commit
to Jehops/freebsd-ports-legacy
that referenced
this pull request
Apr 16, 2020
…ns/bind916). See <libuv/libuv#2791> and <libuv/libuv#2792> PR: 245653 Reported by: lysfjord.daniel(at)smokepit.net and many git-svn-id: svn+ssh://svn.freebsd.org/ports/head@531835 35697150-7ecd-e111-bb59-0022644237b5
That should read pre-v1.36.0, right? |
|
The feature was introduced in v1.35.0, the fix (that fixes one bug but causes another) in v1.36.0. |
cjihrig
approved these changes
Apr 17, 2020
saghul
added a commit
to saghul/libuv
that referenced
this pull request
Apr 18, 2020
Instead of implicitly enabling it by checking the supplied buffer size to alloc_cb, have a dedicated flag that must be set on `uv_udp_init_ex`. Fixes: libuv#2791 Closes: libuv#2792
saghul
added a commit
to saghul/libuv
that referenced
this pull request
Apr 18, 2020
Instead of implicitly enabling it by checking the supplied buffer size to alloc_cb, have a dedicated flag that must be set on `uv_udp_init_ex`. Fixes: libuv#2791 Closes: libuv#2792
saghul
added a commit
to saghul/libuv
that referenced
this pull request
Apr 19, 2020
Instead of implicitly enabling it by checking the supplied buffer size to alloc_cb, have a dedicated flag that must be set on `uv_udp_init_ex`. Fixes: libuv#2791 Closes: libuv#2792 PR-URL: libuv#2799 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
ericbsd
pushed a commit
to ghostbsd/ghostbsd-ports
that referenced
this pull request
Apr 22, 2020
…ns/bind916). See <libuv/libuv#2791> and <libuv/libuv#2792> PR: 245653 Reported by: lysfjord.daniel(at)smokepit.net and many
svmhdvn
pushed a commit
to svmhdvn/freebsd-ports
that referenced
this pull request
Jan 10, 2024
…ns/bind916). See <libuv/libuv#2791> and <libuv/libuv#2792> PR: 245653 Reported by: lysfjord.daniel(at)smokepit.net and many
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 3d71366.
This reverts commit d9cd7d4.
The first reverted commit is the sendmmsg/recvmmsg support, the second
one a fix-up to deliver datagrams in order. The second commit has been
implicated in causing bind9 to crash on freebsd.
A quick review of the code suggests that downstream code written against
pre-v1.35.0 libuv can't safely deal with multi-datagram support because
they are unaware of the
UV_UDP_MMSG_CHUNKflag and what that impliesfor buffer management, hence I'm moving to revert it.
The
UV_UDP_MMSG_CHUNKflag remains part ofuv_udp_flagsforAPI/ABI backwards compatibility reasons but it is no longer used.
Fixes: #2791
Refs: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=245653
CI: https://ci.nodejs.org/job/libuv-test-commit/1856/
This is my quick reading of the code, I don't mind being proven wrong. Is there a way to maintain the multi-datagram feature without breaking existing users?