1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next 0/5] mptcp: fix MPJoinAckHMacFailure, add MPJoinRejected
  4. View patch

[PATCH mptcp-next 4/5] mptcp: add MPJoinRejected MIB counter

Matthieu Baerts (NGI0) posted 5 patches 1 week, 5 days ago
[PATCH mptcp-next 4/5] mptcp: add MPJoinRejected MIB counter
Posted by Matthieu Baerts (NGI0) 1 week, 5 days ago 
This counter is useful to understand why some paths are rejected, and
not created as expected.

It is incremented when receiving a connection request, if the PM didn't
allow the creation of new subflows.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/mib.c      | 1 +
 net/mptcp/mib.h      | 1 +
 net/mptcp/protocol.c | 4 +++-
 net/mptcp/subflow.c  | 2 ++
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index 19eb9292bd6093a760b41f98c1774fd2490c48e3..0c24545f0e8df95b3475bfccc7a2f2ce440f7ad2 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -28,6 +28,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("MPJoinSynAckHMacFailure", MPTCP_MIB_JOINSYNACKMAC),
 	SNMP_MIB_ITEM("MPJoinAckRx", MPTCP_MIB_JOINACKRX),
 	SNMP_MIB_ITEM("MPJoinAckHMacFailure", MPTCP_MIB_JOINACKMAC),
+	SNMP_MIB_ITEM("MPJoinRejected", MPTCP_MIB_JOINREJECTED),
 	SNMP_MIB_ITEM("MPJoinSynTx", MPTCP_MIB_JOINSYNTX),
 	SNMP_MIB_ITEM("MPJoinSynTxCreatSkErr", MPTCP_MIB_JOINSYNTXCREATSKERR),
 	SNMP_MIB_ITEM("MPJoinSynTxBindErr", MPTCP_MIB_JOINSYNTXBINDERR),
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 128282982843a07614a46f9b2c2f7c708306c769..250c6b77977e8f846b5741304f7841a922f51967 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -23,6 +23,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_JOINSYNACKMAC,	/* HMAC was wrong on SYN/ACK + MP_JOIN */
 	MPTCP_MIB_JOINACKRX,		/* Received an ACK + MP_JOIN */
 	MPTCP_MIB_JOINACKMAC,		/* HMAC was wrong on ACK + MP_JOIN */
+	MPTCP_MIB_JOINREJECTED,		/* The PM rejected the JOIN request */
 	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 */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9042a86e003036f8eb9680bc6c520fb57d92dbd0..8ba1a4f225bd2014ebdc973e752702028ee5f31a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3527,8 +3527,10 @@ bool mptcp_finish_join(struct sock *ssk)
 		return true;
 	}
 
-	if (!mptcp_pm_allow_new_subflow(msk))
+	if (!mptcp_pm_allow_new_subflow(msk)) {
+		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_JOINREJECTED);
 		goto err_prohibited;
+	}
 
 	/* If we can't acquire msk socket lock here, let the release callback
 	 * handle it
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index e7951786a97c91190c7341d2c586a1f4acc05ed5..15613d691bfef6800268ae75b62508736865f44a 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -247,6 +247,7 @@ static int subflow_check_req(struct request_sock *req,
 
 		if (unlikely(req->syncookie)) {
 			if (!mptcp_can_accept_new_subflow(subflow_req->msk)) {
+				SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINREJECTED);
 				subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT);
 				return -EPERM;
 			}
@@ -902,6 +903,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 			}
 
 			if (!mptcp_can_accept_new_subflow(owner)) {
+				SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINREJECTED);
 				subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT);
 				goto dispose_child;
 			}

-- 
2.48.1
Re: [PATCH mptcp-next 4/5] mptcp: add MPJoinRejected MIB counter
Posted by Geliang Tang 1 week, 4 days ago 
Hi Matt,

On Sat, 2025-03-29 at 17:26 +0100, Matthieu Baerts (NGI0) wrote:
> This counter is useful to understand why some paths are rejected, and
> not created as expected.
> 
> It is incremented when receiving a connection request, if the PM
> didn't
> allow the creation of new subflows.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/mib.c      | 1 +
>  net/mptcp/mib.h      | 1 +
>  net/mptcp/protocol.c | 4 +++-
>  net/mptcp/subflow.c  | 2 ++
>  4 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
> index
> 19eb9292bd6093a760b41f98c1774fd2490c48e3..0c24545f0e8df95b3475bfccc7a
> 2f2ce440f7ad2 100644
> --- a/net/mptcp/mib.c
> +++ b/net/mptcp/mib.c
> @@ -28,6 +28,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>  	SNMP_MIB_ITEM("MPJoinSynAckHMacFailure",
> MPTCP_MIB_JOINSYNACKMAC),
>  	SNMP_MIB_ITEM("MPJoinAckRx", MPTCP_MIB_JOINACKRX),
>  	SNMP_MIB_ITEM("MPJoinAckHMacFailure", MPTCP_MIB_JOINACKMAC),
> +	SNMP_MIB_ITEM("MPJoinRejected", MPTCP_MIB_JOINREJECTED),
>  	SNMP_MIB_ITEM("MPJoinSynTx", MPTCP_MIB_JOINSYNTX),
>  	SNMP_MIB_ITEM("MPJoinSynTxCreatSkErr",
> MPTCP_MIB_JOINSYNTXCREATSKERR),
>  	SNMP_MIB_ITEM("MPJoinSynTxBindErr",
> MPTCP_MIB_JOINSYNTXBINDERR),
> diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
> index
> 128282982843a07614a46f9b2c2f7c708306c769..250c6b77977e8f846b5741304f7
> 841a922f51967 100644
> --- a/net/mptcp/mib.h
> +++ b/net/mptcp/mib.h
> @@ -23,6 +23,7 @@ enum linux_mptcp_mib_field {
>  	MPTCP_MIB_JOINSYNACKMAC,	/* HMAC was wrong on SYN/ACK
> + MP_JOIN */
>  	MPTCP_MIB_JOINACKRX,		/* Received an ACK + MP_JOIN
> */
>  	MPTCP_MIB_JOINACKMAC,		/* HMAC was wrong on ACK +
> MP_JOIN */
> +	MPTCP_MIB_JOINREJECTED,		/* The PM rejected
> the JOIN request */
>  	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 */
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index
> 9042a86e003036f8eb9680bc6c520fb57d92dbd0..8ba1a4f225bd2014ebdc973e752
> 702028ee5f31a 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3527,8 +3527,10 @@ bool mptcp_finish_join(struct sock *ssk)
>  		return true;
>  	}
>  
> -	if (!mptcp_pm_allow_new_subflow(msk))
> +	if (!mptcp_pm_allow_new_subflow(msk)) {
> +		MPTCP_INC_STATS(sock_net(ssk),
> MPTCP_MIB_JOINREJECTED);

nit:

Although the result is the same as sock_net(ssk), what do you think of
using sock_net(parent) here?

Thanks,
-Geliang

>  		goto err_prohibited;
> +	}
>  
>  	/* If we can't acquire msk socket lock here, let the release
> callback
>  	 * handle it
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index
> e7951786a97c91190c7341d2c586a1f4acc05ed5..15613d691bfef6800268ae75b62
> 508736865f44a 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -247,6 +247,7 @@ static int subflow_check_req(struct request_sock
> *req,
>  
>  		if (unlikely(req->syncookie)) {
>  			if
> (!mptcp_can_accept_new_subflow(subflow_req->msk)) {
> +				SUBFLOW_REQ_INC_STATS(req,
> MPTCP_MIB_JOINREJECTED);
>  				subflow_add_reset_reason(skb,
> MPTCP_RST_EPROHIBIT);
>  				return -EPERM;
>  			}
> @@ -902,6 +903,7 @@ static struct sock *subflow_syn_recv_sock(const
> struct sock *sk,
>  			}
>  
>  			if (!mptcp_can_accept_new_subflow(owner)) {
> +				SUBFLOW_REQ_INC_STATS(req,
> MPTCP_MIB_JOINREJECTED);
>  				subflow_add_reset_reason(skb,
> MPTCP_RST_EPROHIBIT);
>  				goto dispose_child;
>  			}
>
Re: [PATCH mptcp-next 4/5] mptcp: add MPJoinRejected MIB counter
Posted by Matthieu Baerts 1 week, 4 days ago 
Hi Geliang,

On 31/03/2025 03:59, Geliang Tang wrote:
> Hi Matt,
> 
> On Sat, 2025-03-29 at 17:26 +0100, Matthieu Baerts (NGI0) wrote:
>> This counter is useful to understand why some paths are rejected, and
>> not created as expected.
>>
>> It is incremented when receiving a connection request, if the PM
>> didn't
>> allow the creation of new subflows.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/mib.c      | 1 +
>>  net/mptcp/mib.h      | 1 +
>>  net/mptcp/protocol.c | 4 +++-
>>  net/mptcp/subflow.c  | 2 ++
>>  4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
>> index
>> 19eb9292bd6093a760b41f98c1774fd2490c48e3..0c24545f0e8df95b3475bfccc7a
>> 2f2ce440f7ad2 100644
>> --- a/net/mptcp/mib.c
>> +++ b/net/mptcp/mib.c
>> @@ -28,6 +28,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
>>  	SNMP_MIB_ITEM("MPJoinSynAckHMacFailure",
>> MPTCP_MIB_JOINSYNACKMAC),
>>  	SNMP_MIB_ITEM("MPJoinAckRx", MPTCP_MIB_JOINACKRX),
>>  	SNMP_MIB_ITEM("MPJoinAckHMacFailure", MPTCP_MIB_JOINACKMAC),
>> +	SNMP_MIB_ITEM("MPJoinRejected", MPTCP_MIB_JOINREJECTED),
>>  	SNMP_MIB_ITEM("MPJoinSynTx", MPTCP_MIB_JOINSYNTX),
>>  	SNMP_MIB_ITEM("MPJoinSynTxCreatSkErr",
>> MPTCP_MIB_JOINSYNTXCREATSKERR),
>>  	SNMP_MIB_ITEM("MPJoinSynTxBindErr",
>> MPTCP_MIB_JOINSYNTXBINDERR),
>> diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
>> index
>> 128282982843a07614a46f9b2c2f7c708306c769..250c6b77977e8f846b5741304f7
>> 841a922f51967 100644
>> --- a/net/mptcp/mib.h
>> +++ b/net/mptcp/mib.h
>> @@ -23,6 +23,7 @@ enum linux_mptcp_mib_field {
>>  	MPTCP_MIB_JOINSYNACKMAC,	/* HMAC was wrong on SYN/ACK
>> + MP_JOIN */
>>  	MPTCP_MIB_JOINACKRX,		/* Received an ACK + MP_JOIN
>> */
>>  	MPTCP_MIB_JOINACKMAC,		/* HMAC was wrong on ACK +
>> MP_JOIN */
>> +	MPTCP_MIB_JOINREJECTED,		/* The PM rejected
>> the JOIN request */
>>  	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 */
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index
>> 9042a86e003036f8eb9680bc6c520fb57d92dbd0..8ba1a4f225bd2014ebdc973e752
>> 702028ee5f31a 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -3527,8 +3527,10 @@ bool mptcp_finish_join(struct sock *ssk)
>>  		return true;
>>  	}
>>  
>> -	if (!mptcp_pm_allow_new_subflow(msk))
>> +	if (!mptcp_pm_allow_new_subflow(msk)) {
>> +		MPTCP_INC_STATS(sock_net(ssk),
>> MPTCP_MIB_JOINREJECTED);
> 
> nit:
> 
> Although the result is the same as sock_net(ssk), what do you think of
> using sock_net(parent) here?

We could, but why would you prefer using 'parent' instead of 'ssk'? Ii
picked 'ssk' because that's the one passed in argument of this function.
'parent' is obtained from 'ssk'.

I hope you don't mind if I apply the patch as it is because I already
applied the parent patches. I can always change that later of course.

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.