rkitover/net-ssh2

Expose libssh2_keepalive API

jamessan opened this issue · 9 comments

In my environment, we do a lot of communication with hosts that may lose connectivity for various reasons. Having access to the libssh2_keepalive API would make detecting and reacting to these situations a lot easier.

I'll look into it.

Of course if you can give me a pull request that would make things much easier, all you'd need to do is look at similar functions in SSH2.xs and do something similar.

Yeah, I'll try and put together a pull request. I just wanted to put the idea out there in case someone is able to beat me to it. :)

What would be the expected usage pattern for the keepalive functionality? I'm assuming if it has been setup then TIEd interface for reading from a channel should leverage it, but if manual reading/writing is being done should the onus be on the user to call keepalive_send?

I have been also investigating that issue, because I wanted to implement keepalive requests into my Net::SSH::Any module (that uses Net::SSH2 under the hood) but have found that the libssh2 support for sending keep alives is completely unreliable and may actually break the connection when used.

See this mail from libssh2-devel for the details: http://article.gmane.org/gmane.network.ssh.libssh2.devel/5406/match=trying+understand+keepalive

In any case, I have a patch that wraps the keepalive methods, just in case somebody wants to play with it: https://github.com/salva/p5-Net-SSH2/tree/keepalive

92c4b75 may actually be enough for my purposes, as the code I would need this in uses poll/read/write (and in blocking mode). I think that hooking into the TIEd channel API would still be quite useful for the simpler use cases.

Be careful. As the docs now say, calling keepalive_send may break the SSH session because it is not able to recover from an EAGAIN error.

Under the hood, libssh2_keepalive_send tries to write the keepalive packet in just one write syscall. If that syscall fails because of the socket buffer becoming full or some signal arriving at the wrong time, a partial packet would be written into the socket and after that point, the SSH session would be corrupted and useless.

The possibility of getting a EAGAIN when sending the keepalive packet is very low because it is a very short packet so, maybe in some cases, the risk may be acceptable.

Also, I believe there are cases when this method can be used safely, but I still have think further about that...

@salva This can be closed now, right? Looks like you pushed the keepalive changes some time ago.

My last comment still applies. The keep alive implementation in libssh2 is half done and so buggy and can corrupt the session.

So, IIRC, the keep alive feature is wrapped in Net::SSH2, but using it is still a bad idea!

That's fine. The request was to have the functionality available, which it is. I'll keep the risks in mind if/when I end up using it.