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

A more useful UDP echo example #6737

Open
Zoxc opened this issue Jul 31, 2024 · 7 comments
Open

A more useful UDP echo example #6737

Zoxc opened this issue Jul 31, 2024 · 7 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-net Module: tokio/net T-docs Topic: documentation

Comments

@Zoxc
Copy link

Zoxc commented Jul 31, 2024

It would be nice if the UDP echo server example was more realistic and allowed binding to any interface.

The current example doesn't constrain the reply to occur from the same IP it was received on which means it only works when bound to a specific IP.

@Zoxc Zoxc added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Jul 31, 2024
@techtenk
Copy link

techtenk commented Aug 2, 2024

@Zoxc For clarity in my own mind, for the example to be more realistic, you would want the first socket to be open as the "server" and bind to all interfaces, probably on a specified port:
let addr = env::args() .nth(1) .unwrap_or_else(|| "0.0.0.0:6555".to_string());

and then for the client, you would open a socket on another port, use UdpSocket.connect() to specify the destination (using any of the server IP addresses) and then send the PINGS and listen for the PONGS. Replies would be guaranteed to be from the correct IP by virtue of the filtering that .connect() does. Does that sound right? Did I miss anything?

@Darksonn Darksonn added T-docs Topic: documentation M-net Module: tokio/net labels Aug 2, 2024
@Zoxc
Copy link
Author

Zoxc commented Aug 2, 2024

That would run into problems since it reuses the port and seems inefficient. A better way would be following the Sourcing packets from a wildcard socket section in https://blog.cloudflare.com/everything-you-ever-wanted-to-know-about-udp-sockets-but-were-afraid-to-ask-part-1 however that likely would need the addition of suitable APIs in tokio and possibly mio.

@techtenk
Copy link

techtenk commented Aug 2, 2024

Ok that’s what I thought you meant at first but then I saw the thing about the client not even checking if the response is from the same server and thought it might be an easier problem to solve than I first thought.

I read that section of the blog, thanks. In terms of implementing new APIs, specifically it would be an API in Tokio/Mio to be able to get the CSMG Metadata from the underlying Linux/BSD/(Windows?) API?

Is there an argument to iterate all interfaces and bind to each one individually in the example and then noting in the comments that this isn’t ideal for all use cases? That would at least open up the idea and leaves further reading up to the user. Or did you intend this issue to point out this as a limitation of Tokio and then track updating the example as a good “definition of done”.

@Zoxc
Copy link
Author

Zoxc commented Aug 4, 2024

In terms of implementing new APIs, specifically it would be an API in Tokio/Mio to be able to get the CSMG Metadata from the underlying Linux/BSD/(Windows?) API?

Yeah I think some replacement for recv_from and send_to on UdpSocket which allows the right IP to be passed along would be needed.

Or did you intend this issue to point out this as a limitation of Tokio and then track updating the example as a good “definition of done”.

I was wondering if there was an easier way that I missed, but it doesn't appear so.

I also noticed that there's a similar example on UdpSocket itself which uses wildcard IPs, so that's straight up incorrect.

@techtenk
Copy link

techtenk commented Aug 5, 2024

Yeah, I see the other example now and the potential problems with it. After a little digging it sounds like the "replacement" would be recvmsg(2). It looks like exposing that in mio has been discussed and softly rejected in the past: MIO #200 and MIO #550

If we got an implementation of recvmsg, then it's just a matter* of passing some parameters for extra information following the example here: https://blog.powerdns.com/2012/10/08/on-binding-datagram-udp-sockets-to-the-any-addresses

*just a matter - it's still messy in IPv4 but better in IPv6, and the example in the blog doesn't include Windows implementation

I could imagine a few paths forward. Not sure if I love any of them:

  1. Use recvmsg straight from nix::sys::socket in the example, and not update or change any APIs to support the example. Assuming it plays well enough with the Tokio structs to still be a useful example (I haven't dug in enough to see), this could be the most feasible solution that actually uses a wildcard approach.
  2. Write APIs to expose recvmsg through Mio and Tokio. Seems unlikely to get buy-in.
  3. Write an API that is less general "expose the underlying recvmsg" and more specific to handling this more specific use case. This seems more in the nature of Tokio/Mio at least, so buy in might be easier.
  4. Iterate all the interfaces and bind to each one as an approximation of a "wildcard" with comments explaining the limitations.

At the very minimum, if it was decided to do nothing else, the example that uses wildcards should be changed, since it could fail silently.

@techtenk
Copy link

techtenk commented Aug 7, 2024

What about using the Rust implementation of Wireguard as a precedent for how to implement this? Basically make a bind4 and bind6 for the special cases of binding to the INADDR_ANY addresses.

https://github.com/mullvad/wireguard-rs/blob/9b53a9d1a61115a328ca43955153d09cc2e969ef/src/platform/linux/udp.rs#L598

Inside those functions you could do the CMSG work and when the UDPSocket is operating in that mode, it attaches that information to each incoming packet so that you can read it off the packet when sending a response. I guess then all that would be left is to implement a send_to that can take a source IP parameter, like you originally suggested.

*I see the issue you opened in the Rust code base, feel free to carry this over there if you think it's a more appropriate issue to have the discussion.

@techtenk
Copy link

techtenk commented Aug 10, 2024

Here's some code demonstrating the issue, for anyone else coming along later:

add a file in the examples/ directory called broken-udp.rs
add network-interface = "2.0.0" to the examples/Cargo.toml file
run it with cargo run --example broken-udp

#![warn(rust_2018_idioms)]

use std::error::Error;
use std::io;
use std::net::{IpAddr, SocketAddr};
use network_interface::{NetworkInterface, NetworkInterfaceConfig};
use tokio::net::UdpSocket;

struct Server {
    socket: UdpSocket,
    buf: Vec<u8>,
    to_send: Option<(usize, SocketAddr)>,
}

impl Server {


    async fn run(self) -> Result<(), io::Error> {
        let Server {
            socket,
            mut buf,
            mut to_send,
        } = self;

        loop {
            // First we check to see if there's a message we need to echo back.
            // If so then we try to send it back to the original source, waiting
            // until it's writable and we're able to do so.
            if let Some((size, peer)) = to_send {
                let amt = socket.send_to(&buf[..size], &peer).await?;

                println!("Echoed {}/{} bytes to {}", amt, size, peer);
            }

            // If we're here then `to_send` is `None`, so we take a look for the
            // next message we're going to echo back.
            to_send = Some(socket.recv_from(&mut buf).await?);
        }

    }

}
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
    let serv_addr = "0.0.0.0:0";

    let socket = UdpSocket::bind(&serv_addr).await.expect("Failed to open socket");

    // server will echo the bytes
    let server = Server {
        socket,
        buf: vec![0; 1024],
        to_send: None,
    };

    let server_address = server.socket.local_addr().unwrap().clone();
    let port = server_address.port();

    let networks = NetworkInterface::show().unwrap();
    let addresses = networks.iter().fold(Vec::new(), |mut carry, next| -> Vec<IpAddr> {
        carry.push(next.addr.first().unwrap().ip());
        return carry.clone(); // not very efficient
    });

    let socket_addresses = addresses.iter().map(|addr| {
        SocketAddr::new(*addr, port)
    }).collect::<Vec<SocketAddr>>();

    // This starts the server task.
    tokio::spawn(async { server.run().await });

    // set up the client socket
    let client_addr = "127.0.0.1:0";
    let client = UdpSocket::bind(&client_addr).await.expect("Failed to open client socket");

    // iterate through the server socket addresses (one per IP) and see check what IP the server responds from
    for ip in socket_addresses {
        client.send_to(&b"PING"[..], ip).await.expect(format!("Failed to send bytes to {:?}", ip).as_str());
        println!("Sent some bytes to {:?}", ip);
        let mut recv_buf = [0u8; 32];
        let (len, addr) = client.recv_from(&mut recv_buf).await.unwrap();
        println!("Received some bytes! {:?} from {:?}", String::from_utf8_lossy(&recv_buf[..len]), addr);
    }

    Ok(())
}

Example output:

Sent some bytes to 172.21.108.37:35308
Echoed 4/4 bytes to 127.0.0.1:51832
Received some bytes! "PING" from 127.0.0.1:35308
Sent some bytes to 127.0.0.1:35308
Echoed 4/4 bytes to 127.0.0.1:51832
Received some bytes! "PING" from 127.0.0.1:35308

We loop through all the IP addresses the server is hosting from but it always responds with the same default IP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-net Module: tokio/net T-docs Topic: documentation
Projects
None yet
Development

No branches or pull requests

3 participants