Tested-by: Chester A. Unal chester.a.unal@xpedite-tech.com
Thanks a ton! Chester A.
On Mon, Feb 24, 2025 at 6:19 PM 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. Instead, a new dedicated mapping error could have been returned and treated as a special case. However, the commit 31bf11de146c ("mptcp: introduce MAPPING_BAD_CSUM") has been introduced by Paolo Abeni soon after, and backported later on to stable. It better handle the csum case, and it means the exception for valid_csum_seen in subflow_can_fallback(), plus this one for the infinite mapping in subflow_check_data_avail(), are no longer needed.
In other words, the code can be simplified there: a fallback should only be done if msk->allow_infinite_fallback is set. This boolean is set to false once MPTCP-specific operations acting on the whole MPTCP connection vs the initial path have been done, e.g. a second path has been created, or an MPTCP re-injection -- yes, possible even with a single subflow. The subflow_can_fallback() helper can then be dropped, and replaced by this single condition.
This also makes the code clearer: a fallback should only be done if it is possible to do so.
While at it, no need to set map_data_len to 0 in get_mapping_status() for the infinite mapping case: it will be set to skb->len just after, at the end of subflow_check_data_avail(), and not read in between.
Fixes: f8d4bcacff3b ("mptcp: infinite mapping receiving") Cc: stable@vger.kernel.org Reported-by: Chester A. Unal chester.a.unal@xpedite-tech.com Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/544 Acked-by: Paolo Abeni pabeni@redhat.com Signed-off-by: Matthieu Baerts (NGI0) matttbe@kernel.org
net/mptcp/subflow.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index dfcbef9c46246983d21c827d9551d4eb2c95430e..9f18217dddc865bc467a2c5c34aa4bf23bf3ac75 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1142,7 +1142,6 @@ 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; }@@ -1286,18 +1285,6 @@ static void subflow_sched_work_if_closed(struct mptcp_sock *msk, struct sock *ss mptcp_schedule_work(sk); }
-static bool subflow_can_fallback(struct mptcp_subflow_context *subflow) -{
struct mptcp_sock *msk = mptcp_sk(subflow->conn);if (subflow->mp_join)return false;else if (READ_ONCE(msk->csum_enabled))return !subflow->valid_csum_seen;elsereturn READ_ONCE(msk->allow_infinite_fallback);-}
static void mptcp_subflow_fail(struct mptcp_sock *msk, struct sock *ssk) { struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk); @@ -1393,7 +1380,7 @@ static bool subflow_check_data_avail(struct sock *ssk) return true; }
if (!subflow_can_fallback(subflow) && subflow->map_data_len) {
if (!READ_ONCE(msk->allow_infinite_fallback)) { /* fatal protocol error, close the socket. * subflow_error_report() will introduce the appropriate barriers */-- 2.47.1