MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP
@ 2026-05-12 10:33 Shardul Bankar
  2026-05-12 10:33 ` [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets Shardul Bankar
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Shardul Bankar @ 2026-05-12 10:33 UTC (permalink / raw)
  To: mptcp
  Cc: matttbe, martineau, geliang, pabeni, janak, kalpan.jani,
	shardulsb08, Shardul Bankar

MPTCP_RST_EMPTCP (reset reason 1) is currently used as a catch-all for
several distinct conditions across subflow setup, authentication, and
data-path validation.  This series adds per-event MIB counters at each
of those use sites and extends mptcp_join.sh's chk_rst_nr() to validate
them.

Patch 1 adds six new counters (MD5SigReset, MPJoinSynAckNoMPJoin,
MPJoinAckNoMPJoin, MPJoinAckNoCtx, DssReset, MPJoinNotEstablished) and
increments them in mib.c/mib.h, subflow.c and protocol.c.

Patch 2 extends chk_rst_nr() with named env-var expectations for each
new counter (silent on success, gated on availability) and adds one
test inside signal_address_tests that triggers MPJoinSynAckNoMPJoin
from within the existing ns1/ns2 setup.

v3 (this version):
  patch 1: move new SNMP_MIB_ITEM entries and enum values next to
           their semantic neighbors instead of appending them in a
           single block after MPRst*.  No functional change.
  patch 2: drop the third-netns approach for triggering
           MPJoinSynAckNoMPJoin.  ns1 now signals an address that
           is already bound on ns2 (10.0.2.2) with a TCP-only
           mptcp_connect listener on ns2; the client's MP_JOIN
           routes locally to that listener and the SYN/ACK arrives
           without MP_JOIN.  The test moves into
           signal_address_tests.  speed=slow dropped (not needed
           here), and the local-and-assign that tripped shellcheck
           SC2155 is gone with the old test.
v2: https://lore.kernel.org/all/20260509183335.969018-1-shardul.b@mpiricsoftware.com/
v1: https://lore.kernel.org/all/20260421095646.3741956-1-shardul.b@mpiricsoftware.com/

Shardul Bankar (2):
  mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets
  selftests: mptcp: extend chk_rst_nr to validate per-event RST counters

 net/mptcp/mib.c                               |  6 ++
 net/mptcp/mib.h                               |  6 ++
 net/mptcp/protocol.c                          |  1 +
 net/mptcp/subflow.c                           |  9 +++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++++
 5 files changed, 94 insertions(+)


base-commit: 52f68f13d42a1c3752de7a776b6f873988f48401
-- 
2.34.1


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

* [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets
  2026-05-12 10:33 [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP Shardul Bankar
@ 2026-05-12 10:33 ` Shardul Bankar
  2026-05-27  6:50   ` Matthieu Baerts
  2026-05-12 10:33 ` [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters Shardul Bankar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Shardul Bankar @ 2026-05-12 10:33 UTC (permalink / raw)
  To: mptcp
  Cc: matttbe, martineau, geliang, pabeni, janak, kalpan.jani,
	shardulsb08, Shardul Bankar

MPTCP_RST_EMPTCP (reset reason 1) is used as a catch-all for several
distinct error conditions across subflow setup, authentication, and
data-path validation.  The existing MPRstTx/MPRstRx counters only
track aggregate reset volume, making it difficult to diagnose which
code path is triggering subflow resets in production.

Add per-event MIB counters covering each MPTCP_RST_EMPTCP use site
that is not already covered by an existing counter, named after the
underlying event or condition rather than the reset action:

  MD5SigReset           MD5SIG enabled on listener (incompatible)
  MPJoinSynAckNoMPJoin  SYN/ACK missing MP_JOIN option
  MPJoinAckNoMPJoin     server-side ACK missing MP_JOIN option
                          (fallback path, MPJoin required)
  MPJoinAckNoCtx        server-side ACK with no subflow context
  DssReset              data mapping invalid (also fires on
                          MAPPING_NODSS / EMIDDLEBOX path)
  MPJoinNotEstablished  JOIN attempted on a not-fully-established msk

The aggregate MPRstTx/MPRstRx counters are unchanged.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/511
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
v3:
  - Move new SNMP_MIB_ITEM entries and enum values next to their
    semantic neighbors (e.g. MD5SigReset after MD5SigFallback,
    MPJoinSynAckNoMPJoin after the MPJoinSynAck* group) instead of
    appending them in a single block after MPRst*.  No functional
    change.
v2: https://lore.kernel.org/all/20260509183335.969018-1-shardul.b@mpiricsoftware.com/
v1: https://lore.kernel.org/all/20260421095646.3741956-1-shardul.b@mpiricsoftware.com/

 net/mptcp/mib.c      | 6 ++++++
 net/mptcp/mib.h      | 6 ++++++
 net/mptcp/protocol.c | 1 +
 net/mptcp/subflow.c  | 9 +++++++++
 4 files changed, 22 insertions(+)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index f23fda0c55a72..ca1da14d30b08 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -26,9 +26,13 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("MPJoinSynAckRx", MPTCP_MIB_JOINSYNACKRX),
 	SNMP_MIB_ITEM("MPJoinSynAckBackupRx", MPTCP_MIB_JOINSYNACKBACKUPRX),
 	SNMP_MIB_ITEM("MPJoinSynAckHMacFailure", MPTCP_MIB_JOINSYNACKMAC),
+	SNMP_MIB_ITEM("MPJoinSynAckNoMPJoin", MPTCP_MIB_MPJOINSYNACKNOMPJOIN),
 	SNMP_MIB_ITEM("MPJoinAckRx", MPTCP_MIB_JOINACKRX),
 	SNMP_MIB_ITEM("MPJoinAckHMacFailure", MPTCP_MIB_JOINACKMAC),
+	SNMP_MIB_ITEM("MPJoinAckNoMPJoin", MPTCP_MIB_MPJOINACKNOMPJOIN),
+	SNMP_MIB_ITEM("MPJoinAckNoCtx", MPTCP_MIB_MPJOINACKNOCTX),
 	SNMP_MIB_ITEM("MPJoinRejected", MPTCP_MIB_JOINREJECTED),
+	SNMP_MIB_ITEM("MPJoinNotEstablished", MPTCP_MIB_MPJOINNOTESTABLISHED),
 	SNMP_MIB_ITEM("MPJoinSynTx", MPTCP_MIB_JOINSYNTX),
 	SNMP_MIB_ITEM("MPJoinSynTxCreatSkErr", MPTCP_MIB_JOINSYNTXCREATSKERR),
 	SNMP_MIB_ITEM("MPJoinSynTxBindErr", MPTCP_MIB_JOINSYNTXBINDERR),
@@ -81,7 +85,9 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("Blackhole", MPTCP_MIB_BLACKHOLE),
 	SNMP_MIB_ITEM("MPCapableDataFallback", MPTCP_MIB_MPCAPABLEDATAFALLBACK),
 	SNMP_MIB_ITEM("MD5SigFallback", MPTCP_MIB_MD5SIGFALLBACK),
+	SNMP_MIB_ITEM("MD5SigReset", MPTCP_MIB_MD5SIGRESET),
 	SNMP_MIB_ITEM("DssFallback", MPTCP_MIB_DSSFALLBACK),
+	SNMP_MIB_ITEM("DssReset", MPTCP_MIB_DSSRESET),
 	SNMP_MIB_ITEM("SimultConnectFallback", MPTCP_MIB_SIMULTCONNFALLBACK),
 	SNMP_MIB_ITEM("FallbackFailed", MPTCP_MIB_FALLBACKFAILED),
 	SNMP_MIB_ITEM("WinProbe", MPTCP_MIB_WINPROBE),
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 812218b5ed2bf..27fc53367d18f 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -21,9 +21,13 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_JOINSYNACKRX,		/* Received a SYN/ACK + MP_JOIN */
 	MPTCP_MIB_JOINSYNACKBACKUPRX,	/* Received a SYN/ACK + MP_JOIN + backup flag */
 	MPTCP_MIB_JOINSYNACKMAC,	/* HMAC was wrong on SYN/ACK + MP_JOIN */
+	MPTCP_MIB_MPJOINSYNACKNOMPJOIN,	/* MP_RST: missing MP_JOIN in SYN/ACK */
 	MPTCP_MIB_JOINACKRX,		/* Received an ACK + MP_JOIN */
 	MPTCP_MIB_JOINACKMAC,		/* HMAC was wrong on ACK + MP_JOIN */
+	MPTCP_MIB_MPJOINACKNOMPJOIN,	/* MP_RST: missing MP_JOIN in ACK */
+	MPTCP_MIB_MPJOINACKNOCTX,	/* MP_RST: no subflow context on ACK */
 	MPTCP_MIB_JOINREJECTED,		/* The PM rejected the JOIN request */
+	MPTCP_MIB_MPJOINNOTESTABLISHED,	/* MP_RST: JOIN on not-fully-established msk */
 	MPTCP_MIB_JOINSYNTX,		/* Sending a SYN + MP_JOIN */
 	MPTCP_MIB_JOINSYNTXCREATSKERR,	/* Not able to create a socket when sending a SYN + MP_JOIN */
 	MPTCP_MIB_JOINSYNTXBINDERR,	/* Not able to bind() the address when sending a SYN + MP_JOIN */
@@ -84,7 +88,9 @@ enum linux_mptcp_mib_field {
 					 * established packet
 					 */
 	MPTCP_MIB_MD5SIGFALLBACK,	/* Conflicting TCP option enabled */
+	MPTCP_MIB_MD5SIGRESET,		/* MP_RST: MD5SIG enabled on listener */
 	MPTCP_MIB_DSSFALLBACK,		/* Bad or missing DSS */
+	MPTCP_MIB_DSSRESET,		/* MP_RST: bad data mapping */
 	MPTCP_MIB_SIMULTCONNFALLBACK,	/* Simultaneous connect */
 	MPTCP_MIB_FALLBACKFAILED,	/* Can't fallback due to msk status */
 	MPTCP_MIB_WINPROBE,		/* MPTCP-level zero window probe */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ce8372fb3c6af..3b5041fb2588a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3851,6 +3851,7 @@ bool mptcp_finish_join(struct sock *ssk)
 
 	/* mptcp socket already closing? */
 	if (!mptcp_is_fully_established(parent)) {
+		MPTCP_INC_STATS(sock_net(parent), MPTCP_MIB_MPJOINNOTESTABLISHED);
 		subflow->reset_reason = MPTCP_RST_EMPTCP;
 		return false;
 	}
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index d562e149606f6..6fce3f431667c 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -160,6 +160,7 @@ static int subflow_check_req(struct request_sock *req,
 	 * TCP option space.
 	 */
 	if (rcu_access_pointer(tcp_sk(sk_listener)->md5sig_info)) {
+		MPTCP_INC_STATS(sock_net(sk_listener), MPTCP_MIB_MD5SIGRESET);
 		subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP);
 		return -EINVAL;
 	}
@@ -568,6 +569,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 		u8 hmac[SHA256_DIGEST_SIZE];
 
 		if (!(mp_opt.suboptions & OPTION_MPTCP_MPJ_SYNACK)) {
+			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPJOINSYNACKNOMPJOIN);
 			subflow->reset_reason = MPTCP_RST_EMPTCP;
 			goto do_reset;
 		}
@@ -870,6 +872,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 		 */
 		if (!ctx || fallback) {
 			if (fallback_is_fatal) {
+				if (!ctx)
+					MPTCP_INC_STATS(sock_net(sk),
+							MPTCP_MIB_MPJOINACKNOCTX);
+				else
+					MPTCP_INC_STATS(sock_net(sk),
+							MPTCP_MIB_MPJOINACKNOMPJOIN);
 				subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP);
 				goto dispose_child;
 			}
@@ -1421,6 +1429,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
 			 * subflow_error_report() will introduce the appropriate barriers
 			 */
 			subflow->reset_transient = 0;
+			MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSRESET);
 			subflow->reset_reason = status == MAPPING_NODSS ?
 						MPTCP_RST_EMIDDLEBOX :
 						MPTCP_RST_EMPTCP;
-- 
2.34.1


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

* [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters
  2026-05-12 10:33 [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP Shardul Bankar
  2026-05-12 10:33 ` [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets Shardul Bankar
@ 2026-05-12 10:33 ` Shardul Bankar
  2026-05-27  6:24   ` Matthieu Baerts
  2026-05-12 11:47 ` [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP MPTCP CI
  2026-05-27  7:43 ` Matthieu Baerts
  3 siblings, 1 reply; 7+ messages in thread
From: Shardul Bankar @ 2026-05-12 10:33 UTC (permalink / raw)
  To: mptcp
  Cc: matttbe, martineau, geliang, pabeni, janak, kalpan.jani,
	shardulsb08, Shardul Bankar

Extend chk_rst_nr() with named env-var expectations for each
per-event MPTCP_RST_EMPTCP counter, matching the pattern used by
chk_join_nr().  Each counter defaults to 0 and is checked silently
on success; mismatches print a check line and fail the test.
Counters absent from the running kernel are skipped silently so
older kernels do not false-fail.

Add a test at the end of signal_address_tests that triggers
MPJoinSynAckNoMPJoin: ns1 signals an address that is already bound
on the client (ns2), where a TCP-only mptcp_connect listener is
started.  The client's MP_JOIN routes locally to the TCP listener,
which responds with a plain SYN/ACK without the MP_JOIN option, and
the new counter increments on the client side.

Other per-event counters (MD5SigReset, MPJoinAckNoMPJoin,
MPJoinAckNoCtx, DssReset, MPJoinNotEstablished) are not currently
reachable from mptcp_join.sh; the env-var hooks are in place for
future tests to set expectations explicitly.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
v3:
  - Drop the third-netns approach for triggering
    MPJoinSynAckNoMPJoin.  Use ns1 signalling an address that is
    already bound on ns2 (10.0.2.2) with a TCP-only mptcp_connect
    listener on ns2; the client's MP_JOIN routes locally to that
    listener and the SYN/ACK arrives without MP_JOIN.
  - Move the test into signal_address_tests; drop the standalone
    rst_emptcp_tests group and its sorted-list entry.
  - Drop speed=slow (not needed: only one ADD_ADDR, no in-flight
    actions).
  - The local-and-assign that tripped shellcheck SC2155 is gone
    with the old test.
v2: https://lore.kernel.org/all/20260509183335.969018-1-shardul.b@mpiricsoftware.com/

 .../testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 5d4d0f127f795..c6ef7f785a558 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -76,6 +76,13 @@ unset join_create_err
 unset join_bind_err
 unset join_connect_err
 
+unset rst_md5sig_reset
+unset rst_synack_no_mpjoin
+unset rst_ack_no_mpjoin
+unset rst_ack_no_ctx
+unset rst_dss_reset
+unset rst_not_established
+
 unset fb_ns1
 unset fb_ns2
 unset fb_infinite_map_tx
@@ -1349,6 +1356,12 @@ chk_rst_nr()
 	local rst_tx=$1
 	local rst_rx=$2
 	local ns_invert=${3:-""}
+	local md5sig_reset=${rst_md5sig_reset:-0}
+	local synack_no_mpjoin=${rst_synack_no_mpjoin:-0}
+	local ack_no_mpjoin=${rst_ack_no_mpjoin:-0}
+	local ack_no_ctx=${rst_ack_no_ctx:-0}
+	local dss_reset=${rst_dss_reset:-0}
+	local not_established=${rst_not_established:-0}
 	local count
 	local ns_tx=$ns1
 	local ns_rx=$ns2
@@ -1385,6 +1398,43 @@ chk_rst_nr()
 	else
 		print_ok
 	fi
+
+	# Per-event MPTCP_RST_EMPTCP counters; default 0, gated on availability.
+	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMD5SigReset")
+	if [ -n "$count" ] && [ "$count" != "$md5sig_reset" ]; then
+		print_check "MD5SigReset ${tx}"
+		fail_test "got $count MD5SigReset expected $md5sig_reset"
+	fi
+
+	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPJoinSynAckNoMPJoin")
+	if [ -n "$count" ] && [ "$count" != "$synack_no_mpjoin" ]; then
+		print_check "MPJoinSynAckNoMPJoin ${rx}"
+		fail_test "got $count MPJoinSynAckNoMPJoin expected $synack_no_mpjoin"
+	fi
+
+	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPJoinAckNoMPJoin")
+	if [ -n "$count" ] && [ "$count" != "$ack_no_mpjoin" ]; then
+		print_check "MPJoinAckNoMPJoin ${tx}"
+		fail_test "got $count MPJoinAckNoMPJoin expected $ack_no_mpjoin"
+	fi
+
+	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPJoinAckNoCtx")
+	if [ -n "$count" ] && [ "$count" != "$ack_no_ctx" ]; then
+		print_check "MPJoinAckNoCtx ${tx}"
+		fail_test "got $count MPJoinAckNoCtx expected $ack_no_ctx"
+	fi
+
+	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtDssReset")
+	if [ -n "$count" ] && [ "$count" != "$dss_reset" ]; then
+		print_check "DssReset ${rx}"
+		fail_test "got $count DssReset expected $dss_reset"
+	fi
+
+	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPJoinNotEstablished")
+	if [ -n "$count" ] && [ "$count" != "$not_established" ]; then
+		print_check "MPJoinNotEstablished ${rx}"
+		fail_test "got $count MPJoinNotEstablished expected $not_established"
+	fi
 }
 
 chk_infi_nr()
@@ -2378,6 +2428,28 @@ signal_address_tests()
 			chk_add_nr 4 4
 		fi
 	fi
+
+	# signalled address belongs to the client, where a TCP-only
+	# listener is bound at it: the client's MP_JOIN routes locally
+	# to the listener and receives a SYN/ACK without MP_JOIN.
+	# MPJoinSynAckNoMPJoin increments on the client side.
+	if reset "signal address, TCP-only listener on client"; then
+		local extra_bind
+
+		pm_nl_set_limits $ns1 0 1
+		pm_nl_set_limits $ns2 1 1
+		pm_nl_add_endpoint $ns1 10.0.2.2 flags signal
+
+		ip netns exec ${ns2} ./mptcp_connect -l -t -1 -p "$(get_port)" \
+			-s TCP 10.0.2.2 &
+		extra_bind=$!
+
+		run_tests $ns1 $ns2 10.0.1.1
+		rst_synack_no_mpjoin=1 \
+			chk_rst_nr 0 0
+
+		kill ${extra_bind} 2>/dev/null
+	fi
 }
 
 laminar_endp_tests()
-- 
2.34.1


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

* Re: [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP
  2026-05-12 10:33 [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP Shardul Bankar
  2026-05-12 10:33 ` [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets Shardul Bankar
  2026-05-12 10:33 ` [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters Shardul Bankar
@ 2026-05-12 11:47 ` MPTCP CI
  2026-05-27  7:43 ` Matthieu Baerts
  3 siblings, 0 replies; 7+ messages in thread
From: MPTCP CI @ 2026-05-12 11:47 UTC (permalink / raw)
  To: Shardul Bankar; +Cc: mptcp

Hi Shardul,

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): Unstable: 1 failed test(s): bpftest_test_progs_mptcp ⚠️ 
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/25729977608

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


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] 7+ messages in thread

* Re: [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters
  2026-05-12 10:33 ` [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters Shardul Bankar
@ 2026-05-27  6:24   ` Matthieu Baerts
  0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-05-27  6:24 UTC (permalink / raw)
  To: Shardul Bankar, mptcp
  Cc: martineau, geliang, pabeni, janak, kalpan.jani, Shardul Bankar

Hi Shardul,

On 12/05/2026 20:33, Shardul Bankar wrote:
> Extend chk_rst_nr() with named env-var expectations for each
> per-event MPTCP_RST_EMPTCP counter, matching the pattern used by
> chk_join_nr().  Each counter defaults to 0 and is checked silently
> on success; mismatches print a check line and fail the test.
> Counters absent from the running kernel are skipped silently so
> older kernels do not false-fail.
> 
> Add a test at the end of signal_address_tests that triggers
> MPJoinSynAckNoMPJoin: ns1 signals an address that is already bound
> on the client (ns2), where a TCP-only mptcp_connect listener is
> started.  The client's MP_JOIN routes locally to the TCP listener,
> which responds with a plain SYN/ACK without the MP_JOIN option, and
> the new counter increments on the client side.
> 
> Other per-event counters (MD5SigReset, MPJoinAckNoMPJoin,
> MPJoinAckNoCtx, DssReset, MPJoinNotEstablished) are not currently
> reachable from mptcp_join.sh; the env-var hooks are in place for
> future tests to set expectations explicitly.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
> v3:
>   - Drop the third-netns approach for triggering
>     MPJoinSynAckNoMPJoin.  Use ns1 signalling an address that is
>     already bound on ns2 (10.0.2.2) with a TCP-only mptcp_connect
>     listener on ns2; the client's MP_JOIN routes locally to that
>     listener and the SYN/ACK arrives without MP_JOIN.
>   - Move the test into signal_address_tests; drop the standalone
>     rst_emptcp_tests group and its sorted-list entry.
>   - Drop speed=slow (not needed: only one ADD_ADDR, no in-flight
>     actions).
>   - The local-and-assign that tripped shellcheck SC2155 is gone
>     with the old test.
> v2: https://lore.kernel.org/all/20260509183335.969018-1-shardul.b@mpiricsoftware.com/
> 
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 5d4d0f127f795..c6ef7f785a558 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -76,6 +76,13 @@ unset join_create_err
>  unset join_bind_err
>  unset join_connect_err
>  
> +unset rst_md5sig_reset
> +unset rst_synack_no_mpjoin
> +unset rst_ack_no_mpjoin
> +unset rst_ack_no_ctx
> +unset rst_dss_reset
> +unset rst_not_established

See below: I think you can use the 'join_' prefix and attach them above,
except for md5sig and dss that could also simply be called rst_dss and
rst_md5sig.

> +
>  unset fb_ns1
>  unset fb_ns2
>  unset fb_infinite_map_tx
> @@ -1349,6 +1356,12 @@ chk_rst_nr()
>  	local rst_tx=$1
>  	local rst_rx=$2
>  	local ns_invert=${3:-""}
> +	local md5sig_reset=${rst_md5sig_reset:-0}
> +	local synack_no_mpjoin=${rst_synack_no_mpjoin:-0}
> +	local ack_no_mpjoin=${rst_ack_no_mpjoin:-0}
> +	local ack_no_ctx=${rst_ack_no_ctx:-0}
> +	local dss_reset=${rst_dss_reset:-0}
> +	local not_established=${rst_not_established:-0}
>  	local count
>  	local ns_tx=$ns1
>  	local ns_rx=$ns2
> @@ -1385,6 +1398,43 @@ chk_rst_nr()
>  	else
>  		print_ok
>  	fi
> +
> +	# Per-event MPTCP_RST_EMPTCP counters; default 0, gated on availability.
> +	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMD5SigReset")
> +	if [ -n "$count" ] && [ "$count" != "$md5sig_reset" ]; then
> +		print_check "MD5SigReset ${tx}"
> +		fail_test "got $count MD5SigReset expected $md5sig_reset"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPJoinSynAckNoMPJoin")

Here, the netns should be fixed: only the client should get it, and
using "chk_rst_nr ... invert" will not check at the right netns.

In fact, I think this counter and other MPJoin counters here below
should probably be added to the chk_join_nr() helper, no? Leaving here
only the MD5Sig and DSS related one. WDYT?

> +	if [ -n "$count" ] && [ "$count" != "$synack_no_mpjoin" ]; then
> +		print_check "MPJoinSynAckNoMPJoin ${rx}"
> +		fail_test "got $count MPJoinSynAckNoMPJoin expected $synack_no_mpjoin"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPJoinAckNoMPJoin")
> +	if [ -n "$count" ] && [ "$count" != "$ack_no_mpjoin" ]; then
> +		print_check "MPJoinAckNoMPJoin ${tx}"
> +		fail_test "got $count MPJoinAckNoMPJoin expected $ack_no_mpjoin"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPJoinAckNoCtx")
> +	if [ -n "$count" ] && [ "$count" != "$ack_no_ctx" ]; then
> +		print_check "MPJoinAckNoCtx ${tx}"
> +		fail_test "got $count MPJoinAckNoCtx expected $ack_no_ctx"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtDssReset")
> +	if [ -n "$count" ] && [ "$count" != "$dss_reset" ]; then
> +		print_check "DssReset ${rx}"
> +		fail_test "got $count DssReset expected $dss_reset"
> +	fi
> +
> +	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPJoinNotEstablished")
> +	if [ -n "$count" ] && [ "$count" != "$not_established" ]; then
> +		print_check "MPJoinNotEstablished ${rx}"
> +		fail_test "got $count MPJoinNotEstablished expected $not_established"
> +	fi
>  }
>  
>  chk_infi_nr()
> @@ -2378,6 +2428,28 @@ signal_address_tests()
>  			chk_add_nr 4 4
>  		fi
>  	fi
> +
> +	# signalled address belongs to the client, where a TCP-only
> +	# listener is bound at it: the client's MP_JOIN routes locally
> +	# to the listener and receives a SYN/ACK without MP_JOIN.
> +	# MPJoinSynAckNoMPJoin increments on the client side.
> +	if reset "signal address, TCP-only listener on client"; then
> +		local extra_bind
> +
> +		pm_nl_set_limits $ns1 0 1
> +		pm_nl_set_limits $ns2 1 1
> +		pm_nl_add_endpoint $ns1 10.0.2.2 flags signal
> +
> +		ip netns exec ${ns2} ./mptcp_connect -l -t -1 -p "$(get_port)" \
> +			-s TCP 10.0.2.2 &
> +		extra_bind=$!

It would be better to call mptcp_lib_wait_local_port_listen(), just to
be sure it is listening. (Even if 'run_tests' will also call it on the
other netns)

On the other end, I see we didn't call it in similar other cases.

> +		run_tests $ns1 $ns2 10.0.1.1

See above: so here, call chk_join_nr() after, like in most tests, but
here check with join_synack_no_mpjoin=1.

> +		rst_synack_no_mpjoin=1 \
> +			chk_rst_nr 0 0
> +
> +		kill ${extra_bind} 2>/dev/null
> +	fi
>  }
>  
>  laminar_endp_tests()

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


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

* Re: [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets
  2026-05-12 10:33 ` [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets Shardul Bankar
@ 2026-05-27  6:50   ` Matthieu Baerts
  0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-05-27  6:50 UTC (permalink / raw)
  To: Shardul Bankar, mptcp
  Cc: martineau, geliang, pabeni, janak, kalpan.jani, Shardul Bankar

Hi Shardul,

On 12/05/2026 20:33, Shardul Bankar wrote:
> MPTCP_RST_EMPTCP (reset reason 1) is used as a catch-all for several
> distinct error conditions across subflow setup, authentication, and
> data-path validation.  The existing MPRstTx/MPRstRx counters only
> track aggregate reset volume, making it difficult to diagnose which
> code path is triggering subflow resets in production.

(...)

> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index d562e149606f6..6fce3f431667c 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -160,6 +160,7 @@ static int subflow_check_req(struct request_sock *req,
>  	 * TCP option space.
>  	 */
>  	if (rcu_access_pointer(tcp_sk(sk_listener)->md5sig_info)) {
> +		MPTCP_INC_STATS(sock_net(sk_listener), MPTCP_MIB_MD5SIGRESET);
>  		subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP);
>  		return -EINVAL;
>  	}

In the v1, you also had another counter added in this helper, for the
MPJOIN case with !subflow_req->msk: I thought that JOINNOTOKEN was
always sent in this case. I just noticed another possible error that
could come from the PM side if there is no local_id. I don't know if we
should cover this case, but maybe "safer" to?

Maybe adding a MPJoinNoIdFound and use it in subflow_token_join_request,
similar to MPJoinNoTokenFound?

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


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

* Re: [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP
  2026-05-12 10:33 [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP Shardul Bankar
                   ` (2 preceding siblings ...)
  2026-05-12 11:47 ` [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP MPTCP CI
@ 2026-05-27  7:43 ` Matthieu Baerts
  3 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-05-27  7:43 UTC (permalink / raw)
  To: Shardul Bankar, mptcp
  Cc: martineau, geliang, pabeni, janak, kalpan.jani, Shardul Bankar

Hi Shardul,

On 12/05/2026 20:33, Shardul Bankar wrote:
> MPTCP_RST_EMPTCP (reset reason 1) is currently used as a catch-all for
> several distinct conditions across subflow setup, authentication, and
> data-path validation.  This series adds per-event MIB counters at each
> of those use sites and extends mptcp_join.sh's chk_rst_nr() to validate
> them.
Thank you for the v3, and sorry for the delay.

The series looks good to me, just a few minor comments. But I also
noticed that the From: and Signed-off-by: tags mismatch:

  From: Shardul Bankar <shardulsb08@gmail.com>
  Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>

They should be the same, or to be precise: the last From should be the
same as the SoB. In other words, you can send patches with another
email, but you should not change the author.

Don't hesitate to look at 'b4' to send patches, instead of using 'git
send-email' with many options:

https://www.mptcp.dev/contributing.html#it-is-easy-with-b4

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


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

end of thread, other threads:[~2026-05-27  7:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 10:33 [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP Shardul Bankar
2026-05-12 10:33 ` [PATCH v3 1/2] mptcp: add per-event MIB counters for MPTCP_RST_EMPTCP resets Shardul Bankar
2026-05-27  6:50   ` Matthieu Baerts
2026-05-12 10:33 ` [PATCH v3 2/2] selftests: mptcp: extend chk_rst_nr to validate per-event RST counters Shardul Bankar
2026-05-27  6:24   ` Matthieu Baerts
2026-05-12 11:47 ` [PATCH v3 0/2] mptcp: per-event MIB counters for MPTCP_RST_EMPTCP MPTCP CI
2026-05-27  7:43 ` Matthieu Baerts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox