Repository navigation
Raise the forwarded-channel window from 128 KB to 2 MB (vendored JSch class) - #70
Merged
Merged
Conversation
… class) Every connection through the tunnel server's port 2010 is a JSch forwarded-tcpip channel. Its local window is how much the server may send before this client acknowledges: request bodies, uploads, everything a device sends towards the site under test. JSch hard-codes it at 128 KB (16 KB packets), which caps each connection at 128 KB per round trip. JSch has no setting for it. The constants are private, and Channel#setLocalWindowSizeMax only applies before a channel is connected, while Session creates and confirms these when the server opens them. So ChannelForwardedTCPIP is vendored from 2.28.8 with two constants changed (2 MB window, 32 KB packets). Project classes come first on the classpath and in the shaded jar, and JSch has no multi-release copy of this class. JschForwardedWindowTest checks that the vendored class is the one loaded, and fails when JSch is upgraded, so it is re-copied rather than silently replacing a newer one. Measured end to end against a real tunnel VM (uploads through 2009 / 2011): - 20 ms: 3.1 / 3.1 MB/s -> 12.7 / 30.2 MB/s - 50 ms: 1.3 / 1.3 MB/s -> 5.1 / 14.3 MB/s (13 MB/s through 2009 with BBR on the tunnel VM) Downloads, latency, head-of-line blocking, the burst test and CPU are unchanged.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| daemon = c.getDeclaredConstructor().newInstance(); | ||
|
|
||
| PipedOutputStream out = new PipedOutputStream(); | ||
| io.setInputStream(new PassiveInputStream(out, 32 * 1024), false); |
| (_config.factory == null) ? Util.createSocket(_config.target, _config.lport, TIMEOUT) | ||
| : _config.factory.createSocket(_config.target, _config.lport); | ||
| socket.setTcpNoDelay(true); | ||
| io.setInputStream(socket.getInputStream()); |
| this.config = getPort(_session, null, port); | ||
|
|
||
| if (this.config == null) { | ||
| if (_session.getLogger().isEnabled(Logger.ERROR)) { |
| } | ||
| } | ||
|
|
||
| static void addPort(Session session, String _address_to_bind, int port, int allocated_port, |
| setRemotePacketSize(buf.getInt()); | ||
| byte[] addr = buf.getString(); | ||
| int port = buf.getInt(); | ||
| byte[] orgaddr = buf.getString(); |
| byte[] addr = buf.getString(); | ||
| int port = buf.getInt(); | ||
| byte[] orgaddr = buf.getString(); | ||
| int orgport = buf.getInt(); |
probeReverseForward checked in.available() and then channel.isClosed(). The probe proxy closes as soon as it has answered, so the answer and the close can both land between those two calls; the loop then broke out with the answer sitting unread in the buffer and reported the forward as dead. That is how aReverseForwardTheServerRefusedIsRequestedAgainByTheMonitor failed on the Java 25 leg in 94 ms, well short of the 5 s timeout.
705d49f was written against a misdiagnosis: the probe was not losing a buffered answer, its thread had been left interrupted.
JSch wakes the thread waiting for the server's tcpip-forward reply by interrupting it, after setting the reply (Session.java, case SSH_MSG_REQUEST_SUCCESS/FAILURE). On loopback the reply is quick enough that the caller sees it, returns from setPortForwardingR, and receives the interrupt afterwards. The caller was left interrupted and its next blocking call failed: - SSHTunnelEmbeddedServerTest.aReverseForwardTheServerRefusedIsRequestedAgainByTheMonitor: the probe after the re-registration returned false in ~60 ms - AppEmbeddedLifecycleTest: boot() failed starting Jetty with an InterruptedException (also seen on main, run 40093219005) Both call sites now go through a helper that runs the request on a short-lived thread, which absorbs the interrupt. The wait stays uninterruptible and bounded, as JSch's own is.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
Uploads through the tunnel were capped at 128 KB per round trip:
Every connection through the tunnel server's port 2010 is a JSch
forwarded-tcpipchannel. Its local window is how much the server may send before this client acknowledges: request bodies, file uploads, everything a device sends towards the site under test. JSch hard-codes it at0x20000(16 KB packets), and offers no setting:Channel#setLocalWindowSizeMax(public since 2.28.8) only applies before a channel is connected, whileSessioncreates and confirms these channels itself when the server opens them.What changed
src/main/java/com/jcraft/jsch/ChannelForwardedTCPIP.javais vendored from JSch 2.28.8 with two constants changed: a 2 MB window and 32 KB packets. The file header says why.JschForwardedWindowTest:Tests
mvn test: 1044 tests, 0 failures.Unchanged: downloads, request latency, head-of-line blocking during a download, the 300-request burst, and squid/sshd CPU.