Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

streaming API #4

Closed
lun-4 opened this issue Jan 9, 2023 · 2 comments · Fixed by #5
Closed

streaming API #4

lun-4 opened this issue Jan 9, 2023 · 2 comments · Fixed by #5

Comments

@lun-4
Copy link
Owner

lun-4 commented Jan 9, 2023

right now the API assumes you want the entire packet with all of its resource data fields.

however, for std.net.getAddrInfo, this is too much. we only want to parse A, AAAA, and CNAME to resolve to an address.

we should be able to have a streaming API that lets clients ignore full parsing of the parts of the packet they don't need. this might be difficult because of dns pointers

@lun-4 lun-4 changed the title enable more memory-efficient processing streaming API Jan 9, 2023
@lun-4
Copy link
Owner Author

lun-4 commented Jan 23, 2023

the original zig issue made me aware of https://github.com/dantecatalfamo/zig-dns and this gives me an idea i'm totally not stealing.

namely, that pointer resolution becomes an optional part of the library, with that i would be able to create an API that doesn't require any sort of allocation, but can do it if you ask it to. i think at that stage you'd be able to write a stdlib-worthy DNS implementation that trusts the nameservers to give the correct names... 🤔

something like this

var parser = dns.Parser.init(bytes_from_dns);
var name_pool = dns.NamePool.init(allocator);
defer name_pool.deinit();
while (try parser.next()) |part| {
    switch (part) {
        .header => |header| // ...
        .question => |raw_question| // ...
        .answer => |raw_answer| {
            // you can use raw_answer directly, which would be RawAnswer
           _ = try raw_answer.getNames(allocator, .{}); // safe (does not recurse or loop), but gives []LabelComponent
            // you can also use raw_answer.data, decode it, and if you trust the server and its an A/AAAA, you use it
            // or
            var answer = try name_pool.resolve(raw_answer, .{}); // returns Answer instead of RawAnswer
            _ = answer.name; // less safe, but gives you the full [][]const u8
        }
    }
}

you could even build an "easy" api that does the allocations for you and creates the entire packet for the API users that don't want to optimize for that usecase

thanks for your plug, @dantecatalfamo

@lun-4
Copy link
Owner Author

lun-4 commented Feb 8, 2023

progress report:

the streaming api actually works, in streaming-api branch, zigdig-tiny won't allocate anything other than the list of std.net.Address, provided to users through dns.helpers.getAddressList:

luna@arae:~/g/zigdig streaming-api>1 *3
$ zig build && ./zig-out/bin/zigdig-tiny ziglang.org
ziglang.org has address 108.158.122.44:0
ziglang.org has address 108.158.122.128:0
ziglang.org has address 108.158.122.51:0
ziglang.org has address 108.158.122.24:0
ziglang.org has address [2600:9000:2399:400:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:2800:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:1800:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:9600:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:4e00:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:1e00:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:be00:16:958c:6440:93a1]:0
ziglang.org has address [2600:9000:2399:5800:16:958c:6440:93a1]:0

here's how the library client code looks like for the getAddressList case

the full client, however, is not finished, that needs me to finish implementations for name resolution and RDATA parsing.

after that, this issue can be closed and a new one can be made about cleaning things up for future stdlib upstreaming

@lun-4 lun-4 mentioned this issue Feb 9, 2023
@lun-4 lun-4 closed this as completed in #5 Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant