When DSS validating fails, just drop this skb and continue, don't
fallback and reset the subflow.

This patch changes the return value of get_mapping_status() in this
case from MAPPING_INVALID to MAPPING_EMPTY. With this change, in
subflow_check_data_avail(), the code will reach 'goto no_data', instead
of 'goto fallback'.

---
This patch could be put into "BPF redundant scheduler, part 2" series.
It can fix the issue I mentioned in [1].

[1]
https://patchwork.kernel.org/project/mptcp/patch/b0a39d02796da8a214fb8b8b9a597eef59e3ebd7.1666349129.git.geliang.tang@suse.com/
---

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/subflow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 97d301d3bfd3..3944ebf492d9 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1128,7 +1128,8 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 	 */
 	if (!validate_mapping(ssk, skb)) {
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSTCPMISMATCH);
-		return MAPPING_INVALID;
+		sk_eat_skb(ssk, skb);
+		return MAPPING_EMPTY;
 	}
 
 	skb_ext_del(skb, SKB_EXT_MPTCP);
-- 
2.35.3

On Mon, 14 Nov 2022, Geliang Tang wrote:

> When DSS validating fails, just drop this skb and continue, don't
> fallback and reset the subflow.
>
> This patch changes the return value of get_mapping_status() in this
> case from MAPPING_INVALID to MAPPING_EMPTY. With this change, in
> subflow_check_data_avail(), the code will reach 'goto no_data', instead
> of 'goto fallback'.
>
> ---
> This patch could be put into "BPF redundant scheduler, part 2" series.
> It can fix the issue I mentioned in [1].
>
> [1]
> https://patchwork.kernel.org/project/mptcp/patch/b0a39d02796da8a214fb8b8b9a597eef59e3ebd7.1666349129.git.geliang.tang@suse.com/

I don't think this is the solution - the problem is that 
validate_mapping() is finding mappings that don't match with the TCP 
sequence numbers of the packets. This check for the receiver seems ok, 
it's the sender side that needs fixing.

- Mat

> ---
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/subflow.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 97d301d3bfd3..3944ebf492d9 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1128,7 +1128,8 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
> 	 */
> 	if (!validate_mapping(ssk, skb)) {
> 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSTCPMISMATCH);
> -		return MAPPING_INVALID;
> +		sk_eat_skb(ssk, skb);
> +		return MAPPING_EMPTY;
> 	}
>
> 	skb_ext_del(skb, SKB_EXT_MPTCP);
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6482477422739456
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6482477422739456/summary/summary.txt

- KVM Validation: debug:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4652890074120192
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4652890074120192/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/703087448a53


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)