Skip to content

unix,win: add uv_udp_try_send2#4644

Merged
bnoordhuis merged 2 commits intolibuv:v1.xfrom
bnoordhuis:uv_udp_try_send2
Dec 13, 2024
Merged

unix,win: add uv_udp_try_send2#4644
bnoordhuis merged 2 commits intolibuv:v1.xfrom
bnoordhuis:uv_udp_try_send2

Conversation

@bnoordhuis
Copy link
Copy Markdown
Member

Add a version of uv_udp_try_send that can send multiple datagrams.

Uses sendmmsg(2) on platforms that support it (Linux, FreeBSD, macOS),
falls back to a regular sendmsg(2) loop elsewhere.

This work was sponsored by ISC, the Internet Systems Consortium.


The first commit is a NFC that is prep work for the second commit. Same line count, more functionality, improved legibility (IMO)

I moved uv__udp_sendmsg to the bottom of udp.c because the diff is crazy hard to read otherwise.

I don't love the name uv_udp_try_send2 but I guess it's convention by now. Can I see hands for uv_udp_try_sendv? uv_udp_try_send_many? Other suggestions?

Shuffle around and DRY the sendmsg logic in preparation for
uv_udp_try_send2(). NFC barring bugs.

This work was sponsored by ISC, the Internet Systems Consortium.
Add a version of uv_udp_try_send that can send multiple datagrams.

Uses sendmmsg(2) on platforms that support it (Linux, FreeBSD, macOS),
falls back to a regular sendmsg(2) loop elsewhere.

This work was sponsored by ISC, the Internet Systems Consortium.
@saghul
Copy link
Copy Markdown
Member

saghul commented Dec 10, 2024

don't love the name uv_udp_try_send2 but I guess it's convention by now. Can I see hands for uv_udp_try_sendv? uv_udp_try_send_many? Other suggestions?

I like uv_udp_try_send2 and uv_udp_try_sendv. The former is a more used convention, and I don't recall us having any instances of the latter, even if APIs like uv_write take multiple buffers, so there is that...

@bnoordhuis
Copy link
Copy Markdown
Member Author

Okay, uv_udp_try_send2 it is then. Any objections to me merging this?

@bnoordhuis bnoordhuis merged commit e8969bf into libuv:v1.x Dec 13, 2024
@bnoordhuis bnoordhuis deleted the uv_udp_try_send2 branch December 13, 2024 20:53
Comment on lines +433 to +434
fallback loop for platforms that do not support the former. The handle must
be fully initialized; call c:func:`uv_udp_bind` first.
Copy link
Copy Markdown
Contributor

@squeek502 squeek502 Jan 16, 2025

Choose a reason for hiding this comment

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

The handle must be fully initialized; call uv_udp_bind first.

Could I get some clarification about this? uv_udp_bind is not called on the client handle in the updated test-udp-try-send.c.

EDIT: Nevermind, I see now that, in the test code, the earlier uv_udp_try_send call handles the bind.

@ibc
Copy link
Copy Markdown
Contributor

ibc commented Jul 4, 2025

Too late but what's the purpose of the flags argument?

int uv_udp_try_send2(uv_udp_t* handle,
                     unsigned int count,
                     uv_buf_t* bufs[/*count*/],
                     unsigned int nbufs[/*count*/],
                     struct sockaddr* addrs[/*count*/],
                     unsigned int flags) {
  if (count < 1)
    return UV_EINVAL;

  if (flags != 0)
    return UV_EINVAL;

Basically if flags != 0 then the function fails with UV_EINVAL. So what's the purpose of args?

@saghul
Copy link
Copy Markdown
Member

saghul commented Jul 4, 2025

Usually we add those for future usage.

@ibc
Copy link
Copy Markdown
Contributor

ibc commented Jul 4, 2025

Usually we add those for future usage.

Makes sense. Maybe some docs stating that flags must be 0 for now would be helpful to avoid confusion (people maybe tempted to use same flags as in other uv_udp related functions).

@ibc
Copy link
Copy Markdown
Contributor

ibc commented Jul 4, 2025

Another thing I don't understand from the uv_udp_try_send2() function is the meaning of count, specially in the addrs array:

int uv_udp_try_send2(uv_udp_t* handle,
                     unsigned int count,
                     uv_buf_t* bufs[/*count*/],
                     unsigned int nbufs[/*count*/],
                     struct sockaddr* addrs[/*count*/],
                     unsigned int flags)

So imagine I have a single uv_buf_t buf and I want to send it to sockaddr1 and sockaddr2 (2 different UDP destinations) from the same UDP socket in libuv. Would it be something like this (pseudo code)?

uv_udp_try_send2(handle,
                 2 /*count*/,
                 [&buf, &buf] /*bufs[count]*/,
                 [1, 1] /*nbufs[count]*/,
                 [sockaddr1, sockaddr2] /*addrs[count]*/,
                 0 /*flags*/)

@saghul
Copy link
Copy Markdown
Member

saghul commented Jul 4, 2025

So imagine I have a single uv_buf_t buf and I want to send it to sockaddr1 and sockaddr2 (2 different UDP destinations) from the same UDP socket in libuv. Would it be something like this (pseudo code)?

uv_udp_try_send2(handle,
                 2 /*count*/,
                 [&buf, &buf] /*bufs[count]*/,
                 [1, 1] /*nbufs[count]*/,
                 [sockaddr1, sockaddr2] /*addrs[count]*/,
                 0 /*flags*/)

Yep.

If you check the implementation:

static int uv__udp_sendmsgv(int fd,
you can see that it basically sends each buffer list to the given address.

@ibc
Copy link
Copy Markdown
Contributor

ibc commented Jul 4, 2025

If you check the implementation

Yes. The thing is: what about if I just read the docs?

@saghul
Copy link
Copy Markdown
Member

saghul commented Jul 4, 2025

But who does that? :-P

@ibc
Copy link
Copy Markdown
Contributor

ibc commented Jul 4, 2025

But who does that? :-P

I tried. Then I add to dig into the PR and the test it includes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants