| Message ID | 20221224194253.3202231-8-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Various patches to improve DCO behaviour | expand |
Hi, On Sat, Dec 24, 2022 at 08:42:51PM +0100, Arne Schwabe wrote: > Instead of getting the server in a very weird state, we bail out here. This > is only a bandaid solution but better than the alternatives. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> Having a clear failure indication in the log and towards the client is good, so definitely Feature-ACK. I did also observe this when running the gremlin TCP test last night, with 4/9 applied Dec 25 02:49:44 ubuntu2004 tun-tcp-p2mp-username-cn[1828024]: ovpn-dco installed socket with residual read len=0, mi=gremlin84091/194.97.140.21:10133. This connection will probably break. Dec 25 02:49:46 ubuntu2004 tun-tcp-p2mp-username-cn[1828024]: ovpn-dco installed socket with residual read len=0, mi=gremlin84091/194.97.140.21:10133. This connection will probably break. Dec 25 02:49:47 ubuntu2004 tun-tcp-p2mp-username-cn[1828024]: ovpn-dco installed socket with residual read len=0, mi=gremlin84091/194.97.140.21:10133. This connection will probably break. Dec 25 02:49:48 ubuntu2004 tun-tcp-p2mp-username-cn[1828024]: ovpn-dco installed socket with residual read len=0, mi=gremlin84091/194.97.140.21:10133. This connection will probably break. ... with a single instance (gremlin84091) having 2905 lines of log - so this is certainly not a state we want the server to be in (other instances only show up once, or 4 or 22 times... whatever OpenVPN does here). This does not apply as it needs 4/9, and *that* one breaks inbound TCP for me, so postponing this patch until we understand why 4/ doesn't work. gert
diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c index 263f4d994..3837ccbab 100644 --- a/src/openvpn/mtcp.c +++ b/src/openvpn/mtcp.c @@ -591,7 +591,7 @@ multi_tcp_post(struct multi_context *m, struct multi_instance *mi, const int act struct gc_arena gc = gc_new(); msg(M_INFO, "ovpn-dco installed socket with residual read " "len=%d, mi=%s. This connection will probably" - " break.", BLEN(&c->c2.link_socket->stream_buf.residual), + " break.", BLEN(&c->c2.link_socket->stream_buf.buf), multi_instance_string(mi, false, &gc)); gc_free(&gc); break; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 9a20112e2..d29b7efe3 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2316,9 +2316,17 @@ multi_client_setup_dco_initial(struct multi_context *m, { if (!dco_enabled(&mi->context.options)) { - /* DCO not enabled, nothing to do, return sucess */ + /* DCO not enabled, nothing to do, return success */ return true; } + + if (socket_read_residual(mi->context.c2.link_socket)) + { + msg(M_INFO, "TCP socket with half read packet. Cannot install to " + "DCO: %s", multi_instance_string(mi, false, gc)); + return false; + } + int ret = dco_multi_add_new_peer(m, mi); if (ret < 0) {
Instead of getting the server in a very weird state, we bail out here. This is only a bandaid solution but better than the alternatives. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/mtcp.c | 2 +- src/openvpn/multi.c | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-)