[PATCH gwproxy v1 0/3] Initial work for DNS lookup implementation

[PATCH gwproxy v1 0/3] Initial work for DNS lookup implementation

[Ahmad Gani]

Hi Chief,

This is initial work of dns lookup feature, The patches aren't final, but
it's enough to get a grasp of what the interface looks like. I've 
provided a temporary entry point main function to test it.

There are 3 patches in this series:
- create net.h and net.c to store network related functionality.
- allow only port string number in service parameter of gwp_dns_queue
- initial work for implementation of C-ares-like getaddrinfo function

I propose to move the utility or helper function (that is commonly used 
in multiple places) to net.h and net.c, e.g., struct gwp_sockaddr is 
useful, and it possibly will be used in multiple places as the project 
expands.

I think it would be better to limit the service field to only contain the
port number, as SOCKS5 always uses a string port number.

in reply to Telegram chat [1]:
> (( It's the only reason we need a dedicated thread for resolving DNS ))

Is it preferred to use the current model (spawning dedicated DNS threads)
and make the behavior of the resolver the same as getaddrinfo (blocking)?
So far I've created the addrinfo interfaces with C-ares style and tested 
it; although, it's still blocking.

I would like to know the numbers for comparison between the thread model 
vs. the asynchronous model. Which approach is best for this scenario/case
(DNS resolution)?

I feel like multi-threading is much faster than single-threading, as it's
executed in true parallel (on a multi-core system) compared to 
concurrently doing things with an event notification mechanism.

My general understanding of parallel and concurrent is derived from this 
definition on the Stack Overflow answer [2]:
> Concurrency is when two or more tasks can start, run, and complete in 
> overlapping time periods. It doesn't necessarily mean they'll ever both
> be running at the same instant. For example, multitasking on
> a single-core machine. Parallelism is when tasks literally run at the 
> same time, e.g., on a multicore processor.

I also noticed that in the C-ares example [3], they recommend using the 
event thread example, and it is similar to the io_uring model in my 
perspective, where the operation is executed internally instead of 
letting the caller poll for readiness with ares_process_fd. Maybe we can 
mimic this aspect? I will remove the callback and make it blocking to 
make it compatible with the current thread model, but let's hear your 
opinion first.

I would like to request at least 40 revisions [4] to ensure I've learned 
something, so please treat me like I'm a dumb AI agent [5] (prompt me 
anything, I need more input!) and please review, thanks!

[1]:
- https://t.me/GNUWeeb/1192156

[2]:
- https://stackoverflow.com/a/1050257/22382954

[3]:
- https://c-ares.org/docs.html#examples

[4]:
- https://t.me/GNUWeeb/625966
- https://lore.kernel.org/lkml/20201104145430.300542-1-jarkko.sakkinen@linux.intel.com/T/

[5]:
- https://www.youtube.com/watch?v=P_O0ynyLOtI

Ahmad Gani (3):
  dnslookup: split common functionality and struct into net.c
  dnslookup: Allow only port string number
  dnslookup: Initial work for implementation of C-ares-like getaddrinfo
    function

 Makefile                |   1 +
 man/gwp_dns_queue.3     |   2 +-
 src/gwproxy/dns.h       |  12 +-
 src/gwproxy/dnslookup.c | 286 +++++++++++++++++++++++++++++++
 src/gwproxy/dnslookup.h | 110 ++++++++++++
 src/gwproxy/dnsparser.c | 362 ++++++++++++++++++++++++++++++++++++++++
 src/gwproxy/dnsparser.h | 188 +++++++++++++++++++++
 src/gwproxy/gwproxy.c   |  29 ----
 src/gwproxy/net.c       | 108 ++++++++++++
 src/gwproxy/net.h       |  27 +++
 10 files changed, 1085 insertions(+), 40 deletions(-)
 create mode 100644 src/gwproxy/dnslookup.c
 create mode 100644 src/gwproxy/dnslookup.h
 create mode 100644 src/gwproxy/dnsparser.c
 create mode 100644 src/gwproxy/dnsparser.h
 create mode 100644 src/gwproxy/net.c
 create mode 100644 src/gwproxy/net.h


base-commit: 0753f2d766e85fcbffc1f83dfd4e67d6206591cb
-- 
Ahmad Gani

[PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Ahmad Gani]

It's better to split these common function that used in more
than one place.

Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
 Makefile              |   1 +
 src/gwproxy/dns.h     |   9 +---
 src/gwproxy/gwproxy.c |  29 ------------
 src/gwproxy/net.c     | 108 ++++++++++++++++++++++++++++++++++++++++++
 src/gwproxy/net.h     |  27 +++++++++++
 5 files changed, 137 insertions(+), 37 deletions(-)
 create mode 100644 src/gwproxy/net.c
 create mode 100644 src/gwproxy/net.h

diff --git a/Makefile b/Makefile
index d9952a12b2d2..76e12e0ae739 100644
--- a/Makefile
+++ b/Makefile
@@ -37,6 +37,7 @@ GWPROXY_TARGET = gwproxy
 GWPROXY_CC_SOURCES = \
 	$(GWPROXY_DIR)/gwproxy.c \
 	$(GWPROXY_DIR)/log.c \
+	$(GWPROXY_DIR)/net.c \
 	$(GWPROXY_DIR)/ev/epoll.c
 
 
diff --git a/src/gwproxy/dns.h b/src/gwproxy/dns.h
index 7d62d3f9b89e..290325d4f989 100644
--- a/src/gwproxy/dns.h
+++ b/src/gwproxy/dns.h
@@ -9,17 +9,10 @@
 #include <stdatomic.h>
 #include <stdbool.h>
 #include <netinet/in.h>
+#include <gwproxy/net.h>
 
 struct gwp_dns_wrk;
 
-struct gwp_sockaddr {
-	union {
-		struct sockaddr		sa;
-		struct sockaddr_in	i4;
-		struct sockaddr_in6	i6;
-	};
-};
-
 struct gwp_dns_entry {
 	char			*name;
 	char			*service;
diff --git a/src/gwproxy/gwproxy.c b/src/gwproxy/gwproxy.c
index f465f82446ec..5bc4f3ff62bb 100644
--- a/src/gwproxy/gwproxy.c
+++ b/src/gwproxy/gwproxy.c
@@ -270,35 +270,6 @@ static inline ssize_t gwp_eventfd_write(int fd, uint64_t val)
 	return 0;
 }
 
-__hot
-static int convert_ssaddr_to_str(char buf[FULL_ADDRSTRLEN],
-				 const struct gwp_sockaddr *gs)
-{
-	int f = gs->sa.sa_family;
-	uint16_t port = 0;
-	size_t l;
-
-	if (f == AF_INET) {
-		if (!inet_ntop(f, &gs->i4.sin_addr, buf, INET_ADDRSTRLEN))
-			return -EINVAL;
-		l = strlen(buf);
-		port = ntohs(gs->i4.sin_port);
-	} else if (f == AF_INET6) {
-		buf[0] = '[';
-		if (!inet_ntop(f, &gs->i6.sin6_addr, buf + 1, INET6_ADDRSTRLEN))
-			return -EINVAL;
-		l = strlen(buf);
-		buf[l++] = ']';
-		port = ntohs(gs->i6.sin6_port);
-	} else {
-		return -EINVAL;
-	}
-
-	buf[l++] = ':';
-	snprintf(buf + l, FULL_ADDRSTRLEN - l, "%hu", port);
-	return 0;
-}
-
 __hot
 const char *ip_to_str(const struct gwp_sockaddr *gs)
 {
diff --git a/src/gwproxy/net.c b/src/gwproxy/net.c
new file mode 100644
index 000000000000..7427f87ae300
--- /dev/null
+++ b/src/gwproxy/net.c
@@ -0,0 +1,108 @@
+#include <stdio.h>
+#include <stddef.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <gwproxy/net.h>
+
+int init_addr(const char *addr, struct gwp_sockaddr *addr_st, uint16_t port)
+{
+	char *separator, *ipstr, *port_str;
+	char tmp[FULL_ADDRSTRLEN];
+	unsigned short nport, af;
+	struct sockaddr_in6 *i6;
+	struct sockaddr_in *i4;
+	size_t addrlen;
+	int i, hport;
+
+	i6 = (void *)addr_st;
+	i4 = (void *)addr_st;
+	separator = NULL;
+	addrlen = strlen(addr) + 1;
+	if (addrlen > FULL_ADDRSTRLEN)
+		return -EINVAL;
+
+	strncpy(tmp, addr, FULL_ADDRSTRLEN);
+	for (i = addrlen - 1; i > 0; i--) {
+		if (tmp[i] == ':') {
+			separator = &tmp[i];
+			break;
+		}
+	}
+
+	if (separator) {
+		*separator = '\0';
+
+		port_str = separator + 1;
+		hport = atoi(port_str);
+		if (!hport)
+			return -EINVAL;
+		if (hport > 65535 || hport < 0)
+			return -EINVAL;
+		nport = htons(hport);
+	} else {
+		nport = htons(port);
+	}
+
+	if (*tmp == '[') {
+		af = AF_INET6;
+		/* replace ']' with null-terminated byte */
+		if (separator) {
+			if (*(separator - 1) != ']')
+				return -EINVAL;
+			*(separator - 1) = '\0';
+		} else {
+			tmp[addrlen - 1] = '\0';
+		}
+		ipstr = tmp + 1;
+	} else {
+		af = AF_INET;
+		ipstr = tmp;
+	}
+
+	addr_st->sa.sa_family = af;
+	switch (af) {
+	case AF_INET:
+		i4->sin_port = nport;
+		if (!inet_pton(AF_INET, ipstr, &i4->sin_addr))
+			return -EINVAL;
+
+		break;
+	case AF_INET6:
+		i6->sin6_port = nport;
+		if (!inet_pton(AF_INET6, ipstr, &i6->sin6_addr))
+			return -EINVAL;
+
+		break;
+	}
+
+	return 0;
+}
+
+int convert_ssaddr_to_str(char buf[FULL_ADDRSTRLEN],
+			const struct gwp_sockaddr *gs)
+{
+	int f = gs->sa.sa_family;
+	uint16_t port = 0;
+	size_t l;
+
+	if (f == AF_INET) {
+		if (!inet_ntop(f, &gs->i4.sin_addr, buf, INET_ADDRSTRLEN))
+			return -EINVAL;
+		l = strlen(buf);
+		port = ntohs(gs->i4.sin_port);
+	} else if (f == AF_INET6) {
+		buf[0] = '[';
+		if (!inet_ntop(f, &gs->i6.sin6_addr, buf + 1, INET6_ADDRSTRLEN))
+			return -EINVAL;
+		l = strlen(buf);
+		buf[l++] = ']';
+		port = ntohs(gs->i6.sin6_port);
+	} else {
+		return -EINVAL;
+	}
+
+	buf[l++] = ':';
+	snprintf(buf + l, FULL_ADDRSTRLEN - l, "%hu", port);
+	return 0;
+}
diff --git a/src/gwproxy/net.h b/src/gwproxy/net.h
new file mode 100644
index 000000000000..ebfebca5d840
--- /dev/null
+++ b/src/gwproxy/net.h
@@ -0,0 +1,27 @@
+/*
+ * utility for network-related stuff
+ */
+
+#include <arpa/inet.h>
+
+#define FULL_ADDRSTRLEN (INET6_ADDRSTRLEN + sizeof(":65535[]") - 1)
+
+struct gwp_sockaddr {
+	union {
+		struct sockaddr		sa;
+		struct sockaddr_in	i4;
+		struct sockaddr_in6	i6;
+	};
+};
+
+/*
+ * Initialize address structure
+ *
+ * @param addr
+ * @param addr_st
+ * @param port in host byte order when the string does not contain port
+ */
+int init_addr(const char *addr, struct gwp_sockaddr *addr_st, uint16_t port);
+
+int convert_ssaddr_to_str(char buf[FULL_ADDRSTRLEN],
+			const struct gwp_sockaddr *gs);

base-commit: 0753f2d766e85fcbffc1f83dfd4e67d6206591cb
-- 
Ahmad Gani

[PATCH gwproxy v1 2/3] dnslookup: Allow only port string number

[Ahmad Gani]

Instead of adding additional work for parsing well-known services like
http, https, etc... (port in socks5 field is a number anyway).

Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
 man/gwp_dns_queue.3 | 2 +-
 src/gwproxy/dns.h   | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/man/gwp_dns_queue.3 b/man/gwp_dns_queue.3
index dcd06746bbcc..f57807517df5 100644
--- a/man/gwp_dns_queue.3
+++ b/man/gwp_dns_queue.3
@@ -77,7 +77,7 @@ contains:
 The hostname being resolved.
 .TP
 .B char *service
-The service name or port number (e.g., "http", "443").
+The port number in ascii format.
 .TP
 .B _Atomic(int) refcnt
 Atomic reference count for the entry.
diff --git a/src/gwproxy/dns.h b/src/gwproxy/dns.h
index 290325d4f989..3bd437207d99 100644
--- a/src/gwproxy/dns.h
+++ b/src/gwproxy/dns.h
@@ -71,8 +71,7 @@ void gwp_dns_ctx_free(struct gwp_dns_ctx *ctx);
  *
  * @param ctx		Pointer to the DNS context.
  * @param name		Name to resolve.
- * @param service 	Service to resolve (e.g., "http", "https", or port
- *			number).
+ * @param service 	Service to resolve in port number ascii format.
  * @return		Pointer to a gwp_dns_entry on success, NULL on failure.
  */
 struct gwp_dns_entry *gwp_dns_queue(struct gwp_dns_ctx *ctx,
-- 
Ahmad Gani

[PATCH gwproxy v1 3/3] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function

[Ahmad Gani]

Introducing glibc's getaddrinfo replacement, the DNS protocol
implementation is limited to standard query (OPCODE_QUERY) as for now,
but may extended later as necessary.

Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
 src/gwproxy/dnslookup.c | 286 ++++++++++++++++++++++++++++++++
 src/gwproxy/dnslookup.h | 118 +++++++++++++
 src/gwproxy/dnsparser.c | 357 ++++++++++++++++++++++++++++++++++++++++
 src/gwproxy/dnsparser.h | 191 +++++++++++++++++++++
 4 files changed, 952 insertions(+)
 create mode 100644 src/gwproxy/dnslookup.c
 create mode 100644 src/gwproxy/dnslookup.h
 create mode 100644 src/gwproxy/dnsparser.c
 create mode 100644 src/gwproxy/dnsparser.h

diff --git a/src/gwproxy/dnslookup.c b/src/gwproxy/dnslookup.c
new file mode 100644
index 000000000000..5b87e7248d54
--- /dev/null
+++ b/src/gwproxy/dnslookup.c
@@ -0,0 +1,286 @@
+/*
+ * DNS lookup: an implementation of DNS resolution for IPv4 and IPv6.
+ */
+
+#include <gwproxy/net.h>
+#include <gwproxy/dnslookup.h>
+#include <gwproxy/dnsparser.h>
+#include <gwproxy/syscall.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t nport,
+	struct gw_ares_addrinfo *result, struct gw_addrinfo_node **tail)
+{
+	uint8_t send_buff[UDP_MSG_LIMIT];
+	uint8_t recv_buff[UDP_MSG_LIMIT];
+	gwdns_query_pkt *query_pkt;
+	gwdns_answ_data raw_answ;
+	gwdns_question_part q;
+	ssize_t buff_len;
+	int ret;
+
+	q.domain = name;
+	q.type = type;
+	q.dst_buffer = send_buff;
+	q.dst_len = UDP_MSG_LIMIT;
+	buff_len = construct_question(&q);
+	if (buff_len < 0) {
+		/*
+		 * I'm confident that 512 bytes is more than sufficient
+		 * to construct single dns query packet.
+		 */
+		assert(buff_len != -ENOBUFS);
+		ret = GW_ARES_EINVAL;
+		return ret;
+	}
+	query_pkt = (void *)send_buff;
+
+attempt_retry:
+	ret = __sys_send(sockfd, send_buff, buff_len, MSG_NOSIGNAL);
+	if (ret < 0) {
+		ret = GW_ARES_INTERNAL_ERR;
+		return ret;
+	}
+
+	if (ret != buff_len)
+		goto attempt_retry;
+
+	ret = __sys_recv(sockfd, recv_buff, UDP_MSG_LIMIT, MSG_NOSIGNAL);
+	if (ret < 0) {
+		ret = GW_ARES_INTERNAL_ERR;
+		return ret;
+	}
+
+	/* 
+	 * TODO(reyuki): even though it's unlikely,
+	 * but what todo when the connection is closed?
+	 * drop the request or retry?
+	 */
+	assert(ret);
+
+	ret = serialize_answ(query_pkt->hdr.id, recv_buff, ret, &raw_answ);
+	if (ret) {
+		/*
+		 * TODO(reyuki): the reason of failure can vary,
+		 * but what to do in that case? EAGAIN could possibly indicate
+		 * short recv, but this is UDP packet, retry from send?
+		 * or just drop the request?
+		 */
+	}
+	assert(!ret);
+
+	/* TODO(reyuki): hints->ai_family is used to filter results */
+	for (size_t i = 0; i < raw_answ.hdr.ancount; i++) {
+		gwdns_serialized_answ *answ = raw_answ.rr_answ[i];
+		struct gw_addrinfo_node *new_node = malloc(sizeof(*new_node));
+		if (!new_node) {
+			ret = GW_ARES_ENOMEM;
+			goto exit_free;
+		}
+		new_node->ai_next = NULL;
+
+		if (answ->rr_type == TYPE_AAAA) {
+			new_node->ai_family = AF_INET6;
+			new_node->ai_addrlen = sizeof(new_node->ai_addr.i6);
+			new_node->ai_addr.i6.sin6_port = nport;
+			new_node->ai_addr.i6.sin6_family = AF_INET6;
+			/*
+			 * no overflow.
+			 * it's guaranteed to be true by serialize_answ function
+			 */
+			assert(sizeof(new_node->ai_addr.i6.sin6_addr) == answ->rdlength);
+			memcpy(&new_node->ai_addr.i6.sin6_addr, answ->rdata, answ->rdlength);
+		} else {
+			new_node->ai_family = AF_INET;
+			new_node->ai_addrlen = sizeof(new_node->ai_addr.i4);
+			new_node->ai_addr.i4.sin_port = nport;
+			new_node->ai_addr.i4.sin_family = AF_INET;
+			/*
+			 * no overflow.
+			 * it's guaranteed to be true by serialize_answ function
+			 */
+			assert(sizeof(new_node->ai_addr.i4.sin_addr) == answ->rdlength);
+			memcpy(&new_node->ai_addr.i4.sin_addr, answ->rdata, answ->rdlength);
+			new_node->ai_ttl = answ->ttl;
+		}
+
+		if (!*tail)
+			result->nodes = new_node;
+		else
+			(*tail)->ai_next = new_node;
+		*tail = new_node;
+	}
+
+	ret = GW_ARES_SUCCESS;
+exit_free:
+	free_serialize_answ(&raw_answ);
+	return ret;
+}
+
+void gw_ares_getaddrinfo(gw_ares_channel_t *channel,
+			const char *name, const char *service,
+			const struct gw_ares_addrinfo_hints *hints,
+			gw_ares_addrinfo_callback callback, void *arg)
+{
+	struct gw_ares_addrinfo *result;
+	struct gw_addrinfo_node *tail;
+	struct gwp_sockaddr *addr;
+	socklen_t addrlen;
+	int ret, sockfd;
+	uint16_t nport;
+	uint8_t mask;
+
+	switch (hints->ai_family) {
+		case AF_UNSPEC:
+			mask = I6_BIT | I4_BIT;
+			break;
+		case AF_INET:
+			mask = I4_BIT;
+			break;
+		case AF_INET6:
+			mask = I6_BIT;
+			break;
+		default:
+			ret = GW_ARES_EINVAL;
+			goto error;
+	}
+
+	nport = (uint16_t)atoi(service);
+	if (!nport) {
+		ret = GW_ARES_EINVAL;
+		goto error;
+	}
+	nport = htons(nport);
+
+	result = malloc(sizeof(*result));
+	if (!result) {
+		ret = GW_ARES_ENOMEM;
+		goto error;
+	}
+
+	addr = &channel->servers[0];
+	sockfd = __sys_socket(addr->sa.sa_family, SOCK_DGRAM, 0);
+	if (sockfd < 0) {
+		ret = GW_ARES_INTERNAL_ERR;
+		goto error_free;
+	}
+
+	addrlen = addr->sa.sa_family == AF_INET ? sizeof(addr->i4) : sizeof(addr->i6);
+	ret = __sys_connect(sockfd, &addr->sa, addrlen);
+	if (ret < 0) {
+		ret = GW_ARES_INTERNAL_ERR;
+		goto error_close;
+	}
+
+	tail = NULL;
+	if (IS_I6(mask)) {
+		ret = resolve_name(sockfd, name, TYPE_AAAA, nport, result, &tail);
+		if (ret)
+			goto error_close;
+	}
+
+	if (IS_I4(mask)) {
+		ret = resolve_name(sockfd, name, TYPE_A, nport, result, &tail);
+		if (ret)
+			goto error_close;
+	}
+
+	callback(arg, GW_ARES_SUCCESS, result);
+	return;
+error_close:
+	__sys_close(sockfd);
+error_free:
+	free(result);
+error:
+	callback(arg, ret, result);
+}
+
+void gw_ares_freeaddrinfo(struct gw_ares_addrinfo *ai)
+{
+	struct gw_addrinfo_node *tmp, *node = ai->nodes;
+	while (node) {
+		tmp = node->ai_next;
+		free(node);
+		node = tmp;
+	}
+
+	free(ai);
+}
+
+int gw_ares_init(gw_ares_channel_t **channel, struct gw_ares_options *opts)
+{
+	gw_ares_channel_t *c;
+	int ret;
+
+	if (!opts->nr_server)
+		return -EINVAL;
+
+	*channel = malloc(sizeof(**channel));
+	if (!*channel)
+		return -ENOMEM;
+
+	c = *channel;
+	c->nr_server = opts->nr_server;
+	c->servers = malloc(c->nr_server * sizeof(*c->servers));
+	if (!c->servers) {
+		free(*channel);
+		return -ENOMEM;
+	}
+	/*
+	 * TODO(reyuki): validate flags and
+	 * for now use it to control recursion desired (RD) bit?
+	 */
+	c->flags = opts->flags;
+	for (int i = 0; i < c->nr_server; i++) {
+		ret = init_addr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+void gw_ares_deinit(gw_ares_channel_t *channel)
+{
+	free(channel->servers);
+	free(channel);
+}
+
+static void gw_ares_cb(void *arg, int status, struct gw_ares_addrinfo *result)
+{
+	struct gw_addrinfo_node *node;
+	char buf[FULL_ADDRSTRLEN];
+
+	(void)arg;
+
+	assert(!status);
+	node = result->nodes;
+	while (node) {
+		int r = convert_ssaddr_to_str(buf, &node->ai_addr);
+		assert(!r);
+		printf("%s: %s\n", node->ai_family == AF_INET6 ? "IPv6" : "IPv4", buf);
+		node = node->ai_next;
+	}
+	gw_ares_freeaddrinfo(result);
+}
+
+int main(void)
+{
+	gw_ares_channel_t *channel;
+	struct gw_ares_addrinfo_hints hints = {
+		.ai_family = AF_UNSPEC
+	};
+	const char *servers[] = {"1.1.1.1", "8.8.8.8"};
+	struct gw_ares_options opts = {
+		.flags = 0,
+		.nr_server = 1,
+		.servers = servers
+	};
+	int ret;
+	ret = gw_ares_init(&channel, &opts);
+	if (ret)
+		return -EXIT_FAILURE;
+	gw_ares_getaddrinfo(channel, "google.com", "80", &hints, gw_ares_cb, NULL);
+	gw_ares_deinit(channel);
+}
diff --git a/src/gwproxy/dnslookup.h b/src/gwproxy/dnslookup.h
new file mode 100644
index 000000000000..a4ed02cab5f1
--- /dev/null
+++ b/src/gwproxy/dnslookup.h
@@ -0,0 +1,118 @@
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <gwproxy/net.h>
+
+#define DEFAULT_DOMAIN_PORT 53
+#define I6_BIT (1u << 0)
+#define I4_BIT (1u << 1)
+#define IS_I6(X) (((X) & I6_BIT) != 0)
+#define IS_I4(X) (((X) & I4_BIT) != 0)
+
+enum {
+	GW_ARES_SUCCESS		= 0,
+	GW_ARES_ENOMEM		= 1,
+	GW_ARES_EINVAL		= 2,
+
+	/*
+	 * internal error can be interpreted as system call failure, 
+	 * however, the cause of it can be vary.
+	 */
+	GW_ARES_INTERNAL_ERR	= 3
+};
+
+struct gw_ares_options {
+	int		flags;
+	int		nr_server;
+
+	/* 
+	 * the string format is ip:port, the ip may be encapsulated with square
+	 * brackets ([]), and must be if using IPv6.
+	 */
+	const char	**servers;
+};
+
+struct gw_ares_channeldata {
+	int			flags;
+	int			nr_server;
+	/* currently only index 0 is used, and others are ignored. */
+	struct gwp_sockaddr	*servers;
+};
+
+typedef struct gw_ares_channeldata gw_ares_channel_t;
+
+struct gw_ares_addrinfo_hints {
+	int ai_family;
+};
+
+/*
+ * gw_addrinfo_node structure is similar to RFC 3493 addrinfo,
+ * but without canonname and with extra ttl field.
+ * 
+ * - https://c-ares.org/docs/ares_getaddrinfo.html
+ */
+struct gw_addrinfo_node {
+	int			ai_family;
+	int			ai_ttl;
+	socklen_t		ai_addrlen;
+	struct gwp_sockaddr	ai_addr;
+	struct gw_addrinfo_node	*ai_next;
+};
+
+struct gw_ares_addrinfo {
+	struct gw_addrinfo_node *nodes;
+	char			*name;
+};
+
+/*
+ * result is only initialized if status == GW_ARES_SUCCESS.
+ */
+typedef void (*gw_ares_addrinfo_callback)(void *arg, int status,
+					struct gw_ares_addrinfo *result);
+
+/* 
+ * Initiate a host query by name and service
+ * 
+ * Description:
+ * the gw_getaddrinfo function initiate a host query
+ * by @name on the name service channel identified by @channel
+ * 
+ * @param channel
+ * @param name
+ * @param service
+ * @param hints
+ * @param callback
+ * @param arg
+ */
+void gw_ares_getaddrinfo(gw_ares_channel_t *channel,
+			const char *name, const char *service,
+			const struct gw_ares_addrinfo_hints *hints,
+			gw_ares_addrinfo_callback callback, void *arg);
+
+/*
+ * Free the resources allocated by gw_ares_getaddrinfo.
+ * 
+ * @param ai
+ */
+void gw_ares_freeaddrinfo(struct gw_ares_addrinfo *ai);
+
+/*
+ * Initialize name service communication channel.
+ * 
+ * Description:
+ * the gw_ares_init function initialize a communication channel for name
+ * service lookups.
+ * 
+ * It is recommended for an application to have at most one channel and use this
+ * for all DNS queries for the life of the application.
+ * 
+ * gw_ares_init can return any of the following values when an error occured:
+ * -EINVAL invalid options
+ * -ENOMEM insufficient memory
+ * 
+ * @param channel pointer to initialize
+ * @param opts controlling the behavior of the resolver
+ * @return zero on success and negative integer on error
+ */
+int gw_ares_init(gw_ares_channel_t **channel, struct gw_ares_options *opts);
+void gw_ares_deinit(gw_ares_channel_t *channel);
diff --git a/src/gwproxy/dnsparser.c b/src/gwproxy/dnsparser.c
new file mode 100644
index 000000000000..9e84f539ccde
--- /dev/null
+++ b/src/gwproxy/dnsparser.c
@@ -0,0 +1,357 @@
+#define _DEFAULT_SOURCE
+#include <endian.h>
+#include <gwproxy/dnsparser.h>
+
+static ssize_t construct_qname(uint8_t *dst, size_t dst_len, const char *qname)
+{
+	const uint8_t *p = (const uint8_t *)qname;
+	uint8_t *lp = dst; // Length position.
+	uint8_t *sp = lp + 1;  // String position.
+	size_t total = 0;
+	uint16_t l;
+
+	l = 0;
+	while (1) {
+		uint8_t c = *p++;
+
+		total++;
+		if (total >= dst_len)
+			return -ENAMETOOLONG;
+
+		if (c == '.' || c == '\0') {
+			if (l < 1 || l > 255)
+				return -EINVAL;
+
+			*lp = (uint8_t)l;
+			lp = sp++;
+			l = 0;
+			if (!c)
+				break;
+		} else {
+			l++;
+			*sp = c;
+			sp++;
+		}
+	}
+
+	return total;
+}
+
+static ssize_t calculate_question_len(uint8_t *in, size_t in_len)
+{
+	const uint8_t *p = in;
+	size_t tot_len, advance_len;
+
+	tot_len = 0;
+	while (true) {
+		if (*p == 0x0)
+			break;
+
+		if (tot_len >= in_len)
+			return -ENAMETOOLONG;
+
+		advance_len = *p + 1;
+		tot_len += advance_len;
+		p += advance_len;
+	}
+
+	return  tot_len;
+}
+
+int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
+{
+	size_t advance_len, first_len;
+	gwdns_header_pkt *hdr;
+	uint16_t raw_flags;
+	int ret;
+
+	advance_len = sizeof(*hdr);
+	if (in_len < advance_len)
+		return -EAGAIN;
+
+	hdr = (void *)in;
+	if (memcmp(&txid, &hdr->id, sizeof(txid)))
+		return -EINVAL;
+
+	memcpy(&raw_flags, &in[2], sizeof(raw_flags));
+	raw_flags = ntohs(raw_flags);
+	/* QR MUST 1 = response from dns server */
+	if (!DNS_QR(raw_flags))
+		return -EINVAL;
+
+	/* OPCODE MUST 0 = standard query */
+	if (DNS_OPCODE(raw_flags))
+		return -EINVAL;
+	
+	/* RCODE MUST 0 = No error */
+	if (DNS_RCODE(raw_flags))
+		return -EPROTO;
+
+	// is it safe or recommended to alter the in buffer directly?
+	hdr->ancount = ntohs(hdr->ancount);
+	if (!hdr->ancount)
+		return -ENODATA;
+
+	in += advance_len;
+	in_len -= advance_len;
+
+	first_len = 1 + in[0];
+	advance_len = first_len + 1 + 2 + 2;
+	if (in_len < advance_len)
+		return -EAGAIN;
+
+	ret = calculate_question_len(in, in_len);
+	if (ret < 0)
+		return -EINVAL;
+
+	advance_len -= first_len;
+	advance_len += ret;
+	if (in_len < advance_len)
+		return -EAGAIN;
+
+	in += advance_len;
+	in_len -= advance_len;
+	out->hdr.ancount = 0;
+	out->rr_answ = malloc(hdr->ancount * sizeof(uint8_t *));
+	if (!out->rr_answ)
+		return -ENOMEM;
+
+	for (size_t i = 0; i < hdr->ancount; i++) {
+		uint16_t is_compressed, rdlength;
+		gwdns_serialized_answ *item = malloc(sizeof(gwdns_serialized_answ));
+		if (!item) {
+			ret = -ENOMEM;
+			goto exit_free;
+		}
+
+		out->rr_answ[i] = item;
+
+		memcpy(&is_compressed, in, sizeof(is_compressed));
+		is_compressed = DNS_IS_COMPRESSED(ntohs(is_compressed));
+		assert(is_compressed);
+		in += 2; // NAME
+
+		memcpy(&item->rr_type, in, 2);
+		item->rr_type = ntohs(item->rr_type);
+		in += 2; // TYPE
+		memcpy(&item->rr_class, in, 2);
+		item->rr_class = ntohs(item->rr_class);
+		in += 2; // CLASS
+		memcpy(&item->ttl, in, 4);
+		item->ttl = be32toh(item->ttl);
+		in += 4; // TTL
+
+		memcpy(&rdlength, in, sizeof(rdlength));
+		rdlength = ntohs(rdlength);
+		if (item->rr_type != TYPE_AAAA && item->rr_type != TYPE_A) {
+			ret = -EINVAL;
+			free(item);
+			goto exit_free;
+		}
+		if (item->rr_type == TYPE_AAAA && rdlength != sizeof(struct in6_addr)) {
+			ret = -EINVAL;
+			free(item);
+			goto exit_free;
+		}
+		if (item->rr_type == TYPE_A && rdlength != sizeof(struct in_addr)) {
+			ret = -EINVAL;
+			free(item);
+			goto exit_free;
+		}
+		item->rdlength = rdlength;
+		in += 2;
+
+		/*
+		 * considering if condition above,
+		 * maybe we don't need a malloc and just allocate fixed size
+		 * for rdata? however if this parser want to be expanded for
+		 * other dns operation (e.g OPCODE_IQUERY, etc), rdata maybe
+		 * contain more than sizeof in6_addr.
+		 */
+		item->rdata = malloc(rdlength);
+		if (!item->rdata) {
+			ret = -ENOMEM;
+			free(item);
+			goto exit_free;
+		}
+		memcpy(item->rdata, in, rdlength);
+		in += rdlength;
+		out->hdr.ancount++;
+	}
+
+	return 0;
+exit_free:
+	for (size_t i = 0; i < out->hdr.ancount; i++) {
+		free(out->rr_answ[i]->rdata);
+		free(out->rr_answ[i]);
+	}
+	free(out->rr_answ);
+	return ret;
+}
+
+void free_serialize_answ(gwdns_answ_data *answ)
+{
+	for (size_t i = 0; i < answ->hdr.ancount; i++) {
+		free(answ->rr_answ[i]->rdata);
+		free(answ->rr_answ[i]);
+	}
+	free(answ->rr_answ);
+}
+
+ssize_t construct_question(gwdns_question_part *question)
+{
+	gwdns_header_pkt *hdr;
+	gwdns_query_pkt pkt;
+	uint16_t qtype, qclass;
+	size_t required_len;
+	ssize_t bw;
+
+	if (question->type != TYPE_AAAA && question->type != TYPE_A)
+		return -EINVAL;
+
+	hdr = &pkt.hdr;
+	/*
+	* the memset implicitly set opcode to OPCODE_QUERY
+	*/
+	memset(hdr, 0, sizeof(*hdr));
+	hdr->id = htons((uint16_t)rand());
+	DNS_SET_RD(hdr->flags, true);
+	hdr->flags = htons(hdr->flags);
+	hdr->qdcount = htons(1);
+
+	/*
+	* pkt.body is interpreted as question section
+	* for layout and format, see RFC 1035 4.1.2. Question section format
+	*/
+	bw = construct_qname(pkt.body, sizeof(pkt.body) - 3, question->domain);
+	if (bw < 0)
+		return bw;
+
+	pkt.body[bw++] = 0x0;
+	qtype = htons(question->type);
+	qclass = htons(CLASS_IN);
+	memcpy(&pkt.body[bw], &qtype, 2);
+	bw += 2;
+	memcpy(&pkt.body[bw], &qclass, 2);
+	bw += 2;
+
+	required_len = sizeof(pkt.hdr) + bw;
+	if (question->dst_len < required_len)
+		return -ENOBUFS;
+
+	memcpy(question->dst_buffer, &pkt, required_len);
+
+	return required_len;
+}
+
+#ifdef RUNTEST
+
+void test_simulate_ipv4query(void)
+{
+	char buff[UDP_MSG_LIMIT];
+	gwdns_query_pkt *send_pkt;
+	uint8_t recv_pkt[] = {
+		/* Header (12 bytes) */
+		0x00, 0x00,		/* transaction ID - STUB! */
+		0x81, 0x80,		/* Flags: QR=1, AA=0, RD=1, RA=1, RCODE=0 */
+		0x00, 0x01,		/* QDCOUNT = 1 */
+		0x00, 0x06,		/* ANCOUNT = 6 */
+		0x00, 0x00,		/* NSCOUNT = 0 */
+		0x00, 0x00,		/* ARCOUNT = 0 */
+		
+		/* Question Section */
+		/* Pointer label compression may be used in answers */
+		0x06, 'g','o','o','g','l','e',
+		0x03, 'c','o','m',
+		0x00,			/* Terminate name */
+		0x00, 0x01,		/* QTYPE = A */
+		0x00, 0x01,		/* QCLASS = IN */
+
+		/* Answer Section (6 records) */
+		/* Each Answer record: name pointer, type, class, ttl, rdlength, rdata */
+		/* First Answer */
+		0xC0, 0x0C,		/* Name: pointer to offset 0x0C (start of question name) */
+		0x00, 0x01,		/* TYPE = A */
+		0x00, 0x01,		/* CLASS = IN */
+		0x00, 0x00, 0x08, 0x62, /* TTL = 0x00000862 = 2146 sec */
+		0x00, 0x04,		/* RDLENGTH = 4 */
+		0x4A, 0x7D, 0x18, 0x71,	/* RDATA = 74.125.24.113 */
+
+		/* Second Answer */
+		0xC0, 0x0C,
+		0x00, 0x01,
+		0x00, 0x01,
+		0x00, 0x00, 0x08, 0x62,
+		0x00, 0x04,
+		0x4A, 0x7D, 0x18, 0x65, /* 74.125.24.101 */
+
+		/* Third Answer */
+		0xC0, 0x0C,
+		0x00, 0x01,
+		0x00, 0x01,
+		0x00, 0x00, 0x08, 0x62,
+		0x00, 0x04,
+		0x4A, 0x7D, 0x18, 0x8B, /* 74.125.24.139 */
+
+		/* Fourth Answer */
+		0xC0, 0x0C,
+		0x00, 0x01,
+		0x00, 0x01,
+		0x00, 0x00, 0x08, 0x62,
+		0x00, 0x04,
+		0x4A, 0x7D, 0x18, 0x8A, /* 74.125.24.138 */
+
+		/* Fifth Answer */
+		0xC0, 0x0C,
+		0x00, 0x01,
+		0x00, 0x01,
+		0x00, 0x00, 0x08, 0x62,
+		0x00, 0x04,
+		0x4A, 0x7D, 0x18, 0x64, /* 74.125.24.100 */
+
+		/* Sixth Answer */
+		0xC0, 0x0C,
+		0x00, 0x01,
+		0x00, 0x01,
+		0x00, 0x00, 0x08, 0x62,
+		0x00, 0x04,
+		0x4A, 0x7D, 0x18, 0x66, /* 74.125.24.102 */
+	};
+	gwdns_answ_data d;
+	char first_label[] = "google";
+	char second_label[] = "com";
+
+	memset(&d, 0, sizeof(d));
+	gwdns_question_part q = {
+		.domain = "google.com",
+		.dst_buffer = (uint8_t *)buff,
+		.dst_len = sizeof(buff)
+	};
+	assert(construct_question(&q) > 0);
+
+	assert(buff[12] == 6);
+	assert(!memcmp(&buff[13], first_label, 6));
+
+	assert(buff[13 + 6] == 3);
+	assert(!memcmp(&buff[13 + 6 + 1], second_label, 3));
+
+	// fill the STUB
+	memcpy(recv_pkt, buff, 2);
+
+	send_pkt = (void *)buff;
+	assert(!serialize_answ(send_pkt->hdr.id, recv_pkt, sizeof(recv_pkt), &d));
+}
+
+void run_all_tests(void)
+{
+	test_simulate_ipv4query();
+	fprintf(stderr, "all tests passed!\n");
+}
+
+int main(void)
+{
+	run_all_tests();
+	return 0;
+}
+
+#endif
diff --git a/src/gwproxy/dnsparser.h b/src/gwproxy/dnsparser.h
new file mode 100644
index 000000000000..41048568240b
--- /dev/null
+++ b/src/gwproxy/dnsparser.h
@@ -0,0 +1,191 @@
+#include <stdint.h>
+#include <stddef.h>
+#include <assert.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <arpa/inet.h>
+#include <sys/socket.h>
+#include <liburing.h>
+
+#ifndef __packed
+#define __packed __attribute__((__packed__))
+#endif
+
+/*
+ * 4. MESSAGES
+ * 4.1. Format
+ *
+ * All communications inside of the domain protocol are carried in a single
+ * format called a message. The top-level format of a message is divided
+ * into 5 sections (some of which may be empty in certain cases), shown below:
+ *
+ *     +---------------------+
+ *     |        Header       |
+ *     +---------------------+
+ *     |       Question      | the question for the name server
+ *     +---------------------+
+ *     |        Answer       | RRs answering the question
+ *     +---------------------+
+ *     |      Authority      | RRs pointing toward an authority
+ *     +---------------------+
+ *     |      Additional     | RRs holding additional information
+ *     +---------------------+
+ *
+ * These sections are defined in RFC 1035 §4.1. The Header section is always
+ * present and includes fields that specify which of the other sections follow,
+ * as well as metadata such as whether the message is a query or response,
+ * the opcode, etc.
+ */
+
+/* Flag bit position in little-endian machine */
+#define DNS_QR_BIT		0xF
+#define DNS_OPCODE_BIT		0xB	// 4-bit field
+#define DNS_AA_BIT		0xA
+#define DNS_TC_BIT		0x9
+#define DNS_RD_BIT		0x8
+#define DNS_RA_BIT		0x7
+#define DNS_Z_BIT		0x4	// 3-bit field
+#define DNS_RCODE_BIT		0x0	// 4-bit field
+#define DNS_COMPRESSION_BIT	(0x3 << 0xE)
+
+/* Flag extraction macros for listtle-endian machine */
+#define DNS_QR(flags)		(((flags) >> DNS_QR_BIT) & 0x1)
+#define DNS_OPCODE(flags)	(((flags) >> DNS_OPCODE_BIT) & 0xF)
+#define DNS_RCODE(flags)	((flags) & 0xF)
+#define DNS_IS_COMPRESSED(mask) ((mask) & DNS_COMPRESSION_BIT)
+
+/* Flag construction macros for little-endian machine */
+#define DNS_SET_RD(flags, val)	(flags) = ((flags) & ~(1 << DNS_RD_BIT)) | ((!!(val)) << DNS_RD_BIT)
+
+/* as per RFC 1035 §2.3.4. Size limits */
+#define DOMAIN_LABEL_LIMIT 63
+#define DOMAIN_NAME_LIMIT 255
+#define UDP_MSG_LIMIT 512
+
+typedef enum {
+	OPCODE_QUERY		= 0,	// Standard query (QUERY)
+	OPCODE_IQUERY		= 1,	// Inverse query (IQUERY)
+	OPCODE_STATUS		= 2,	// Server status request (STATUS)
+	OPCODE_RESERVED_MIN	= 3,	// Reserved for future use (inclusive)
+	OPCODE_RESERVED_MAX	= 15	// Reserved for future use (inclusive)
+} gwdns_op;
+
+typedef enum {
+	TYPE_A		= 1,	// IPv4 host address
+	TYPE_NS		= 2,	// an authoritative name server
+	TYPE_CNAME	= 5,	// the canonical name for an alias
+	TYPE_SOA	= 6,	// marks the start of a zone of authority
+	TYPE_MB		= 7,	// a mailbox domain name (EXPERIMENTAL)
+	TYPE_MG		= 8,	// a mail group member (EXPERIMENTAL)
+	TYPE_MR		= 9,	// a mail rename domain name (EXPERIMENTAL)
+	TYPE_NULL	= 10,	// a null RR (EXPERIMENTAL)
+	TYPE_WKS	= 11,	// a well known service description
+	TYPE_PTR	= 12,	// a domain name pointer
+	TYPE_HINFO	= 13,	// host information
+	TYPE_MINFO	= 14,	// mailbox or mail list information
+	TYPE_MX		= 15,	// mail exchange
+	TYPE_TXT	= 16,	// text strings
+	TYPE_AAAA	= 28,	// IPv6 host address
+	QTYPE_AXFR	= 252,	// A request for a transfer of an entire zone
+	QTYPE_MAILB	= 253,	// A request for mailbox-related records (MB, MG or MR)
+	QTYPE_ALL	= 255	// A request for all records
+} gwdns_type;
+
+typedef enum {
+	CLASS_IN	= 1,	// Internet
+	CLASS_CH	= 3,	// CHAOS class
+	CLASS_HS	= 4,	// Hesiod
+	QCLASS_ANY	= 255	// ANY class (matches any class)
+} gwdns_class;
+
+typedef struct {
+	uint16_t id;
+	uint16_t flags;
+	uint16_t qdcount;
+	uint16_t ancount;
+	uint16_t nscount;
+	uint16_t arcount;
+} __packed gwdns_header_pkt;
+
+typedef struct {
+	uint8_t question[UDP_MSG_LIMIT];
+	char answr[UDP_MSG_LIMIT];
+} gwdns_question_buffer;
+
+typedef struct {
+	uint8_t *dst_buffer;
+	uint16_t type;
+	size_t dst_len;
+	const char *domain;
+} gwdns_question_part;
+
+/*
+ * 4.1.3. Resource record format
+ *
+ * The answer, authority, and additional sections all share the same
+ * format: a variable number of resource records, where the number of
+ * records is specified in the corresponding count field in the header.
+ */
+typedef struct {
+	uint8_t  *name;		// DOMAIN NAME: variable‑length sequence of labels (length-byte followed by label, ending in 0), possibly compressed
+	uint16_t  rr_type;	// TYPE: two-octet code identifying the RR type (see gwdns_type)
+	uint16_t  rr_class;	// CLASS: two-octet code identifying the RR class (see gwdns_class)
+	uint32_t  ttl;		// TTL: 32-bit unsigned, time to live in seconds
+	uint16_t  rdlength;	// RDLENGTH: length in octets of RDATA
+	uint8_t  *rdata;	// RDATA: variable-length data, format depends on TYPE and CLASS
+} gwdns_serialized_rr;
+
+typedef struct {
+	char qname[DOMAIN_NAME_LIMIT];
+	uint16_t qtype;
+	uint16_t qclass;
+} gwdns_serialized_question;
+
+typedef gwdns_serialized_rr gwdns_serialized_answ;
+
+typedef struct {
+	gwdns_header_pkt hdr;
+	uint8_t body[UDP_MSG_LIMIT];
+} gwdns_query_pkt;
+
+typedef struct {
+	gwdns_header_pkt hdr;
+	gwdns_serialized_question question;
+	gwdns_serialized_answ **rr_answ;
+} gwdns_answ_data;
+
+/*
+ * Construct question packet
+ * 
+ * The caller may need to check for potential transaction ID collisions.
+ * 
+ * possible error are:
+ * - ENAMETOOLONG	domain name in question.name is too long.
+ * - ENOBUFS		length in question.dst_len is not sufficient.
+ * - EINVAL		malformed or unsupported value in question data
+ *
+ * @param	prepared question
+ * @return	length of bytes written into dst_buffer on success,
+ * 		or a negative integer on failure.
+ */
+ssize_t construct_question(gwdns_question_part *question);
+
+/*
+ * Serialize name server's answer
+ *
+ * possible error are:
+ * -EAGAIN	in buffer is not sufficient, no bytes are processed, need more data.
+ * -EINVAL	the content of in buffer is not valid.
+ * -ENOMEM	failed to allocate dynamic memory.
+ * -ENODATA	the packet didn't contain any answers.
+ * -EPROTO	the DNS server can't understand your question
+ *
+ * @param txid	transaction id of question.
+ * @param in	a pointer to buffer that want to be parsed
+ * @param out	a pointer to serialized buffer of answer to question
+ * @return	zero on success or a negative number on failure
+ */
+int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out);
+void free_serialize_answ(gwdns_answ_data *answ);
-- 
Ahmad Gani

Re: [PATCH gwproxy v1 0/3] Initial work for DNS lookup implementation

[Ammar Faizi]

On Thu, Jul 31, 2025 at 10:07:43AM +0700, Ahmad Gani wrote:
> Is it preferred to use the current model (spawning dedicated DNS threads)
> and make the behavior of the resolver the same as getaddrinfo (blocking)?
> So far I've created the addrinfo interfaces with C-ares style and tested 
> it; although, it's still blocking.
> 
> I would like to know the numbers for comparison between the thread model 
> vs. the asynchronous model. Which approach is best for this scenario/case
> (DNS resolution)?

It's better if the DNS resolution can be done non-blocking via epoll
than a separate thread.

With dedicated DNS worker threads, you need:

  1) Queue.
  2) Mutex.
  3) Condvar.
  4) An evenfd to notify the sleeping epoll.
  5) A reference count to avoid UAF on cancellation.
  6) Open-and-close a SOCK_DGRAM for each query.
  7) eventfd_write() from the producer.
  8) eventfd_read() from the consumer (sleeping epoll).

The steps to perform just a single query are unecessarily compilated.
The communication between multiple threads costs could have been elided
with a non-blocking pollable socket.

With a non-blocking pollable socket, everything is done more efficiently:

  1) No contention waiting on queue mutex lock.
  2) Only one SOCK_DGRAM socket is needed (can be reused forever).
  3) No event fd is needed.
  4) No mutex is not needed (maybe only for the cache, but even that,
     it's very minimal with rwlock, not a full mutex protection).

You can scale up the number of SOCK_DGRAM sockets if you ever want to
use multiple DNS servers. Anyway, since SOCK_DGRAM is stateless, you
can even use one SOCK_DGRAM to send-and-recv to-from multiple DNS
servers (see sendto(2) and recvfrom(2)).

Also, with a very busy proxy server, you'll be more far away from
hitting RLIMIT_NOFILE as the number of fds is probably cut in half
(no event fd + socket fds created by getaddrinfo() internally).

> I feel like multi-threading is much faster than single-threading, as it's
> executed in true parallel (on a multi-core system) compared to 
> concurrently doing things with an event notification mechanism.

Yes, but you should add more threads that call epoll_wait(). Not the
number of DNS threads. The reason why we have so many DNS threads is
because we can't poll it. Not because we have maxed out a CPU core to
100%.

For now, multithread makes things faster when performing getaddrinfo()
because multiple queries are done asynchronously. It's not because
getaddrinfo() calls have fully eaten your CPU core. Right?

Ideally, the same thing could be achieved with epoll_wait() without
extra mutex, condvar and eventfd (which is cheaper).

Communication between threads is costly, avoid it if possible. If you
can have multithreaded workload with zero communication between threads,
that's very good. And that's one of the reasons we want to invent our own
DNS resolver. It's an effort to reduce the communication between threads.

> I also noticed that in the C-ares example [3], they recommend using the 
> event thread example, and it is similar to the io_uring model in my 
> perspective, where the operation is executed internally instead of 
> letting the caller poll for readiness with ares_process_fd. Maybe we can 
> mimic this aspect?

That's wrong, io_uring arms poll for networking workloads, it does not
create io-wq threads (except for shutdown()).

This has been discussed previously:

  that'll just arm a poll trigger to retry the operation
  when a connection comes in

  if you see io-wq activity for something that does
  connect/accept/recv/send/etc only, then there's either
  a bug in the kernel or you are doing something wrong
  in your app

https://discord.com/channels/1241076672589991966/1241076672589991970/1398781840546074624

-- 
Ammar Faizi

Re: [PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Ammar Faizi]

On Thu, Jul 31, 2025 at 10:07:44AM +0700, Ahmad Gani wrote:
> +int init_addr(const char *addr, struct gwp_sockaddr *addr_st, uint16_t port)

This is a no-no. We don't need it.

In gwproxy.c, there is convert_str_to_ssaddr(). It's also better than
your version because with it you can have:

  ./gwproxy --target google.com:80 --bind localhost:1111

Not restricted to IP address format, but domain name is also ok.

-- 
Ammar Faizi

Re: [PATCH gwproxy v1 3/3] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function

[Alviro Iskandar Setiawan]

On Thu, Jul 31, 2025 at 10:09 AM Ahmad Gani wrote:
> +int gw_ares_init(gw_ares_channel_t **channel, struct gw_ares_options *opts)
> +{
> +       gw_ares_channel_t *c;
> +       int ret;
> +
> +       if (!opts->nr_server)
> +               return -EINVAL;
> +
> +       *channel = malloc(sizeof(**channel));
> +       if (!*channel)
> +               return -ENOMEM;
> +
> +       c = *channel;
> +       c->nr_server = opts->nr_server;
> +       c->servers = malloc(c->nr_server * sizeof(*c->servers));
> +       if (!c->servers) {
> +               free(*channel);
> +               return -ENOMEM;
> +       }

This can set:

   c = malloc(sizeof(*c))

first, then on on success set:

   *channel = c;

I don't see a valid reason to fill a dangling pointer if it fails.

-- Viro

Re: [PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Alviro Iskandar Setiawan]

On Thu, Jul 31, 2025 at 9:01 PM Ammar Faizi wrote:
> On Thu, Jul 31, 2025 at 10:07:44AM +0700, Ahmad Gani wrote:
> > +int init_addr(const char *addr, struct gwp_sockaddr *addr_st, uint16_t port)
>
> This is a no-no. We don't need it.
>
> In gwproxy.c, there is convert_str_to_ssaddr(). It's also better than
> your version because with it you can have:
>
>   ./gwproxy --target google.com:80 --bind localhost:1111
>
> Not restricted to IP address format, but domain name is also ok.

The init_addr() function is used in the subsequent patch within the
gw_ares_init() function to transform IP address strings into its
binary form. That IP address is expected to be used as the DNS server.

I think a variant of convert_str_to_ssaddr() that accepts only an IP
address format should be created to handle the task in gw_ares_init(),
as domain name resolution is not expected yet at that point.

Another option would be to make init_addr() private to dnslookup.c
because no one else uses it.

-- Viro

Re: [PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Ammar Faizi]

On Fri, Aug 01, 2025 at 01:28:22AM +0700, Alviro Iskandar Setiawan wrote:
> On Thu, Jul 31, 2025 at 9:01 PM Ammar Faizi wrote:
> > On Thu, Jul 31, 2025 at 10:07:44AM +0700, Ahmad Gani wrote:
> > > +int init_addr(const char *addr, struct gwp_sockaddr *addr_st, uint16_t port)
> >
> > This is a no-no. We don't need it.
> >
> > In gwproxy.c, there is convert_str_to_ssaddr(). It's also better than
> > your version because with it you can have:
> >
> >   ./gwproxy --target google.com:80 --bind localhost:1111
> >
> > Not restricted to IP address format, but domain name is also ok.
> 
> The init_addr() function is used in the subsequent patch within the
> gw_ares_init() function to transform IP address strings into its
> binary form. That IP address is expected to be used as the DNS server.
> 
> I think a variant of convert_str_to_ssaddr() that accepts only an IP
> address format should be created to handle the task in gw_ares_init(),
> as domain name resolution is not expected yet at that point.

That's not true. A DNS server IP address can be specificed in a domain
name format too. For example:

  $ nslookup google.com one.one.one.one
  Server:		one.one.one.one
  Address:	1.1.1.1#53

  Non-authoritative answer:
  Name:	google.com
  Address: 142.250.188.238
  Name:	google.com
  Address: 2607:f8b0:4007:809::200e

will resolve one.one.one.one first, then it uses the resolved address
1.1.1.1 to resolve google.com.

-- 
Ammar Faizi

Re: [PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Alviro Iskandar Setiawan]

On Fri, Aug 1, 2025 at 1:36 AM Ammar Faizi wrote:
> On Fri, Aug 01, 2025 at 01:28:22AM +0700, Alviro Iskandar Setiawan wrote:
> > On Thu, Jul 31, 2025 at 9:01 PM Ammar Faizi wrote:
> > > On Thu, Jul 31, 2025 at 10:07:44AM +0700, Ahmad Gani wrote:
> > > > +int init_addr(const char *addr, struct gwp_sockaddr *addr_st, uint16_t port)
> > >
> > > This is a no-no. We don't need it.
> > >
> > > In gwproxy.c, there is convert_str_to_ssaddr(). It's also better than
> > > your version because with it you can have:
> > >
> > >   ./gwproxy --target google.com:80 --bind localhost:1111
> > >
> > > Not restricted to IP address format, but domain name is also ok.
> >
> > The init_addr() function is used in the subsequent patch within the
> > gw_ares_init() function to transform IP address strings into its
> > binary form. That IP address is expected to be used as the DNS server.
> >
> > I think a variant of convert_str_to_ssaddr() that accepts only an IP
> > address format should be created to handle the task in gw_ares_init(),
> > as domain name resolution is not expected yet at that point.
>
> That's not true. A DNS server IP address can be specificed in a domain
> name format too. For example:
>
>   $ nslookup google.com one.one.one.one
>   Server:               one.one.one.one
>   Address:      1.1.1.1#53
>
>   Non-authoritative answer:
>   Name: google.com
>   Address: 142.250.188.238
>   Name: google.com
>   Address: 2607:f8b0:4007:809::200e
>
> will resolve one.one.one.one first, then it uses the resolved address
> 1.1.1.1 to resolve google.com.

Ah, ic ic. I didn't know that. So it's fine to keep using glibc's
getaddrinfo() to initialize ares?

-- Viro

Re: [PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Ammar Faizi]

On Fri, Aug 01, 2025 at 01:42:41AM +0700, Alviro Iskandar Setiawan wrote:
> Ah, ic ic. I didn't know that. So it's fine to keep using glibc's
> getaddrinfo() to initialize ares?

Yes, it's fine at initialization. Cold paths are NOT something we
should further optimize. The point of this pollable DNS resolver plan
is to optimize hot paths. That's the event loop.

But since that initialization needs a default port. The function
convert_str_to_ssaddr() may be modified to handle IP addr with no port.

Maybe something like:

  static int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs, uint16_t default_port);

After that, gw_ares_init() can use convert_str_to_ssaddr() with
default_port=53.

-- 
Ammar Faizi

Re: [PATCH gwproxy v1 1/3] dnslookup: split common functionality and struct into net.c

[Alviro Iskandar Setiawan]

On Fri, Aug 1, 2025 at 1:53 AM Ammar Faizi wrote:
> On Fri, Aug 01, 2025 at 01:42:41AM +0700, Alviro Iskandar Setiawan wrote:
> > Ah, ic ic. I didn't know that. So it's fine to keep using glibc's
> > getaddrinfo() to initialize ares?
>
> Yes, it's fine at initialization. Cold paths are NOT something we
> should further optimize. The point of this pollable DNS resolver plan
> is to optimize hot paths. That's the event loop.
>
> But since that initialization needs a default port. The function
> convert_str_to_ssaddr() may be modified to handle IP addr with no port.
>
> Maybe something like:
>
>   static int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs, uint16_t default_port);
>
> After that, gw_ares_init() can use convert_str_to_ssaddr() with
> default_port=53.

Sounds good to me.

-- Viro

Re: [PATCH gwproxy v1 3/3] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function

[Alviro Iskandar Setiawan]

On Thu, Jul 31, 2025 at 10:09 AM Ahmad Gani wrote:
> +static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t nport,
> +       struct gw_ares_addrinfo *result, struct gw_addrinfo_node **tail)
> +{
> +       uint8_t send_buff[UDP_MSG_LIMIT];
> +       uint8_t recv_buff[UDP_MSG_LIMIT];
> +       gwdns_query_pkt *query_pkt;
> +       gwdns_answ_data raw_answ;
> +       gwdns_question_part q;
> +       ssize_t buff_len;
> +       int ret;
> [...]
> +attempt_retry:
> +       ret = __sys_send(sockfd, send_buff, buff_len, MSG_NOSIGNAL);
> +       if (ret < 0) {
> +               ret = GW_ARES_INTERNAL_ERR;
> +               return ret;
> +       }
> +
> +       if (ret != buff_len)
> +               goto attempt_retry;

Why is this attempt_retry needed?

-- Viro

Re: [PATCH gwproxy v1 0/3] Initial work for DNS lookup implementation

[reyuki]

On Thu, Jul 31, 2025 at 8:39 PM Ammar Faizi wrote:
>
> On Thu, Jul 31, 2025 at 10:07:43AM +0700, Ahmad Gani wrote:
>> Is it preferred to use the current model (spawning dedicated DNS threads)
>> and make the behavior of the resolver the same as getaddrinfo (blocking)?
>> So far I've created the addrinfo interfaces with C-ares style and tested
>> it; although, it's still blocking.
>>
>> I would like to know the numbers for comparison between the thread model
>> vs. the asynchronous model. Which approach is best for this scenario/case
>> (DNS resolution)?
>
> It's better if the DNS resolution can be done non-blocking via epoll
> than a separate thread.
>
> With dedicated DNS worker threads, you need:
>
>   1) Queue.
>   2) Mutex.
>   3) Condvar.
>   4) An evenfd to notify the sleeping epoll.
>   5) A reference count to avoid UAF on cancellation.
>   6) Open-and-close a SOCK_DGRAM for each query.
>   7) eventfd_write() from the producer.
>   8) eventfd_read() from the consumer (sleeping epoll).
>
> The steps to perform just a single query are unecessarily compilated.
> The communication between multiple threads costs could have been elided
> with a non-blocking pollable socket.
>
> With a non-blocking pollable socket, everything is done more efficiently:
>
>   1) No contention waiting on queue mutex lock.
>   2) Only one SOCK_DGRAM socket is needed (can be reused forever).
>   3) No event fd is needed.
>   4) No mutex is not needed (maybe only for the cache, but even that,
>      it's very minimal with rwlock, not a full mutex protection).
>
> You can scale up the number of SOCK_DGRAM sockets if you ever want to
> use multiple DNS servers. Anyway, since SOCK_DGRAM is stateless, you
> can even use one SOCK_DGRAM to send-and-recv to-from multiple DNS
> servers (see sendto(2) and recvfrom(2)).
>
> Also, with a very busy proxy server, you'll be more far away from
> hitting RLIMIT_NOFILE as the number of fds is probably cut in half
> (no event fd + socket fds created by getaddrinfo() internally).
>
>> I feel like multi-threading is much faster than single-threading, as it's
>> executed in true parallel (on a multi-core system) compared to
>> concurrently doing things with an event notification mechanism.
>
> Yes, but you should add more threads that call epoll_wait(). Not the
> number of DNS threads. The reason why we have so many DNS threads is
> because we can't poll it. Not because we have maxed out a CPU core to
> 100%.
>
> For now, multithread makes things faster when performing getaddrinfo()
> because multiple queries are done asynchronously. It's not because
> getaddrinfo() calls have fully eaten your CPU core. Right?
>
> Ideally, the same thing could be achieved with epoll_wait() without
> extra mutex, condvar and eventfd (which is cheaper).
>
> Communication between threads is costly, avoid it if possible. If you
> can have multithreaded workload with zero communication between threads,
> that's very good. And that's one of the reasons we want to invent our own
> DNS resolver. It's an effort to reduce the communication between threads.

Thanks for convincing me, now I can decide to use the polling option instead
of blocking getaddrinfo option that's executed in dedicated threads.

I think I get the general idea of using pollable socket:
- mark the UDP socket as non block
- let the caller have reference to the socket, so they can poll it

But have difficulties in the technical implementation:

When the caller is notified, they must call what?

It is a UDP socket, it's said to be stateless, does connect can possibly
return EINPROGRESS? It seems to be a common case in non-blocking
TCP socket. And if EAGAIN is received from recv, do we retry from send?

I find it unclear how the library and the program interact when using the
non-blocking behavior, but I think I will give it a thought (maybe add an
internal state to address these concerns).

>> I also noticed that in the C-ares example [3], they recommend using the
>> event thread example, and it is similar to the io_uring model in my
>> perspective, where the operation is executed internally instead of
>> letting the caller poll for readiness with ares_process_fd. Maybe we can
>> mimic this aspect?
>
> That's wrong, io_uring arms poll for networking workloads, it does not
> create io-wq threads (except for shutdown()).

I didn't talk about io-wq threads; I don't have any technical
understanding or knowledge about them. When I said 'the operation is
executed internally,' what I meant was my general understanding of the
io_uring model that tells of the completion of certain operations you've
requested. which is different compared to the epoll model that tells
readiness for a specific operation, and you execute that operation on
your own.

> https://discord.com/channels/1241076672589991966/1241076672589991970/1398781840546074624

I can't view the linked message from the given link, is it a private community?

--
Ahmad Gani

Re: [PATCH gwproxy v1 3/3] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function

[reyuki]

On Fri, Aug 1, 2025 at 2:15 AM Alviro Iskandar Setiawan wrote:
>
> On Thu, Jul 31, 2025 at 10:09 AM Ahmad Gani wrote:
> > +static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t nport,
> > +       struct gw_ares_addrinfo *result, struct gw_addrinfo_node **tail)
> > +{
> > +       uint8_t send_buff[UDP_MSG_LIMIT];
> > +       uint8_t recv_buff[UDP_MSG_LIMIT];
> > +       gwdns_query_pkt *query_pkt;
> > +       gwdns_answ_data raw_answ;
> > +       gwdns_question_part q;
> > +       ssize_t buff_len;
> > +       int ret;
> > [...]
> > +attempt_retry:
> > +       ret = __sys_send(sockfd, send_buff, buff_len, MSG_NOSIGNAL);
> > +       if (ret < 0) {
> > +               ret = GW_ARES_INTERNAL_ERR;
> > +               return ret;
> > +       }
> > +
> > +       if (ret != buff_len)
> > +               goto attempt_retry;
>
> Why is this attempt_retry needed?

Oh, do you mean short send is not possible in blocking socket? Sorry for
the confusion, I think I'm not aware of that fact. I will remove it.

Re: [PATCH gwproxy v1 0/3] Initial work for DNS lookup implementation

[Ammar Faizi]

On Fri, Aug 01, 2025 at 08:49:22AM +0700, reyuki wrote:
> I think I get the general idea of using pollable socket:
> - mark the UDP socket as non block
> - let the caller have reference to the socket, so they can poll it
> 
> But have difficulties in the technical implementation:
> 
> When the caller is notified, they must call what?

The same thing as other sockets. If you get EPOLLIN, that means there
is data to recv, so call recv/recvfrom.

> It is a UDP socket, it's said to be stateless, does connect can possibly
> return EINPROGRESS? It seems to be a common case in non-blocking

Most likely not. That will only set the dst addr. Apart from that, a
connect() call is optional if you use a UDP socket as a client.

sendto(2) and recvfrom(2) can be used if you ever want to use one
SOCK_DGRAM with multiple dst addresses.

> TCP socket. And if EAGAIN is received from recv, do we retry from send?

No, -EAGAIN from recv simply means the socket is non-blocking and there
is no data to be read at the moment.

> I find it unclear how the library and the program interact when using the
> non-blocking behavior, but I think I will give it a thought (maybe add an
> internal state to address these concerns).

The socks5 lib can do that. Why the DNS lib not?

> I can't view the linked message from the given link, is it a private community?

Oh, I didn't know if it was private. It seems the admins recently made
it private. I remember the days when it was public. But I have quoted
the needed sentences, I suppose that's enough.

-- 
Ammar Faizi

Re: [PATCH gwproxy v1 3/3] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function

[Alviro Iskandar Setiawan]

On Fri, Aug 1, 2025 at 8:52 AM reyuki wrote:
> On Fri, Aug 1, 2025 at 2:15 AM Alviro Iskandar Setiawan wrote:
> > On Thu, Jul 31, 2025 at 10:09 AM Ahmad Gani wrote:
> > > +static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t nport,
> > > +       struct gw_ares_addrinfo *result, struct gw_addrinfo_node **tail)
> > > +{
> > > +       uint8_t send_buff[UDP_MSG_LIMIT];
> > > +       uint8_t recv_buff[UDP_MSG_LIMIT];
> > > +       gwdns_query_pkt *query_pkt;
> > > +       gwdns_answ_data raw_answ;
> > > +       gwdns_question_part q;
> > > +       ssize_t buff_len;
> > > +       int ret;
> > > [...]
> > > +attempt_retry:
> > > +       ret = __sys_send(sockfd, send_buff, buff_len, MSG_NOSIGNAL);
> > > +       if (ret < 0) {
> > > +               ret = GW_ARES_INTERNAL_ERR;
> > > +               return ret;
> > > +       }
> > > +
> > > +       if (ret != buff_len)
> > > +               goto attempt_retry;
> >
> > Why is this attempt_retry needed?
>
> Oh, do you mean short send is not possible in blocking socket? Sorry for
> the confusion, I think I'm not aware of that fact. I will remove it.

I didn't say that. I asked because I don't know.

-- Viro

Re: [PATCH gwproxy v1 0/3] Initial work for DNS lookup implementation

[reyuki]

On Fri, Aug 1, 2025 at 9:19 AM Ammar Faizi wrote:
> The socks5 lib can do that. Why the DNS lib not?

In the case of the SOCKS5 lib, it focuses on parsing, and all network-related
tasks are handled directly by the caller. That’s why I was a bit confused
at first.

But wait—I think I’ve figured it out. I got an idea, I just need some time
to think it through. Thanks for helping me think through the problem.

I can make the DNS lib behave similarly. Instead of delegating the tasks
directly, I can provide an interface that helps the caller handle the
socket-related work.

Sorry for the late reply—I’ve been feeling a bit out of control lately.
My mind keeps telling me to finish this as fast as possible, but when I don’t
understand something, I get easily frustrated and end up watching anime
to distract myself, instead of calmly working through the problem.

I really need to fix that mindset.

Anyway, just give me a moment—I’ll send the patches shortly.

--
Ahmad Gani