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

[WIP] - Forward reuse_port and reuse_addr when it's possible to #5312

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rodjjo
Copy link

@rodjjo rodjjo commented Dec 19, 2024

The runtime exposes functions to allow us to set SO_REUSEADDR and SO_REUSEPORT, however it's not working:

wasmer run --net examples/rust_udp/target/wasm32-wasmer-wasi/debug/udp_example.rustc.wasm
Starting UDP server
Binding to 0.0.0.0:8080
Error: Os { code: 3, kind: AddrInUse, message: "Address in use" }

The example code:

use tokio::net::UdpSocket;
use std::io;
use std::net::{SocketAddr};
use socket2::{Socket, Domain, Type};


#[tokio::main]
async fn main() -> io::Result<()> {
    println!("Starting UDP server");
    let std_sock = Socket::new(Domain::IPV4, Type::DGRAM, None)?;
    std_sock.set_reuse_address(true)?;

    println!("Binding to 0.0.0.0:8080");

    let address: SocketAddr = "0.0.0.0:8080".parse().unwrap();
    let address = address.into();
    std_sock.bind(&address)?;
   
    let sock = UdpSocket::from_std(std_sock.into())?;

    let mut buf = [0; 1024];
    loop {
        let (len, addr) = sock.recv_from(&mut buf).await?;
        println!("{:?} bytes received from {:?}", len, addr);

        let len = sock.send_to(&buf[..len], addr).await?;
        println!("{:?} bytes sent", len);
    }
}

This pull request intend to solve issue-5247

I need help to understand the impact of the changes on other platforms, I tested it in my use case and I was able to reuse the address when I ran Wasmer on Ubuntu.

@rodjjo rodjjo force-pushed the reuse-addr-port-udp-socket branch from 7b51254 to ba5be63 Compare December 19, 2024 13:47
@rodjjo rodjjo marked this pull request as draft December 19, 2024 13:52
@rodjjo rodjjo marked this pull request as ready for review December 19, 2024 14:06
@rodjjo
Copy link
Author

rodjjo commented Dec 19, 2024

@maminrayej what do you think, would be that the way ?

Copy link
Contributor

@maminrayej maminrayej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although it needs a test to pin this behavior. You could try to add a WASIX test by looking at other tests and how they are set up.

As a bonus (not required for the PR to be merged), you could fix the same issue in other places such as listen_tcp.

if let Some(ruleset) = self.ruleset.as_ref() {
if !ruleset.allows_socket(addr, Direction::Inbound) {
tracing::warn!(%addr, "bind_udp blocked by firewall rule");
return Err(NetworkError::PermissionDenied);
}
}

#[cfg(not(windows))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we are not using the same logic for Windows? It seems that the API for socket reusing is also available there, so the more commonalities between systems, the better

@syrusakbary
Copy link
Member

syrusakbary commented Jan 2, 2025

Hey @rodjjo, we are happy to merge the PR once we make the logic with Windows similar as with Unix and hopefully a test that verifies the fix to make sure we are shielded of this issue in the future.

Let us know if you need any help with that

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 this pull request may close these issues.

3 participants