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

PM: create new subflow after one got closed #235

Closed
matttbe opened this issue Oct 13, 2021 · 2 comments
Closed

PM: create new subflow after one got closed #235

matttbe opened this issue Oct 13, 2021 · 2 comments
Assignees

Comments

@matttbe
Copy link
Member

matttbe commented Oct 13, 2021

It looks interesting to have the in-kernel PM checking if we need to create a new subflow once one has been closed.

A few reasons for that:

  • maybe other subflows didn't get created in the past because we were at the limit (subflows_max)
  • maybe the subflow can be re-established if it has been closed due to an error, after a NAT timeout
  • (maybe we want to retry later, e.g. we got a RST and we might want to retry later → maybe more something for the userspace PM?)

This has been discussed on the ML with @pabeni, see: https://lore.kernel.org/mptcp/d4be3022ec8e5087a1a336e9c487aa4d3b045dfb.camel@redhat.com/T/#mcedd98703c3280f107f94f86ef8925441fcc1ede

@pabeni pabeni self-assigned this Nov 18, 2021
@pgreenland
Copy link

Follow on from initial discussion on the mailing list: Original Mailing List Email, see extract below covering issue description.

Our current use case for MPTCP is relative simple. We have a limited number of connections originating from a client embedded system, destined for a server on the internet. The client has multiple WAN links, which go up and down periodically with network conditions. We were hoping to largely transparently maintain a logical link to our server, utilising any and all links when available.

Currently when subflows fail, they aren't re-established, which given our use-case is pretty important.

While it may be better suited for the userspace path manager, a separate application would be overkill for our initial plan. A simple strategy in the kernel would be preferred Potentially with a configurable delay between reconnection attempted and max count.

A related issue we noticed with subflow creation, is that it appears that after the initial connection subflow creation is chained. With the successful creation of one subflow triggering the creation of the next. Until all available endpoints (or subflow limit) is reached. If the first subflow creation fails, no further attempts will be made.

For this problem re-connection attempts would be good, possibly combined with a big bang, all subflows established at once, rather than one at a time.

@matttbe
Copy link
Member Author

matttbe commented Dec 16, 2021

This has been fixed in our export branch thanks to the modifications done by @pabeni:

  • 7dff646: mptcp: fix per socket endpoint accounting

  • b3b3136: mptcp: clean-up MPJ option writing

  • 93ddc71: mptcp: keep track of local endpoint still available foreach msk

  • b3adca8: mptcp: do not block subflows creation on errors

  • 0f4945b: selftests: mptcp: add tests for subflow creation failure

    • 07368dd: selftests: mptcp: apply Mat's comment
  • 1d0e64a: mptcp: cleanup MPJ subflow list handling

  • Results: 8c25dcd..c94ca03

  • bcccc44: mptcp: avoid atomic bit manipulation when possible

  • Results: c85bd12..1fdc9db

@pgreenland do you mind checking this new version? Please re-open this ticket if you still have issues.

@matttbe matttbe closed this as completed Dec 16, 2021
matttbe pushed a commit that referenced this issue Aug 17, 2023
Add several new tcx test cases to improve test coverage. This also includes
a few new tests with ingress instead of clsact qdisc, to cover the fix from
commit dc644b5 ("tcx: Fix splat in ingress_destroy upon tcx_entry_free").

  # ./test_progs -t tc
  [...]
  #234     tc_links_after:OK
  #235     tc_links_append:OK
  #236     tc_links_basic:OK
  #237     tc_links_before:OK
  #238     tc_links_chain_classic:OK
  #239     tc_links_chain_mixed:OK
  #240     tc_links_dev_cleanup:OK
  #241     tc_links_dev_mixed:OK
  #242     tc_links_ingress:OK
  #243     tc_links_invalid:OK
  #244     tc_links_prepend:OK
  #245     tc_links_replace:OK
  #246     tc_links_revision:OK
  #247     tc_opts_after:OK
  #248     tc_opts_append:OK
  #249     tc_opts_basic:OK
  #250     tc_opts_before:OK
  #251     tc_opts_chain_classic:OK
  #252     tc_opts_chain_mixed:OK
  #253     tc_opts_delete_empty:OK
  #254     tc_opts_demixed:OK
  #255     tc_opts_detach:OK
  #256     tc_opts_detach_after:OK
  #257     tc_opts_detach_before:OK
  #258     tc_opts_dev_cleanup:OK
  #259     tc_opts_invalid:OK
  #260     tc_opts_mixed:OK
  #261     tc_opts_prepend:OK
  #262     tc_opts_replace:OK
  #263     tc_opts_revision:OK
  [...]
  Summary: 44/38 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/r/8699efc284b75ccdc51ddf7062fa2370330dc6c0.1692029283.git.daniel@iogearbox.net
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants