Re: [PATCH v13 5/6] migrate: Migrate TCP flows

On Mon, 10 Feb 2025 10:51:31 +0100
Stefano Brivio <sbrivio(a)redhat.com> wrote:

...
  On Mon, 10 Feb 2025 17:05:08 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
 
  On Sun, Feb 09, 2025 at 11:20:04PM +0100, Stefano
Brivio wrote:  
 
 +++ b/migrate.c
 @@ -98,11 +98,30 @@ static int seen_addrs_target_v1(struct ctx *c,
  
  /* Stages for version 1 */
  static const struct migrate_stage stages_v1[] = {
 +	/* FIXME: With this step, close() in tcp_flow_migrate_source_ext()
 +	 * *sometimes* closes the connection for real.
 +	 */
 +/*	{
 +		.name = "shrink TCP windows",
 +		.source = flow_migrate_source_early,
 +		.target = NULL,
 +	},
 +*/     
 
 Given we're not sure if this will help, and it adds some
 complications, probably makes sense to split this into a separate
 patch.   
 
 I'd rather not because, due to this, the code for the case *without it*
 is a bit different anyway, and we avoid some code churn. I would also
 like to merge it unused to make our lives a bit easier the day we retry
 to work on it. 
Oh, I missed the fact you already split this out in v14... fine then.
There are some stray FIXMEs in 11/14, tcp_flow_migrate_source_ext():

+	/* FIXME: Reenable dump in tcp_flow_migrate_shrink_window() */

+	/* FIXME: it's either this or flow_migrate_source_early(), why? */

but I'll just drop them in the next version.

-- 
Stefano