From: Geliang Tang <tanggeliang@kylinos.cn>

v14:
 - address Matt's comments in v13.
 - I have to use these lines in patch #8:

      local cmd_ret=0
      local out=$(${cmd} 2>${err}) || cmd_ret=${?}

   The following expected code breaks pm_netlink.sh, I don't know why:

      local out cmd_ret=0
      out=$(${cmd} 2>${err}) || cmd_ret=${?}

   If this is still a problem, I'll send a squash-to patch to fix it.

Geliang Tang (14):
  mptcp: export mptcp_genl_family & mptcp_nl_fill_addr
  mptcp: implement mptcp_userspace_pm_dump_addr
  mptcp: add token for get-addr in yaml
  mptcp: dump addrs in userspace pm list
  mptcp: check userspace pm flags
  selftests: mptcp: add userspace pm subflow flag
  selftests: mptcp: add token for dump_addr
  selftests: mptcp: add mptcp_lib_check_output helper
  selftests: mptcp: dump userspace addrs list
  mptcp: add userspace_pm_lookup_addr_by_id helper
  mptcp: implement mptcp_userspace_pm_get_addr
  mptcp: get addr in userspace pm list
  selftests: mptcp: add token for get_addr
  selftests: mptcp: userspace pm get addr tests

 Documentation/netlink/specs/mptcp_pm.yaml     |   3 +-
 net/mptcp/mptcp_pm_gen.c                      |   7 +-
 net/mptcp/mptcp_pm_gen.h                      |   2 +-
 net/mptcp/pm.c                                |  16 ++
 net/mptcp/pm_netlink.c                        |  26 ++-
 net/mptcp/pm_userspace.c                      | 180 ++++++++++++++++--
 net/mptcp/protocol.h                          |  13 ++
 .../testing/selftests/net/mptcp/mptcp_join.sh |  91 +++++++++
 .../testing/selftests/net/mptcp/mptcp_lib.sh  |  19 ++
 .../testing/selftests/net/mptcp/pm_netlink.sh |  18 +-
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c |  39 +++-
 11 files changed, 368 insertions(+), 46 deletions(-)

-- 
2.40.1

Hi Geliang, Mat,

On 20/02/2024 10:58 am, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v14:
>  - address Matt's comments in v13.
>  - I have to use these lines in patch #8:
> 
>       local cmd_ret=0
>       local out=$(${cmd} 2>${err}) || cmd_ret=${?}
> 
>    The following expected code breaks pm_netlink.sh, I don't know why:
> 
>       local out cmd_ret=0
>       out=$(${cmd} 2>${err}) || cmd_ret=${?}
> 
>    If this is still a problem, I'll send a squash-to patch to fix it.
> 
> Geliang Tang (14):
>   mptcp: export mptcp_genl_family & mptcp_nl_fill_addr
>   mptcp: implement mptcp_userspace_pm_dump_addr
>   mptcp: add token for get-addr in yaml
>   mptcp: dump addrs in userspace pm list
>   mptcp: check userspace pm flags
>   selftests: mptcp: add userspace pm subflow flag
>   selftests: mptcp: add token for dump_addr
>   selftests: mptcp: add mptcp_lib_check_output helper
>   selftests: mptcp: dump userspace addrs list
>   mptcp: add userspace_pm_lookup_addr_by_id helper
>   mptcp: implement mptcp_userspace_pm_get_addr
>   mptcp: get addr in userspace pm list
>   selftests: mptcp: add token for get_addr
>   selftests: mptcp: userspace pm get addr tests

Thank you for the new version and the reviews!

Now in our tree (feat. for next):

New patches for t/upstream:
- 7d868c62af9f: mptcp: export mptcp_genl_family & mptcp_nl_fill_addr
- 67cb691f1093: mptcp: implement mptcp_userspace_pm_dump_addr
- 60ef9f34e3d3: mptcp: add token for get-addr in yaml
- ffb2fe640112: mptcp: dump addrs in userspace pm list
- 76eac7d3c80c: mptcp: check userspace pm flags
- 409cea20942c: selftests: mptcp: add userspace pm subflow flag
- 6353432774ce: selftests: mptcp: add token for dump_addr
- d09f50cf57ae: selftests: mptcp: add mptcp_lib_check_output helper
- c6ad331dc5ab: selftests: mptcp: dump userspace addrs list
- 47038dcd3675: mptcp: add userspace_pm_lookup_addr_by_id helper
- 2a5b3503d500: mptcp: implement mptcp_userspace_pm_get_addr
- bbfcefc51c19: mptcp: get addr in userspace pm list
- 1cda2a037f12: selftests: mptcp: add token for get_addr
- d0c5cd880322: selftests: mptcp: userspace pm get addr tests
- Results: 23f1f4962d2e..bf8465c7dc96 (export)

- ae4144118862: "squashed" in "selftests: mptcp: add
mptcp_lib_check_output helper"
- Results: bf8465c7dc96..55a829ff16aa (export)

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

Hi Geliang,

On 20/02/2024 10:58, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v14:
>  - address Matt's comments in v13.

Thank you for the new version!

I didn't look at it yet, but:

>  - I have to use these lines in patch #8:
> 
>       local cmd_ret=0
>       local out=$(${cmd} 2>${err}) || cmd_ret=${?}
> 
>    The following expected code breaks pm_netlink.sh, I don't know why:
> 
>       local out cmd_ret=0
>       out=$(${cmd} 2>${err}) || cmd_ret=${?}
> 
>    If this is still a problem, I'll send a squash-to patch to fix it.

Yes, we should either fix warnings or explicitly mark the false positive
ones as ignored.

Can you try with the attached patch please? It is not clear to me why
the second block doesn't do the same as the first one.

I can squash it when applying the series if there are no other comments.

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

On Tue, Feb 20, 2024 at 12:50:49PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 20/02/2024 10:58, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > v14:
> >  - address Matt's comments in v13.
> 
> Thank you for the new version!
> 
> I didn't look at it yet, but:
> 
> >  - I have to use these lines in patch #8:
> > 
> >       local cmd_ret=0
> >       local out=$(${cmd} 2>${err}) || cmd_ret=${?}
> > 
> >    The following expected code breaks pm_netlink.sh, I don't know why:
> > 
> >       local out cmd_ret=0
> >       out=$(${cmd} 2>${err}) || cmd_ret=${?}
> > 
> >    If this is still a problem, I'll send a squash-to patch to fix it.
> 
> Yes, we should either fix warnings or explicitly mark the false positive
> ones as ignored.
> 
> Can you try with the attached patch please? It is not clear to me why
> the second block doesn't do the same as the first one.

Thanks Matt, the attached patch works well. It fixes the issue I
mentioned perfectly.

> 
> I can squash it when applying the series if there are no other comments.

Great, thanks.

-Geliang

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

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index aca35376006b..556a7d9784d7 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -352,20 +352,24 @@ mptcp_lib_check_tools() {
>  }
>  
>  mptcp_lib_check_output() {
> -	local err="$1"
> -	local cmd="$2"
> -	local expected="$3"
> +	local err="${1}"
> +	local cmd="${2}"
> +	local expected="${3}"
>  	local cmd_ret=0
> -	local out=$(${cmd} 2>${err}) || cmd_ret=${?}
> +	local out
>  
> -	if [ $cmd_ret -ne 0 ]; then
> -		mptcp_lib_print_err "[FAIL] command execution '$cmd' stderr"
> +	if ! out=$(${cmd} 2>"${err}"); then
> +		cmd_ret=${?}
> +	fi
> +
> +	if [ ${cmd_ret} -ne 0 ]; then
> +		mptcp_lib_print_err "[FAIL] command execution '${cmd}' stderr"
>  		cat "${err}"
>  		return 2
> -	elif [ "$out" = "$expected" ]; then
> +	elif [ "${out}" = "${expected}" ]; then
>  		return 0
>  	else
> -		mptcp_lib_print_err "[FAIL] expected '$expected' got '$out'"
> +		mptcp_lib_print_err "[FAIL] expected '${expected}' got '${out}'"
>  		return 1
>  	fi
>  }

Hi Geliang, Mat,

On 20/02/2024 12:50 pm, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 20/02/2024 10:58, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> v14:
>>  - address Matt's comments in v13.
> 
> Thank you for the new version!
> 
> I didn't look at it yet, but:
> 
>>  - I have to use these lines in patch #8:
>>
>>       local cmd_ret=0
>>       local out=$(${cmd} 2>${err}) || cmd_ret=${?}
>>
>>    The following expected code breaks pm_netlink.sh, I don't know why:
>>
>>       local out cmd_ret=0
>>       out=$(${cmd} 2>${err}) || cmd_ret=${?}
>>
>>    If this is still a problem, I'll send a squash-to patch to fix it.
> 
> Yes, we should either fix warnings or explicitly mark the false positive
> ones as ignored.
> 
> Can you try with the attached patch please? It is not clear to me why
> the second block doesn't do the same as the first one.
> 
> I can squash it when applying the series if there are no other comments.

There are no other comments from me, thanks for the v14.

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

@Geliang: OK for you if I squash the suggested patch?

@Mat: still OK for you with this v14? There are only modifications in
the selftests (patches 8, 9 and 14) except a small change in patch 5.

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

On Tue, 20 Feb 2024, Matthieu Baerts wrote:

> Hi Geliang, Mat,
>
> On 20/02/2024 12:50 pm, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 20/02/2024 10:58, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> v14:
>>>  - address Matt's comments in v13.
>>
>> Thank you for the new version!
>>
>> I didn't look at it yet, but:
>>
>>>  - I have to use these lines in patch #8:
>>>
>>>       local cmd_ret=0
>>>       local out=$(${cmd} 2>${err}) || cmd_ret=${?}
>>>
>>>    The following expected code breaks pm_netlink.sh, I don't know why:
>>>
>>>       local out cmd_ret=0
>>>       out=$(${cmd} 2>${err}) || cmd_ret=${?}
>>>
>>>    If this is still a problem, I'll send a squash-to patch to fix it.
>>
>> Yes, we should either fix warnings or explicitly mark the false positive
>> ones as ignored.
>>
>> Can you try with the attached patch please? It is not clear to me why
>> the second block doesn't do the same as the first one.
>>
>> I can squash it when applying the series if there are no other comments.
>
> There are no other comments from me, thanks for the v14.
>
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
> @Geliang: OK for you if I squash the suggested patch?
>
> @Mat: still OK for you with this v14? There are only modifications in
> the selftests (patches 8, 9 and 14) except a small change in patch 5.
>

Yes, v14 looks good, my tag still applies:

Reviewed-by: Mat Martineau <martineau@kernel.org>

Thanks for your review Matthieu!


- Mat