Skip to content

Conversation

@Protryon
Copy link
Contributor

Adds a fake port option for wonky firewall shenanigans.

@ylow
Copy link
Contributor

ylow commented Aug 9, 2024

Apologies as you might have heard we just got acquired by HuggingFace. Things have been hectic for a while, but now I have bandwidth to review this. Thanks for the patience!

@rrauch
Copy link
Contributor

rrauch commented Aug 22, 2024

How about adding a new, optional advertised_port field to NFSTcpListener and then change how local_port in RPCContext is assigned?

Something like this:

pub struct NFSTcpListener<T: NFSFileSystem + Send + Sync + 'static> {
    listener: TcpListener,
    port: u16,
    advertised_port: Option<u16>,
    arcfs: Arc<T>,
    mount_signal: Option<mpsc::Sender<bool>>,
}

...

async fn handle_forever(&self) -> io::Result<()> {
        loop {
            let (socket, _) = self.listener.accept().await?;
            let context = RPCContext {
                local_port: self.advertised_port.unwrap_or(self.port),
                client_addr: socket.peer_addr().unwrap().to_string(),
...                

I think the ability to advertise a different port from the actual listening port is very useful, especially when dealing with Windows clients.

@ylow
Copy link
Contributor

ylow commented Sep 23, 2024

Can we maybe split this into 2 PRs? This looks like it has 2 pieces. One which implements NLM4 protocol (thanks!), and another for a port number.

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