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

[Stefano Brivio <sbrivio@redhat.com>]

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

> On Mon, 10 Feb 2025 17:05:08 +1100
> David Gibson <david@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