| Message ID | 20250329-mptcp-mpj-reject-v1-4-2396d5666e8f@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7b0845a435658dc42fcf80775a471a3f21684145 |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | mptcp: fix MPJoinAckHMacFailure, add MPJoinRejected | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | warning | total: 0 errors, 2 warnings, 0 checks, 39 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | success | Success! ✅ |
| matttbe/KVM_Validation__btf-normal__only_bpftest_all_ | success | Success! ✅ |
| matttbe/KVM_Validation__btf-debug__only_bpftest_all_ | success | Success! ✅ |
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; > } >
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
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; }
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(-)