* [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
@ 2025-08-06 3:57 Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c Ahmad Gani
` (8 more replies)
0 siblings, 9 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
Hi Chief,
This is revision v4 of 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 and
uses epoll to make it asynchronous.
There are 6 patches in this series:
- create net.h and net.c for network related functionality and struct
- add default_port parameter to convert_str_to_ssaddr function
- allow only port string number in service parameter of gwp_dns_queue
- initial work for implementation of C-ares-like getaddrinfo function
- make gw_ares_getaddrinfo function asynchronous
- transaction id creation is delegated to caller
# Remove TODO(reyuki): hints->ai_family is used to filter results
The filtering has already been completed using a bitmask. As for sorting
the results, that responsibility is delegated to the caller, allowing
them to choose their preferred sorting method.
# Make UDP socket non-blocking
As discussed previously [1], I made the UDP socket non-blocking for
asynchronous operation and provide a function helper for caller to handle
socket-related operation.
# Move socket creation to library initialization
Since we use the UDP protocol, socket creation is moved to library
initialization, so it is just created once and reused as many times as
possible.
# Make UDP socket unconnected
Avoid connect syscall on UDP socket to allow communicating with multiple
recursive resolvers or name servers with sendto and recvfrom syscalls.
# Restructure internal struct and program execution flow
Unlike C-ares, this design requires the getaddrinfo callback to be specified
during initialization, since it typically points to the same handler and
rarely changes between calls.
To support non-blocking behavior, the program needs to track the state of
each request, so I need to make changes to the structure and program
execution flow.
Each gw_ares_getaddrinfo call creates a new request on the internal side
of dnslookup; you can see the gw_ares_request struct for what's stored.
## The changes are made based on the following principles:
1. The DNS-specific details are intentionally hidden from the program
The DNS-specific tasks and operations are intentionally hidden from the
program using the library. The program doesn't need to know anything about
the DNS protocol.
2. How the library and the program interact
The library MUST tell the program if there's an error inside the
gw_ares_getaddrinfo call, or if the getaddrinfo operation has completed.
This is achieved by calling the getaddrinfo_cb function.
The library MUST tell the program if a socket file descriptor is created or
a particular event is adjusted. This is achieved by calling the
sockstate_cb function.
The program is responsible for handling this appropriately using the
callback it has specified.
Although no event is currently adjusted, it may be implemented in a future
patch series.
The program MUST tell the library if a particular socket file descriptor
is ready to be used for a certain operation, like recv or send. This is
achieved by calling the gw_ares_process_event function.
Although currently only recv is executed, other operations may be added
in a future patch series.
## And here's how the library is supposed to work after these changes:
Initialize the library will create a UDP socket that can be used
internally by the library to communicate with the DNS server.
There are two callbacks, and to avoid the confusion of what callback
I'm talking about, let's name them getaddrinfo_cb and sockstate_cb.
You must prepare both and pass it to gw_ares_init through gw_ares_options,
the sockstate_cb will receive a socket file descriptor from the callback
and it will be used in your event notification setup (poll, epoll, etc...).
You can specify ai_family member in the hints structure to filter the
addrinfo in the results, use AF_UNSPEC to select both IPv4 and IPv6.
When you call gw_ares_getaddrinfo, it will attempt to send DNS packet to
DNS server you have specified in gw_ares_options. It will not block your
program, however you are supposed to call gw_ares_process_event later in
your own event loop.
The function gw_ares_process_event is an abstraction provided by the
library to handle socket-related tasks and give you the results by
calling the callback you've specified in the gw_ares_options struct.
# Update unit test of dns parser
Because I wanted to test how the DNS server behaves when handling multiple
requests, I updated the old unit test. As expected, it's possible to send
multiple requests—as long as you don't batch them (i.e., QDCOUNT > 1) [2].
As Sir Ammar pointed out, the UDP protocol is stateless. That means you
can use a single UDP socket to send packets to and receive packets from
multiple destinations. So there's no need to wait for a response (recv)
before sending the next request—you can send several first, then receive
them later.
# Transaction id creation is delegated to caller
Because UDP protocol is unreliable, there's no guarantee in the order of
packet arrival.
A single request can have two transaction IDs if the address family hint
is AF_UNSPEC. To simplify this, the program now uses the same txid for both
IPv4 and IPv6. This avoids the need to store two txids in an array, which
can cause issues when responses arrive out of order.
# Fix logic bug in serialize_answ that can lead to memory error: invalid read
I accidentally found a logic bug in the DNS parser while updating the
unit test code. It was first detected by Valgrind, but the error report
did not directly address the real issue:
==764369== Conditional jump or move depends on uninitialised value(s)
==764369== at 0x49B8C9A: inet_ntop6 (inet_ntop.c:126)
==764369== by 0x49B8C9A: inet_ntop (inet_ntop.c:59)
==764369== by 0x4002473: convert_ssaddr_to_str (net.c:91)
==764369== by 0x4001FFA: test_parse_ipv6 (dnsparser.c:320)
==764369== by 0x400206B: run_all_tests (dnsparser.c:341)
==764369== by 0x4002097: main (dnsparser.c:347)
I wonder why Valgrind said that even though I already memset gs? So I
tried with ASAN, and it detected a stack-buffer-overflow. It is triggered
by a malformed DNS response packet; probably I was copy-pasting the raw
hex that was incomplete, and it led to this finding.
The solution is simple: check how many bytes remain before reading—never
read if there are none left, along with other changes made to fix the problem.
# Changelog
v1 -> v2:
- use existing convert_str_to_ssaddr instead of init_addr
- fix memory leak when init_addr (now it's replaced) failed
- modify convert_str_to_ssaddr to support default port
- bring back __cold attribute on convert_ssaddr_to_str
- don't fill a dangling pointer as Sir Alviro said [3]
- for now it's blocking, attempt_retry label is not needed
v2 -> v3:
- remove TODO(reyuki): hints->ai_family is used to filter results
- make gw_ares_getaddrinfo asynchronous by change UDP socket to non-blocking
- move socket creation to library initialization
- make UDP socket unconnected
- restructure internal struct and program execution flow to support async
- update unit test of dns parser
- transaction id creation is delegated to caller
- fix logic bug in serialize_answ that can lead to memory error: invalid read
- rename parameter prt and split commit message
- move variable declaration in for loop according to the Sir Alviro coding-style
v3 -> v4:
- add else block in the convert_str_to_ssaddr function
- use format specifier %hu for uint16_t in printf
- update base commit to branch master in upstream remote repository
[1]:
- https://lore.gnuweeb.org/gwml/aIwkO9Aw+IFht0vd@biznet-home.integral.gnuweeb.org/
[2]:
- www.ietf.org/rfc/rfc9619.pdf
- https://stackoverflow.com/questions/4082081/requesting-a-and-aaaa-records-in-single-dns-query/4083071
[3]:
- https://lore.gnuweeb.org/gwml/CAOG64qMPmFL-+7OrVv4psmyOt8G-3OZJGa=bQ1a_wuV06WTyng@mail.gmail.com/
Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
Ahmad Gani (6):
dnslookup: Split common functionality and struct into net.h and net.c
dnslookup: Add a new parameter default_port
dnslookup: Allow only port string number
dnslookup: Initial work for implementation of C-ares-like getaddrinfo
function
dnsparser: Transaction id creation is delegated to caller
dnslookup: Make gw_ares_getaddrinfo asynchronous
Makefile | 1 +
man/gwp_dns_queue.3 | 2 +-
src/gwproxy/dns.h | 12 +-
src/gwproxy/dnslookup.c | 432 ++++++++++++++++++++++++++++++++++++++++
src/gwproxy/dnslookup.h | 161 +++++++++++++++
src/gwproxy/dnsparser.c | 415 ++++++++++++++++++++++++++++++++++++++
src/gwproxy/dnsparser.h | 192 ++++++++++++++++++
src/gwproxy/gwproxy.c | 93 +--------
src/gwproxy/net.c | 106 ++++++++++
src/gwproxy/net.h | 36 ++++
10 files changed, 1348 insertions(+), 102 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: a583bb879e3f043dfe7d723a74c7910c0c8ad5b4
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
@ 2025-08-06 3:57 ` Ahmad Gani
2025-08-06 6:18 ` Alviro Iskandar Setiawan
2025-08-06 3:57 ` [PATCH gwproxy v4 2/6] dnslookup: Add a new parameter default_port Ahmad Gani
` (7 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
List of extracted stuff:
- convert_str_to_ssaddr function to net.c.
- convert_ssaddr_to_str function to net.c.
- struct gwp_sockaddr to net.h
- FULL_ADDRSTRLEN constant to net.h
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 | 89 --------------------------------------
src/gwproxy/net.c | 99 +++++++++++++++++++++++++++++++++++++++++++
src/gwproxy/net.h | 34 +++++++++++++++
5 files changed, 135 insertions(+), 97 deletions(-)
create mode 100644 src/gwproxy/net.c
create mode 100644 src/gwproxy/net.h
diff --git a/Makefile b/Makefile
index 6c2e9be0d857..007ba47d1bf5 100644
--- a/Makefile
+++ b/Makefile
@@ -29,6 +29,7 @@ GWPROXY_TARGET = gwproxy
GWPROXY_CC_SOURCES = \
$(GWPROXY_DIR)/gwproxy.c \
$(GWPROXY_DIR)/log.c \
+ $(GWPROXY_DIR)/net.c \
$(GWPROXY_DIR)/ev/epoll.c
GWPROXY_OBJECTS = $(GWPROXY_CC_SOURCES:%.c=%.c.o)
diff --git a/src/gwproxy/dns.h b/src/gwproxy/dns.h
index a4a7e2c37a41..a4038fc0b254 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 dac99033b680..2bc853e46299 100644
--- a/src/gwproxy/gwproxy.c
+++ b/src/gwproxy/gwproxy.c
@@ -258,35 +258,6 @@ einval:
#define FULL_ADDRSTRLEN (INET6_ADDRSTRLEN + sizeof(":65535[]") - 1)
-__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)
{
@@ -297,66 +268,6 @@ const char *ip_to_str(const struct gwp_sockaddr *gs)
return convert_ssaddr_to_str(bp, gs) ? NULL : bp;
}
-__cold
-static int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs)
-{
- static const struct addrinfo hints = {
- .ai_family = AF_UNSPEC,
- .ai_socktype = SOCK_STREAM,
- };
- char host[NI_MAXHOST], port[6], *p;
- struct addrinfo *res, *ai;
- bool found = false;
- size_t l;
- int r;
-
- if (*str == '[') {
- p = strchr(++str, ']');
- if (!p)
- return -EINVAL;
- l = p - str;
- if (l >= sizeof(host))
- return -EINVAL;
- p++;
- if (*p != ':')
- return -EINVAL;
- } else {
- p = strchr(str, ':');
- if (!p)
- return -EINVAL;
- l = p - str;
- if (l >= sizeof(host))
- return -EINVAL;
- }
-
- strncpy(host, str, l);
- host[l] = '\0';
- strncpy(port, p + 1, sizeof(port) - 1);
- port[sizeof(port) - 1] = '\0';
-
- r = getaddrinfo(host, port, &hints, &res);
- if (r)
- return -EINVAL;
- if (!res)
- return -EINVAL;
-
- memset(gs, 0, sizeof(*gs));
- for (ai = res; ai; ai = ai->ai_next) {
- if (ai->ai_family == AF_INET) {
- gs->i4 = *(struct sockaddr_in *)ai->ai_addr;
- found = true;
- break;
- } else if (ai->ai_family == AF_INET6) {
- gs->i6 = *(struct sockaddr_in6 *)ai->ai_addr;
- found = true;
- break;
- }
- }
-
- freeaddrinfo(res);
- return found ? 0 : -EINVAL;
-}
-
__cold
static int gwp_ctx_init_log(struct gwp_ctx *ctx)
{
diff --git a/src/gwproxy/net.c b/src/gwproxy/net.c
new file mode 100644
index 000000000000..12c6c0b1275d
--- /dev/null
+++ b/src/gwproxy/net.c
@@ -0,0 +1,99 @@
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stddef.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <netdb.h>
+#include <gwproxy/net.h>
+#include <gwproxy/common.h>
+
+__cold
+int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs)
+{
+ static const struct addrinfo hints = {
+ .ai_family = AF_UNSPEC,
+ .ai_socktype = SOCK_STREAM,
+ };
+ char host[NI_MAXHOST], port[6], *p;
+ struct addrinfo *res, *ai;
+ bool found = false;
+ size_t l;
+ int r;
+
+ if (*str == '[') {
+ p = strchr(++str, ']');
+ if (!p)
+ return -EINVAL;
+ l = p - str;
+ if (l >= sizeof(host))
+ return -EINVAL;
+ p++;
+ if (*p != ':')
+ return -EINVAL;
+ } else {
+ p = strchr(str, ':');
+ if (!p)
+ return -EINVAL;
+ l = p - str;
+ if (l >= sizeof(host))
+ return -EINVAL;
+ }
+
+ strncpy(host, str, l);
+ host[l] = '\0';
+ strncpy(port, p + 1, sizeof(port) - 1);
+ port[sizeof(port) - 1] = '\0';
+
+ r = getaddrinfo(host, port, &hints, &res);
+ if (r)
+ return -EINVAL;
+ if (!res)
+ return -EINVAL;
+
+ memset(gs, 0, sizeof(*gs));
+ for (ai = res; ai; ai = ai->ai_next) {
+ if (ai->ai_family == AF_INET) {
+ gs->i4 = *(struct sockaddr_in *)ai->ai_addr;
+ found = true;
+ break;
+ } else if (ai->ai_family == AF_INET6) {
+ gs->i6 = *(struct sockaddr_in6 *)ai->ai_addr;
+ found = true;
+ break;
+ }
+ }
+
+ freeaddrinfo(res);
+ return found ? 0 : -EINVAL;
+}
+
+__cold
+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..627a70f926f9
--- /dev/null
+++ b/src/gwproxy/net.h
@@ -0,0 +1,34 @@
+/*
+ * 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;
+ };
+};
+
+/*
+ * Convert address string to network address
+ *
+ * @param str source
+ * @param gs destination
+ * @return zero on success and a negative integer on failure.
+ */
+int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs);
+
+/*
+ * Convert network address to string format
+ *
+ * @param buf destination
+ * @param gs source
+ * @return zero on success and a negative integer on failure.
+ */
+int convert_ssaddr_to_str(char buf[FULL_ADDRSTRLEN],
+ const struct gwp_sockaddr *gs);
base-commit: a583bb879e3f043dfe7d723a74c7910c0c8ad5b4
--
Ahmad Gani
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH gwproxy v4 2/6] dnslookup: Add a new parameter default_port
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c Ahmad Gani
@ 2025-08-06 3:57 ` Ahmad Gani
2025-08-06 6:20 ` Alviro Iskandar Setiawan
2025-08-06 3:57 ` [PATCH gwproxy v4 3/6] dnslookup: Allow only port string number Ahmad Gani
` (6 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
default_port as parameter in convert_str_to_ssaddr function is needed
when separator ':' does not exists in the provided address string.
Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
src/gwproxy/gwproxy.c | 4 ++--
src/gwproxy/net.c | 25 ++++++++++++++++---------
src/gwproxy/net.h | 4 +++-
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/src/gwproxy/gwproxy.c b/src/gwproxy/gwproxy.c
index 2bc853e46299..aa3d07440284 100644
--- a/src/gwproxy/gwproxy.c
+++ b/src/gwproxy/gwproxy.c
@@ -528,7 +528,7 @@ static int gwp_ctx_init_threads(struct gwp_ctx *ctx)
return -EINVAL;
}
- r = convert_str_to_ssaddr(cfg->bind, &bind_addr);
+ r = convert_str_to_ssaddr(cfg->bind, &bind_addr, 0);
if (r) {
pr_err(&ctx->lh, "Invalid bind address '%s'\n", cfg->bind);
return r;
@@ -747,7 +747,7 @@ static int gwp_ctx_init(struct gwp_ctx *ctx)
if (!ctx->cfg.as_socks5) {
const char *t = ctx->cfg.target;
- r = convert_str_to_ssaddr(t, &ctx->target_addr);
+ r = convert_str_to_ssaddr(t, &ctx->target_addr, 0);
if (r) {
pr_err(&ctx->lh, "Invalid target address '%s'", t);
goto out_free_log;
diff --git a/src/gwproxy/net.c b/src/gwproxy/net.c
index 12c6c0b1275d..268d5f968dcf 100644
--- a/src/gwproxy/net.c
+++ b/src/gwproxy/net.c
@@ -10,7 +10,8 @@
#include <gwproxy/common.h>
__cold
-int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs)
+int convert_str_to_ssaddr(const char *str,
+ struct gwp_sockaddr *gs, uint16_t default_port)
{
static const struct addrinfo hints = {
.ai_family = AF_UNSPEC,
@@ -27,24 +28,30 @@ int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs)
if (!p)
return -EINVAL;
l = p - str;
- if (l >= sizeof(host))
- return -EINVAL;
p++;
- if (*p != ':')
+ if (*p != ':' && !default_port)
return -EINVAL;
- } else {
+ } else if (!default_port) {
p = strchr(str, ':');
if (!p)
return -EINVAL;
l = p - str;
- if (l >= sizeof(host))
- return -EINVAL;
+ } else {
+ l = strlen(str);
+ p = NULL;
}
+ if (l >= sizeof(host))
+ return -EINVAL;
+
strncpy(host, str, l);
host[l] = '\0';
- strncpy(port, p + 1, sizeof(port) - 1);
- port[sizeof(port) - 1] = '\0';
+ if (default_port) {
+ snprintf(port, 6, "%hu", default_port);
+ } else {
+ strncpy(port, p + 1, sizeof(port) - 1);
+ port[sizeof(port) - 1] = '\0';
+ }
r = getaddrinfo(host, port, &hints, &res);
if (r)
diff --git a/src/gwproxy/net.h b/src/gwproxy/net.h
index 627a70f926f9..ff4ca1fc1965 100644
--- a/src/gwproxy/net.h
+++ b/src/gwproxy/net.h
@@ -19,9 +19,11 @@ struct gwp_sockaddr {
*
* @param str source
* @param gs destination
+ * @param default_port fill it with zero if unspecified
* @return zero on success and a negative integer on failure.
*/
-int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs);
+int convert_str_to_ssaddr(const char *str,
+ struct gwp_sockaddr *gs, uint16_t default_port);
/*
* Convert network address to string format
--
Ahmad Gani
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH gwproxy v4 3/6] dnslookup: Allow only port string number
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 2/6] dnslookup: Add a new parameter default_port Ahmad Gani
@ 2025-08-06 3:57 ` Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function Ahmad Gani
` (5 subsequent siblings)
8 siblings, 0 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
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 a4038fc0b254..10c7cea2ebe4 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
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
` (2 preceding siblings ...)
2025-08-06 3:57 ` [PATCH gwproxy v4 3/6] dnslookup: Allow only port string number Ahmad Gani
@ 2025-08-06 3:57 ` Ahmad Gani
2025-08-06 6:40 ` Alviro Iskandar Setiawan
2025-08-06 11:09 ` Alviro Iskandar Setiawan
2025-08-06 3:57 ` [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller Ahmad Gani
` (4 subsequent siblings)
8 siblings, 2 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
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 | 285 ++++++++++++++++++++++++++++
src/gwproxy/dnslookup.h | 118 ++++++++++++
src/gwproxy/dnsparser.c | 408 ++++++++++++++++++++++++++++++++++++++++
src/gwproxy/dnsparser.h | 191 +++++++++++++++++++
4 files changed, 1002 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..de286fb5ab14
--- /dev/null
+++ b/src/gwproxy/dnslookup.c
@@ -0,0 +1,285 @@
+/*
+ * 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;
+
+ ret = __sys_send(sockfd, send_buff, buff_len, MSG_NOSIGNAL);
+ if (ret < 0) {
+ ret = GW_ARES_INTERNAL_ERR;
+ return ret;
+ }
+
+ 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;
+
+ c = malloc(sizeof(*c));
+ if (!c)
+ return -ENOMEM;
+
+ *channel = c;
+ c->nr_server = opts->nr_server;
+ c->servers = malloc(c->nr_server * sizeof(*c->servers));
+ if (!c->servers) {
+ free(c);
+ 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 = convert_str_to_ssaddr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
+ if (ret) {
+ free(c->servers);
+ free(c);
+ 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..744dc61581ef
--- /dev/null
+++ b/src/gwproxy/dnsparser.c
@@ -0,0 +1,408 @@
+#define _DEFAULT_SOURCE
+#include <endian.h>
+#include <gwproxy/dnsparser.h>
+#include <gwproxy/net.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) {
+ tot_len++;
+ break;
+ }
+
+ if (tot_len >= in_len)
+ return -ENAMETOOLONG;
+
+ advance_len = *p + 1;
+ tot_len += advance_len;
+ p += advance_len;
+ }
+
+ tot_len += 4;
+
+ return tot_len;
+}
+
+int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
+{
+ size_t idx;
+ gwdns_header_pkt *hdr;
+ uint16_t raw_flags;
+ int ret;
+
+ idx = sizeof(*hdr);
+ if (idx >= in_len)
+ return -EAGAIN;
+
+ hdr = (void *)in;
+ if (memcmp(&txid, &hdr->id, sizeof(txid)))
+ return -EINVAL;
+
+ memcpy(&raw_flags, &hdr->flags, 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;
+
+ /*
+ * Check the sizes upfront.
+ *
+ * 1 bytes for variable-length
+ * in[idx] for the length of first name
+ * 1 bytes for null terminator
+ * 2 bytes for qtype
+ * 2 bytes for qclass
+ */
+ if ((size_t)(1 + in[idx] + 1 + 2 + 2) >= in_len)
+ return -EINVAL;
+
+ ret = calculate_question_len(&in[idx], in_len - idx);
+ if (ret <= 0)
+ return -EINVAL;
+
+ idx += ret;
+ if (idx >= in_len)
+ return -EAGAIN;
+
+ 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[idx], sizeof(is_compressed));
+ is_compressed = DNS_IS_COMPRESSED(ntohs(is_compressed));
+ assert(is_compressed);
+ idx += 2; // NAME
+ if (idx >= in_len) {
+ ret = -EAGAIN;
+ goto exit_free;
+ }
+
+ memcpy(&item->rr_type, &in[idx], 2);
+ item->rr_type = ntohs(item->rr_type);
+ idx += 2; // TYPE
+ if (idx >= in_len) {
+ ret = -EAGAIN;
+ goto exit_free;
+ }
+ memcpy(&item->rr_class, &in[idx], 2);
+ item->rr_class = ntohs(item->rr_class);
+ idx += 2; // CLASS
+ if (idx >= in_len) {
+ ret = -EAGAIN;
+ goto exit_free;
+ }
+ memcpy(&item->ttl, &in[idx], 4);
+ item->ttl = be32toh(item->ttl);
+ idx += 4; // TTL
+ if (idx >= in_len) {
+ ret = -EAGAIN;
+ goto exit_free;
+ }
+
+ memcpy(&rdlength, &in[idx], 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;
+ idx += 2;
+ if (idx >= in_len) {
+ ret = -EAGAIN;
+ goto exit_free;
+ }
+
+ /*
+ * 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[idx], rdlength);
+ idx += rdlength;
+ if (idx > in_len) {
+ ret = -EINVAL;
+ goto exit_free;
+ }
+ 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_parse_ipv4(void)
+{
+ gwdns_answ_data d;
+ uint16_t txid;
+
+ uint8_t recv_pkt[] = {
+ 0x23, 0xc6, 0x81, 0x80, 0x00, 0x01, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00,
+ 0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03, 0x63, 0x6f, 0x6d, 0x00,
+ 0x00, 0x01, 0x00, 0x01, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x8a, 0xc0, 0x0c, 0x00, 0x01,
+ 0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x66,
+ 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04,
+ 0x4a, 0x7d, 0x18, 0x64, 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+ 0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x8b, 0xc0, 0x0c, 0x00, 0x01,
+ 0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04, 0x4a, 0x7d, 0x18, 0x65,
+ 0xc0, 0x0c, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x35, 0x00, 0x04,
+ 0x4a, 0x7d, 0x18, 0x71
+ };
+
+ memcpy(&txid, recv_pkt, 2);
+ assert(!serialize_answ(txid, recv_pkt, sizeof(recv_pkt), &d));
+ for (size_t i = 0; i < d.hdr.ancount; i++) {
+ struct gwp_sockaddr gs;
+ gwdns_serialized_answ *answ;
+ char buff[FULL_ADDRSTRLEN];
+
+ memset(&gs, 0, sizeof(gs));
+ answ = d.rr_answ[i];
+ assert(answ->rdlength == 4);
+ gs.sa.sa_family = AF_INET;
+ memcpy(&gs.i4.sin_addr, answ->rdata, 4);
+ convert_ssaddr_to_str(buff, &gs);
+ printf("IPv4: %s\n", buff);
+ }
+ free_serialize_answ(&d);
+}
+
+void test_parse_ipv6(void)
+{
+ gwdns_answ_data d;
+ uint16_t txid;
+ int ret;
+
+ uint8_t recv_pkt[] = {
+ 0x45, 0x67,
+ 0x81, 0x80,
+ 0x00, 0x01,
+ 0x00, 0x04,
+ 0x00, 0x00,
+ 0x00, 0x00,
+
+ 0x06, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65,
+ 0x03, 0x63, 0x6f, 0x6d,
+ 0x00,
+ 0x00, 0x1c,
+ 0x00, 0x01,
+
+ 0xc0, 0x0c,
+ 0x00, 0x1c,
+ 0x00, 0x01,
+ 0x00, 0x00, 0x09, 0x06,
+ 0x00, 0x10,
+ 0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x71,
+
+ 0xc0, 0x0c,
+ 0x00, 0x1c, 0x00, 0x01, 0x00, 0x00, 0x09, 0x06,
+ 0x00, 0x10,
+ 0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x8a,
+
+ 0xc0, 0x0c,
+ 0x00, 0x1c,
+ 0x00, 0x01,
+ 0x00, 0x00, 0x09, 0x06,
+ 0x00, 0x10,
+ 0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x65,
+
+ 0xc0, 0x0c,
+ 0x00, 0x1c,
+ 0x00, 0x01,
+ 0x00, 0x00, 0x0c, 0x16,
+ 0x00, 0x10,
+ 0x24, 0x04, 0x68, 0x00, 0x40, 0x03, 0x0c, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x71
+ };
+
+ memcpy(&txid, recv_pkt, sizeof(txid));
+
+ ret = serialize_answ(txid, recv_pkt, sizeof(recv_pkt), &d);
+ assert(!ret);
+ for (size_t i = 0; i < d.hdr.ancount; i++) {
+ struct gwp_sockaddr gs;
+ gwdns_serialized_answ *answ;
+ char buff[FULL_ADDRSTRLEN];
+
+ memset(&gs, 0, sizeof(gs));
+ answ = d.rr_answ[i];
+ assert(answ->rdlength == 16);
+ gs.sa.sa_family = AF_INET6;
+ memcpy(&gs.i6.sin6_addr, answ->rdata, 16);
+ convert_ssaddr_to_str(buff, &gs);
+ printf("IPv6: %s\n", buff);
+ }
+ free_serialize_answ(&d);
+}
+
+/*
+ * test mock data of recv in both IPv4 and IPv6
+ *
+ * the mock data are produced by this script:
+ * https://gist.github.com/realyukii/d7b450b4ddc305c66a2d8cd5600f23c4
+ */
+void run_all_tests(void)
+{
+ /*
+ * We can't use serialize_answ to parse multiple response at once.
+ * The caller MUST call serialize_answ more than one time if there's
+ * more than one response, because txid is passed to only verify one
+ * response.
+ */
+ test_parse_ipv4();
+ test_parse_ipv6();
+ 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
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
` (3 preceding siblings ...)
2025-08-06 3:57 ` [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function Ahmad Gani
@ 2025-08-06 3:57 ` Ahmad Gani
2025-08-06 6:28 ` Alviro Iskandar Setiawan
2025-08-06 3:57 ` [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous Ahmad Gani
` (3 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
It gives the caller more flexibility in handling DNS requests.
A single domain name lookup can have two responses: one for IPv4 and one
for IPv6. With this change, the caller may use the same txid for both queries,
making it easier to group related DNS responses.
Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
src/gwproxy/dnsparser.c | 7 +++++--
src/gwproxy/dnsparser.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/gwproxy/dnsparser.c b/src/gwproxy/dnsparser.c
index 744dc61581ef..2d8af1da2893 100644
--- a/src/gwproxy/dnsparser.c
+++ b/src/gwproxy/dnsparser.c
@@ -65,9 +65,9 @@ static ssize_t calculate_question_len(uint8_t *in, size_t in_len)
int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
{
- size_t idx;
gwdns_header_pkt *hdr;
uint16_t raw_flags;
+ size_t idx;
int ret;
idx = sizeof(*hdr);
@@ -244,7 +244,10 @@ ssize_t construct_question(gwdns_question_part *question)
* the memset implicitly set opcode to OPCODE_QUERY
*/
memset(hdr, 0, sizeof(*hdr));
- hdr->id = htons((uint16_t)rand());
+ /*
+ * no need to htons, so no ntohs for comparison in serialize_answ.
+ */
+ hdr->id = question->txid;
DNS_SET_RD(hdr->flags, true);
hdr->flags = htons(hdr->flags);
hdr->qdcount = htons(1);
diff --git a/src/gwproxy/dnsparser.h b/src/gwproxy/dnsparser.h
index 41048568240b..502281ad9d57 100644
--- a/src/gwproxy/dnsparser.h
+++ b/src/gwproxy/dnsparser.h
@@ -116,6 +116,7 @@ typedef struct {
typedef struct {
uint8_t *dst_buffer;
+ uint16_t txid;
uint16_t type;
size_t dst_len;
const char *domain;
--
Ahmad Gani
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
` (4 preceding siblings ...)
2025-08-06 3:57 ` [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller Ahmad Gani
@ 2025-08-06 3:57 ` Ahmad Gani
2025-08-06 6:22 ` Alviro Iskandar Setiawan
2025-08-06 7:03 ` [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ammar Faizi
` (2 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 3:57 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Ahmad Gani, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
The previous gw_ares_getaddrinfo version is blocking and no different
with glibc's getaddrinfo, this changes introduce asynchronous version of
gw_ares_getaddrinfo function.
Signed-off-by: Ahmad Gani <reyuki@gnuweeb.org>
---
src/gwproxy/dnslookup.c | 367 ++++++++++++++++++++++++++++------------
src/gwproxy/dnslookup.h | 99 ++++++++---
src/gwproxy/dnsparser.c | 14 +-
3 files changed, 337 insertions(+), 143 deletions(-)
diff --git a/src/gwproxy/dnslookup.c b/src/gwproxy/dnslookup.c
index de286fb5ab14..4832394adbed 100644
--- a/src/gwproxy/dnslookup.c
+++ b/src/gwproxy/dnslookup.c
@@ -6,48 +6,45 @@
#include <gwproxy/dnslookup.h>
#include <gwproxy/dnsparser.h>
#include <gwproxy/syscall.h>
-#include <sys/syscall.h>
+#include <sys/epoll.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)
+int gw_ares_process_event(gw_ares_channel_t *channel)
{
- uint8_t send_buff[UDP_MSG_LIMIT];
uint8_t recv_buff[UDP_MSG_LIMIT];
- gwdns_query_pkt *query_pkt;
+ struct gw_ares_request *req;
gwdns_answ_data raw_answ;
- gwdns_question_part q;
- ssize_t buff_len;
+ socklen_t addrlen;
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;
-
- ret = __sys_send(sockfd, send_buff, buff_len, MSG_NOSIGNAL);
+ addrlen = sizeof(*channel->addr);
+ ret = __sys_recvfrom(channel->sockfd, recv_buff, UDP_MSG_LIMIT,
+ MSG_NOSIGNAL, &channel->addr->sa, &addrlen);
if (ret < 0) {
- ret = GW_ARES_INTERNAL_ERR;
+ /* GW_ARES_WAITING is unused for now. */
+ if (ret == -EAGAIN)
+ ret = GW_ARES_WAITING;
+ else
+ ret = GW_ARES_INTERNAL_ERR;
return ret;
}
- ret = __sys_recv(sockfd, recv_buff, UDP_MSG_LIMIT, MSG_NOSIGNAL);
- if (ret < 0) {
- ret = GW_ARES_INTERNAL_ERR;
- return ret;
+ if (ret < 2)
+ return GW_ARES_EINVAL;
+
+ printf("%d bytes received\n", ret);
+ req = NULL;
+ int idx;
+ for (idx = 0; idx < channel->req_nr; idx++) {
+ if (!memcmp(&channel->reqs[idx]->txid, recv_buff, 2)) {
+ req = channel->reqs[idx];
+ break;
+ }
}
+ if (!req)
+ return GW_ARES_NOENT;
+
/*
* TODO(reyuki): even though it's unlikely,
* but what todo when the connection is closed?
@@ -55,7 +52,8 @@ static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t np
*/
assert(ret);
- ret = serialize_answ(query_pkt->hdr.id, recv_buff, ret, &raw_answ);
+ memset(&raw_answ, 0, sizeof(raw_answ));
+ ret = serialize_answ(req->txid, recv_buff, ret, &raw_answ);
if (ret) {
/*
* TODO(reyuki): the reason of failure can vary,
@@ -63,13 +61,20 @@ static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t np
* short recv, but this is UDP packet, retry from send?
* or just drop the request?
*/
+ assert(ret == -ENODATA);
}
- 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));
+ printf("ancount=%hu\n", raw_answ.hdr.ancount);
+
+ size_t i;
+ for (i = 0; i < raw_answ.hdr.ancount; i++) {
+ struct gw_addrinfo_node *new_node;
+ gwdns_serialized_answ *answ;
+ struct sockaddr_in6 *i6;
+ struct sockaddr_in *i4;
+
+ answ = raw_answ.rr_answ[i];
+ new_node = malloc(sizeof(*new_node));
if (!new_node) {
ret = GW_ARES_ENOMEM;
goto exit_free;
@@ -77,119 +82,193 @@ static int resolve_name(int sockfd, const char *name, uint16_t type, uint16_t np
new_node->ai_next = NULL;
if (answ->rr_type == TYPE_AAAA) {
+ i6 = &new_node->ai_addr.i6;
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;
+ new_node->ai_addrlen = sizeof(i6);
+ i6->sin6_port = req->dst_port;
+ 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);
+ assert(sizeof(i6->sin6_addr) == answ->rdlength);
+ memcpy(&i6->sin6_addr, answ->rdata, answ->rdlength);
} else {
+ i4 = &new_node->ai_addr.i4;
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;
+ new_node->ai_addrlen = sizeof(i4);
+ i4->sin_port = req->dst_port;
+ 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);
+ assert(sizeof(i4->sin_addr) == answ->rdlength);
+ memcpy(&i4->sin_addr, answ->rdata, answ->rdlength);
new_node->ai_ttl = answ->ttl;
}
- if (!*tail)
- result->nodes = new_node;
+ if (!req->tail)
+ req->results->nodes = new_node;
else
- (*tail)->ai_next = new_node;
- *tail = new_node;
+ req->tail->ai_next = new_node;
+ req->tail = new_node;
}
ret = GW_ARES_SUCCESS;
exit_free:
free_serialize_answ(&raw_answ);
+ req->refcnt--;
+ assert(req->refcnt >= 0);
+ if (!req->refcnt) {
+ channel->getaddrinfo_cb(
+ req->callback_args, GW_ARES_SUCCESS, req->results
+ );
+ free(req);
+ channel->req_nr--;
+ req = channel->reqs[channel->req_nr];
+ channel->reqs[idx] = req;
+ channel->reqs[channel->req_nr] = NULL;
+ }
return ret;
}
+static int resolve_name(gw_ares_channel_t *channel, const char *name,
+ uint16_t type, uint16_t txid)
+{
+ uint8_t send_buff[UDP_MSG_LIMIT];
+ gwdns_question_part q;
+ ssize_t buff_len;
+ int ret;
+
+ q.domain = name;
+ q.type = type;
+ q.txid = txid;
+ 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;
+ }
+
+ ret = __sys_sendto(
+ channel->sockfd, send_buff, buff_len, MSG_NOSIGNAL,
+ &channel->addr->sa, channel->addrlen
+ );
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int resize_reqs(gw_ares_channel_t *channel)
+{
+ struct gw_ares_request **ptr;
+ int new_cap;
+
+ new_cap = channel->req_cap * 2;
+ ptr = realloc(channel->reqs, new_cap);
+ if (!ptr)
+ return -ENOMEM;
+
+ memset(ptr[channel->req_nr], 0, channel->req_nr * sizeof(*channel->reqs));
+ channel->reqs = ptr;
+ channel->req_cap = new_cap;
+
+ return 0;
+}
+
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)
+ const struct gw_ares_addrinfo_hints *hints, void *arg)
{
- struct gw_ares_addrinfo *result;
- struct gw_addrinfo_node *tail;
- struct gwp_sockaddr *addr;
- socklen_t addrlen;
- int ret, sockfd;
+ struct gw_ares_addrinfo *results;
+ struct gw_ares_request *req;
uint16_t nport;
uint8_t mask;
+ int ret;
+
+ if (channel->req_nr == channel->req_cap) {
+ ret = resize_reqs(channel);
+ if (ret) {
+ ret = GW_ARES_ENOMEM;
+ goto error;
+ }
+ }
+ assert(channel->req_nr < channel->req_cap);
+
+ req = malloc(sizeof(*req));
+ if (!req) {
+ ret = GW_ARES_ENOMEM;
+ goto error;
+ }
+ channel->reqs[channel->req_nr] = req;
+ channel->req_nr++;
+
+ req->tail = NULL;
+ req->txid = (uint16_t)rand();
switch (hints->ai_family) {
case AF_UNSPEC:
mask = I6_BIT | I4_BIT;
+ req->refcnt = 2;
break;
case AF_INET:
+ req->refcnt = 1;
mask = I4_BIT;
break;
case AF_INET6:
+ req->refcnt = 1;
mask = I6_BIT;
break;
default:
ret = GW_ARES_EINVAL;
- goto error;
+ goto error_free_req;
}
nport = (uint16_t)atoi(service);
if (!nport) {
ret = GW_ARES_EINVAL;
- goto error;
+ goto error_free_req;
}
nport = htons(nport);
- result = malloc(sizeof(*result));
- if (!result) {
+ results = malloc(sizeof(*results));
+ if (!results) {
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;
+ goto error_free_req;
}
- 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;
- }
+ req->dst_port = nport;
+ req->results = results;
- tail = NULL;
if (IS_I6(mask)) {
- ret = resolve_name(sockfd, name, TYPE_AAAA, nport, result, &tail);
+ ret = resolve_name(channel, name, TYPE_AAAA, req->txid);
if (ret)
- goto error_close;
+ goto error_free_all;
}
if (IS_I4(mask)) {
- ret = resolve_name(sockfd, name, TYPE_A, nport, result, &tail);
+ ret = resolve_name(channel, name, TYPE_A, req->txid);
if (ret)
- goto error_close;
+ goto error_free_all;
}
- callback(arg, GW_ARES_SUCCESS, result);
return;
-error_close:
- __sys_close(sockfd);
-error_free:
- free(result);
+error_free_all:
+ free(results);
+error_free_req:
+ free(req);
+ channel->req_nr--;
+ channel->reqs[channel->req_nr] = NULL;
error:
- callback(arg, ret, result);
+ channel->getaddrinfo_cb(arg, ret, results);
}
void gw_ares_freeaddrinfo(struct gw_ares_addrinfo *ai)
@@ -218,68 +297,136 @@ int gw_ares_init(gw_ares_channel_t **channel, struct gw_ares_options *opts)
*channel = c;
c->nr_server = opts->nr_server;
+
+ ret = -ENOMEM;
+
+ c->reqs = calloc(DEFAULT_REQ_CAP, sizeof(*c->reqs));
+ if (!c->reqs)
+ goto error_free_c;
+
c->servers = malloc(c->nr_server * sizeof(*c->servers));
- if (!c->servers) {
- free(c);
- return -ENOMEM;
- }
+ if (!c->servers)
+ goto error_free_reqs;
+
/*
- * TODO(reyuki): validate flags and
- * for now use it to control recursion desired (RD) bit?
+ * TODO(reyuki): validate flags
+ * For now flags is unused but it may be used to control
+ * recursion desired (RD) bit and other things in the future.
*/
c->flags = opts->flags;
- for (int i = 0; i < c->nr_server; i++) {
- ret = convert_str_to_ssaddr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
- if (ret) {
- free(c->servers);
- free(c);
- return ret;
- }
+ int i;
+ for (i = 0; i < c->nr_server; i++) {
+ ret = convert_str_to_ssaddr(
+ opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT
+ );
+ if (ret)
+ goto error_free_all;
}
+ ret = __sys_socket(c->servers[0].sa.sa_family, SOCK_DGRAM | SOCK_NONBLOCK, 0);
+ if (ret < 0)
+ goto error_free_all;
+
+ opts->sockstate_cb(opts->sockstate_data, ret);
+ c->req_cap = DEFAULT_REQ_CAP;
+ c->req_nr = 0;
+ c->getaddrinfo_cb = opts->getaddrinfo_cb;
+ c->sockfd = ret;
+ c->addr = &c->servers[0];
+ c->addrlen = c->addr->sa.sa_family == AF_INET ?
+ sizeof(c->addr->i4) : sizeof(c->addr->i6);
+
return 0;
+error_free_all:
+ free(c->servers);
+error_free_reqs:
+ free(c->reqs);
+error_free_c:
+ free(c);
+ return ret;
}
void gw_ares_deinit(gw_ares_channel_t *channel)
{
free(channel->servers);
+ free(channel->reqs);
free(channel);
}
-static void gw_ares_cb(void *arg, int status, struct gw_ares_addrinfo *result)
+static void sockstate_cb(void *data, int socket)
+{
+ int epfd = *(int *)data;
+ struct epoll_event ev = {
+ .events = EPOLLIN
+ };
+
+ __sys_epoll_ctl(epfd, EPOLL_CTL_ADD, socket, &ev);
+}
+
+static void getaddrinfo_cb(void *arg, int status, struct gw_ares_addrinfo *result)
{
struct gw_addrinfo_node *node;
char buf[FULL_ADDRSTRLEN];
(void)arg;
+ if (status)
+ printf("status=%d\n", status);
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);
+ printf(
+ "%s: %s\n",
+ node->ai_family == AF_INET6 ? "IPv6" : "IPv4", buf
+ );
node = node->ai_next;
}
+
gw_ares_freeaddrinfo(result);
}
int main(void)
{
+ struct gw_ares_addrinfo_hints hints;
+ struct gw_ares_options opts;
gw_ares_channel_t *channel;
- struct gw_ares_addrinfo_hints hints = {
- .ai_family = AF_UNSPEC
- };
+ struct epoll_event ev;
+ int ret, epfd;
+
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;
+
+ epfd = __sys_epoll_create1(0);
+ if (epfd < 0)
+ return -EXIT_FAILURE;
+
+ opts.flags = 0;
+ opts.nr_server = 1;
+ opts.servers = servers;
+ opts.sockstate_data = &epfd;
+ opts.sockstate_cb = sockstate_cb;
+ opts.getaddrinfo_cb = getaddrinfo_cb;
ret = gw_ares_init(&channel, &opts);
if (ret)
return -EXIT_FAILURE;
- gw_ares_getaddrinfo(channel, "google.com", "80", &hints, gw_ares_cb, NULL);
+
+ hints.ai_family = AF_UNSPEC;
+ gw_ares_getaddrinfo(channel, "google.com", "80", &hints, NULL);
+ gw_ares_getaddrinfo(channel, "facebook.com", "80", &hints, NULL);
+ gw_ares_getaddrinfo(channel, "github.com", "80", &hints, NULL);
+
+ while (true) {
+ if (!channel->req_nr)
+ break;
+
+ ret = __sys_epoll_wait(epfd, &ev, 1024, -1);
+ if (ret < 0)
+ return -EXIT_FAILURE;
+
+ if (ev.events & EPOLLIN)
+ gw_ares_process_event(channel);
+ }
+
gw_ares_deinit(channel);
}
diff --git a/src/gwproxy/dnslookup.h b/src/gwproxy/dnslookup.h
index a4ed02cab5f1..a98ed6cff23c 100644
--- a/src/gwproxy/dnslookup.h
+++ b/src/gwproxy/dnslookup.h
@@ -3,6 +3,7 @@
#include <errno.h>
#include <gwproxy/net.h>
+#define DEFAULT_REQ_CAP 50
#define DEFAULT_DOMAIN_PORT 53
#define I6_BIT (1u << 0)
#define I4_BIT (1u << 1)
@@ -18,31 +19,35 @@ enum {
* internal error can be interpreted as system call failure,
* however, the cause of it can be vary.
*/
- GW_ARES_INTERNAL_ERR = 3
+ GW_ARES_INTERNAL_ERR = 3,
+ GW_ARES_WAITING = 4,
+ GW_ARES_NOENT = 5
};
-struct gw_ares_options {
- int flags;
- int nr_server;
+typedef void (*gw_ares_sockstate_cb)(void *data, int socket);
- /*
- * 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_addrinfo {
+ struct gw_addrinfo_node *nodes;
+ char *name;
};
-struct gw_ares_channeldata {
+/*
+ * result is only initialized if status == GW_ARES_SUCCESS.
+ */
+typedef void (*gw_ares_addrinfo_cb)(void *arg, int status,
+ struct gw_ares_addrinfo *result);
+
+struct gw_ares_options {
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;
+ /*
+ * the string format is ip:port, the ip may be encapsulated with square
+ * brackets ([]), and must be if using IPv6.
+ */
+ const char **servers;
+ gw_ares_sockstate_cb sockstate_cb;
+ gw_ares_addrinfo_cb getaddrinfo_cb;
+ void *sockstate_data;
};
/*
@@ -59,16 +64,36 @@ struct gw_addrinfo_node {
struct gw_addrinfo_node *ai_next;
};
-struct gw_ares_addrinfo {
- struct gw_addrinfo_node *nodes;
- char *name;
+struct gw_ares_request {
+ int refcnt;
+ uint16_t dst_port;
+ uint16_t txid;
+ struct gw_ares_addrinfo *results;
+ struct gw_addrinfo_node *tail;
+ void *callback_args;
};
-/*
- * result is only initialized if status == GW_ARES_SUCCESS.
- */
-typedef void (*gw_ares_addrinfo_callback)(void *arg, int status,
- struct gw_ares_addrinfo *result);
+struct gw_ares_channeldata {
+ int flags;
+ int nr_server;
+ /* currently only index 0 is used, and others are ignored. */
+ struct gwp_sockaddr *servers;
+ gw_ares_addrinfo_cb getaddrinfo_cb;
+
+ int sockfd;
+ struct gwp_sockaddr *addr;
+ socklen_t addrlen;
+
+ struct gw_ares_request **reqs;
+ int req_nr;
+ int req_cap;
+};
+
+typedef struct gw_ares_channeldata gw_ares_channel_t;
+
+struct gw_ares_addrinfo_hints {
+ int ai_family;
+};
/*
* Initiate a host query by name and service
@@ -86,8 +111,7 @@ typedef void (*gw_ares_addrinfo_callback)(void *arg, int status,
*/
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);
+ const struct gw_ares_addrinfo_hints *hints, void *arg);
/*
* Free the resources allocated by gw_ares_getaddrinfo.
@@ -115,4 +139,23 @@ void gw_ares_freeaddrinfo(struct gw_ares_addrinfo *ai);
* @return zero on success and negative integer on error
*/
int gw_ares_init(gw_ares_channel_t **channel, struct gw_ares_options *opts);
+
+/*
+ * Free the resources allocated by gw_ares_init.
+ *
+ * @param channel
+ */
void gw_ares_deinit(gw_ares_channel_t *channel);
+
+/*
+ * Process event
+ *
+ * Description:
+ * the gw_ares_process_event function performing socket-related operation,
+ * you MUST call this when certain I/O in DNS socket file descriptor
+ * is ready to use.
+ *
+ * @param channel
+ * @return zero on success and negative integer on error
+ */
+int gw_ares_process_event(gw_ares_channel_t *channel);
diff --git a/src/gwproxy/dnsparser.c b/src/gwproxy/dnsparser.c
index 2d8af1da2893..442ea5558174 100644
--- a/src/gwproxy/dnsparser.c
+++ b/src/gwproxy/dnsparser.c
@@ -122,7 +122,8 @@ int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *o
if (!out->rr_answ)
return -ENOMEM;
- for (size_t i = 0; i < hdr->ancount; i++) {
+ size_t i;
+ for (i = 0; i < hdr->ancount; i++) {
uint16_t is_compressed, rdlength;
gwdns_serialized_answ *item = malloc(sizeof(gwdns_serialized_answ));
if (!item) {
@@ -211,7 +212,7 @@ int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *o
return 0;
exit_free:
- for (size_t i = 0; i < out->hdr.ancount; i++) {
+ for (i = 0; i < out->hdr.ancount; i++) {
free(out->rr_answ[i]->rdata);
free(out->rr_answ[i]);
}
@@ -221,7 +222,8 @@ exit_free:
void free_serialize_answ(gwdns_answ_data *answ)
{
- for (size_t i = 0; i < answ->hdr.ancount; i++) {
+ size_t i;
+ for (i = 0; i < answ->hdr.ancount; i++) {
free(answ->rr_answ[i]->rdata);
free(answ->rr_answ[i]);
}
@@ -300,7 +302,8 @@ void test_parse_ipv4(void)
memcpy(&txid, recv_pkt, 2);
assert(!serialize_answ(txid, recv_pkt, sizeof(recv_pkt), &d));
- for (size_t i = 0; i < d.hdr.ancount; i++) {
+ size_t i;
+ for (i = 0; i < d.hdr.ancount; i++) {
struct gwp_sockaddr gs;
gwdns_serialized_answ *answ;
char buff[FULL_ADDRSTRLEN];
@@ -367,7 +370,8 @@ void test_parse_ipv6(void)
ret = serialize_answ(txid, recv_pkt, sizeof(recv_pkt), &d);
assert(!ret);
- for (size_t i = 0; i < d.hdr.ancount; i++) {
+ size_t i;
+ for (i = 0; i < d.hdr.ancount; i++) {
struct gwp_sockaddr gs;
gwdns_serialized_answ *answ;
char buff[FULL_ADDRSTRLEN];
--
Ahmad Gani
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c
2025-08-06 3:57 ` [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c Ahmad Gani
@ 2025-08-06 6:18 ` Alviro Iskandar Setiawan
0 siblings, 0 replies; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 6:18 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 2/6] dnslookup: Add a new parameter default_port
2025-08-06 3:57 ` [PATCH gwproxy v4 2/6] dnslookup: Add a new parameter default_port Ahmad Gani
@ 2025-08-06 6:20 ` Alviro Iskandar Setiawan
0 siblings, 0 replies; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 6:20 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous
2025-08-06 3:57 ` [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous Ahmad Gani
@ 2025-08-06 6:22 ` Alviro Iskandar Setiawan
2025-08-06 8:42 ` Ahmad Gani
0 siblings, 1 reply; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 6:22 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> @@ -122,7 +122,8 @@ int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *o
> if (!out->rr_answ)
> return -ENOMEM;
>
> - for (size_t i = 0; i < hdr->ancount; i++) {
> + size_t i;
> + for (i = 0; i < hdr->ancount; i++) {
This kind of change should be squashed. And var declaration should be
placed on the top.
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller
2025-08-06 3:57 ` [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller Ahmad Gani
@ 2025-08-06 6:28 ` Alviro Iskandar Setiawan
2025-08-06 10:20 ` Ahmad Gani
0 siblings, 1 reply; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 6:28 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> --- a/src/gwproxy/dnsparser.c
> +++ b/src/gwproxy/dnsparser.c
> @@ -65,9 +65,9 @@ static ssize_t calculate_question_len(uint8_t *in, size_t in_len)
>
> int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
> {
> - size_t idx;
> gwdns_header_pkt *hdr;
> uint16_t raw_flags;
> + size_t idx;
> int ret;
Don't yield unrelated changes. Something like this, variable
reordering, should be squashed. It has no relevance with "Transaction
id creation is delegated to caller".
The subject should be changed too. Use imperative form.
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 3:57 ` [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function Ahmad Gani
@ 2025-08-06 6:40 ` Alviro Iskandar Setiawan
2025-08-06 6:51 ` Alviro Iskandar Setiawan
2025-08-06 8:40 ` Ahmad Gani
2025-08-06 11:09 ` Alviro Iskandar Setiawan
1 sibling, 2 replies; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 6:40 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 10:57 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;
> +
> + c = malloc(sizeof(*c));
> + if (!c)
> + return -ENOMEM;
> +
> + *channel = c;
> + c->nr_server = opts->nr_server;
> + c->servers = malloc(c->nr_server * sizeof(*c->servers));
> + if (!c->servers) {
> + free(c);
> + 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 = convert_str_to_ssaddr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
> + if (ret) {
> + free(c->servers);
> + free(c);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
Again, I don't see any point in filling `*channel` with a dangling
pointer on error.
This part "*channel = c", can't you do that only when successful?
Please address the previous review properly. Don't ignore comments.
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 6:40 ` Alviro Iskandar Setiawan
@ 2025-08-06 6:51 ` Alviro Iskandar Setiawan
2025-08-06 8:43 ` Ahmad Gani
2025-08-06 8:40 ` Ahmad Gani
1 sibling, 1 reply; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 6:51 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 1:40 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 10:57 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;
> > +
> > + c = malloc(sizeof(*c));
> > + if (!c)
> > + return -ENOMEM;
> > +
> > + *channel = c;
> > + c->nr_server = opts->nr_server;
> > + c->servers = malloc(c->nr_server * sizeof(*c->servers));
> > + if (!c->servers) {
> > + free(c);
> > + 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 = convert_str_to_ssaddr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
> > + if (ret) {
> > + free(c->servers);
> > + free(c);
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
>
> Again, I don't see any point in filling `*channel` with a dangling
> pointer on error.
>
> This part "*channel = c", can't you do that only when successful?
>
> Please address the previous review properly. Don't ignore comments.
Also, the for loop needs a coding style fix. Don't declare the
variable inside the for loop argument. Please stop repeating the same
mistake again and again.
DO NOT DO THIS:
for (int i = 0; i < N; i++) {
something;
something;
}
DO THIS:
int i;
for (i = 0; i < N; i++) {
something;
something;
}
You should also incorporate other rules like placing variable
declarations on the top (beginning of the scope). For example, I am
still seeing this mistake:
DO NOT DO THIS:
void func(void)
{
func2();
func3();
int i;
for (i = 0; i < N; i++) {
func4();
func5();
}
}
DO THIS:
void func(void)
{
int i;
func2();
func3();
for (i = 0; i < N; i++) {
func4();
func5();
}
}
How damn hard is it to understand?
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
` (5 preceding siblings ...)
2025-08-06 3:57 ` [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous Ahmad Gani
@ 2025-08-06 7:03 ` Ammar Faizi
2025-08-06 7:08 ` Ammar Faizi
2025-08-06 7:08 ` (subset) " Ammar Faizi
8 siblings, 0 replies; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 7:03 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 10:57:18AM +0700, Ahmad Gani wrote:
> Ahmad Gani (6):
> dnslookup: Split common functionality and struct into net.h and net.c
> dnslookup: Add a new parameter default_port
> dnslookup: Allow only port string number
> dnslookup: Initial work for implementation of C-ares-like getaddrinfo
> function
> dnsparser: Transaction id creation is delegated to caller
> dnslookup: Make gw_ares_getaddrinfo asynchronous
Please don't leave trailing whitespaces. Patch #4 and patch #6 are dirty.
--------
dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
.git/rebase-apply/patch:68: trailing whitespace.
/*
.git/rebase-apply/patch:326: trailing whitespace.
* internal error can be interpreted as system call failure,
.git/rebase-apply/patch:336: trailing whitespace.
/*
.git/rebase-apply/patch:359: trailing whitespace.
*
.git/rebase-apply/patch:381: trailing whitespace.
/*
warning: squelched 11 whitespace errors
warning: 16 lines add whitespace errors.
---------
dnslookup: Make gw_ares_getaddrinfo asynchronous
.git/rebase-apply/patch:583: trailing whitespace.
/*
.git/rebase-apply/patch:656: trailing whitespace.
*
.git/rebase-apply/patch:663: trailing whitespace.
*
.git/rebase-apply/patch:668: trailing whitespace.
*
warning: 4 lines add whitespace errors.
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
` (6 preceding siblings ...)
2025-08-06 7:03 ` [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ammar Faizi
@ 2025-08-06 7:08 ` Ammar Faizi
2025-08-06 8:37 ` Ahmad Gani
2025-08-06 7:08 ` (subset) " Ammar Faizi
8 siblings, 1 reply; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 7:08 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 10:57:18AM +0700, Ahmad Gani wrote:
> dnslookup: Split common functionality and struct into net.h and net.c
> dnslookup: Add a new parameter default_port
I have applied these two patches as they're already in a good shape.
You can rebase on top of master branch.
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: (subset) [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
` (7 preceding siblings ...)
2025-08-06 7:08 ` Ammar Faizi
@ 2025-08-06 7:08 ` Ammar Faizi
8 siblings, 0 replies; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 7:08 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, 6 Aug 2025 10:57:18 +0700, Ahmad Gani wrote:
> This is revision v4 of 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 and
> uses epoll to make it asynchronous.
>
> There are 6 patches in this series:
> - create net.h and net.c for network related functionality and struct
> - add default_port parameter to convert_str_to_ssaddr function
> - allow only port string number in service parameter of gwp_dns_queue
> - initial work for implementation of C-ares-like getaddrinfo function
> - make gw_ares_getaddrinfo function asynchronous
> - transaction id creation is delegated to caller
>
> [...]
Applied, thanks!
[1/6] dnslookup: Split common functionality and struct into net.h and net.c
commit: 70b88aa9a05411f52418a71240ad4a050e91bcd0
[2/6] dnslookup: Add a new parameter default_port
commit: e6bd4242be57a8a7356f37f7137a907822d58cc4
Best regards,
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 7:08 ` Ammar Faizi
@ 2025-08-06 8:37 ` Ahmad Gani
2025-08-06 8:58 ` Ammar Faizi
2025-08-06 22:14 ` Ammar Faizi
0 siblings, 2 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 8:37 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 2:08 PM Ammar Faizi wrote:
> On Wed, Aug 06, 2025 at 10:57:18AM +0700, Ahmad Gani wrote:
> > dnslookup: Split common functionality and struct into net.h and net.c
> > dnslookup: Add a new parameter default_port
>
> I have applied these two patches as they're already in a good shape.
> You can rebase on top of master branch.
Err... I made a small change to net.c, but it looks like the patch was
already pulled. I just added a macro guard for _GNU_SOURCE to get rid of
compiler warning about a macro constant being redeclared. What should I
do in this case?
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 6:40 ` Alviro Iskandar Setiawan
2025-08-06 6:51 ` Alviro Iskandar Setiawan
@ 2025-08-06 8:40 ` Ahmad Gani
1 sibling, 0 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 8:40 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 1:41 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 10:57 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;
> > +
> > + c = malloc(sizeof(*c));
> > + if (!c)
> > + return -ENOMEM;
> > +
> > + *channel = c;
> > + c->nr_server = opts->nr_server;
> > + c->servers = malloc(c->nr_server * sizeof(*c->servers));
> > + if (!c->servers) {
> > + free(c);
> > + 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 = convert_str_to_ssaddr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
> > + if (ret) {
> > + free(c->servers);
> > + free(c);
> > + return ret;
> > + }
> > + }
> > +
> > + return 0;
> > +}
>
> Again, I don't see any point in filling `*channel` with a dangling
> pointer on error.
>
> This part "*channel = c", can't you do that only when successful?
>
> Please address the previous review properly. Don't ignore comments.
Eh, sorry—I overlooked that. I didn't recheck it because I assumed it was
acceptable, since there were no comments on patchset v2 where the change
was first made. By "avoiding a dangling pointer", I thought the request
meant not to assign to the channel if memory allocation for c failed.
Anyway, don't worry, I will update it in the next patchset.
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous
2025-08-06 6:22 ` Alviro Iskandar Setiawan
@ 2025-08-06 8:42 ` Ahmad Gani
0 siblings, 0 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 8:42 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 1:22 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > @@ -122,7 +122,8 @@ int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *o
> > if (!out->rr_answ)
> > return -ENOMEM;
> >
> > - for (size_t i = 0; i < hdr->ancount; i++) {
> > + size_t i;
> > + for (i = 0; i < hdr->ancount; i++) {
>
> This kind of change should be squashed.
Alright, I will squash it.
> And var declaration should be placed on the top.
I was actually thinking of moving it to the top as well, but when you
previously [1] mentioned moving the declaration, I assumed you meant it
literally—just relocating it to the top of the loop.
[1]:
- https://lore.gnuweeb.org/gwml/CAOG64qMMV1F5OnD6wXj+xgmqpn59VagC3gBos5ESO7xRs5Qv7g@mail.gmail.com/
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 6:51 ` Alviro Iskandar Setiawan
@ 2025-08-06 8:43 ` Ahmad Gani
0 siblings, 0 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 8:43 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 1:51 PM Alviro Iskandar Setiawan wrote:
> Also, the for loop needs a coding style fix. Don't declare the
> variable inside the for loop argument. Please stop repeating the same
> mistake again and again.
>
> DO NOT DO THIS:
>
> for (int i = 0; i < N; i++) {
> something;
> something;
> }
>
> DO THIS:
>
> int i;
> for (i = 0; i < N; i++) {
> something;
> something;
> }
Hmm, I already removed the declaration inside the for loop statements, but
it seems the patches weren't synchronized with my local file. I apologize
for overlooking this, I will try to recheck each patch to avoid the
same mistakes.
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 8:37 ` Ahmad Gani
@ 2025-08-06 8:58 ` Ammar Faizi
2025-08-06 9:47 ` Ammar Faizi
2025-08-06 22:14 ` Ammar Faizi
1 sibling, 1 reply; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 8:58 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 03:37:15PM +0700, Ahmad Gani wrote:
> On Wed, Aug 6, 2025 at 2:08 PM Ammar Faizi wrote:
> > On Wed, Aug 06, 2025 at 10:57:18AM +0700, Ahmad Gani wrote:
> > > dnslookup: Split common functionality and struct into net.h and net.c
> > > dnslookup: Add a new parameter default_port
> >
> > I have applied these two patches as they're already in a good shape.
> > You can rebase on top of master branch.
>
> Err... I made a small change to net.c, but it looks like the patch was
> already pulled. I just added a macro guard for _GNU_SOURCE to get rid of
> compiler warning about a macro constant being redeclared. What should I
> do in this case?
The CI build on GitHub didn't catch that warning. Not sure. It probably
missed -Werror flag on GitHub Actions, I'll take a look later. Still
cleaning my kitchen now...
You can send a follow up patch with a Fixes tag.
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 8:58 ` Ammar Faizi
@ 2025-08-06 9:47 ` Ammar Faizi
2025-08-06 20:50 ` Ammar Faizi
0 siblings, 1 reply; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 9:47 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 03:58:30PM +0700, Ammar Faizi wrote:
> The CI build on GitHub didn't catch that warning. Not sure. It probably
> missed -Werror flag on GitHub Actions, I'll take a look later. Still
> cleaning my kitchen now...
Yeah, it's missing -Werror flag on the CI, so it didn't fail. That warning
actually exists:
x86_64-linux-gnu-gcc -ggdb3 -fvisibility=hidden -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-stack-protector -Wunsafe-loop-optimizations -Wstrict-aliasing=3 -Wstrict-prototypes -Wmissing-prototypes -Wstack-usage=8192 -Wformat -Wformat-security -Wformat-signedness -Wunreachable-code -Wsequence-point -Wextra -Wall -O2 -fpic -fPIC -D_GNU_SOURCE -include /home/runner/work/gwproxy/gwproxy/config.h -I./src/ -DNDEBUG -MMD -MP -MF src/gwproxy/ev/epoll.c.o.d -c src/gwproxy/ev/epoll.c -o src/gwproxy/ev/epoll.c.o
src/gwproxy/net.c:1: warning: "_GNU_SOURCE" redefined
1 | #define _GNU_SOURCE
|
<command-line>: note: this is the location of the previous definition
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller
2025-08-06 6:28 ` Alviro Iskandar Setiawan
@ 2025-08-06 10:20 ` Ahmad Gani
2025-08-06 10:51 ` Alviro Iskandar Setiawan
0 siblings, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 10:20 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 1:28 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > --- a/src/gwproxy/dnsparser.c
> > +++ b/src/gwproxy/dnsparser.c
> > @@ -65,9 +65,9 @@ static ssize_t calculate_question_len(uint8_t *in, size_t in_len)
> >
> > int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwdns_answ_data *out)
> > {
> > - size_t idx;
> > gwdns_header_pkt *hdr;
> > uint16_t raw_flags;
> > + size_t idx;
> > int ret;
>
> Don't yield unrelated changes. Something like this, variable
> reordering, should be squashed. It has no relevance with "Transaction
> id creation is delegated to caller".
Err.. How do you usually clean up commits like this? I accidentally amended
the changes to the current commit, and splitting it manually without a diff
seems like a nightmare. Is there a way to make splitting a patch easier?
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller
2025-08-06 10:20 ` Ahmad Gani
@ 2025-08-06 10:51 ` Alviro Iskandar Setiawan
2025-08-06 12:29 ` Ahmad Gani
0 siblings, 1 reply; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 10:51 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 5:21 PM Ahmad Gani wrote:
> Err.. How do you usually clean up commits like this? I accidentally amended
> the changes to the current commit, and splitting it manually without a diff
> seems like a nightmare. Is there a way to make splitting a patch easier?
What do you mean by "splitting it manually without a diff"?
I don't understand the exact activity you meant.
This splitting problem is because you didn't do it in the first place.
I don't often do that.
You can unstage the unrelated hunk, then amend a previous commit to
apply the same changes where that hunk should be applied.
That's just one hunk specific to variable reordering, it should not be
a problem. If you had a large chunk, you could copy the hunk and apply
it using "git apply" anw.
But this one is not a nightmare, this is just one line change.
And even If you lost the change due to a mistake in performing "git
commit --amend", you can still fetch the old changes from git reflog.
And even if your git reflog is fully destroyed, you still have the
patches you sent via email that can be reapplied, resulting in the
same git history, just that with different SHA1 hashes.
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 3:57 ` [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function Ahmad Gani
2025-08-06 6:40 ` Alviro Iskandar Setiawan
@ 2025-08-06 11:09 ` Alviro Iskandar Setiawan
2025-08-06 11:20 ` Alviro Iskandar Setiawan
1 sibling, 1 reply; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 11:09 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> 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.
Thinking about this further, I think this design is wrong. The DNS
library is expected to only handle bytes processing, leaving the
networking stack to the caller.
Think, how is it going to be integrated with io_uring?
We should let the caller perform the recv and send. This library
doesn't need to care about the networking stack at all. Make it the
caller's responsibility to feed the library with bytes from the
network.
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 11:09 ` Alviro Iskandar Setiawan
@ 2025-08-06 11:20 ` Alviro Iskandar Setiawan
2025-08-06 11:39 ` Ammar Faizi
2025-08-06 12:30 ` Ahmad Gani
0 siblings, 2 replies; 35+ messages in thread
From: Alviro Iskandar Setiawan @ 2025-08-06 11:20 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 6:09 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > 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.
>
> Thinking about this further, I think this design is wrong. The DNS
> library is expected to only handle bytes processing, leaving the
> networking stack to the caller.
>
> Think, how is it going to be integrated with io_uring?
>
> We should let the caller perform the recv and send. This library
> doesn't need to care about the networking stack at all. Make it the
> caller's responsibility to feed the library with bytes from the
> network.
Please take a look at src/gwproxy/socks5.c
That's a perfect example of socks5 bytes processing without
networking. Purely bytes parsing.
That way you can use io_uring, epoll, or whatever shiny features, even
statically allocated fake bytes work.
-- Viro
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 11:20 ` Alviro Iskandar Setiawan
@ 2025-08-06 11:39 ` Ammar Faizi
2025-08-06 12:30 ` Ahmad Gani
2025-08-06 12:30 ` Ahmad Gani
1 sibling, 1 reply; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 11:39 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ahmad Gani, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 06:20:10PM +0700, Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 6:09 PM Alviro Iskandar Setiawan wrote:
> > On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > > 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.
> >
> > Thinking about this further, I think this design is wrong. The DNS
> > library is expected to only handle bytes processing, leaving the
> > networking stack to the caller.
> >
> > Think, how is it going to be integrated with io_uring?
> >
> > We should let the caller perform the recv and send. This library
> > doesn't need to care about the networking stack at all. Make it the
> > caller's responsibility to feed the library with bytes from the
> > network.
>
> Please take a look at src/gwproxy/socks5.c
>
> That's a perfect example of socks5 bytes processing without
> networking. Purely bytes parsing.
>
> That way you can use io_uring, epoll, or whatever shiny features, even
> statically allocated fake bytes work.
That's right, it was the last unfinished discussion. Nice that you
bring that point again.
I think we should not be following c-ares. The io_uring design simply
can't work with c-ares' style abstraction. gwproxy DNS library does not
need to have the same interface as c-ares.
And yes, socks5.c is a perfect example.
JFYI, the other day Ahmad also wrote a similar library that accepts
only bytes (for socks5). It should not be hard to convert the current
design to be like that given the experience.
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller
2025-08-06 10:51 ` Alviro Iskandar Setiawan
@ 2025-08-06 12:29 ` Ahmad Gani
0 siblings, 0 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 12:29 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 5:51 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 5:21 PM Ahmad Gani wrote:
> > Err.. How do you usually clean up commits like this? I accidentally amended
> > the changes to the current commit, and splitting it manually without a diff
> > seems like a nightmare. Is there a way to make splitting a patch easier?
>
> What do you mean by "splitting it manually without a diff"?
>
> I don't understand the exact activity you meant.
>
> This splitting problem is because you didn't do it in the first place.
> I don't often do that.
Yeah, after a second thought, this isn't a nightmare at all.
All I need to do is just git rebase the previous commit that isn't tainted
and copy-paste necessary and relevant changes from the mixed patch file
while listening to Bad Apple with full bass; it is such a relief.
/s
Anyway, after reading an email on another thread, I guess I don't need to
fix this if it's going to be redesigned anyway.
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 11:20 ` Alviro Iskandar Setiawan
2025-08-06 11:39 ` Ammar Faizi
@ 2025-08-06 12:30 ` Ahmad Gani
2025-08-06 21:33 ` Ammar Faizi
1 sibling, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 12:30 UTC (permalink / raw)
To: Alviro Iskandar Setiawan; +Cc: Ammar Faizi, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 6:20 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 6:09 PM Alviro Iskandar Setiawan wrote:
> > On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > > 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.
> >
> > Thinking about this further, I think this design is wrong. The DNS
> > library is expected to only handle bytes processing, leaving the
> > networking stack to the caller.
> >
> > Think, how is it going to be integrated with io_uring?
> >
> > We should let the caller perform the recv and send. This library
> > doesn't need to care about the networking stack at all. Make it the
> > caller's responsibility to feed the library with bytes from the
> > network.
>
> Please take a look at src/gwproxy/socks5.c
>
> That's a perfect example of socks5 bytes processing without
> networking. Purely bytes parsing.
>
> That way you can use io_uring, epoll, or whatever shiny features, even
> statically allocated fake bytes work.
Well, dnsparser.c contains pure byte parsing code, so you might want to
take a look there.
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 11:39 ` Ammar Faizi
@ 2025-08-06 12:30 ` Ahmad Gani
2025-08-06 20:54 ` Ammar Faizi
0 siblings, 1 reply; 35+ messages in thread
From: Ahmad Gani @ 2025-08-06 12:30 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 6, 2025 at 6:39 PM Ammar Faizi wrote:
> On Wed, Aug 06, 2025 at 06:20:10PM +0700, Alviro Iskandar Setiawan wrote:
> > On Wed, Aug 6, 2025 at 6:09 PM Alviro Iskandar Setiawan wrote:
> > > On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > > > 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.
> > >
> > > Thinking about this further, I think this design is wrong. The DNS
> > > library is expected to only handle bytes processing, leaving the
> > > networking stack to the caller.
> > >
> > > Think, how is it going to be integrated with io_uring?
> > >
> > > We should let the caller perform the recv and send. This library
> > > doesn't need to care about the networking stack at all. Make it the
> > > caller's responsibility to feed the library with bytes from the
> > > network.
> >
> > Please take a look at src/gwproxy/socks5.c
> >
> > That's a perfect example of socks5 bytes processing without
> > networking. Purely bytes parsing.
> >
> > That way you can use io_uring, epoll, or whatever shiny features, even
> > statically allocated fake bytes work.
>
> That's right, it was the last unfinished discussion. Nice that you
> bring that point again.
>
> I think we should not be following c-ares. The io_uring design simply
> can't work with c-ares' style abstraction. gwproxy DNS library does not
> need to have the same interface as c-ares.
Alright, let's start over again. Should I send the unfinished v5 patch series
for archival purposes? It will be a good artefak in the GNU/Weeb mailing list.
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 9:47 ` Ammar Faizi
@ 2025-08-06 20:50 ` Ammar Faizi
0 siblings, 0 replies; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 20:50 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 04:47:05PM +0700, Ammar Faizi wrote:
> Yeah, it's missing -Werror flag on the CI, so it didn't fail. That warning
> actually exists:
>
> x86_64-linux-gnu-gcc -ggdb3 -fvisibility=hidden -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-stack-protector -Wunsafe-loop-optimizations -Wstrict-aliasing=3 -Wstrict-prototypes -Wmissing-prototypes -Wstack-usage=8192 -Wformat -Wformat-security -Wformat-signedness -Wunreachable-code -Wsequence-point -Wextra -Wall -O2 -fpic -fPIC -D_GNU_SOURCE -include /home/runner/work/gwproxy/gwproxy/config.h -I./src/ -DNDEBUG -MMD -MP -MF src/gwproxy/ev/epoll.c.o.d -c src/gwproxy/ev/epoll.c -o src/gwproxy/ev/epoll.c.o
> src/gwproxy/net.c:1: warning: "_GNU_SOURCE" redefined
> 1 | #define _GNU_SOURCE
> |
> <command-line>: note: this is the location of the previous definition
I was a bit busy deep cleaning my house and had several video calls.
Now I am back to this CI thing, it turned out that when enabling
io_uring support, it breaks 32-bit architectures. This one is from
i686:
```
src/gwproxy/ev/io_uring.c: In function ‘prep_timer_del_target’:
src/gwproxy/ev/io_uring.c:327:56: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
327 | io_uring_prep_timeout_remove(s, EV_BIT_TIMER | (uint64_t)gcp, 0);
| ^
src/gwproxy/ev/io_uring.c: In function ‘handle_event’:
src/gwproxy/ev/io_uring.c:773:23: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
773 | void *udata = (void *)CLEAR_EV_BIT(cqe->user_data);
| ^
In file included from ./src/gwproxy/gwproxy.h:15,
from ./src/gwproxy/ev/io_uring.h:8,
from src/gwproxy/ev/io_uring.c:11:
src/gwproxy/ev/io_uring.c:857:26: error: format ‘%ld’ expects argument of type ‘long int’, but argument 6 has type ‘int’ [-Werror=format=]
857 | pr_err(&ctx->lh, "Bug, invalid %s: res=%d, fd=%ld, s=%s", inv_op,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
858 | cqe->res, (intptr_t)udata, strerror(-cqe->res));
| ~~~~~~~~~~~~~~~
| |
| int
./src/gwproxy/log.h:40:41: note: in definition of macro ‘pr_log’
40 | __pr_log(__hd->handle, __level, FMT, ##__VA_ARGS__); \
| ^~~
src/gwproxy/ev/io_uring.c:857:9: note: in expansion of macro ‘pr_err’
857 | pr_err(&ctx->lh, "Bug, invalid %s: res=%d, fd=%ld, s=%s", inv_op,
| ^~~~~~
src/gwproxy/ev/io_uring.c:857:57: note: format string is defined here
857 | pr_err(&ctx->lh, "Bug, invalid %s: res=%d, fd=%ld, s=%s", inv_op,
| ~~^
| |
| long int
| %d
```
Apparently this happens because a pointer size on 32-bit architectures
is 32-bit. We need double-casts to fix mess. I'll fix it.
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 12:30 ` Ahmad Gani
@ 2025-08-06 20:54 ` Ammar Faizi
0 siblings, 0 replies; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 20:54 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 07:30:55PM +0700, Ahmad Gani wrote:
> Alright, let's start over again. Should I send the unfinished v5 patch series
> for archival purposes? It will be a good artefak in the GNU/Weeb mailing list.
You can continue to v5, just drop the c-ares style thing, make it
possible for both, io_uring and epoll pattern.
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 12:30 ` Ahmad Gani
@ 2025-08-06 21:33 ` Ammar Faizi
2025-08-14 4:54 ` Ahmad Gani
0 siblings, 1 reply; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 21:33 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 07:30:08PM +0700, Ahmad Gani wrote:
> Well, dnsparser.c contains pure byte parsing code, so you might want to
> take a look there.
That part is good. You're half way to get there. But that parser doesn't
provide all necessary parts to resolve a domain.
The problem with the current design is the primary interfaces you
provide. They are gw_ares_init(), gw_ares_getaddrinfo(), etc. Those
things do 'full networking setup' which is not suitable for io_uring
pattern.
Let's make a simple rule. No syscalls in the DNS library. An exception
maybe reading /etc/hosts, but you can skip that part for now.
The DNS lib should at least accept:
- Domain name.
- Service name (port).
- Address families to be resolved (AF_INET or AF_INET6).
- Received bytes from the network.
- Pointer to recv and send buffer (including their lengths).
The DNS lib should at least generate:
- Resolved addresses in 'struct gwp_sockaddr' form.
- Bytes to be sent to the network.
If you provide such interfaces, it can be integrated with all networking
patterns. And that's what we're trying to achieve by inventing our own
library.
I didn't catch this mistake earlier because I only skimmed the code.
Thanks to Alviro for pointing it out yesterday.
You should also now realize that your "gw_ares" design is not compatible
with prep_{recv,send} calls from io_uring. Right?
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation
2025-08-06 8:37 ` Ahmad Gani
2025-08-06 8:58 ` Ammar Faizi
@ 2025-08-06 22:14 ` Ammar Faizi
1 sibling, 0 replies; 35+ messages in thread
From: Ammar Faizi @ 2025-08-06 22:14 UTC (permalink / raw)
To: Ahmad Gani; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
On Wed, Aug 06, 2025 at 03:37:15PM +0700, Ahmad Gani wrote:
> Err... I made a small change to net.c, but it looks like the patch was
> already pulled. I just added a macro guard for _GNU_SOURCE to get rid of
> compiler warning about a macro constant being redeclared. What should I
> do in this case?
FWIW, I have fixed the _GNU_SOURCE redefinition warning together with other
32-bit archs warnings.
https://github.com/GNUWeeb/gwproxy/commits/cff091cf6441845c1a8fc56b9e1216bf6cf48dc
--
Ammar Faizi
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function
2025-08-06 21:33 ` Ammar Faizi
@ 2025-08-14 4:54 ` Ahmad Gani
0 siblings, 0 replies; 35+ messages in thread
From: Ahmad Gani @ 2025-08-14 4:54 UTC (permalink / raw)
To: Ammar Faizi; +Cc: Alviro Iskandar Setiawan, GNU/Weeb Mailing List
Sorry for the late reply! I've been caught up reading manga [1] and kind of
slacking on the development of this feature. But I'm feeling motivated
again—Anyway, let's pick up where we left off.
So, the goal is to have an abstraction for name resolution without relying
on glibc's getaddrinfo, allowing asynchronous operation without separate,
dedicated DNS thread(s), right?
On Thu, 7 Aug 2025 04:33:46 +0700 Ammar Faizi wrote:
> Let's make a simple rule. No syscalls in the DNS library. An exception
> maybe reading /etc/hosts, but you can skip that part for now.
>
> The DNS lib should at least accept:
> - Domain name.
> - Service name (port).
> - Address families to be resolved (AF_INET or AF_INET6).
> - Received bytes from the network.
> - Pointer to recv and send buffer (including their lengths).
>
> The DNS lib should at least generate:
> - Resolved addresses in 'struct gwp_sockaddr' form.
> - Bytes to be sent to the network.
I've created `gwdns_build_query` and `gwdns_parse_query`, which should
meet those criteria, please take a look at new email threads [2].
[1]:
- https://nostarch.com/microprocessors
[2]:
- https://lore.gnuweeb.org/gwml/20250814044658.252579-1-reyuki@gnuweeb.org/
--
Ahmad Gani
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2025-08-14 4:55 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 3:57 [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 1/6] dnslookup: Split common functionality and struct into net.h and net.c Ahmad Gani
2025-08-06 6:18 ` Alviro Iskandar Setiawan
2025-08-06 3:57 ` [PATCH gwproxy v4 2/6] dnslookup: Add a new parameter default_port Ahmad Gani
2025-08-06 6:20 ` Alviro Iskandar Setiawan
2025-08-06 3:57 ` [PATCH gwproxy v4 3/6] dnslookup: Allow only port string number Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function Ahmad Gani
2025-08-06 6:40 ` Alviro Iskandar Setiawan
2025-08-06 6:51 ` Alviro Iskandar Setiawan
2025-08-06 8:43 ` Ahmad Gani
2025-08-06 8:40 ` Ahmad Gani
2025-08-06 11:09 ` Alviro Iskandar Setiawan
2025-08-06 11:20 ` Alviro Iskandar Setiawan
2025-08-06 11:39 ` Ammar Faizi
2025-08-06 12:30 ` Ahmad Gani
2025-08-06 20:54 ` Ammar Faizi
2025-08-06 12:30 ` Ahmad Gani
2025-08-06 21:33 ` Ammar Faizi
2025-08-14 4:54 ` Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 5/6] dnsparser: Transaction id creation is delegated to caller Ahmad Gani
2025-08-06 6:28 ` Alviro Iskandar Setiawan
2025-08-06 10:20 ` Ahmad Gani
2025-08-06 10:51 ` Alviro Iskandar Setiawan
2025-08-06 12:29 ` Ahmad Gani
2025-08-06 3:57 ` [PATCH gwproxy v4 6/6] dnslookup: Make gw_ares_getaddrinfo asynchronous Ahmad Gani
2025-08-06 6:22 ` Alviro Iskandar Setiawan
2025-08-06 8:42 ` Ahmad Gani
2025-08-06 7:03 ` [PATCH gwproxy v4 0/6] Initial work for DNS lookup implementation Ammar Faizi
2025-08-06 7:08 ` Ammar Faizi
2025-08-06 8:37 ` Ahmad Gani
2025-08-06 8:58 ` Ammar Faizi
2025-08-06 9:47 ` Ammar Faizi
2025-08-06 20:50 ` Ammar Faizi
2025-08-06 22:14 ` Ammar Faizi
2025-08-06 7:08 ` (subset) " Ammar Faizi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox