All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test
@ 2026-08-13 10:29 Matthieu Baerts (NGI0)
  2026-08-13 10:29 ` [PATCH mptcp-net 1/2] selftests: mptcp: lib: dump nstat " Matthieu Baerts (NGI0)
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Matthieu Baerts (NGI0) @ 2026-08-13 10:29 UTC (permalink / raw)
  To: MPTCP Linux; +Cc: Matthieu Baerts (NGI0)

In case of errors, mptcp_lib_pr_nstat is called to dump the nstat
counters, but for some tests, it was dumping the counters for all
subtests, not just the current one.

That's an issue for tests that don't recreate the netns for each
subtest, e.g. mptcp_connect.sh. In this case, 'nstat -a' will look at
the absolute counters since the creation of the netns, making
debugging harder.

Instead, it should dump the counters for the current test, by using the
history recorded in /tmp/<ns>.nstat, if available, and not using '-a'
which was dumping the absolute values instead of calculating increments.

While at it, rename the previous 'hist' variable to 'cache' as it was
used to look at the cache, not the nstat history.

Do the same in mptcp_lib_get_counter, even if it wasn't an issue for
the moment. Still, good to fix it for future usage, and to align it
with the rest.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (2):
      selftests: mptcp: lib: dump nstat for the right test
      selftests: mptcp: lib: get counters for the right test

 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)
---
base-commit: ab76ed72e0bed8352d3b8e2b4d1be7c823e5fba3
change-id: 20260813-sft-mptcp-nstat-hist-33411708430f

Best regards,
--  
Matthieu Baerts (NGI0) <matttbe@kernel.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH mptcp-net 1/2] selftests: mptcp: lib: dump nstat for the right test
  2026-08-13 10:29 [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test Matthieu Baerts (NGI0)
@ 2026-08-13 10:29 ` Matthieu Baerts (NGI0)
  2026-08-13 10:29 ` [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters " Matthieu Baerts (NGI0)
  2026-08-13 11:41 ` [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB " MPTCP CI
  2 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts (NGI0) @ 2026-08-13 10:29 UTC (permalink / raw)
  To: MPTCP Linux; +Cc: Matthieu Baerts (NGI0)

In case of errors, mptcp_lib_pr_nstat is called to dump the nstat
counters, but for some tests, it was dumping the counters for all
subtests, not just the current one.

That's an issue for tests that don't recreate the netns for each
subtest, e.g. mptcp_connect.sh. In this case, 'nstat -a' will look at
the absolute counters since the creation of the netns, making
debugging harder.

Instead, it should dump the counters for the current test, by using the
history recorded in /tmp/<ns>.nstat if available, and not using '-a'
which was dumping the absolute values instead of calculating increments.

While at it, rename the previous 'hist' variable to 'cache' as it was
used to look at the cache, not the nstat history.

Fixes: 658e53141780 ("selftests: mptcp: join: dump stats from history")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 5ef6033775c8..da1da414c30f 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -108,12 +108,14 @@ mptcp_lib_pr_info() {
 
 mptcp_lib_pr_nstat() {
 	local ns="${1}"
-	local hist="/tmp/${ns}.out"
+	local cache="/tmp/${ns}.out"
+	local hist="/tmp/${ns}.nstat"
 
-	if [ -f "${hist}" ]; then
-		awk '$2 != 0 { print "  "$0 }' "${hist}"
+	if [ -f "${cache}" ]; then
+		awk '$2 != 0 { print "  "$0 }' "${cache}"
 	else
-		ip netns exec "${ns}" nstat -as | grep Tcp
+		NSTAT_HISTORY="${hist}" ip netns exec "${ns}" nstat -s |
+			grep Tcp
 	fi
 }
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters for the right test
  2026-08-13 10:29 [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test Matthieu Baerts (NGI0)
  2026-08-13 10:29 ` [PATCH mptcp-net 1/2] selftests: mptcp: lib: dump nstat " Matthieu Baerts (NGI0)
@ 2026-08-13 10:29 ` Matthieu Baerts (NGI0)
  2026-08-19  8:34   ` Geliang Tang
  2026-08-13 11:41 ` [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB " MPTCP CI
  2 siblings, 1 reply; 8+ messages in thread
From: Matthieu Baerts (NGI0) @ 2026-08-13 10:29 UTC (permalink / raw)
  To: MPTCP Linux; +Cc: Matthieu Baerts (NGI0)

When the value for a MIB counter is required, mptcp_lib_get_counter is
called. It tries to use the cache, if available. If not it falls back to
calling 'nstat' directly by looking at the absolute counters.

That's an issue for tests that don't recreate the netns for each
subtest. In this case, 'nstat -a' will look at the counters for the
netns.

Instead, it should look at the increment for the current test, by using
the history recorded in /tmp/<ns>.nstat, if available, and not using
'-a' which was dumping the absolute values.

While at it, rename the previous 'hist' variable to 'cache' as it was
used to look at the cache, not the nstat history.

Fixes: 71388a9f331d ("selftests: mptcp: lib: get counters from nstat history")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index da1da414c30f..b9d14647f401 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -416,19 +416,21 @@ mptcp_lib_nstat_get() {
 }
 
 # $1: ns, $2: MIB counter
-# Get the counter from the history (mptcp_lib_nstat_{init,get}()) if available.
-# If not, get the counter from nstat ignoring any history.
+# Get the counter from the cache (mptcp_lib_nstat_{init,get}()) if available.
+# If not, get the counter from nstat ignoring any cache, but using the history.
 mptcp_lib_get_counter() {
 	local ns="${1}"
 	local counter="${2}"
-	local hist="/tmp/${ns}.out"
+	local cache="/tmp/${ns}.out"
+	local hist="/tmp/${ns}.nstat"
 	local count
 
-	if [[ -s "${hist}" && "${counter}" == *"Tcp"* ]]; then
-		count=$(awk "/^${counter} / {print \$2; exit}" "${hist}")
+	if [[ -s "${cache}" && "${counter}" == *"Tcp"* ]]; then
+		count=$(awk "/^${counter} / {print \$2; exit}" "${cache}")
 	else
-		count=$(ip netns exec "${ns}" nstat -asz "${counter}" |
-			awk 'NR==1 {next} {print $2}')
+		count=$(NSTAT_HISTORY="${hist}" ip netns exec "${ns}" \
+			nstat -sz "${counter}" |
+				awk 'NR==1 {next} {print $2}')
 	fi
 	if [ -z "${count}" ]; then
 		mptcp_lib_fail_if_expected_feature "${counter} counter"

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test
  2026-08-13 10:29 [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test Matthieu Baerts (NGI0)
  2026-08-13 10:29 ` [PATCH mptcp-net 1/2] selftests: mptcp: lib: dump nstat " Matthieu Baerts (NGI0)
  2026-08-13 10:29 ` [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters " Matthieu Baerts (NGI0)
@ 2026-08-13 11:41 ` MPTCP CI
  2026-08-19  9:22   ` Geliang Tang
  2 siblings, 1 reply; 8+ messages in thread
From: MPTCP CI @ 2026-08-13 11:41 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/31693194883

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/1737d20f1a46
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1145356


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters for the right test
  2026-08-13 10:29 ` [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters " Matthieu Baerts (NGI0)
@ 2026-08-19  8:34   ` Geliang Tang
  2026-08-19  9:02     ` Matthieu Baerts
  0 siblings, 1 reply; 8+ messages in thread
From: Geliang Tang @ 2026-08-19  8:34 UTC (permalink / raw)
  To: Matthieu Baerts (NGI0), MPTCP Linux

Hi Matt,

On Thu, 2026-08-13 at 12:29 +0200, Matthieu Baerts (NGI0) wrote:
> When the value for a MIB counter is required, mptcp_lib_get_counter
> is
> called. It tries to use the cache, if available. If not it falls back
> to
> calling 'nstat' directly by looking at the absolute counters.
> 
> That's an issue for tests that don't recreate the netns for each
> subtest. In this case, 'nstat -a' will look at the counters for the
> netns.
> 
> Instead, it should look at the increment for the current test, by
> using
> the history recorded in /tmp/<ns>.nstat, if available, and not using
> '-a' which was dumping the absolute values.
> 
> While at it, rename the previous 'hist' variable to 'cache' as it was
> used to look at the cache, not the nstat history.
> 
> Fixes: 71388a9f331d ("selftests: mptcp: lib: get counters from nstat
> history")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index da1da414c30f..b9d14647f401 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -416,19 +416,21 @@ mptcp_lib_nstat_get() {
>  }
>  
>  # $1: ns, $2: MIB counter
> -# Get the counter from the history (mptcp_lib_nstat_{init,get}()) if
> available.
> -# If not, get the counter from nstat ignoring any history.
> +# Get the counter from the cache (mptcp_lib_nstat_{init,get}()) if
> available.
> +# If not, get the counter from nstat ignoring any cache, but using
> the history.
>  mptcp_lib_get_counter() {
>  	local ns="${1}"
>  	local counter="${2}"
> -	local hist="/tmp/${ns}.out"
> +	local cache="/tmp/${ns}.out"
> +	local hist="/tmp/${ns}.nstat"
>  	local count
>  
> -	if [[ -s "${hist}" && "${counter}" == *"Tcp"* ]]; then
> -		count=$(awk "/^${counter} / {print \$2; exit}"
> "${hist}")
> +	if [[ -s "${cache}" && "${counter}" == *"Tcp"* ]]; then
> +		count=$(awk "/^${counter} / {print \$2; exit}"
> "${cache}")
>  	else
> -		count=$(ip netns exec "${ns}" nstat -asz
> "${counter}" |
> -			awk 'NR==1 {next} {print $2}')
> +		count=$(NSTAT_HISTORY="${hist}" ip netns exec
> "${ns}" \
> +			nstat -sz "${counter}" |
> +				awk 'NR==1 {next} {print $2}')
>  	fi
>  	if [ -z "${count}" ]; then
>  		mptcp_lib_fail_if_expected_feature "${counter}
> counter"

I noticed that the code being modified in mptcp_lib_pr_nstat and
mptcp_lib_get_counter is duplicated. I'm wondering if we could remove
this redundancy - for example, by creating a new helper for this logic.

# $1: ns ; $@: nstat arguments
# If cache exists, return it; otherwise run nstat.
mptcp_lib_nstat_cmd() {
    local ns="${1}"
    local cache="/tmp/${ns}.out"
    local hist="/tmp/${ns}.nstat"

    if [ -s "${cache}" ]; then
        cat "${cache}"
    else
        NSTAT_HISTORY="${hist}" ip netns exec "${ns}" nstat -sz "${@}"
    fi
}

Then mptcp_lib_pr_nstat can be simplified to:

mptcp_lib_pr_nstat() {
    local ns="${1}"

    mptcp_lib_nstat_cmd "${ns}" |
            awk '/Tcp/ { print "  "$0 }'
}

And mptcp_lib_get_counter can be simplified to:

mptcp_lib_get_counter() {
    local ns="${1}"
    local counter="${2}"
    local count

    count=$(mptcp_lib_nstat_cmd "${ns}" "${counter}" |
            awk -v c="${counter}" '$1 == c {print $2; exit}')
    if [ -z "${count}" ]; then
        mptcp_lib_fail_if_expected_feature "${counter} counter"
        return 1
    fi

    echo "${count}"
}

What do you think - would this work?

Thanks,
-Geliang

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters for the right test
  2026-08-19  8:34   ` Geliang Tang
@ 2026-08-19  9:02     ` Matthieu Baerts
  2026-08-19  9:18       ` Geliang Tang
  0 siblings, 1 reply; 8+ messages in thread
From: Matthieu Baerts @ 2026-08-19  9:02 UTC (permalink / raw)
  To: Geliang Tang, MPTCP Linux

Hi Geliang,

On 19/08/2026 10:34, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, 2026-08-13 at 12:29 +0200, Matthieu Baerts (NGI0) wrote:
>> When the value for a MIB counter is required, mptcp_lib_get_counter
>> is
>> called. It tries to use the cache, if available. If not it falls back
>> to
>> calling 'nstat' directly by looking at the absolute counters.
>>
>> That's an issue for tests that don't recreate the netns for each
>> subtest. In this case, 'nstat -a' will look at the counters for the
>> netns.
>>
>> Instead, it should look at the increment for the current test, by
>> using
>> the history recorded in /tmp/<ns>.nstat, if available, and not using
>> '-a' which was dumping the absolute values.
>>
>> While at it, rename the previous 'hist' variable to 'cache' as it was
>> used to look at the cache, not the nstat history.
>>
>> Fixes: 71388a9f331d ("selftests: mptcp: lib: get counters from nstat
>> history")
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_lib.sh | 16 +++++++++-------
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> index da1da414c30f..b9d14647f401 100644
>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> @@ -416,19 +416,21 @@ mptcp_lib_nstat_get() {
>>  }
>>  
>>  # $1: ns, $2: MIB counter
>> -# Get the counter from the history (mptcp_lib_nstat_{init,get}()) if
>> available.
>> -# If not, get the counter from nstat ignoring any history.
>> +# Get the counter from the cache (mptcp_lib_nstat_{init,get}()) if
>> available.
>> +# If not, get the counter from nstat ignoring any cache, but using
>> the history.
>>  mptcp_lib_get_counter() {
>>  	local ns="${1}"
>>  	local counter="${2}"
>> -	local hist="/tmp/${ns}.out"
>> +	local cache="/tmp/${ns}.out"
>> +	local hist="/tmp/${ns}.nstat"
>>  	local count
>>  
>> -	if [[ -s "${hist}" && "${counter}" == *"Tcp"* ]]; then
>> -		count=$(awk "/^${counter} / {print \$2; exit}"
>> "${hist}")
>> +	if [[ -s "${cache}" && "${counter}" == *"Tcp"* ]]; then
>> +		count=$(awk "/^${counter} / {print \$2; exit}"
>> "${cache}")
>>  	else
>> -		count=$(ip netns exec "${ns}" nstat -asz
>> "${counter}" |
>> -			awk 'NR==1 {next} {print $2}')
>> +		count=$(NSTAT_HISTORY="${hist}" ip netns exec
>> "${ns}" \
>> +			nstat -sz "${counter}" |
>> +				awk 'NR==1 {next} {print $2}')
>>  	fi
>>  	if [ -z "${count}" ]; then
>>  		mptcp_lib_fail_if_expected_feature "${counter}
>> counter"
> 
> I noticed that the code being modified in mptcp_lib_pr_nstat and
> mptcp_lib_get_counter is duplicated. I'm wondering if we could remove
> this redundancy - for example, by creating a new helper for this logic.

Thank you for the review and suggestion.

> # $1: ns ; $@: nstat arguments
> # If cache exists, return it; otherwise run nstat.
> mptcp_lib_nstat_cmd() {
>     local ns="${1}"
>     local cache="/tmp/${ns}.out"
>     local hist="/tmp/${ns}.nstat"
> 
>     if [ -s "${cache}" ]; then
>         cat "${cache}"
>     else
>         NSTAT_HISTORY="${hist}" ip netns exec "${ns}" nstat -sz "${@}"
>     fi
> }
> 
> Then mptcp_lib_pr_nstat can be simplified to:
> 
> mptcp_lib_pr_nstat() {
>     local ns="${1}"
> 
>     mptcp_lib_nstat_cmd "${ns}" |
>             awk '/Tcp/ { print "  "$0 }'
> }
> 
> And mptcp_lib_get_counter can be simplified to:
> 
> mptcp_lib_get_counter() {
>     local ns="${1}"
>     local counter="${2}"
>     local count
> 
>     count=$(mptcp_lib_nstat_cmd "${ns}" "${counter}" |
>             awk -v c="${counter}" '$1 == c {print $2; exit}')

The cache cannot be used to check non-Tcp counters because it only
contains *Tcp* counters. Then that makes this code adding extra
conditions, and I'm not sure the new helper is worth it. WDYT?

Maybe it could be added if other modifications are needed around these
helpers, but here, I wouldn't do that in a series targeting -net.

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters for the right test
  2026-08-19  9:02     ` Matthieu Baerts
@ 2026-08-19  9:18       ` Geliang Tang
  0 siblings, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2026-08-19  9:18 UTC (permalink / raw)
  To: Matthieu Baerts, MPTCP Linux

On Wed, 2026-08-19 at 11:02 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 19/08/2026 10:34, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Thu, 2026-08-13 at 12:29 +0200, Matthieu Baerts (NGI0) wrote:
> > > When the value for a MIB counter is required,
> > > mptcp_lib_get_counter
> > > is
> > > called. It tries to use the cache, if available. If not it falls
> > > back
> > > to
> > > calling 'nstat' directly by looking at the absolute counters.
> > > 
> > > That's an issue for tests that don't recreate the netns for each
> > > subtest. In this case, 'nstat -a' will look at the counters for
> > > the
> > > netns.
> > > 
> > > Instead, it should look at the increment for the current test, by
> > > using
> > > the history recorded in /tmp/<ns>.nstat, if available, and not
> > > using
> > > '-a' which was dumping the absolute values.
> > > 
> > > While at it, rename the previous 'hist' variable to 'cache' as it
> > > was
> > > used to look at the cache, not the nstat history.
> > > 
> > > Fixes: 71388a9f331d ("selftests: mptcp: lib: get counters from
> > > nstat
> > > history")
> > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > > ---
> > >  tools/testing/selftests/net/mptcp/mptcp_lib.sh | 16 +++++++++---
> > > ----
> > >  1 file changed, 9 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > index da1da414c30f..b9d14647f401 100644
> > > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > @@ -416,19 +416,21 @@ mptcp_lib_nstat_get() {
> > >  }
> > >  
> > >  # $1: ns, $2: MIB counter
> > > -# Get the counter from the history
> > > (mptcp_lib_nstat_{init,get}()) if
> > > available.
> > > -# If not, get the counter from nstat ignoring any history.
> > > +# Get the counter from the cache (mptcp_lib_nstat_{init,get}())
> > > if
> > > available.
> > > +# If not, get the counter from nstat ignoring any cache, but
> > > using
> > > the history.
> > >  mptcp_lib_get_counter() {
> > >  	local ns="${1}"
> > >  	local counter="${2}"
> > > -	local hist="/tmp/${ns}.out"
> > > +	local cache="/tmp/${ns}.out"
> > > +	local hist="/tmp/${ns}.nstat"
> > >  	local count
> > >  
> > > -	if [[ -s "${hist}" && "${counter}" == *"Tcp"* ]]; then
> > > -		count=$(awk "/^${counter} / {print \$2; exit}"
> > > "${hist}")
> > > +	if [[ -s "${cache}" && "${counter}" == *"Tcp"* ]]; then
> > > +		count=$(awk "/^${counter} / {print \$2; exit}"
> > > "${cache}")
> > >  	else
> > > -		count=$(ip netns exec "${ns}" nstat -asz
> > > "${counter}" |
> > > -			awk 'NR==1 {next} {print $2}')
> > > +		count=$(NSTAT_HISTORY="${hist}" ip netns exec
> > > "${ns}" \
> > > +			nstat -sz "${counter}" |
> > > +				awk 'NR==1 {next} {print $2}')
> > >  	fi
> > >  	if [ -z "${count}" ]; then
> > >  		mptcp_lib_fail_if_expected_feature "${counter}
> > > counter"
> > 
> > I noticed that the code being modified in mptcp_lib_pr_nstat and
> > mptcp_lib_get_counter is duplicated. I'm wondering if we could
> > remove
> > this redundancy - for example, by creating a new helper for this
> > logic.
> 
> Thank you for the review and suggestion.
> 
> > # $1: ns ; $@: nstat arguments
> > # If cache exists, return it; otherwise run nstat.
> > mptcp_lib_nstat_cmd() {
> >     local ns="${1}"
> >     local cache="/tmp/${ns}.out"
> >     local hist="/tmp/${ns}.nstat"
> > 
> >     if [ -s "${cache}" ]; then
> >         cat "${cache}"
> >     else
> >         NSTAT_HISTORY="${hist}" ip netns exec "${ns}" nstat -sz
> > "${@}"
> >     fi
> > }
> > 
> > Then mptcp_lib_pr_nstat can be simplified to:
> > 
> > mptcp_lib_pr_nstat() {
> >     local ns="${1}"
> > 
> >     mptcp_lib_nstat_cmd "${ns}" |
> >             awk '/Tcp/ { print "  "$0 }'
> > }
> > 
> > And mptcp_lib_get_counter can be simplified to:
> > 
> > mptcp_lib_get_counter() {
> >     local ns="${1}"
> >     local counter="${2}"
> >     local count
> > 
> >     count=$(mptcp_lib_nstat_cmd "${ns}" "${counter}" |
> >             awk -v c="${counter}" '$1 == c {print $2; exit}')
> 
> The cache cannot be used to check non-Tcp counters because it only
> contains *Tcp* counters. Then that makes this code adding extra
> conditions, and I'm not sure the new helper is worth it. WDYT?
> 
> Maybe it could be added if other modifications are needed around
> these
> helpers, but here, I wouldn't do that in a series targeting -net.

Sure. I can also send a follow-up patch later. I have no other comments
on this series - I'll reply to the cover letter with my Reviewed-by tag
shortly.

> 
> Cheers,
> Matt

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test
  2026-08-13 11:41 ` [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB " MPTCP CI
@ 2026-08-19  9:22   ` Geliang Tang
  0 siblings, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2026-08-19  9:22 UTC (permalink / raw)
  To: mptcp, Matthieu Baerts

Hi Matt,

On Thu, 2026-08-13 at 11:41 +0000, MPTCP CI wrote:
> Hi Matthieu,
> 
> Thank you for your modifications, that's great!
> 
> Our CI did some validations and here is its report:
> 
> - KVM Validation: normal (except selftest_mptcp_join): Success! ✅
> - KVM Validation: normal (only selftest_mptcp_join): Success! ✅
> - KVM Validation: debug (except selftest_mptcp_join): Success! ✅
> - KVM Validation: debug (only selftest_mptcp_join): Success! ✅
> - KVM Validation: btf-normal (only bpftest_all): Success! ✅
> - KVM Validation: btf-debug (only bpftest_all): Success! ✅

This series looks good to me!

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> - Task:
> https://github.com/multipath-tcp/mptcp_net-next/actions/runs/31693194883
> 
> Initiator: Patchew Applier
> Commits:
> https://github.com/multipath-tcp/mptcp_net-next/commits/1737d20f1a46
> Patchwork:
> https://patchwork.kernel.org/project/mptcp/list/?series=1145356
> 
> 
> If there are some issues, you can reproduce them using the same
> environment as
> the one used by the CI thanks to a docker image, e.g.:
> 
>     $ cd [kernel source code]
>     $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm
> -it \
>         --pull always mptcp/mptcp-upstream-virtme-docker:latest \
>         auto-normal
> 
> For more details:
> 
>     https://github.com/multipath-tcp/mptcp-upstream-virtme-docker
> 
> 
> Please note that despite all the efforts that have been already done
> to have a
> stable tests suite when executed on a public CI like here, it is
> possible some
> reported issues are not due to your modifications. Still, do not
> hesitate to
> help us improve that ;-)
> 
> Cheers,
> MPTCP GH Action bot
> Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-19  9:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 10:29 [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB counters for the right test Matthieu Baerts (NGI0)
2026-08-13 10:29 ` [PATCH mptcp-net 1/2] selftests: mptcp: lib: dump nstat " Matthieu Baerts (NGI0)
2026-08-13 10:29 ` [PATCH mptcp-net 2/2] selftests: mptcp: lib: get counters " Matthieu Baerts (NGI0)
2026-08-19  8:34   ` Geliang Tang
2026-08-19  9:02     ` Matthieu Baerts
2026-08-19  9:18       ` Geliang Tang
2026-08-13 11:41 ` [PATCH mptcp-net 0/2] selftests: mptcp: lib: get MIB " MPTCP CI
2026-08-19  9:22   ` Geliang Tang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.