[PATCH mptcp-next 0/3] Upstreaming cleanup

Mat Martineau posted 3 patches 2 years, 2 months ago
Failed in applying to current master (apply log)
tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 -
1 file changed, 1 deletion(-)
[PATCH mptcp-next 0/3] Upstreaming cleanup
Posted by Mat Martineau 2 years, 2 months ago
The most recent patch series I upstreamed for -net needed a couple of
tweaks to mitigate net/net-next merge conflicts. The first two patches
here update the commits in "fixes net" on the export branch to match
what I sent upstream, and the third patch restores Paolo's function
rename in mptcp_join.sh.

Mat Martineau (3):
  Squash to "selftests: mptcp: improve 'fair usage on close' stability"
  Squash to "selftests: mptcp: more robust signal race test"
  selftests: mptcp: Rename wait function

 tools/testing/selftests/net/mptcp/mptcp_join.sh | 1 -
 1 file changed, 1 deletion(-)


base-commit: d08b615cca7c8389461c7bf83d46ca8bc1fd72db
-- 
2.35.1


Re: [PATCH mptcp-next 0/3] Upstreaming cleanup
Posted by Matthieu Baerts 2 years, 2 months ago
Hi Mat,

On 18/02/2022 23:18, Mat Martineau wrote:
> The most recent patch series I upstreamed for -net needed a couple of
> tweaks to mitigate net/net-next merge conflicts. The first two patches
> here update the commits in "fixes net" on the export branch to match
> what I sent upstream, and the third patch restores Paolo's function
> rename in mptcp_join.sh.

Thank you for these patches!

I was thinking a bit about how to avoid such "issues" in the future.
Here are some ideas:

- there could be 2 separated trees periodically updated:
  - one for net-next, the one we have for the moment
  - one for -net: a new one
  → all patches for the -net tree would be duplicated in both trees
  → when applying patches in the -net tree, we will notice the conflicts
  → applying the different patches can be automated with the scripts I'm
currently using

- there could be 2 mixed trees periodically updated:


            top

             ^
             |

    commits for net-next

             ^
             |

    merge net + net-next

             ^
            / \
               \
    net-next    \
                 \

                commits for -net (+ commits for the CI)

                   ^
                   |

                  net


  → no duplicated -net commits
  → the "export" branch will be on top of net and net-next
  → "mptcp: use kmalloc on kasan build" could be cherry-picked on top of
the "export" branch for the -net tree not to have it "under" the ones
for net-next.

In both cases, it would be easy to export the different commits and have
different tags that can be used by the CI.

The second case looks interesting to me but there are at least two main
points to be careful with:

 - if the "export" branch is on top of the net-next branch, it means the
patches we have in the tree might no longer apply as is in net-next:
that's the opposite situation as of today where the patches for -net are
on top of net-next. I think that's fine because we would have this issue
in any case if a patch for net-next depend on one that has been
queued/applied in -net but is not in net-next yet.

 - there might be conflicts / build / validation issues from elsewhere
to resolve when merging net-next with -net: conflicts in non MPTCP
parts. They will have to be resolved before they are resolved in
net-next tree. Workaround is not update the "net" base in case of
conflicts until the sync is done by the -net maintainers: easy to put in
place in case of conflicts, a bit less for the build / validation issues
but that's probably rare. A lot of stuff are modified in the -net tree
with all the different protocols, drivers and BPF! It is not rare to
have conflicts if you look at:

  $ git log --grep 'Merge
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net' net-next

What do you think?
First, do we need a solution?
Then would it be OK with the second solution, especially in case of
conflicts/build/validation issues? If not, would the first solution
helps? Or another one?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH mptcp-next 0/3] Upstreaming cleanup
Posted by Mat Martineau 2 years, 2 months ago
On Sat, 19 Feb 2022, Matthieu Baerts wrote:

> Hi Mat,
>
> On 18/02/2022 23:18, Mat Martineau wrote:
>> The most recent patch series I upstreamed for -net needed a couple of
>> tweaks to mitigate net/net-next merge conflicts. The first two patches
>> here update the commits in "fixes net" on the export branch to match
>> what I sent upstream, and the third patch restores Paolo's function
>> rename in mptcp_join.sh.
>
> Thank you for these patches!
>
> I was thinking a bit about how to avoid such "issues" in the future.
> Here are some ideas:
>
> - there could be 2 separated trees periodically updated:
>  - one for net-next, the one we have for the moment
>  - one for -net: a new one
>  → all patches for the -net tree would be duplicated in both trees
>  → when applying patches in the -net tree, we will notice the conflicts
>  → applying the different patches can be automated with the scripts I'm
> currently using
>
> - there could be 2 mixed trees periodically updated:
>
>
>            top
>
>             ^
>             |
>
>    commits for net-next
>
>             ^
>             |
>
>    merge net + net-next
>
>             ^
>            / \
>               \
>    net-next    \
>                 \
>
>                commits for -net (+ commits for the CI)
>
>                   ^
>                   |
>
>                  net
>
>
>  → no duplicated -net commits
>  → the "export" branch will be on top of net and net-next
>  → "mptcp: use kmalloc on kasan build" could be cherry-picked on top of
> the "export" branch for the -net tree not to have it "under" the ones
> for net-next.
>
> In both cases, it would be easy to export the different commits and have
> different tags that can be used by the CI.
>
> The second case looks interesting to me but there are at least two main
> points to be careful with:
>
> - if the "export" branch is on top of the net-next branch, it means the
> patches we have in the tree might no longer apply as is in net-next:
> that's the opposite situation as of today where the patches for -net are
> on top of net-next. I think that's fine because we would have this issue
> in any case if a patch for net-next depend on one that has been
> queued/applied in -net but is not in net-next yet.
>

I don't think it changes the situation for the mptcp-next patches - even 
today, they are on top of the mptcp-net patches which is what would 
potentially cause problems sending to net-next.

Since I apply the patches to net-next and resolve conflicts before sending 
to netdev, it's not too big of a concern. I think your proposal here is an 
improvement because fewer adjustments will be needed when sending the -net 
patches to netdev.


I would adjust your diagram this way:


            export

              ^
              |

     kmalloc on ksan build

              ^
              |

     commits for net-next

              ^
              |

     merge net + net-next

              ^
             / \        export-net
                \      /
     net-next    \    kmalloc on ksan build
     (rebase      \  /
      daily)
                 commits for -net (+ commits for the CI)

                    ^
                    |

                   net (rebase on upstream net-next sync,
                        as you suggest below)

Then people could check out export-net when developing mptcp-net patches.

> - there might be conflicts / build / validation issues from elsewhere
> to resolve when merging net-next with -net: conflicts in non MPTCP
> parts. They will have to be resolved before they are resolved in
> net-next tree. Workaround is not update the "net" base in case of
> conflicts until the sync is done by the -net maintainers: easy to put in
> place in case of conflicts, a bit less for the build / validation issues
> but that's probably rare. A lot of stuff are modified in the -net tree
> with all the different protocols, drivers and BPF! It is not rare to
> have conflicts if you look at:
>
>  $ git log --grep 'Merge
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net' net-next
>

That sync-net-when-upstream-does workaround sounds good to me. I agree, 
it's not rare for those net/net-next conflicts to happen, and since we are 
carrying our own patches until the -net rebase anyway I think it all works 
out. 'git merge-base net/master net-next/master' gives the relevant -net 
commit.

If we wanted advance warning, the CI could attempt a rebase of the 
mptcp-net commits on the current net/master (sort of a dry run) and notify 
if that had any conflicts.

> What do you think?
> First, do we need a solution?

It's not absolutely critical, but I think it's a worthwhile improvement.

This wouldn't prevent the net/net-next conflicts that I was asking about 
in this thread, since those were due to upstreaming some net-next patches 
before the conflicting -net patch was even available. Normally it will be 
better to prioritize the -net patches for upstreaming, and check for 
net-next conflicts while there are MPTCP patches in -net that haven't been 
synced yet. This kind of conflicting net-next patch can typically be 
delayed until the net/net-next sync.


Even so, updating the export structure is worth it given that the -net 
patches are somewhat more sensitive (as we have less opportunity to 
correct problems before release). This makes placing the mptcp-net commits 
on top of the -net history a safer way to go.

> Then would it be OK with the second solution, especially in case of
> conflicts/build/validation issues? If not, would the first solution
> helps? Or another one?
>

I prefer the second solution (with the adjustments for the export-net 
branch). The way the first proposal maintains two versions of the 
mptcp-net patches would be harder to keep track of.

--
Mat Martineau
Intel
Re: [PATCH mptcp-next 0/3] Upstreaming cleanup
Posted by Matthieu Baerts 2 years, 2 months ago
Hi Mat,

On 24/02/2022 01:25, Mat Martineau wrote:
> On Sat, 19 Feb 2022, Matthieu Baerts wrote:
> 
>> Hi Mat,
>>
>> On 18/02/2022 23:18, Mat Martineau wrote:
>>> The most recent patch series I upstreamed for -net needed a couple of
>>> tweaks to mitigate net/net-next merge conflicts. The first two patches
>>> here update the commits in "fixes net" on the export branch to match
>>> what I sent upstream, and the third patch restores Paolo's function
>>> rename in mptcp_join.sh.
>>
>> Thank you for these patches!
>>
>> I was thinking a bit about how to avoid such "issues" in the future.
>> Here are some ideas:
>>
>> - there could be 2 separated trees periodically updated:
>>  - one for net-next, the one we have for the moment
>>  - one for -net: a new one
>>  → all patches for the -net tree would be duplicated in both trees
>>  → when applying patches in the -net tree, we will notice the conflicts
>>  → applying the different patches can be automated with the scripts I'm
>> currently using
>>
>> - there could be 2 mixed trees periodically updated:
>>
>>
>>            top
>>
>>             ^
>>             |
>>
>>    commits for net-next
>>
>>             ^
>>             |
>>
>>    merge net + net-next
>>
>>             ^
>>            / \
>>               \
>>    net-next    \
>>                 \
>>
>>                commits for -net (+ commits for the CI)
>>
>>                   ^
>>                   |
>>
>>                  net
>>
>>
>>  → no duplicated -net commits
>>  → the "export" branch will be on top of net and net-next
>>  → "mptcp: use kmalloc on kasan build" could be cherry-picked on top of
>> the "export" branch for the -net tree not to have it "under" the ones
>> for net-next.
>>
>> In both cases, it would be easy to export the different commits and have
>> different tags that can be used by the CI.
>>
>> The second case looks interesting to me but there are at least two main
>> points to be careful with:
>>
>> - if the "export" branch is on top of the net-next branch, it means the
>> patches we have in the tree might no longer apply as is in net-next:
>> that's the opposite situation as of today where the patches for -net are
>> on top of net-next. I think that's fine because we would have this issue
>> in any case if a patch for net-next depend on one that has been
>> queued/applied in -net but is not in net-next yet.
>>
> 
> I don't think it changes the situation for the mptcp-next patches - even
> today, they are on top of the mptcp-net patches which is what would
> potentially cause problems sending to net-next.
> 
> Since I apply the patches to net-next and resolve conflicts before
> sending to netdev, it's not too big of a concern. I think your proposal
> here is an improvement because fewer adjustments will be needed when
> sending the -net patches to netdev.
> 
> 
> I would adjust your diagram this way:
> 
> 
>            export
> 
>              ^
>              |
> 
>     kmalloc on ksan build
> 
>              ^
>              |
> 
>     commits for net-next
> 
>              ^
>              |
> 
>     merge net + net-next
> 
>              ^
>             / \        export-net
>                \      /
>     net-next    \    kmalloc on ksan build
>     (rebase      \  /
>      daily)
>                 commits for -net (+ commits for the CI)
> 
>                    ^
>                    |
> 
>                   net (rebase on upstream net-next sync,
>                        as you suggest below)
> 
> Then people could check out export-net when developing mptcp-net patches.

Yes, that's what I had in mind for the extra patches for the CI (for
KASAN, GitHub Actions and Cirrus).

> 
>> - there might be conflicts / build / validation issues from elsewhere
>> to resolve when merging net-next with -net: conflicts in non MPTCP
>> parts. They will have to be resolved before they are resolved in
>> net-next tree. Workaround is not update the "net" base in case of
>> conflicts until the sync is done by the -net maintainers: easy to put in
>> place in case of conflicts, a bit less for the build / validation issues
>> but that's probably rare. A lot of stuff are modified in the -net tree
>> with all the different protocols, drivers and BPF! It is not rare to
>> have conflicts if you look at:
>>
>>  $ git log --grep 'Merge
>> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net' net-next
>>
> 
> That sync-net-when-upstream-does workaround sounds good to me. I agree,
> it's not rare for those net/net-next conflicts to happen, and since we
> are carrying our own patches until the -net rebase anyway I think it all
> works out. 'git merge-base net/master net-next/master' gives the
> relevant -net commit.

Ah yes, that's a good idea to avoid having to manage various issues.

I was thinking doing that only in case of merge conflicts but this is
probably not needed.

> If we wanted advance warning, the CI could attempt a rebase of the
> mptcp-net commits on the current net/master (sort of a dry run) and
> notify if that had any conflicts.

Yes, we can also do some updates manually. We can also add an option for
the GH Action job to do that on requests.

>> What do you think?
>> First, do we need a solution?
> 
> It's not absolutely critical, but I think it's a worthwhile improvement.
> 
> This wouldn't prevent the net/net-next conflicts that I was asking about
> in this thread, since those were due to upstreaming some net-next
> patches before the conflicting -net patch was even available. Normally
> it will be better to prioritize the -net patches for upstreaming, and
> check for net-next conflicts while there are MPTCP patches in -net that
> haven't been synced yet. This kind of conflicting net-next patch can
> typically be delayed until the net/net-next sync.
> 
> 
> Even so, updating the export structure is worth it given that the -net
> patches are somewhat more sensitive (as we have less opportunity to
> correct problems before release). This makes placing the mptcp-net
> commits on top of the -net history a safer way to go.

That's right. And we can setup CI jobs when the future export-net branch
is modified!

>> Then would it be OK with the second solution, especially in case of
>> conflicts/build/validation issues? If not, would the first solution
>> helps? Or another one?
>>
> 
> I prefer the second solution (with the adjustments for the export-net
> branch). The way the first proposal maintains two versions of the
> mptcp-net patches would be harder to keep track of.

Indeed, if we have to modify a patch (not often but it happens), it will
not be nice.

I'm adding that to my TODO list!

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH mptcp-next 0/3] Upstreaming cleanup
Posted by Matthieu Baerts 2 years, 1 month ago
Hi Paolo,

On 24/02/2022 15:41, Matthieu Baerts wrote:
> Hi Mat,
> 
> On 24/02/2022 01:25, Mat Martineau wrote:
>> On Sat, 19 Feb 2022, Matthieu Baerts wrote:
>>
>>> Hi Mat,
>>>
>>> On 18/02/2022 23:18, Mat Martineau wrote:
>>>> The most recent patch series I upstreamed for -net needed a couple of
>>>> tweaks to mitigate net/net-next merge conflicts. The first two patches
>>>> here update the commits in "fixes net" on the export branch to match
>>>> what I sent upstream, and the third patch restores Paolo's function
>>>> rename in mptcp_join.sh.
>>>
>>> Thank you for these patches!
>>>
>>> I was thinking a bit about how to avoid such "issues" in the future.
>>> Here are some ideas:
>>>
>>> - there could be 2 separated trees periodically updated:
>>>  - one for net-next, the one we have for the moment
>>>  - one for -net: a new one
>>>  → all patches for the -net tree would be duplicated in both trees
>>>  → when applying patches in the -net tree, we will notice the conflicts
>>>  → applying the different patches can be automated with the scripts I'm
>>> currently using
>>>
>>> - there could be 2 mixed trees periodically updated:
>>>
>>>
>>>            top
>>>
>>>             ^
>>>             |
>>>
>>>    commits for net-next
>>>
>>>             ^
>>>             |
>>>
>>>    merge net + net-next
>>>
>>>             ^
>>>            / \
>>>               \
>>>    net-next    \
>>>                 \
>>>
>>>                commits for -net (+ commits for the CI)
>>>
>>>                   ^
>>>                   |
>>>
>>>                  net
>>>
>>>
>>>  → no duplicated -net commits
>>>  → the "export" branch will be on top of net and net-next
>>>  → "mptcp: use kmalloc on kasan build" could be cherry-picked on top of
>>> the "export" branch for the -net tree not to have it "under" the ones
>>> for net-next.
>>>
>>> In both cases, it would be easy to export the different commits and have
>>> different tags that can be used by the CI.
>>>
>>> The second case looks interesting to me but there are at least two main
>>> points to be careful with:
>>>
>>> - if the "export" branch is on top of the net-next branch, it means the
>>> patches we have in the tree might no longer apply as is in net-next:
>>> that's the opposite situation as of today where the patches for -net are
>>> on top of net-next. I think that's fine because we would have this issue
>>> in any case if a patch for net-next depend on one that has been
>>> queued/applied in -net but is not in net-next yet.
>>>
>>
>> I don't think it changes the situation for the mptcp-next patches - even
>> today, they are on top of the mptcp-net patches which is what would
>> potentially cause problems sending to net-next.
>>
>> Since I apply the patches to net-next and resolve conflicts before
>> sending to netdev, it's not too big of a concern. I think your proposal
>> here is an improvement because fewer adjustments will be needed when
>> sending the -net patches to netdev.
>>
>>
>> I would adjust your diagram this way:
>>
>>
>>            export
>>
>>              ^
>>              |
>>
>>     kmalloc on ksan build
>>
>>              ^
>>              |
>>
>>     commits for net-next
>>
>>              ^
>>              |
>>
>>     merge net + net-next
>>
>>              ^
>>             / \        export-net
>>                \      /
>>     net-next    \    kmalloc on ksan build
>>     (rebase      \  /
>>      daily)
>>                 commits for -net (+ commits for the CI)
>>
>>                    ^
>>                    |
>>
>>                   net (rebase on upstream net-next sync,
>>                        as you suggest below)
>>
>> Then people could check out export-net when developing mptcp-net patches.
> 
> Yes, that's what I had in mind for the extra patches for the CI (for
> KASAN, GitHub Actions and Cirrus).

I was looking at updating the tree as discussed above. I started by
doing some tests with a new Git Project to simulate the situation.

For the moment I have something like (from 'git log --graph'):


        * export
        *   (...)
        *   MPTCP commits for net-next
        *   (...)
        |\
        | *   (...)
        | * MPTCP commits for -net
        | *   (...)
        * | top of net-next branch
        |\|
        | * top of -net branch
        | | (...)

Would that work for you?
Or do you prefer if I do some tests on a separated repo?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH mptcp-next 0/3] Upstreaming cleanup
Posted by Matthieu Baerts 2 years, 2 months ago
Hi Mat,

On 18/02/2022 23:18, Mat Martineau wrote:
> The most recent patch series I upstreamed for -net needed a couple of
> tweaks to mitigate net/net-next merge conflicts. The first two patches
> here update the commits in "fixes net" on the export branch to match
> what I sent upstream, and the third patch restores Paolo's function
> rename in mptcp_join.sh.

Thank you for these patches!

Now in our tree to be ready for the next sync with -net:

- 74d3b29e0057: "squashed" patch 1/3 in "selftests: mptcp: improve 'fair
usage on close' stability"
- ab0f4168d96d: "squashed" patch 2/3 in "selftests: mptcp: more robust
signal race test"
- 1c3bc536df32: selftests: mptcp: Rename wait function
- Results: 93c0eb10f07a..8dda577d8414

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220219T164531
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net