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

include: Add MPTCPD_ADDR_FLAG_FULLMESH flag. #252

Merged
merged 8 commits into from
Aug 10, 2022

Conversation

ossama-othman
Copy link
Member

@ossama-othman ossama-othman commented Aug 9, 2022

Add per-endpoint in-kernel path management MPTCPD_ADDR_FLAG_FULLMESH flag, corresponding
to the kernel API flag MPTCP_PM_ADDR_FLAG_FULLMESH.

Add per-endpoint in-kernel path management flags
MPTCPD_ADDR_FLAG_FULLMESH and MPTCPD_ADDR_FLAG_IMPLICIT, corresponding
to the kernel API flags MPTCP_PM_ADDR_FLAG_FULLMESH and
MPTCP_PM_ADDR_FLAG_IMPLICIT, respectively.
@ossama-othman ossama-othman added the enhancement New feature or request label Aug 9, 2022
@ossama-othman ossama-othman self-assigned this Aug 9, 2022
@coveralls
Copy link

coveralls commented Aug 9, 2022

Pull Request Test Coverage Report for Build 2829218048

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 74 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 64.601%

Files with Coverage Reduction New Missed Lines %
src/configuration.c 74 54.72%
Totals Coverage Status
Change from base Build 2821836556: 0.0%
Covered Lines: 1376
Relevant Lines: 2130

💛 - Coveralls

* The in-kernel path manager may implictly create endpoints. Set
* this flag to allow user-provided endpoints to replace them.
*/
#define MPTCPD_ADDR_FLAG_IMPLICIT (1U << 4)
Copy link
Member Author

@ossama-othman ossama-othman Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could use another eye on the MPTCPD_ADDR_FLAG_FULLMESH and MPTCPD_ADDR_FLAG_IMPLICIT flag documentation. Both were based on git commit logs and the implementation of the corresponding kernel flags.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MPTCPD_ADDR_FLAG_IMPLICIT is only set by the kernel when it creates implicit endpoints without userspace involvement.

The flag is not set by userspace when it tries to replace implicit endpoints. So, it only shows up as a possible attribute of a kernel-created endpoint, but it is not used when creating or replacing endpoints.

Copy link
Member

@mjmartineau mjmartineau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments

#define MPTCPD_ADDR_FLAG_SIGNAL (1U << 0)

/// Create a new subflow.
#define MPTCPD_ADDR_FLAG_SUBFLOW (1U << 1)

/// Set backup priority on the subflow.
#define MPTCPD_ADDR_FLAG_BACKUP (1U << 2)

/**
* @brief Add remote address to in-kernel fullmesh path management.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the most complete description of the fullmesh flag is in the iproute2 man ip mptcp page:

   fullmesh
          If this is a subflow endpoint and additional subflow creation is
          allowed by the MPTCP limits, the MPTCP path manager will try to
          create an additional subflow for each known peer address, using
          this endpoint as the source address. This will occur after the
          MPTCP connection is established. If the peer did not announce
          any additional addresses using the MPTCP ADD_ADDR sub-option,
          this will behave the same as a plain subflow endpoint. When the
          peer does announce addresses, each received ADD_ADDR sub-option
          will trigger creation of an additional subflow to generate a
          full mesh topology.

(see https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/man/man8/ip-mptcp.8#n165)

This @brief line should say this local address is the one added to in-kernel fullmesh path management.

/**
* @brief Add remote address to in-kernel fullmesh path management.
*
* Create a subflow for each remote address through the local address.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested rephrasing: If this flag is set, create a subflow connection to each known remote address, originating from this local address. The total number of subflows is subject to the configured limits.

* The in-kernel path manager may implictly create endpoints. Set
* this flag to allow user-provided endpoints to replace them.
*/
#define MPTCPD_ADDR_FLAG_IMPLICIT (1U << 4)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MPTCPD_ADDR_FLAG_IMPLICIT is only set by the kernel when it creates implicit endpoints without userspace involvement.

The flag is not set by userspace when it tries to replace implicit endpoints. So, it only shows up as a possible attribute of a kernel-created endpoint, but it is not used when creating or replacing endpoints.

MPTCP_PM_ADDR_FLAG_IMPLICIT is only used internally by the kernel.
Drop the corresponding mptcpd MPTCPD_ADDR_FLAG_IMPLICIT flag.
@ossama-othman ossama-othman changed the title include: Support FULLMESH and IMPLICIT addr flags. include: Add MPTCPD_ADDR_FLAG_FULLMESH flag. Aug 10, 2022
@ossama-othman
Copy link
Member Author

ossama-othman commented Aug 10, 2022

I updated the MPTCPD_ADDR_FLAG_FULLMESH documentation per your recommendations, and removed MPTCPD_ADDR_FLAG_IMPLICIT.

@ossama-othman ossama-othman merged commit 0670d5f into multipath-tcp:master Aug 10, 2022
@ossama-othman ossama-othman deleted the update-addr-flags branch August 10, 2022 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants