Re: [PATCH mptcp-next v2 1/2] mptcp: pm: userspace: drop delete_local_addr helper

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 15/04/2025 11:18, Geliang Tang wrote:
> On Tue, 2025-04-15 at 11:09 +0800, Geliang Tang wrote:
>> On Fri, 2025-04-11 at 17:26 -0700, Mat Martineau wrote:
>>> On Thu, 10 Apr 2025, Geliang Tang wrote:
>>>
>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>>
>>>> Address entries should not be removed from local_addr_list when a
>>>> subflow
>>>> is deleted by the userspace PM, should only be removed when
>>>> sending
>>>> a
>>>> REMOVE_ADDR.
>>>>
>>>> So mptcp_userspace_pm_delete_local_addr() helper shouldn't be
>>>> called in
>>>> mptcp_pm_nl_subflow_create_doit() and
>>>> mptcp_pm_nl_subflow_destroy_doit().
>>>>
>>>> Since this helper is open-coding in mptcp_pm_nl_remove_doit(), it
>>>> can be
>>>> dropped now.
>>>>
>>>> Address entries are removed from local_addr_list when sending a
>>>> REMOVE_ADDR
>>>> by the userspace PM, the local_addr_used counter of PM should
>>>> also
>>>> be
>>>> decremented accordingly.
>>>>
>>>
>>> Hi Geliang -
>>>
>>> Have you tried this patch with mptcpd? Does it affect any tests
>>> there?
>>
>> I haven't tested it. To be honest, I don't know how to run the test
>> items of mptcpd. Can you give me some guidance?

'make check' should execute all the tests. Do you mind trying this with
and without your modifications?

Please make sure relevant ones are not skipped because of missing
dependences or tools.

> I just tested it this way, and here are the test results:

(...)

> These results appear to be unchanged from before. And in fact these
> tests do not test userspace pm.

I don't remember what is being tested, but I guess some parts of the
userspace PM are tested somewhere, no?

Coveralls [1] seems to indicate it is tested, but I don't see the details.

[1] https://coveralls.io/github/multipath-tcp/mptcpd

> So it seems that this patch will not affect the tests of mptcpd.

Regarding your patch 2/2 of this series: did you have to modify the
selftests to make the tests passing again due to a changed behaviour, or
did you modify them to extend the behaviour.

It looks like the behaviour has changed, but it is unclear to me if it
is to fix something or add a new feature.

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