Re: [PATCH mptcp-next v2 00/14] mptcp: pm: code reorganisation

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 03/03/2025 12:07, Matthieu Baerts wrote:
> On 03/03/2025 11:55, Geliang Tang wrote:
>> On Sat, 2025-03-01 at 12:10 +0100, Matthieu Baerts wrote:

(...)

>>> I think it would be better to apply these patches ASAP, not to block
>>> further developments, or having to rebase this kind of series. WDYT?
>>
>> Patches that move code to another location are not very friendly to
>> backporting, especially large patches like this set, which almost
>> always conflict. When conflicts occur, we need to deal with them
>> manually.
>>
>> All my suggestions are to reduce the workload of manually dealing with
>> conflicts when they occur.
>>
>> If the code is moved in one piece, we can handle it easily. If the
>> function location changes during the code move, we need to move each
>> function one by one, which increases the workload. If the function name
>> changes, we need to compile and verify it at the same time, which
>> further increases the workload. If the code logic also changes, the
>> situation will be worse.
>>
>> It is possible that this patch does not need to be backported, but the
>> subsequent PM code is modified based on the new code location, and the
>> subsequent new features are developed based on it, so there is still a
>> possibility that this patch needs to be backported.
> 
> Thank you for your reply!
> 
> Yes, clearly this series can cause troubles for future fixes in the PM
> manager, but it looks worth it, and the right time to do so with all
> other cleanups.
> 
> Usually, conflicts in smaller chunk are easier to deal with. Now, here
> it is a bit different: big blocks of code are being moved from one side
> to another. The idea is not to backport this series. We might want to do
> that to ease the backport of some fixes, but it feels to me that we
> would either take all patches from this series, or none.
> 
> So far, I think I did the backports of all MPTCP patches having
> conflicts, and my feeling is that the v2 is OK like that. It was good to
> extract all functions renaming in separated patches and only move code
> from one file to another as you suggested after having reviewed the v1.
> This will help for other reviews, but also for the backports, where we
> will clearly understand what has moved where. I don't think we need more.
> 
>> Since all my comments do not involve any code modifications, and the
>> code itself in this set looks good to me, I am also willing to add my
>> reviewed-by tag:
>>
>> Reviewed-by: Geliang Tang <geliang@kernel.org>
> Thank you! I'm then going to apply this series in our tree ASAP.

Now in our tree (feat. for net-next):

New patches for t/upstream:
- caa77b6a80a2: mptcp: pm: remove '_nl' from mptcp_pm_nl_addr_send_ack
- be18d814ea8b: mptcp: pm: remove '_nl' from mptcp_pm_nl_mp_prio_send_ack
- c6112330af30: mptcp: pm: remove '_nl' from mptcp_pm_nl_work
- 33602d19dcfd: mptcp: pm: remove '_nl' from mptcp_pm_nl_rm_addr_received
- d68e766378ca: mptcp: pm: remove '_nl' from mptcp_pm_nl_subflow_chk_stale()
- 8497c3d80054: mptcp: pm: remove '_nl' from mptcp_pm_nl_is_init_remote_addr
- 6df9239877ee: mptcp: pm: kernel: add '_pm' to mptcp_nl_set_flags
- 901068cd09dc: mptcp: pm: avoid calling PM specific code from core
- dde36f90e966: mptcp: pm: worker: split in-kernel and common tasks
- 309274c12f21: mptcp: pm: export mptcp_remote_address
- 73dff270e144: mptcp: pm: move generic helper at the top
- 6295203b8677: mptcp: pm: move generic PM helpers to pm.c
- 0ac713141723: mptcp: pm: split in-kernel PM specific code
- d96862b4bb41: mptcp: pm: move Netlink PM helpers to pm_netlink.c
- Results: a2a605f37c1b..b084c48057bb (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/f4b35b693104bc77bd7419868548eba9649a4d9a/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.