From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=O6PzovHb; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 4B07F5A0276 for ; Wed, 29 Jan 2025 09:46:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1738140380; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=S2mBz5G748/d8s7P/1sTd7UxXJ/40jFS5RhERK+ZTvY=; b=O6PzovHbG5dYT20JhuDgdL4PZ2xPLIMeNJ4hGAu5R+cXrNWa6eyom0+wZKFKURHqtIUeUL yXX8Puz3E/2RR+54//RSlGvOuFqgAQcxCRUZSvtbq5Pkw0/4k4jj2IWRu3OEx6EcbXOkVa yrjycZOPCWTq6UvWE1ko8/2KaYeWcM0= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-656-lejKcZj1OxOJRyUJfxJ-KA-1; Wed, 29 Jan 2025 03:46:18 -0500 X-MC-Unique: lejKcZj1OxOJRyUJfxJ-KA-1 X-Mimecast-MFC-AGG-ID: lejKcZj1OxOJRyUJfxJ-KA Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-386333ea577so2422531f8f.1 for ; Wed, 29 Jan 2025 00:46:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738140377; x=1738745177; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=S2mBz5G748/d8s7P/1sTd7UxXJ/40jFS5RhERK+ZTvY=; b=drcpFVWIm1jZoLBAoqiHTWDvUdE4rvRRjThF1gFVIi9UKDqZUZLk+pzDW1rTocrf/T CC/XD3poYo+kx+R17pXguqe1bbCp62knxNJJnbFnNV/wQjs2frtNJ8jwkK9IUdHL6QvN K4TRRQ6wCourTCf+PxMr/M2nV4bklV68pHoAYHEP/iWFlneD0DAIXB22zLTBioKw4C17 OxqkBmoQ0lARUzeYeMDkoxsPWsFOUTeFNJtJVCsAgnqNH1UZiHKazfrPg+PThtUtfq7T gCMtW5wZBptebTxZOJkbjMMhVGnFrXlldTuPwVzPiQuNSenK3qRnSdfBpvtsWbaB7wX0 KUGA== X-Gm-Message-State: AOJu0Yz/JXfj/ihPj/X97cc0KJPOFKTCpvhIb6/hrfNAIWWHAhXt1hoG T+2JnNUqanKRF8cYOi+DTi1cTcTO3cEYMtIN9DlAMFnDe28zotXcxO7jK7i9mMAC+HbV+CxgHEd MGsf43iLzc0lSbX1WuSi1lxs8jQnyonJCjU2U3SqaqgfIHKQ5RQ== X-Gm-Gg: ASbGncvq1tLbiHMiVP6OH6gmnZlhN9GSuZ0XpQFkIbVOMXSSfCg+tVUp8Jxp/zBsC/G VKswO93MJ3oChbTRsBKkncStDQLmaJPLp8IZMyU8ZpjQ0fb+EkSD7L9Z+dxXrESWq2XL1FgndQO ZlerIrdb3t6IMXF7dRiTqe5yjGnf6DCCDjkaXI/NCljO7MCaIqFa+6+nWuhY7gIZeozgRirmfDf jcbO9Hk4d4VMzOdHPQOwF6HItphJ1l6qgvMbkjGrlS0Dhn9C/Vq/2wSq8i14RYQLWiYqY1rmXTE WBv2d+3Zh3MdcT9hkr3oDuKpjKZesp5SMA== X-Received: by 2002:adf:ce0f:0:b0:385:dc45:ea06 with SMTP id ffacd0b85a97d-38c51b85db0mr1419888f8f.13.1738140377021; Wed, 29 Jan 2025 00:46:17 -0800 (PST) X-Google-Smtp-Source: AGHT+IGqgiIYAjQf0DKZP61lPPYjaMGCCfhXJv0eMPWJsQSsaBomP9phMTgXFW8/zFyy3GxHtcvmUg== X-Received: by 2002:adf:ce0f:0:b0:385:dc45:ea06 with SMTP id ffacd0b85a97d-38c51b85db0mr1419873f8f.13.1738140376595; Wed, 29 Jan 2025 00:46:16 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38c2a17d6a6sm16158557f8f.26.2025.01.29.00.46.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jan 2025 00:46:15 -0800 (PST) Date: Wed, 29 Jan 2025 09:46:13 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 8/8] flow, tcp: Basic pre-migration source handler to dump sequence numbers Message-ID: <20250129094613.30baf0b6@elisabeth> In-Reply-To: References: <20250128233940.1235855-1-sbrivio@redhat.com> <20250128233940.1235855-9-sbrivio@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: yPiAC6GJXjWS5iazfMWs4oJBmGtpgPeq6OdjM5mEaLc_1738140377 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 2U4OWFW3XONIP3VWB72WBAOMSTGDDMLJ X-Message-ID-Hash: 2U4OWFW3XONIP3VWB72WBAOMSTGDDMLJ X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Laurent Vivier X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 29 Jan 2025 17:15:41 +1100 David Gibson wrote: > On Wed, Jan 29, 2025 at 12:39:40AM +0100, Stefano Brivio wrote: > > Very much draft quality, but it works. Ask passt-repair to switch > > TCP sockets to repair mode and dump their current sequence numbers to > > the flow table, which will be transferred and used by the target in > > the next step. > > > > Signed-off-by: Stefano Brivio > > --- > > flow.c | 43 +++++++++++++++++++++++++++++++++++++++++ > > flow.h | 1 + > > migrate.c | 1 + > > tcp.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > tcp_conn.h | 5 +++++ > > 5 files changed, 106 insertions(+) > > > > diff --git a/flow.c b/flow.c > > index ee1221b..e7148b2 100644 > > --- a/flow.c > > +++ b/flow.c > > @@ -19,6 +19,7 @@ > > #include "inany.h" > > #include "flow.h" > > #include "flow_table.h" > > +#include "repair.h" > > > > const char *flow_state_str[] = { > > [FLOW_STATE_FREE] = "FREE", > > @@ -874,6 +875,48 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now) > > *last_next = FLOW_MAX; > > } > > > > +/** > > + * flow_migrate_source_pre() - Prepare all source flows for migration > > + * @c: Execution context > > + * @m: Migration metadata > > + * > > + * Return: 0 on success > > + */ > > +int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m) > > +{ > > + unsigned i; > > + int rc; > > + > > + (void)m; > > + > > + for (i = 0; i < FLOW_MAX; i++) { /* TODO: iterator with skip */ > > + union flow *flow = &flowtab[i]; > > + > > + if (flow->f.state == FLOW_STATE_FREE) > > + i += flow->free.n - 1; > > + else if (flow->f.state == FLOW_STATE_ACTIVE && > > We should probably just abort any flows that are in pre-ACTIVE state > at migration time. Wait... IIRC flows have to be in ACTIVE state (or > already cancelled) once we get to the next epoll cycle. So we can > possibly just assert that state is either ACTIVE or FREE. "IIRC" doesn't quite fit with "just assert". Let's not make things crash here, it's really not the right point to get issues reported. I can add a print. > > + flow->f.type == FLOW_TCP) > > + rc = tcp_flow_repair_on(c, &flow->tcp); > > + > > + if (rc) > > + return rc; /* TODO: rollback */ > > + } > > + > > + repair_flush(c); /* TODO: move to TCP logic */ > > + > > + for (i = 0; i < FLOW_MAX; i++) { /* TODO: iterator with skip */ > > + union flow *flow = &flowtab[i]; > > + > > + if (flow->f.state == FLOW_STATE_FREE) > > + i += flow->free.n - 1; > > + else if (flow->f.state == FLOW_STATE_ACTIVE && > > + flow->f.type == FLOW_TCP) > > + tcp_flow_dump_seq(c, &flow->tcp); > > + } > > + > > + return 0; > > +} > > + > > /** > > * flow_init() - Initialise flow related data structures > > */ > > diff --git a/flow.h b/flow.h > > index 8eb5964..ff390a6 100644 > > --- a/flow.h > > +++ b/flow.h > > @@ -255,6 +255,7 @@ union flow; > > > > void flow_init(void); > > void flow_defer_handler(const struct ctx *c, const struct timespec *now); > > +int flow_migrate_source_pre(struct ctx *c, struct migrate_meta *m); > > > > void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...) > > __attribute__((format(printf, 3, 4))); > > diff --git a/migrate.c b/migrate.c > > index b8b79e0..6707c02 100644 > > --- a/migrate.c > > +++ b/migrate.c > > @@ -56,6 +56,7 @@ static struct migrate_data data_versions[] = { > > > > /* Handlers to call in source before sending data */ > > struct migrate_handler handlers_source_pre[] = { > > + { flow_migrate_source_pre }, > > { 0 }, > > }; > > > > diff --git a/tcp.c b/tcp.c > > index c89f323..3a3038b 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -299,6 +299,7 @@ > > #include "log.h" > > #include "inany.h" > > #include "flow.h" > > +#include "repair.h" > > #include "linux_dep.h" > > > > #include "flow_table.h" > > @@ -868,6 +869,61 @@ void tcp_defer_handler(struct ctx *c) > > tcp_payload_flush(c); > > } > > > > +/** > > + * tcp_flow_repair_on() - Enable repair mode for a single TCP flow > > + * @c: Execution context > > + * @conn: Pointer to the TCP connection structure > > + * > > + * Return: 0 on success, negative error code on failure > > + */ > > +int tcp_flow_repair_on(struct ctx *c, const struct tcp_tap_conn *conn) > > +{ > > + int rc = 0; > > + > > + if ((rc = repair_set(c, conn->sock, TCP_REPAIR_ON))) > > + err("Failed to set TCP_REPAIR for socket %i", conn->sock); > > Well.. except that the error could just as easily have been on a > previous socket that wasn't flushed yet. Right, I should probably just drop the socket number from here. -- Stefano