Subject: Re: [PATCH net-next] tcp: Revert no longer abort SYN_SENT when receiving some ICMP

From: Leon Romanovsky <[email-masked]>
Date: Tue, 2 Jan 2024 20:01:02 +0200

On Tue, Jan 02, 2024 at 04:31:15PM +0100, Eric Dumazet wrote:
> On Tue, Jan 2, 2024 at 12:41 PM Leon Romanovsky <[email-masked]> wrote:
> >
> > On Tue, Jan 02, 2024 at 11:03:55AM +0100, Eric Dumazet wrote:
> > > On Tue, Jan 2, 2024 at 10:58 AM Leon Romanovsky <[email-masked]> wrote:
> > > >
> > > > On Tue, Jan 02, 2024 at 10:46:13AM +0100, Eric Dumazet wrote:
> > > > > On Tue, Jan 2, 2024 at 10:01 AM Leon Romanovsky <[email-masked]> wrote:
> > > > > >
> > > > > > From: Shachar Kagan <[email-masked]>
> > > > > >
> > > > > > This reverts commit 0a8de364ff7a14558e9676f424283148110384d6.
> > > > > >
> > > > > > Shachar reported that Vagrant (https://www.vagrantup.com/), which is
> > > > > > very popular tool to manage fleet of VMs stopped to work after commit
> > > > > > citied in Fixes line.
> > > > > >
> > > > > > The issue appears while using Vagrant to manage nested VMs.
> > > > > > The steps are:
> > > > > > * create vagrant file
> > > > > > * vagrant up
> > > > > > * vagrant halt (VM is created but shut down)
> > > > > > * vagrant up - fail
> > > > > >
> > > > >
> > > > > I would rather have an explanation, instead of reverting a valid patch.
> > > > >
> > > > > I have been on vacation for some time. I may have missed a detailed
> > > > > explanation, please repost if needed.
> > > >
> > > > Our detailed explanation that revert worked. You provided the patch that
> > > > broke, so please let's not require from users to debug it.
> > > >
> > > > If you need a help to reproduce and/or test some hypothesis, Shachar
> > > > will be happy to help you, just ask.
> > >
> > > I have asked already, and received files that showed no ICMP relevant
> > > interactions.
> > >
> > > Can someone from your team help Shachar to get  a packet capture of
> > > both TCP _and_ ICMP packets ?
> >
> > I or Gal will help her, but for now let's revert it, before we will see
> > this breakage in merge window and later in all other branches which will
> > be based on -rc1.
> 
> Patch is in net-next, we have at least four weeks to find the root cause.

I saw more than once claims that netdev is fast to take patches but also
fast in reverts. There is no need to keep patch with known regression,
while we are in -rc8.

> 
> I am a TCP maintainer, I will ask you to respect my choice, we have
> tests and reverting the patch is breaking one of them.

At least for ipv6, you changed code from 2016 and the patch which I'm asking
to revert is not even marked as a fix. So I don't understand the urgency to keep
the patch.

There are two things to consider:
1. Linux rule number one is "do not break userspace".
2. Linux is a community project and people can have different opinions,
which can be different from your/mine.

Thanks