| Message ID | 20240406014848.71739-1-kerneljasonxing@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 382c60019ee769a8f3743e8340c063e4aa71231e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] mptcp: add reset reason options in some places | expand |
On Sat, 2024-04-06 at 09:48 +0800, Jason Xing wrote: > From: Jason Xing <kernelxing@tencent.com> > > The reason codes are handled in two ways nowadays (quoting Mat Martineau): > 1. Sending in the MPTCP option on RST packets when there is no subflow > context available (these use subflow_add_reset_reason() and directly call > a TCP-level send_reset function) > 2. The "normal" way via subflow->reset_reason. This will propagate to both > the outgoing reset packet and to a local path manager process via netlink > in mptcp_event_sub_closed() > > RFC 8684 defines the skb reset reason behaviour which is not required > even though in some places: > > A host sends a TCP RST in order to close a subflow or reject > an attempt to open a subflow (MP_JOIN). In order to let the > receiving host know why a subflow is being closed or rejected, > the TCP RST packet MAY include the MP_TCPRST option (Figure 15). > The host MAY use this information to decide, for example, whether > it tries to re-establish the subflow immediately, later, or never. > > Since the commit dc87efdb1a5cd ("mptcp: add mptcp reset option support") > introduced this feature about three years ago, we can fully use it. > There remains some places where we could insert reason into skb as > we can see in this patch. > > Many thanks to Mat and Paolo for help:) > > Signed-off-by: Jason Xing <kernelxing@tencent.com> > --- > v2 > Link: https://lore.kernel.org/all/5046e1867c65f39e07822beb0a19a22743b1064b.camel@redhat.com/ > 1. complete all the possible reset reasons in the subflow_check_req() (Paolo) > --- > net/mptcp/subflow.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > index 1626dd20c68f..b7ce2ca1782c 100644 > --- a/net/mptcp/subflow.c > +++ b/net/mptcp/subflow.c > @@ -150,8 +150,10 @@ static int subflow_check_req(struct request_sock *req, > /* no MPTCP if MD5SIG is enabled on this socket or we may run out of > * TCP option space. > */ > - if (rcu_access_pointer(tcp_sk(sk_listener)->md5sig_info)) > + if (rcu_access_pointer(tcp_sk(sk_listener)->md5sig_info)) { > + subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP); > return -EINVAL; > + } > #endif > > mptcp_get_options(skb, &mp_opt); > @@ -219,6 +221,7 @@ static int subflow_check_req(struct request_sock *req, > ntohs(inet_sk((struct sock *)subflow_req->msk)->inet_sport)); > if (!mptcp_pm_sport_in_anno_list(subflow_req->msk, sk_listener)) { > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTSYNRX); > + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); > return -EPERM; > } > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINPORTSYNRX); > @@ -227,10 +230,12 @@ static int subflow_check_req(struct request_sock *req, > subflow_req_create_thmac(subflow_req); > > if (unlikely(req->syncookie)) { > - if (mptcp_can_accept_new_subflow(subflow_req->msk)) > - subflow_init_req_cookie_join_save(subflow_req, skb); > - else > + if (!mptcp_can_accept_new_subflow(subflow_req->msk)) { > + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); > return -EPERM; > + } > + > + subflow_init_req_cookie_join_save(subflow_req, skb); > } > > pr_debug("token=%u, remote_nonce=%u msk=%p", subflow_req->token, > @@ -873,13 +878,18 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, > ntohs(inet_sk((struct sock *)owner)->inet_sport)); > if (!mptcp_pm_sport_in_anno_list(owner, sk)) { > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTACKRX); > + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); > goto dispose_child; > } > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINPORTACKRX); > } > > - if (!mptcp_finish_join(child)) > + if (!mptcp_finish_join(child)) { > + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(child); > + > + subflow_add_reset_reason(skb, subflow->reset_reason); > goto dispose_child; > + } > > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX); > tcp_rsk(req)->drop_req = true; LGTM, Acked-by: Paolo Abeni <pabeni@redhat.com>
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Sat, 6 Apr 2024 09:48:48 +0800 you wrote: > From: Jason Xing <kernelxing@tencent.com> > > The reason codes are handled in two ways nowadays (quoting Mat Martineau): > 1. Sending in the MPTCP option on RST packets when there is no subflow > context available (these use subflow_add_reset_reason() and directly call > a TCP-level send_reset function) > 2. The "normal" way via subflow->reset_reason. This will propagate to both > the outgoing reset packet and to a local path manager process via netlink > in mptcp_event_sub_closed() > > [...] Here is the summary with links: - [net-next,v2] mptcp: add reset reason options in some places https://git.kernel.org/netdev/net-next/c/382c60019ee7 You are awesome, thank you!
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 1626dd20c68f..b7ce2ca1782c 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -150,8 +150,10 @@ static int subflow_check_req(struct request_sock *req, /* no MPTCP if MD5SIG is enabled on this socket or we may run out of * TCP option space. */ - if (rcu_access_pointer(tcp_sk(sk_listener)->md5sig_info)) + if (rcu_access_pointer(tcp_sk(sk_listener)->md5sig_info)) { + subflow_add_reset_reason(skb, MPTCP_RST_EMPTCP); return -EINVAL; + } #endif mptcp_get_options(skb, &mp_opt); @@ -219,6 +221,7 @@ static int subflow_check_req(struct request_sock *req, ntohs(inet_sk((struct sock *)subflow_req->msk)->inet_sport)); if (!mptcp_pm_sport_in_anno_list(subflow_req->msk, sk_listener)) { SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTSYNRX); + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); return -EPERM; } SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINPORTSYNRX); @@ -227,10 +230,12 @@ static int subflow_check_req(struct request_sock *req, subflow_req_create_thmac(subflow_req); if (unlikely(req->syncookie)) { - if (mptcp_can_accept_new_subflow(subflow_req->msk)) - subflow_init_req_cookie_join_save(subflow_req, skb); - else + if (!mptcp_can_accept_new_subflow(subflow_req->msk)) { + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); return -EPERM; + } + + subflow_init_req_cookie_join_save(subflow_req, skb); } pr_debug("token=%u, remote_nonce=%u msk=%p", subflow_req->token, @@ -873,13 +878,18 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, ntohs(inet_sk((struct sock *)owner)->inet_sport)); if (!mptcp_pm_sport_in_anno_list(owner, sk)) { SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTACKRX); + subflow_add_reset_reason(skb, MPTCP_RST_EPROHIBIT); goto dispose_child; } SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINPORTACKRX); } - if (!mptcp_finish_join(child)) + if (!mptcp_finish_join(child)) { + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(child); + + subflow_add_reset_reason(skb, subflow->reset_reason); goto dispose_child; + } SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX); tcp_rsk(req)->drop_req = true;