* [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
* 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
* [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
* 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
* [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
* 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 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 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 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 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 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 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 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 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
* [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
* 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 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 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
* [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 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 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 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: [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 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 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 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: (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
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