dns fails due to ECONNREFUSED with c-ares 1.33 and up

[tchaikov]

in c-ares 1.33, it introduced DNS cookie support, which tracks the local ip in server_connection::self_ip when a socket is created. but in seastar, what we return to c-ares is an index into dns_resolver::impl::_sockets, and the underlying socket is hidden underneath of posix_connected_socket_impl. see https://github.com/c-ares/c-ares/blob/759343ae00bd0dd562f4469ef030a71264ed543e/src/lib/ares_conn.c#L144-L154

so when c-ares tries to retrieve the socket's local address in ares_conn_set_self_ip() , getsockname() fails with ENOTSOCK, and ares_conn_set_self_ip() returns ARES_ECONNREFUSED in this case.

please note, fedora 41 comes with c-ares 1.33.

[tchaikov]

we intend to encapsulate the socket fd to enable support for both userspace and kernel-level TCP stacks. This abstraction allows our system to work seamlessly with different TCP implementations, providing greater flexibility and portability. but if c-ares tries to use the fd directly, we have at least four options

• drop the support for c-ares >= 1.33

• propose upstream changes to c-ares:

  we could suggest adding a new entry in the ares_socket_functions struct. this would allow c-ares to call a custom function instead of getsockname() if the function is provided, like

   struct ares_socket_functions {
     ares_socket_t (*asocket)(int, int, int, void *);
     int (*aclose)(ares_socket_t, void *);
     int (*aconnect)(ares_socket_t, const struct sockaddr *, ares_socklen_t,
                     void *);
     ares_ssize_t (*arecvfrom)(ares_socket_t, void *, size_t, int,
                               struct sockaddr *, ares_socklen_t *, void *);
     ares_ssize_t (*asendv)(ares_socket_t, const struct iovec *, int, void *);
     int (*agetsockname)(ares_socket_t, struct sockaddr *, ares_socklen_t *);
  };

• propose upstream changes to c-ares: to disable DNS cookie support if user managed socket is used.

• override getsockname() using library preloading. to define our own version of getsockname(). use library preloading to override the version provided by libc. (i don't like this option though)

[tchaikov]

@elcallio hi Calle, what do you think?

[elcallio]

Sending a PR to c-ares adding a virtual function for socket name seems the most prudent solution. Until then, we can just cap c-ares version to pre-1.33.

[tchaikov]

thank you Calle, i will try.

[elcallio]

Might be worth implementing in a fork and re-introducing the submodule again until propagated. :-)

[tchaikov]

thank you Calle, i will try.

c-ares/c-ares#893

[tchaikov]

Might be worth implementing in a fork and re-introducing the submodule again until propagated. :-)

i will check if 1.33 or newer versions has something interesting to us. if yes, yeah, we might need to have a (temporary) fork.

and, let's see if this change is acceptable from the upstream's perspective. if it's dead on arrival, we would need to find another way out. and in that case, option 1 will be probably the "fix".

[tchaikov]

some random thoughts on the implementation of the new virtual function. because getsockname() is a system call, we have following options:

• provide a fake local address. i think this will be a temporary solution.
• pretend that it's not. and provide it in network_stack.
• retrieve the local address beforehand, and cache it. then always return the cached address from this virtual function. also, we need to update network_stack to expose this interface.

[elcallio]

We should implement it in the nw stack imho. See also network_interface (i.e. maybe something like socket::get_interface()).

[bradh352]

please see c-ares/c-ares#894

The current list of callbacks in that PR for overriding OS calls are:

• socket()
• close()
• setsockopt()
• connect()
• recvfrom()
• sendto()
• getsockname()
• bind()

I do need to know, however, if your network stack would need to override the OS for these functions as well:

• if_nametoindex()
• if_indextoname()
• gethostname()

Finally, I should mention this note is no longer valid in your source code:

seastar/src/net/dns.cc

Line 1017 in bddc7b2

// 1.) c-ares does not handle EWOULDBLOCK for

And you also should never use ares_fds(), you're just asking for issues for multiple reasons (such as if a file descriptor value goes > 1024, crash!), the fact that you're doing an exhaustive O(n) search, and there's no ability to notify you if a new file descriptor needs to be monitored due to a new enqueued query coming from a separate thread. You really need to use the socket state callbacks, we have an example here: https://c-ares.org/docs.html#examples (after the event thread example).

Speaking of event thread, if you did want to be able to use that (rather than your own event loop), we'd need to extend the callbacks even more to support some sort of polling api.

[tchaikov]

I do need to know, however, if your network stack would need to override the OS for these functions as well:

* if_nametoindex()

* if_indextoname()

* gethostname()

replied at c-ares/c-ares#893

Finally, I should mention this note is no longer valid in your source code:

seastar/src/net/dns.cc

Line 1017 in bddc7b2
// 1.) c-ares does not handle EWOULDBLOCK for

thank you for pointing this out. will send a PR shortly.

And you also should never use ares_fds(), you're just asking for issues for multiple reasons (such as if a file descriptor value goes > 1024, crash!), the fact that you're doing an exhaustive O(n) search, and there's no ability to notify you if a new file descriptor needs to be monitored due to a new enqueued query coming from a separate thread. You really need to use the socket state callbacks, we have an example here: https://c-ares.org/docs.html#examples (after the event thread example).

thank you again. we do have an issue tracking this, see #2197 . it seems that ARES_OPT_SOCK_STATE_CB is a better fit in our use case, i've been experimenting it recently. still need to figure out some timeout issues -- probably i just fail to call ares_process_fd() somewhere.

Speaking of event thread, if you did want to be able to use that (rather than your own event loop), we'd need to extend the callbacks even more to support some sort of polling api.