From: Geliang Tang <tanggeliang@kylinos.cn>

To avoid duplicated code in different MPTCP selftests, we can add and
use helpers defined in mptcp_lib.sh.

This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
and mptcp_join.sh into a helper mptcp_lib_events(). Define it in
mptcp_lib.sh and use it in both scripts.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh   |  8 ++------
 tools/testing/selftests/net/mptcp/mptcp_lib.sh    | 14 ++++++++++++++
 tools/testing/selftests/net/mptcp/userspace_pm.sh | 14 ++------------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 2d9cf6f3bbf3..292fccb1f7f4 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -430,12 +430,8 @@ reset_with_events()
 {
 	reset "${1}" || return 1
 
-	:> "$evts_ns1"
-	:> "$evts_ns2"
-	ip netns exec $ns1 ./pm_nl_ctl events >> "$evts_ns1" 2>&1 &
-	evts_ns1_pid=$!
-	ip netns exec $ns2 ./pm_nl_ctl events >> "$evts_ns2" 2>&1 &
-	evts_ns2_pid=$!
+	mptcp_lib_events "${ns1}" "${evts_ns1}" evts_ns1_pid
+	mptcp_lib_events "${ns2}" "${evts_ns2}" evts_ns2_pid
 }
 
 reset_with_tcp_filter()
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index f65a31594829..5b5ad9947c21 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -399,3 +399,17 @@ mptcp_lib_ns_exit() {
 		rm -f /tmp/"$netns".{nstat,out}
 	done
 }
+
+mptcp_lib_events() {
+	local ns="${1}"
+	local evts="${2}"
+	declare -n pid="${3}"
+
+	:>"${evts}"
+
+	if [ ${pid} -ne 0 ]; then
+		mptcp_lib_kill_wait "${pid}"
+	fi
+	ip netns exec "${ns}" ./pm_nl_ctl events >> "${evts}" 2>&1 &
+	pid=$!
+}
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 6d71bf36a1b9..3200d0b96d53 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -178,21 +178,11 @@ make_connection()
 	if [ -z "$client_evts" ]; then
 		client_evts=$(mktemp)
 	fi
-	:>"$client_evts"
-	if [ $client_evts_pid -ne 0 ]; then
-		mptcp_lib_kill_wait $client_evts_pid
-	fi
-	ip netns exec "$ns2" ./pm_nl_ctl events >> "$client_evts" 2>&1 &
-	client_evts_pid=$!
+	mptcp_lib_events "${ns2}" "${client_evts}" client_evts_pid
 	if [ -z "$server_evts" ]; then
 		server_evts=$(mktemp)
 	fi
-	:>"$server_evts"
-	if [ $server_evts_pid -ne 0 ]; then
-		mptcp_lib_kill_wait $server_evts_pid
-	fi
-	ip netns exec "$ns1" ./pm_nl_ctl events >> "$server_evts" 2>&1 &
-	server_evts_pid=$!
+	mptcp_lib_events "${ns1}" "${server_evts}" server_evts_pid
 	sleep 0.5
 
 	# Run the server
-- 
2.40.1

Hi Geliang,

On 22/02/2024 4:40 am, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To avoid duplicated code in different MPTCP selftests, we can add and
> use helpers defined in mptcp_lib.sh.
> 
> This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
> and mptcp_join.sh into a helper mptcp_lib_events(). Define it in
> mptcp_lib.sh and use it in both scripts.

(...)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index f65a31594829..5b5ad9947c21 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -399,3 +399,17 @@ mptcp_lib_ns_exit() {
>  		rm -f /tmp/"$netns".{nstat,out}
>  	done
>  }
> +
> +mptcp_lib_events() {
> +	local ns="${1}"
> +	local evts="${2}"
> +	declare -n pid="${3}"
> +
> +	:>"${evts}"
> +
> +	if [ ${pid} -ne 0 ]; then

Yet another new shellcheck warning introduced here:

> 
> In mptcp_lib.sh line 410:
>         if [ ${pid} -ne 0 ]; then
>              ^----^ SC2086 (info): Double quote to prevent globbing and word splitting.
> 
> Did you mean: 
>         if [ "${pid}" -ne 0 ]; then

I guess the CI should check that and report it, so I would not have to
fix that just before applying patches.

I just added the suggested double-quotes when applying the patch.

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

On 22/02/2024 12:07 pm, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 22/02/2024 4:40 am, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> To avoid duplicated code in different MPTCP selftests, we can add and
>> use helpers defined in mptcp_lib.sh.
>>
>> This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
>> and mptcp_join.sh into a helper mptcp_lib_events(). Define it in
>> mptcp_lib.sh and use it in both scripts.
> 
> (...)
> 
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> index f65a31594829..5b5ad9947c21 100644
>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> @@ -399,3 +399,17 @@ mptcp_lib_ns_exit() {
>>  		rm -f /tmp/"$netns".{nstat,out}
>>  	done
>>  }
>> +
>> +mptcp_lib_events() {
>> +	local ns="${1}"
>> +	local evts="${2}"
>> +	declare -n pid="${3}"
>> +
>> +	:>"${evts}"
>> +
>> +	if [ ${pid} -ne 0 ]; then
> 
> Yet another new shellcheck warning introduced here:
> 
>>
>> In mptcp_lib.sh line 410:
>>         if [ ${pid} -ne 0 ]; then
>>              ^----^ SC2086 (info): Double quote to prevent globbing and word splitting.
>>
>> Did you mean: 
>>         if [ "${pid}" -ne 0 ]; then
> 
> I guess the CI should check that and report it, so I would not have to
> fix that just before applying patches.

In fact, we don't need this check because mptcp_lib_kill_wait() does
nothing if the pid is set to 0. So I simplified the fix by removing the
condition.

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