[PATCH v3 3/3] migrate, tcp: Don't attempt to carry on migration after flow_alloc_cancel()

In tcp_flow_migrate_target(), if we're unable to create and bind the new socket, we print an error, cancel the flow and carry on. This seems to make sense based on our policy of generally letting the migration complete even if some or all flows are lost in the process. However, it can't quite work: the flow_alloc_cancel() means that the flows in the target's flow table are no longer one to one match to the flows which the source is sending data for. This means that data for later flows will be mismatched to a different flow. Most likely that will cause some nasty error later, but even worse it might appear to succeed but lead to data corruption due to incorrectly restoring one of the flows. Instead, we should leave the flow in the table until we've read all the data for it, *then* discard it. Technically removing the flow_alloc_cancel() would be enough for this: if tcp_flow_repair_socket() fails it leaves conn->sock == -1, which will cause the restore functions in tcp_flow_migrate_target_ext() to fail, discarding the flow. To make what's going on clearer, though, put an explicit test for a bad socket fd in tcp_flow_migrate_target_ext() and discard at that point. Signed-off-by: David Gibson --- tcp.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tcp.c b/tcp.c index e3c0a53b..f713fa99 100644 --- a/tcp.c +++ b/tcp.c @@ -3376,7 +3376,6 @@ int tcp_flow_migrate_target(struct ctx *c, int fd) if ((rc = tcp_flow_repair_socket(c, conn))) { flow_err(flow, "Can't set up socket: %s, drop", strerror_(rc)); - flow_alloc_cancel(flow); return 0; } @@ -3452,6 +3451,10 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd return rc; } + if (conn->sock < 0) + /* We weren't able to create the socket, discard flow */ + goto fail; + if (tcp_flow_select_queue(s, TCP_SEND_QUEUE)) goto fail; -- 2.48.1