Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MIB counters for fallback to TCP #514

Open
matttbe opened this issue Aug 7, 2024 · 4 comments
Open

Add MIB counters for fallback to TCP #514

matttbe opened this issue Aug 7, 2024 · 4 comments
Assignees

Comments

@matttbe
Copy link
Member

matttbe commented Aug 7, 2024

Because there is nothing about that, hard to track what's wrong.

@Dwyane-Yan
Copy link

Dwyane-Yan commented Sep 27, 2024

Hi Matthieu:
I noticed there exists a patch named "mptcp: handle consistently DSS corruption." which adds two MIB counters, but it seems not the same as this issue, right?
Furthermore, upon reviewing the code, I believe that simply adding the code snippet below should work. Do you have any additional suggestions?

--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1394,7 +1394,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
                        WRITE_ONCE(subflow->data_avail, false);
                        return false;
                }
-
+               MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MIDDLEFALLBACK);
                mptcp_do_fallback(ssk);
        }

@matttbe matttbe changed the title Add MIB counters for fallback to TCP in the middle of a connection Add MIB counters for fallback to TCP Sep 27, 2024
@matttbe
Copy link
Member Author

matttbe commented Sep 27, 2024

Hi Gang Yan,

I noticed there exists a patch named "mptcp: handle consistently DSS corruption." which adds two MIB counters, but it seems not the same as this issue, right?

Correct: this patch adds new MIB counters, but for new code triggering a fallback.

The goal of this ticket here is to add counters where we do a fallback / reset subflow, due to an issue with the MPTCP option. Something likely triggered by a middlebox missing up with MPTCP options.

Furthermore, upon reviewing the code, I believe that simply adding the code snippet below should work. Do you have any additional suggestions?

Is there no other places where a fallback to TCP / subflows reset are done, and there are no counters? For example, I think that there are already counters when such fallback / reset are done at the beginning of the connection, but better to double-check that as well (e.g. in case of simultaneous connects, no MPTCP options after packet establishment, if MD5 (or TCP_AO?) is used, etc.).

Also, it would be good to cover that with tests, probably easier with packetdrill tests.

@Dwyane-Yan
Copy link

Dwyane-Yan commented Sep 27, 2024

Is there no other places where a fallback to TCP / subflows reset are done, and there are no counters? For example, I think that there are already counters when such fallback / reset are done at the beginning of the connection, but better to double-check that as well (e.g. in case of simultaneous connects, no MPTCP options after packet establishment, if MD5 (or TCP_AO?) is used, etc.).

OK, I'll check it carefully.

Also, it would be good to cover that with tests, probably easier with packetdrill tests.

Thanks for your advice, I'll look into how to use it.

@geliangtang
Copy link
Member

Hi Matt, I assigned this issue to myself since my colleague Gang Yan (Dwyane-Yan) in my group is looking at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants