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

Global request response handling #250

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

amtelekom
Copy link
Contributor

Right now, the handling to global request responses differs between Client and Server.

At the moment, the only situation where we receive those is in response to keepalive requests (everything else is sent with want_reply false), so I've also added a little bit of an explaining comment instead of a leftover "todo".

The old if is not really sufficient to filter for global requests in response to keepalives, since the value it checks, alive_timeouts is reset to 0 with the first response, but due to TCP resends multiple keepalives and their responses might arrive.

This also stops russh client from repeatedly sending an info-level log message about an unknown packet when keepalive responses are received, downgrading unknown messages to debug in general (same as it was in server already), and REQUEST_SUCCESS / REQUEST_FAILURE to trace.

@amtelekom
Copy link
Contributor Author

I guess w.r.t. #148 we could also look into implementing a mechanism for handling global request responses, I'd be open to proposing something if you'd rather handle this

@amtelekom
Copy link
Contributor Author

amtelekom commented Feb 12, 2024

The second commit actually implements global request response handling based on the order and type of global requests sent - although I currently have no setup for in-depth tests of the reverse TcpIp Tunnel stuff, so that is just built by spec without much validation, so maybe someone could validate it a bit further? @jimmyvanhest are you still interested in this stuff?

@amtelekom amtelekom changed the title Global request response logging / Keepalive response handling Global request response handling Feb 12, 2024
@Eugeny
Copy link
Owner

Eugeny commented Feb 15, 2024

The implementation looks good to me - thanks!

@Eugeny Eugeny merged commit 5c60d30 into Eugeny:main Feb 15, 2024
4 checks 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.

2 participants