Regression are sometimes merged slowly, maybe optimize the downstream interaction?

Regression are sometimes merged slowly, maybe optimize the downstream interaction?

[Thorsten Leemhuis]

Hi Jakub, Hi Dave!

I was wondering if you and your downstream maintainers could consider
slightly optimizing your working habits to get regression fixes from
downstream git repos a bit quicker into mainline. A slightly different
timing afaics might already help a lot; or some timing optimizations in
the interaction with downstream maintainers.

I ask, because in my regression tracking work I noticed that quite a few
regression fixes take a long time towards mainline when they need to go
through net.git; that imho is especially bad for regressions caused by
commits in earlier development cycles, as they can only be fixed in new
stable releases after the fix was mainlined.

Often the fixes progress slowly due to the habits of the downstream
maintainers -- some for example are imho not asking you often enough to
pull fixes. I guess that might need to be discussed down the road as
well, but there is something else that imho needs to be addressed first.

At least from the outside it often looks like bad timing is the reason
why some fixes take quite long journey to mainline. Take for example the
latest pull requests for bluetooth and ipsec:

https://lore.kernel.org/netdev/20220224210838.197787-1-luiz.dentz@gmail.com/
https://lore.kernel.org/netdev/20220225074733.118664-1-steffen.klassert@secunet.com/

One is from Thursday, the other from early Friday; both contain fixes
for regressions in earlier mainline releases that afaics need to get
backported to stable and longterm releases to finally get the regression
erased from this world. The ipsec fix has been in -next already for a
while, the bluetooth fix afaics wasn't.

Sadly, both patch sets missed rc6 as Jakub already had sent his pull
request to Linus on Thursday:
https://lore.kernel.org/all/20220224195305.1584666-1-kuba@kernel.org/

This is not the first time I noticed such bad timing. That made me
wonder: would it be possible for you to optimize the workflow here?
Maybe a simple advice to downstream maintainers could do the trick, e.g.
"ideally sent pull request by Friday morning[some timezone] at the
latest, as then the net maintainers can review, merge, and sent them
onwards to Linus in a pull request later that day".

FWIW, I don't know anything about the inner working of your subsystem,
if you need more time to review or process merge requests from
downstream maintainers the "Friday morning" obviously needs to be adjusted.

Or is there something like that already and the timing just has been bad
a few times when I looked closer?

Ciao, Thorsten

Re: Regression are sometimes merged slowly, maybe optimize the downstream interaction?

[Jakub Kicinski]

On Mon, 28 Feb 2022 14:45:47 +0100 Thorsten Leemhuis wrote:
> Hi Jakub, Hi Dave!

Let's CC Luiz and Steffen.

> I was wondering if you and your downstream maintainers could consider
> slightly optimizing your working habits to get regression fixes from
> downstream git repos a bit quicker into mainline. A slightly different
> timing afaics might already help a lot; or some timing optimizations in
> the interaction with downstream maintainers.
> 
> I ask, because in my regression tracking work I noticed that quite a few
> regression fixes take a long time towards mainline when they need to go
> through net.git; that imho is especially bad for regressions caused by
> commits in earlier development cycles, as they can only be fixed in new
> stable releases after the fix was mainlined.
> 
> Often the fixes progress slowly due to the habits of the downstream
> maintainers -- some for example are imho not asking you often enough to
> pull fixes. I guess that might need to be discussed down the road as
> well, but there is something else that imho needs to be addressed first.
> 
> At least from the outside it often looks like bad timing is the reason
> why some fixes take quite long journey to mainline. Take for example the
> latest pull requests for bluetooth and ipsec:
> 
> https://lore.kernel.org/netdev/20220224210838.197787-1-luiz.dentz@gmail.com/
> https://lore.kernel.org/netdev/20220225074733.118664-1-steffen.klassert@secunet.com/

Yeah, we also narrowly missed the BPF pr a week back :/
Or should I say BPF pr missed the net pr..

> One is from Thursday, the other from early Friday; both contain fixes
> for regressions in earlier mainline releases that afaics need to get
> backported to stable and longterm releases to finally get the regression
> erased from this world. The ipsec fix has been in -next already for a
> while, the bluetooth fix afaics wasn't.
> 
> Sadly, both patch sets missed rc6 as Jakub already had sent his pull
> request to Linus on Thursday:
> https://lore.kernel.org/all/20220224195305.1584666-1-kuba@kernel.org/
> 
> This is not the first time I noticed such bad timing. That made me
> wonder: would it be possible for you to optimize the workflow here?
> Maybe a simple advice to downstream maintainers could do the trick, e.g.
> "ideally sent pull request by Friday morning[some timezone] at the
> latest, as then the net maintainers can review, merge, and sent them
> onwards to Linus in a pull request later that day".

These are fair complaints. We've been sending PRs with fixes every
Thursday for, I'd say, a year or so now. If the sub-tree PR is posted 
by Wednesday it will definitely make the cut. Either folks don't know 
this or they want changes to sit in the networking tree for a couple
of days? Hm.

> FWIW, I don't know anything about the inner working of your subsystem,
> if you need more time to review or process merge requests from
> downstream maintainers the "Friday morning" obviously needs to be adjusted.
> 
> Or is there something like that already and the timing just has been bad
> a few times when I looked closer?

I think it's a particularly unfortunate time with a few "missed prs"
in a short span of time. When Dave was handling all the prs he used
to decide the timing based on contents of the tree, maybe that's 
a better model for prioritizing fixes getting to Linus, but I lack 
the skills necessary to make such calls.

I'll try to advertise the Wednesday rule more, although creating
deadlines has proven to lead to rushed work. Which IMHO is much 
worse :(

BTW there's also something weird with the flow of fixes lately.
Maybe it's just my perception but I feel like it has been disrupted
in Dec and hasn't fully recovered. Normally we get an influx of "new
code / regression fixes" after rc2, and they peter out around rc5.
This release it feels like the fixes _started_ flowing at rc5 :S

Anyway, thanks for raising the issue, and please keep us posted on how
things look from your perspective. It's a balancing act, it'd be great
if we can improve things over time without sudden changes.

Thanks again!

Re: Regression are sometimes merged slowly, maybe optimize the downstream interaction?

[Thorsten Leemhuis]

On 28.02.22 18:46, Jakub Kicinski wrote:
> On Mon, 28 Feb 2022 14:45:47 +0100 Thorsten Leemhuis wrote:
>> I was wondering if you and your downstream maintainers could consider
>> slightly optimizing your working habits to get regression fixes from
>> downstream git repos a bit quicker into mainline. A slightly different
>> timing afaics might already help a lot; or some timing optimizations in
>> the interaction with downstream maintainers.
>>
>> I ask, because in my regression tracking work I noticed that quite a few
>> regression fixes take a long time towards mainline when they need to go
>> through net.git; that imho is especially bad for regressions caused by
>> commits in earlier development cycles, as they can only be fixed in new
>> stable releases after the fix was mainlined.
>>
>> Often the fixes progress slowly due to the habits of the downstream
>> maintainers -- some for example are imho not asking you often enough to
>> pull fixes. I guess that might need to be discussed down the road as
>> well, but there is something else that imho needs to be addressed first.
>>
>> At least from the outside it often looks like bad timing is the reason
>> why some fixes take quite long journey to mainline. Take for example the
>> latest pull requests for bluetooth and ipsec:
>>
>> https://lore.kernel.org/netdev/20220224210838.197787-1-luiz.dentz@gmail.com/
>> https://lore.kernel.org/netdev/20220225074733.118664-1-steffen.klassert@secunet.com/
> 
> Yeah, we also narrowly missed the BPF pr a week back :/
> Or should I say BPF pr missed the net pr..

:-D

I guess things like that will always happen and that is nothing to loose
sleep over -- but maybe with some optimizations we can reduce the number
of times they happen.

>> One is from Thursday, the other from early Friday; both contain fixes
>> for regressions in earlier mainline releases that afaics need to get
>> backported to stable and longterm releases to finally get the regression
>> erased from this world. The ipsec fix has been in -next already for a
>> while, the bluetooth fix afaics wasn't.
>>
>> Sadly, both patch sets missed rc6 as Jakub already had sent his pull
>> request to Linus on Thursday:
>> https://lore.kernel.org/all/20220224195305.1584666-1-kuba@kernel.org/
>>
>> This is not the first time I noticed such bad timing. That made me
>> wonder: would it be possible for you to optimize the workflow here?
>> Maybe a simple advice to downstream maintainers could do the trick, e.g.
>> "ideally sent pull request by Friday morning[some timezone] at the
>> latest, as then the net maintainers can review, merge, and sent them
>> onwards to Linus in a pull request later that day".
>
> These are fair complaints. We've been sending PRs with fixes every
> Thursday for, I'd say, a year or so now. If the sub-tree PR is posted 
> by Wednesday it will definitely make the cut. Either folks don't know 
> this or they want changes to sit in the networking tree for a couple
> of days? Hm.

Just wondering: some (most?) of those sub-trees afaics have a stable
branch included in -next, so would sitting in the networking tree
actually make much of a difference for them?

>> FWIW, I don't know anything about the inner working of your subsystem,
>> if you need more time to review or process merge requests from
>> downstream maintainers the "Friday morning" obviously needs to be adjusted.
>>
>> Or is there something like that already and the timing just has been bad
>> a few times when I looked closer?
> 
> I think it's a particularly unfortunate time with a few "missed prs"
> in a short span of time. When Dave was handling all the prs he used
> to decide the timing based on contents of the tree, maybe that's 
> a better model for prioritizing fixes getting to Linus, but I lack 
> the skills necessary to make such calls.
> 
> I'll try to advertise the Wednesday rule more, 

Thx

> although creating
> deadlines has proven to lead to rushed work. Which IMHO is much 
> worse :(

Don't call it a deadline then. :-D But joking aside, I know what you
mean, and yes, that is a reasonable concern.

Maybe one thing could help in here sometimes: if sub-tree maintainers
with your permission ask Linus to pick up a single patch directly from a
repo or a list, *if* there is a good reason to get a fix quickly merged.

> [...]
> Anyway, thanks for raising the issue, and please keep us posted on how
> things look from your perspective. It's a balancing act, it'd be great
> if we can improve things over time without sudden changes.

Yeah, it's definitely a balancing act.

And as you asked for how things look from here, let me get back to the
one issue I already briefly in my mail. To repeat a quote from above:

>> Often the fixes progress slowly due to the habits of the downstream
>> maintainers -- some for example are imho not asking you often enough to
>> pull fixes. I guess that might need to be discussed down the road as
>> well, but there is something else that imho needs to be addressed first.

To give an example, but fwiw: that is in no way special, I've seen
similar turn of events for a few other regressions fixes in sub-tree of
net, so it really is just meant as an example for a general issue (sorry
Steffen).

See this fix:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=master&id=a6d95c5a628a09be129f25d5663a7e9db8261f51

The regression was introduced in 5.14-rc1 and the fix was posted on
2022-01-14, so 46 days ago.

Jiri, who provided the patch, actually wrote "being a regression maybe
we want the fastest track possible" here:
https://lore.kernel.org/netdev/20220119091233.pzqdlzpcyicjavk5@dwarf.suse.cz/

Steffen applied the fix 27 days ago:
https://lore.kernel.org/netdev/20220201064639.GS1223722@gauss3.secunet.de/

Three days later it was in -next. After some time I asked when it will
get merged, to which Steffen replied "It will be merged with the next
pull request for the ipsec tree that will happen likely next week."
https://lore.kernel.org/regressions/20220216110252.GJ17351@gauss3.secunet.de/

He sent that PR on Friday, so the fix will finally be merged to mainline
on Thursday. If Greg immediately picks it up after -rc7 the issue can
finally get fixed in 5.15.y and 5.16.y mid next week -- more than 50
days after the patch for the regression was posted.

Again: I had similar issues with the bluetooth maintainers and the
wireless maintainers (the latter already seem to have slightly changed
their workflow to improve things).

Anyway: sure, reviewing takes time and ideally every fix is in -next for
a while, but I think 50 days is way too long.

Maybe sub-tree maintainers should send PRs more often? Or just tell you
to directly pick up a fix once they reviewed them to avoid the sub-tree
and a merge commits that brings in just one patch?

Fwiw, situations like that in the end let to the creation of a docs
patch that is en-route to 5.18:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=d2b40ba2cce207ecea8a740f71e113f03cc75fd5

To quote:

> +Prioritize work on fixing regressions
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +You should fix any reported regression as quickly as possible, to provide
> +affected users with a solution in a timely manner and prevent more users from
> +running into the issue; nevertheless developers need to take enough time and
> +care to ensure regression fixes do not cause additional damage.
> +
> +In the end though, developers should give their best to prevent users from
> +running into situations where a regression leaves them only three options: "run
> +a kernel with a regression that seriously impacts usage", "continue running an
> +outdated and thus potentially insecure kernel version for more than two weeks
> +after a regression's culprit was identified", and "downgrade to a still
> +supported kernel series that lack required features".
> +
> +How to realize this depends a lot on the situation. Here are a few rules of
> +thumb for you, in order or importance:
> +
> + * Prioritize work on handling regression reports and fixing regression over all
> +   other Linux kernel work, unless the latter concerns acute security issues or
> +   bugs causing data loss or damage.
> +
> + * Always consider reverting the culprit commits and reapplying them later
> +   together with necessary fixes, as this might be the least dangerous and
> +   quickest way to fix a regression.
> +
> + * Developers should handle regressions in all supported kernel series, but are
> +   free to delegate the work to the stable team, if the issue probably at no
> +   point in time occurred with mainline.
> +
> + * Try to resolve any regressions introduced in the current development before
> +   its end. If you fear a fix might be too risky to apply only days before a new
> +   mainline release, let Linus decide: submit the fix separately to him as soon
> +   as possible with the explanation of the situation. He then can make a call
> +   and postpone the release if necessary, for example if multiple such changes
> +   show up in his inbox.
> +
> + * Address regressions in stable, longterm, or proper mainline releases with
> +   more urgency than regressions in mainline pre-releases. That changes after
> +   the release of the fifth pre-release, aka "-rc5": mainline then becomes as
> +   important, to ensure all the improvements and fixes are ideally tested
> +   together for at least one week before Linus releases a new mainline version.
> +
> + * Fix regressions within two or three days, if they are critical for some
> +   reason -- for example, if the issue is likely to affect many users of the
> +   kernel series in question on all or certain architectures. Note, this
> +   includes mainline, as issues like compile errors otherwise might prevent many
> +   testers or continuous integration systems from testing the series.
> +
> + * Aim to fix regressions within one week after the culprit was identified, if
> +   the issue was introduced in either:
> +
> +    * a recent stable/longterm release
> +
> +    * the development cycle of the latest proper mainline release
> +
> +   In the latter case (say Linux v5.14), try to address regressions even
> +   quicker, if the stable series for the predecessor (v5.13) will be abandoned
> +   soon or already was stamped "End-of-Life" (EOL) -- this usually happens about
> +   three to four weeks after a new mainline release.
> +
> + * Try to fix all other regressions within two weeks after the culprit was
> +   found. Two or three additional weeks are acceptable for performance
> +   regressions and other issues which are annoying, but don't prevent anyone
> +   from running Linux (unless it's an issue in the current development cycle,
> +   as those should ideally be addressed before the release). A few weeks in
> +   total are acceptable if a regression can only be fixed with a risky change
> +   and at the same time is affecting only a few users; as much time is
> +   also okay if the regression is already present in the second newest longterm
> +   kernel series.
> +
> +Note: The aforementioned time frames for resolving regressions are meant to
> +include getting the fix tested, reviewed, and merged into mainline, ideally with
> +the fix being in linux-next at least briefly. This leads to delays you need to
> +account for.
> +
> +Subsystem maintainers are expected to assist in reaching those periods by doing
> +timely reviews and quick handling of accepted patches. They thus might have to
> +send git-pull requests earlier or more often than usual; depending on the fix,
> +it might even be acceptable to skip testing in linux-next. Especially fixes for
> +regressions in stable and longterm kernels need to be handled quickly, as fixes
> +need to be merged in mainline before they can be backported to older series.
> +

The patch description explains some of the reasons for this. Linus was
CCed on this, sadly didn't state if this is actually what he expects. So
it still needs to be proven if this holds in the field.

Ciao, Thorsten

Re: Regression are sometimes merged slowly, maybe optimize the downstream interaction?

[Steffen Klassert]

On Tue, Mar 01, 2022 at 11:06:14AM +0100, Thorsten Leemhuis wrote:
> On 28.02.22 18:46, Jakub Kicinski wrote:
> > On Mon, 28 Feb 2022 14:45:47 +0100 Thorsten Leemhuis wrote:
> >>
> >> Or is there something like that already and the timing just has been bad
> >> a few times when I looked closer?
> > 
> > I think it's a particularly unfortunate time with a few "missed prs"
> > in a short span of time. When Dave was handling all the prs he used
> > to decide the timing based on contents of the tree, maybe that's 
> > a better model for prioritizing fixes getting to Linus, but I lack 
> > the skills necessary to make such calls.
> > 
> > I'll try to advertise the Wednesday rule more, 

I did not know that net pull requests always happen at Wednesdays,
it was always a bit unpredictable to me when that is going to happen.
Knowing about that would already improve the workflow.

> 
> Thx
> 
> > although creating
> > deadlines has proven to lead to rushed work. Which IMHO is much 
> > worse :(
> 
> Don't call it a deadline then. :-D But joking aside, I know what you
> mean, and yes, that is a reasonable concern.
> 
> Maybe one thing could help in here sometimes: if sub-tree maintainers
> with your permission ask Linus to pick up a single patch directly from a
> repo or a list, *if* there is a good reason to get a fix quickly merged.
> 
> > [...]
> > Anyway, thanks for raising the issue, and please keep us posted on how
> > things look from your perspective. It's a balancing act, it'd be great
> > if we can improve things over time without sudden changes.
> 
> Yeah, it's definitely a balancing act.
> 
> And as you asked for how things look from here, let me get back to the
> one issue I already briefly in my mail. To repeat a quote from above:
> 
> >> Often the fixes progress slowly due to the habits of the downstream
> >> maintainers -- some for example are imho not asking you often enough to
> >> pull fixes. I guess that might need to be discussed down the road as
> >> well, but there is something else that imho needs to be addressed first.
> 
> To give an example, but fwiw: that is in no way special, I've seen
> similar turn of events for a few other regressions fixes in sub-tree of
> net, so it really is just meant as an example for a general issue (sorry
> Steffen).

No problem.

> 
> See this fix:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=master&id=a6d95c5a628a09be129f25d5663a7e9db8261f51
> 
> The regression was introduced in 5.14-rc1 and the fix was posted on
> 2022-01-14, so 46 days ago.

Let me give some background on this issue. The PMTU handling with
for IPsec was basically broken forever in the one or the other
subtile way. One attempt to fix it was in 2012 with

commit 91657eafb64b4cb53ec3a2fbc4afc3497f735788
xfrm: take net hdr len into account for esp payload size calculation

nine years later Sabrina noticed that the above fix does not work
for IPv6 if the packet sizes are close to the minimum IPv6 MTU.

She tried to fix it with

commit b515d2637276a3810d6595e10ab02c13bfd0b63a
xfrm: xfrm_state_mtu should return at least 1280 for ipv6

Another year later Jiri noticed that this fix also works
only partially and added his fixes. And honestly, I'm not
sure if this is the end of the story.

This issue has already a long history and never really worked
for all corner cases. That's why I was not in hurry with it at
all.

Btw. how do you pick the regressions you are tracking? Why
this issue and not the many others?

What is the aim of your tracking? Is it to make sure a regression
fix will hit the manline, or/and the stable trees?

Just curious.

> 
> Jiri, who provided the patch, actually wrote "being a regression maybe
> we want the fastest track possible" here:
> https://lore.kernel.org/netdev/20220119091233.pzqdlzpcyicjavk5@dwarf.suse.cz/
> 
> Steffen applied the fix 27 days ago:
> https://lore.kernel.org/netdev/20220201064639.GS1223722@gauss3.secunet.de/
> 
> Three days later it was in -next. After some time I asked when it will
> get merged, to which Steffen replied "It will be merged with the next
> pull request for the ipsec tree that will happen likely next week."
> https://lore.kernel.org/regressions/20220216110252.GJ17351@gauss3.secunet.de/
> 
> He sent that PR on Friday, so the fix will finally be merged to mainline
> on Thursday. If Greg immediately picks it up after -rc7 the issue can
> finally get fixed in 5.15.y and 5.16.y mid next week -- more than 50
> days after the patch for the regression was posted.
> 
> Again: I had similar issues with the bluetooth maintainers and the
> wireless maintainers (the latter already seem to have slightly changed
> their workflow to improve things).
> 
> Anyway: sure, reviewing takes time and ideally every fix is in -next for
> a while, but I think 50 days is way too long.
> 
> Maybe sub-tree maintainers should send PRs more often? Or just tell you
> to directly pick up a fix once they reviewed them to avoid the sub-tree
> and a merge commits that brings in just one patch?

If the patches are applied to the ipsec tree they get much more testing
from our test environment before they move to the net and the mainline
tree. In case I apply an urgent patch, I do a pull request immediately.
But as said, I did not consider the above mentioned commit as too
urgent.

Re: Regression are sometimes merged slowly, maybe optimize the downstream interaction?

[Thorsten Leemhuis]

On 01.03.22 14:26, Steffen Klassert wrote:
> On Tue, Mar 01, 2022 at 11:06:14AM +0100, Thorsten Leemhuis wrote:
>> On 28.02.22 18:46, Jakub Kicinski wrote:
>>> On Mon, 28 Feb 2022 14:45:47 +0100 Thorsten Leemhuis wrote:
> [...]
>>>> Often the fixes progress slowly due to the habits of the downstream
>>>> maintainers -- some for example are imho not asking you often enough to
>>>> pull fixes. I guess that might need to be discussed down the road as
>>>> well, but there is something else that imho needs to be addressed first.
>>
>> To give an example, but fwiw: that is in no way special, I've seen
>> similar turn of events for a few other regressions fixes in sub-tree of
>> net, so it really is just meant as an example for a general issue (sorry
>> Steffen).
> No problem.

Great, thx!

>>
>> See this fix:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=master&id=a6d95c5a628a09be129f25d5663a7e9db8261f51
>>
>> The regression was introduced in 5.14-rc1 and the fix was posted on
>> 2022-01-14, so 46 days ago.
> Let me give some background on this issue. [...]

thx for this, from the outside it's often hard to grasp how bad a
regression is, that's why I handle most of them the same way.
> This issue has already a long history and never really worked
> for all corner cases. That's why I was not in hurry with it at
> all.

Which I guess is fine. And if I had been under the the impression that
it was urgent I would have spoken up a few weeks ago when I asked when
the fix was going to get merged.

> Btw. how do you pick the regressions you are tracking? Why
> this issue and not the many others?

Some people already get regzbot directly involved to make sure their
report doesn't fall through the cracks.

I add most to the tracking that are CCed to the regressions list.

I also run a search for the word "regression" on lore and look at new
bugzilla tickets after a week. I add most of the regression reports I
find that way, but sometimes decide not doing so -- for example if the
fix already is on its way or if the regression is reported by a kernel
developer that knows to keep an eye on things.

Not perfect (just like regzbot), but you have to start somewhere.
> What is the aim of your tracking? Is it to make sure a regression
> fix will hit the manline, or/and the stable trees?

To ensure no reported regression falls through the cracks; the focus
currently is on mainline, but I keep any eye on stable as well.

Ciao, Thorsten