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

Close partially-established TCP connections #392

Merged
merged 8 commits into from
Jun 7, 2018
Merged

Conversation

djs55
Copy link
Collaborator

@djs55 djs55 commented Jun 6, 2018

If a client sends SYN, we connect the external socket and reply with SYN ACK. If the client responds with RST ACK then previously we would leak the connection.

This patch refactors the connection closing mechanism, creating an idempotent close_flow function which is called

  • on normal close when the proxy receives FIN etc
  • on a reset, including during the handshake
  • when a switch port is being timed-out.

This replaces the previous on_destroy promise which was used in Lwt.pick since closing the connection should cause the proxy to receive EOF.

Related to [docker/for-mac#1132]

Signed-off-by: David Scott dave.scott@docker.com

If a client sends SYN, we connect the external socket and reply with
SYN ACK. If the client responds with RST ACK then previously we would
leak the connection.

This patch refactors the connection closing mechanism, creating an
idempotent `close_flow` function which is called

- on normal close when the proxy receives `FIN` etc
- on a reset, including during the handshake
- when a switch port is being timed-out.

This replaces the previous `on_destroy` promise which was used in
`Lwt.pick` since closing the connection should cause the proxy to receive
EOF.

Related to [docker/for-mac#1132]

Signed-off-by: David Scott <dave.scott@docker.com>
djs55 added 4 commits June 7, 2018 13:16
This is needed because we don't support the keepalive stuff.

Signed-off-by: David Scott <dave.scott@docker.com>
This fixes the build probably broken by a jbuilder upgrade.

Signed-off-by: David Scott <dave.scott@docker.com>
In particular the archives of ppx_import, a test dependency of uwt
are missing.

In future we will manually reference the test dependencies of the packages
that we actually want to test.

Signed-off-by: David Scott <dave.scott@docker.com>
Signed-off-by: David Scott <dave.scott@docker.com>
djs55 added 3 commits June 7, 2018 14:38
`opam install vpnkit -t` will install the test dependencies of our
dependencies, some of which don't exist (ppx_import)

This patch manually installs dependencies used by vpnkit.

Signed-off-by: David Scott <dave.scott@docker.com>
Signed-off-by: David Scott <dave.scott@docker.com>
Signed-off-by: David Scott <dave.scott@docker.com>
@djs55 djs55 merged commit a9305d8 into moby:master Jun 7, 2018
@djs55 djs55 deleted the fix-syn-rst branch June 7, 2018 14:34
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.

1 participant