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

userspace pm: subflows counter: not updated when a subflows is removed from the other end #492

Open
matttbe opened this issue May 23, 2024 · 0 comments
Labels

Comments

@matttbe
Copy link
Member

matttbe commented May 23, 2024

It looks like the subflow counter is not updated when the other end initiates the removal of a subflow, e.g. after having received a RM_ADDR:

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 2b66c5fa71eb..424745490b78 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3338,17 +3335,19 @@ userspace_tests()
        if reset_with_events "userspace pm add & remove address" &&
           continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
                set_userspace_pm $ns1
-               pm_nl_set_limits $ns2 2 2
+               pm_nl_set_limits $ns2 2 5
+               pm_nl_add_endpoint $ns2 10.0.1.2 flags subflow,fullmesh
+               pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow,fullmesh
                speed=5 \
                        run_tests $ns1 $ns2 10.0.1.1 &
                local tests_pid=$!
                wait_mpj $ns1
                userspace_pm_add_addr $ns1 10.0.2.1 10
                userspace_pm_add_addr $ns1 10.0.3.1 20
-               chk_join_nr 2 2 2
+               chk_join_nr 5 5 5
                chk_add_nr 2 2
-               chk_mptcp_info subflows 2 subflows 2
-               chk_subflows_total 3 3
+               chk_mptcp_info subflows 5 subflows 5
+               chk_subflows_total 6 6
                chk_mptcp_info add_addr_signal 2 add_addr_accepted 2
                userspace_pm_chk_dump_addr "${ns1}" \
                        $'id 10 flags signal 10.0.2.1\nid 20 flags signal 10.0.3.1' \
@@ -3356,15 +3355,13 @@ userspace_tests()
                userspace_pm_chk_get_addr "${ns1}" "10" "id 10 flags signal 10.0.2.1"
                userspace_pm_chk_get_addr "${ns1}" "20" "id 20 flags signal 10.0.3.1"
                userspace_pm_rm_addr $ns1 10
-               userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $MPTCP_LIB_EVENT_SUB_ESTABLISHED
                userspace_pm_chk_dump_addr "${ns1}" \
                        "id 20 flags signal 10.0.3.1" "after rm_addr 10"
                userspace_pm_rm_addr $ns1 20
-               userspace_pm_rm_sf $ns1 10.0.3.1 $MPTCP_LIB_EVENT_SUB_ESTABLISHED
                userspace_pm_chk_dump_addr "${ns1}" "" "after rm_addr 20"
-               chk_rm_nr 2 2 invert
-               chk_mptcp_info subflows 0 subflows 0
-               chk_subflows_total 1 1
+               chk_rm_nr 2 0 invert
+               chk_mptcp_info subflows 1 subflows 1
+               chk_subflows_total 2 2
                kill_events_pids
                mptcp_lib_kill_wait $tests_pid
        fi

We get the following errors:

mptcp_info subflows=1:1             [FAIL] got 5:1 subflows:subflows expected 1:1
mptcp_info subflows_total=2:2       [FAIL] got 6:2 subflows_total:subflows_total expected 2:2

The counter should be the same on each side.

I don't know why this test sends 2 remove address, each time followed by the removal of the subflow: this hid the bug. I suggest removing only one subflow, not the two of them. @geliangtang: WDYT?

We are certainly missing some actions when a subflow is removed, e.g. retry if we are under the limit, etc.

(Note: I modified the test to validate https://patchwork.kernel.org/project/mptcp/patch/[email protected]/ : this patch is needed not to have errors with chk_rm_nr 2 0 invert)

@matttbe matttbe added the bug label May 23, 2024
matttbe pushed a commit that referenced this issue Jun 17, 2024
Add a test case for the jmp32/k fix to ensure selftests have coverage.

Before fix:

  # ./vmtest.sh -- ./test_progs -t verifier_or_jmp32_k
  [...]
  ./test_progs -t verifier_or_jmp32_k
  tester_init:PASS:tester_log_buf 0 nsec
  process_subtest:PASS:obj_open_mem 0 nsec
  process_subtest:PASS:specs_alloc 0 nsec
  run_subtest:PASS:obj_open_mem 0 nsec
  run_subtest:FAIL:unexpected_load_success unexpected success: 0
  #492/1   verifier_or_jmp32_k/or_jmp32_k: bit ops + branch on unknown value:FAIL
  #492     verifier_or_jmp32_k:FAIL
  Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED

After fix:

  # ./vmtest.sh -- ./test_progs -t verifier_or_jmp32_k
  [...]
  ./test_progs -t verifier_or_jmp32_k
  #492/1   verifier_or_jmp32_k/or_jmp32_k: bit ops + branch on unknown value:OK
  #492     verifier_or_jmp32_k:OK
  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: John Fastabend <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Alexei Starovoitov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant