| Message ID | 20250217-mptcp-dss-drop-mpj-v1-1-d671d6b9a153@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Paolo Abeni |
| Headers | show |
| Series | mptcp: reset when MPTCP opts are dropped after MPJ | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | warning | total: 0 errors, 2 warnings, 0 checks, 33 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! ✅ |
Tested-by: Chester A. Unal <chester.a.unal@xpedite-tech.com> Thanks a lot! Chester A. On Mon, Feb 17, 2025 at 8:38 AM Matthieu Baerts (NGI0) <matttbe@kernel.org> wrote: > > Before this patch, if the checksum was not used, the subflow was only > reset if map_data_len was != 0. If there were no MPTCP options or an > invalid mapping, map_data_len was not set to the data len, and then the > subflow was not reset as it should have been, leaving the MPTCP > connection in a wrong fallback mode. > > This map_data_len condition has been introduced to handle the reception > of the infinite mapping. So instead, a new dedicated mapping error is > now returned (infinite), and this special case is properly handled: the > exception is only applied to this case now, and not other ones by > mistake. > > While at it, no need to set map_data_len to 0 as it will be set to > skb->len just after, at the end of subflow_check_data_avail(). > > Fixes: f8d4bcacff3b ("mptcp: infinite mapping receiving") > Reported-by: Chester A. Unal <chester.a.unal@xpedite-tech.com> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/544 > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > net/mptcp/subflow.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > index d2caffa56bdd98f5fd9ef07fdcb3610ea186b848..6ec03580ccec12fb50fc3aacf3d22413647b32b5 100644 > --- a/net/mptcp/subflow.c > +++ b/net/mptcp/subflow.c > @@ -969,6 +969,7 @@ enum mapping_status { > MAPPING_DATA_FIN, > MAPPING_DUMMY, > MAPPING_BAD_CSUM, > + MAPPING_INFINITE, > MAPPING_NODSS > }; > > @@ -1139,8 +1140,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk, > if (data_len == 0) { > pr_debug("infinite mapping received\n"); > MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX); > - subflow->map_data_len = 0; > - return MAPPING_INVALID; > + return MAPPING_INFINITE; > } > > if (mpext->data_fin == 1) { > @@ -1357,7 +1357,8 @@ static bool subflow_check_data_avail(struct sock *ssk) > status = get_mapping_status(ssk, msk); > trace_subflow_check_data_avail(status, skb_peek(&ssk->sk_receive_queue)); > if (unlikely(status == MAPPING_INVALID || status == MAPPING_DUMMY || > - status == MAPPING_BAD_CSUM || status == MAPPING_NODSS)) > + status == MAPPING_BAD_CSUM || status == MAPPING_INFINITE || > + status == MAPPING_NODSS)) > goto fallback; > > if (status != MAPPING_OK) > @@ -1405,7 +1406,7 @@ static bool subflow_check_data_avail(struct sock *ssk) > return true; > } > > - if (!subflow_can_fallback(subflow) && subflow->map_data_len) { > + if (!subflow_can_fallback(subflow) && status != MAPPING_INFINITE) { > /* fatal protocol error, close the socket. > * subflow_error_report() will introduce the appropriate barriers > */ > > -- > 2.47.1 >
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index d2caffa56bdd98f5fd9ef07fdcb3610ea186b848..6ec03580ccec12fb50fc3aacf3d22413647b32b5 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -969,6 +969,7 @@ enum mapping_status { MAPPING_DATA_FIN, MAPPING_DUMMY, MAPPING_BAD_CSUM, + MAPPING_INFINITE, MAPPING_NODSS }; @@ -1139,8 +1140,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk, if (data_len == 0) { pr_debug("infinite mapping received\n"); MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX); - subflow->map_data_len = 0; - return MAPPING_INVALID; + return MAPPING_INFINITE; } if (mpext->data_fin == 1) { @@ -1357,7 +1357,8 @@ static bool subflow_check_data_avail(struct sock *ssk) status = get_mapping_status(ssk, msk); trace_subflow_check_data_avail(status, skb_peek(&ssk->sk_receive_queue)); if (unlikely(status == MAPPING_INVALID || status == MAPPING_DUMMY || - status == MAPPING_BAD_CSUM || status == MAPPING_NODSS)) + status == MAPPING_BAD_CSUM || status == MAPPING_INFINITE || + status == MAPPING_NODSS)) goto fallback; if (status != MAPPING_OK) @@ -1405,7 +1406,7 @@ static bool subflow_check_data_avail(struct sock *ssk) return true; } - if (!subflow_can_fallback(subflow) && subflow->map_data_len) { + if (!subflow_can_fallback(subflow) && status != MAPPING_INFINITE) { /* fatal protocol error, close the socket. * subflow_error_report() will introduce the appropriate barriers */
Before this patch, if the checksum was not used, the subflow was only reset if map_data_len was != 0. If there were no MPTCP options or an invalid mapping, map_data_len was not set to the data len, and then the subflow was not reset as it should have been, leaving the MPTCP connection in a wrong fallback mode. This map_data_len condition has been introduced to handle the reception of the infinite mapping. So instead, a new dedicated mapping error is now returned (infinite), and this special case is properly handled: the exception is only applied to this case now, and not other ones by mistake. While at it, no need to set map_data_len to 0 as it will be set to skb->len just after, at the end of subflow_check_data_avail(). Fixes: f8d4bcacff3b ("mptcp: infinite mapping receiving") Reported-by: Chester A. Unal <chester.a.unal@xpedite-tech.com> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/544 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- net/mptcp/subflow.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)