Retrieve multiple IP addresses when resolving a hostname.

[lachlansneff]

This PR changes the Dns::get_host_by_name function to Dns::get_hosts_by_name and makes it return an iterator.

[lachlansneff]

I also want to point out it doesn't make sense to return nb::Result because there's no sensible way to resume a lookup if it returns WouldBlock.

[ryan-summers]

Do you have an implementation where this would make sense? It would be helpful to have an example to demonstrate how this is intended to be used in an embedded context (although I do not know if the DNS traits are widely used yet).

[ryan-summers]

cc @MathiasKoch as I believe he's the only one using the DNS traits so far (although I intend to leverage them in the coming weeks).

[MathiasKoch]

I do use them yeah, but they are in no way optimal or ergonomic to use currently, so i am open to anything that improves the current design flaws.

I would like to see some kind of reference implementation as well though?

Also, if the case is that nb doesn't make sense, which i tend to agree with, perhaps we should just remove it?

[Sympatron]

It seems to be difficult to implement something this trait, because the Iterator would either need to keep borrowing the stack or own the list of ip addresses, which can be difficult without alloc (you could still only return a predefined maximum number of addresses). I am not entirely sure about this, but I can't see an easy way to solve this. maybe passing a &mut [IpAddr] would be simpler.

Also, if the case is that nb doesn't make sense, which i tend to agree with, perhaps we should just remove it?

I think embedded-hal is also getting rid of nb, so I would be in favor of that, too. To my understanding they are going to provide different traits for synchronous, async and future based APIs.

Some kind of asynchronous API is really important for a network stack though, because operations can take a really long time.

[lachlansneff]

If we use GATs, we could return an Iterator that borrows self.

[ryan-summers]

GATs are not stable, so we cannot yet use them.

[lachlansneff]

That's true. To take a slice, it should really take &mut [MaybeUninit<IpAddr>] and return &[IpAddr] .

[ryan-summers]

That's true. To take a slice, it should really take &mut [MaybeUninit<IpAddr>] and return &[IpAddr] .

Can you please develop a reference implementation to show how these changes would be used? I do not want to merge changes that we aren't sure will work how we'd like.

[lachlansneff]

Sure, I'll hook it up to nrfxlib.

[lachlansneff]

@ryan-summers Here's an example implementation: https://github.com/lachlansneff/nrfxlib/blob/109392c6ad2d6d85ab93330ea399a2df2be69e49/src/tcp.rs#L281-L358

One thing I'm unsure of is the intended implementation target of the trait. Is it intended to be implemented on the same type as TcpStack or UdpStack?

[ryan-summers]

I think we can just tie the lifetime of 'a to that of self. That way, any struct that impl Dns can just borrow out internal storage. E.g.

struct MyDns {
   storage: [IpAddr; 5],
}

impl Dns for MyDns {
    fn get_hosts_by_name<'s>(&'s mut self, hostname: &str, addr_type: AddrType) -> Results<&'s [IpAddr], Self::Error> {
        &self.storage[..]
    }
}

Or something similar? I don't think the user should need to pass in a storage buffer - that's a very c-like paradigm that I don't think is required with the ownership semantics Rust provides

[lachlansneff]

I'm not really sure about that. It seems like it would restrict implementations to do this one thing. And it would mean that the implementer of the network stack decides how many items a DNS lookup can have.

Returning an iterator would, by far, be the most flexible but as you said, it would require nightly because of GATs.

[ryan-summers]

Can we add a const-generic to this function argument to allow a list of addresses to be returned?

fn get_hosts_by_name<const T: usize>(&mut self, hostname: &str, addr_type: AddrType) -> Result<heapless::Vec<IpAddr, T>, Self::Error>;

[lachlansneff]

Yeah, that'd be fine. I think that'd optimize to pretty much the same thing as passing in a slice.

[ryan-summers]

One thing I'm unsure of is the intended implementation target of the trait. Is it intended to be implemented on the same type as TcpStack or UdpStack?

Generally speaking, yes. There's a single "Network stack" that provides a socket-like API to the application (e.g. smoltcp or the w5500). Thus, you're allowed to have any internal storage for the implementing structure as well. If you want to see an example, take a look at https://github.com/quartiq/smoltcp-nal (although this doesn't have DNS yet)

[lachlansneff]

I came up with an example implementation that returns an iterator that can borrow self without GATs. https://github.com/lachlansneff/nrfxlib/blob/9c814bb69f2cd7cc77ca58dbe095cb6c43b688cc/src/tcp.rs#L267-L379

Apologies for the weird indentation.

[lachlansneff]

Another option is to call a callback for each IP address returned.

[lachlansneff]

I also want to point out that because the dns methods take &mut self, any returned item that borrows self means that connect can't be called unless the addresses are first moved somewhere else, which defeats the whole point. Therefore, I think it's reasonable to return an owned iterator.

[lachlansneff]

I'm going to close this PR and open an issue about changing the embedded-nal API to more closely resemble std::net.

[Sympatron]

I came up with an example implementation that returns an iterator that can borrow self without GATs. https://github.com/lachlansneff/nrfxlib/blob/9c814bb69f2cd7cc77ca58dbe095cb6c43b688cc/src/tcp.rs#L267-L379

Apologies for the weird indentation.

This only works, because the API you are using internally calls malloc/free. Without a heap it's not possible.