Skip to content

Conversation

@marco-a-itl
Copy link
Contributor

@marco-a-itl marco-a-itl commented Nov 28, 2025

Add error parameter to subflow_closed callback to allow plugins to identify the reason for subflow closure, such as network errors.

The error value corresponds to the MPTCP_ATTR_ERROR attribute from the kernel, which equals sk_err from struct sock. If the error attribute is not present in the Netlink message, it defaults to 0.

Fixes #330

Changes

  • Updated mptcpd_plugin_ops.subflow_closed signature to include uint8_t error parameter
  • Modified all plugins and tests to use the new signature
  • Added test validation for the error parameter

Add error parameter to subflow_closed callback to allow plugins
to identify the reason for subflow closure, such as network errors.

The error value corresponds to the MPTCP_ATTR_ERROR attribute from
the kernel, which equals sk_err from struct sock. If the error
attribute is not present in the netlink message, it defaults to 0.

Fixes multipath-tcp#330
@coveralls
Copy link

Pull Request Test Coverage Report for Build 19768471729

Details

  • 1 of 2 (50.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 67.014%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/path_manager.c 0 1 0.0%
Totals Coverage Status
Change from base Build 19644312326: -0.03%
Covered Lines: 1481
Relevant Lines: 2210

💛 - Coveralls

Copy link
Member

@matttbe matttbe left a comment

Choose a reason for hiding this comment

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

Thank you for having looked at that!

I just have two small suggestions if you don't mind.

static bool const test_backup_4 = true;
static bool const test_server_side_4 = false;
static bool const test_deny_join_id0_4 = false;
static uint8_t const test_error_4 = 0;
Copy link
Member

Choose a reason for hiding this comment

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

small detail: maybe good to put a non 0 value for one of them, just to check something different, no? That will not change much, but just by principle, just in case?

* @param[in] laddr Local address information.
* @param[in] raddr Remote address information.
* @param[in] backup Backup priority flag.
* @param[in] error Subflow related event error.
Copy link
Member

Choose a reason for hiding this comment

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

small detail, do you mind adding a bit more details, e.g. there might not be any error, it is an errno, and the value is a copy of sk_err.

Something like that?

Subflow closing error, if any. The 'errno'
set in 'sk_err', e.g. reset, timeout, etc.

Do you mind adding that (or something similar) in include/mptcpd/private/plugin.h as well please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

expose pm_event_attrs->error to plugins when subflow is closed

3 participants