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

timeout and shutdown #23

Merged
merged 2 commits into from
Mar 4, 2024
Merged

timeout and shutdown #23

merged 2 commits into from
Mar 4, 2024

Conversation

ssrlive
Copy link
Contributor

@ssrlive ssrlive commented Mar 4, 2024

No description provided.

@SajjadPourali
Copy link
Collaborator

Checking the timeout on pool_read functions is sufficient, as the timeout registers their context.

@ssrlive
Copy link
Contributor Author

ssrlive commented Mar 4, 2024

emm, i have removed the code. please review the rest modifications.

@SajjadPourali SajjadPourali merged commit 6147d5a into narrowlink:main Mar 4, 2024
3 checks passed
@ssrlive
Copy link
Contributor Author

ssrlive commented Mar 4, 2024

Not fixed yet.
image

@SajjadPourali
Copy link
Collaborator

Not fixed yet. image

I see. Ignoring poll_shutdown causes other issues. Let me explore other options to fix it.

@SajjadPourali
Copy link
Collaborator

@ssrlive please check the log. I just fixed this issue; however, there is a minor race condition when both parties try to close sockets simultaneously.

@SajjadPourali
Copy link
Collaborator

@ssrlive could you please check the latest changes and verify if they work as expected?

@ssrlive
Copy link
Contributor Author

ssrlive commented Mar 5, 2024

Looks fine.
image
I think the bug is fixed.

@ssrlive
Copy link
Contributor Author

ssrlive commented Mar 5, 2024

please publish a new version.

@SajjadPourali
Copy link
Collaborator

please publish a new version.

Waiting for #24
@xmh0511

@SajjadPourali
Copy link
Collaborator

05988a3

@ssrlive
Copy link
Contributor Author

ssrlive commented Mar 8, 2024

ipstack/src/stream/tcp.rs

Lines 222 to 233 in 6b23ebf

if matches!(
Pin::new(&mut self.tcb.timeout).poll(cx),
std::task::Poll::Ready(_)
) {
#[cfg(feature = "log")]
trace!("timeout reached for {:?}", self.dst_addr);
let flags = tcp_flags::RST | tcp_flags::ACK;
self.packet_sender
.send(self.create_rev_packet(flags, TTL, None, Vec::new())?)
.map_err(|_| ErrorKind::UnexpectedEof)?;
return std::task::Poll::Ready(Err(Error::from(ErrorKind::TimedOut)));
}

between L231 and L232, should we add this?

                self.tcb.change_state(TcpState::Closed);
                self.shutdown.ready();

@SajjadPourali
Copy link
Collaborator

ipstack/src/stream/tcp.rs

Lines 222 to 233 in 6b23ebf

if matches!(
Pin::new(&mut self.tcb.timeout).poll(cx),
std::task::Poll::Ready(_)
) {
#[cfg(feature = "log")]
trace!("timeout reached for {:?}", self.dst_addr);
let flags = tcp_flags::RST | tcp_flags::ACK;
self.packet_sender
.send(self.create_rev_packet(flags, TTL, None, Vec::new())?)
.map_err(|_| ErrorKind::UnexpectedEof)?;
return std::task::Poll::Ready(Err(Error::from(ErrorKind::TimedOut)));
}

between L231 and L232, should we add this?

                self.tcb.change_state(TcpState::Closed);
                self.shutdown.ready();

Good catch! The AsyncWrite should also be closed when AsyncRead is closed.

@SajjadPourali
Copy link
Collaborator

@ssrlive : Could you also please check if rpref works after this fix?

@ssrlive
Copy link
Contributor Author

ssrlive commented Mar 9, 2024

you can install rperf with this command

cargo install --git https://github.com/ssrlive/rperf.git --branch nat

UDP testing passed,
TCP forward mode passed,
but TCP reverse-NAT mode failed.

I think it's not ipstack issues.

image

And all tests with iperf3 passed.

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.

None yet

2 participants