From e8c69d81f331e686f16fd81efe033ea9f50d4677 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 10:41:50 -0800 Subject: [PATCH 01/74] include: Add remaining user space PM commands. --- include/linux/mptcp_upstream.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/linux/mptcp_upstream.h b/include/linux/mptcp_upstream.h index 8e9c3d6c..b20098e0 100644 --- a/include/linux/mptcp_upstream.h +++ b/include/linux/mptcp_upstream.h @@ -50,6 +50,7 @@ enum { MPTCP_PM_ATTR_SUBFLOWS, /* u32 */ MPTCP_PM_ATTR_TOKEN, /* u32 */ MPTCP_PM_ATTR_LOC_ID, /* u8 */ + MPTCP_PM_ATTR_ADDR_REMOTE, /* nested address */ __MPTCP_PM_ATTR_MAX }; @@ -86,8 +87,10 @@ enum { MPTCP_PM_CMD_SET_LIMITS, MPTCP_PM_CMD_GET_LIMITS, MPTCP_PM_CMD_SET_FLAGS, - MPTCP_PM_CMD_ANNOUNCE, - MPTCP_PM_CMD_REMOVE, + MPTCP_PM_CMD_ANNOUNCE, + MPTCP_PM_CMD_REMOVE, + MPTCP_PM_CMD_SUBFLOW_CREATE, + MPTCP_PM_CMD_SUBFLOW_DESTROY, __MPTCP_PM_CMD_AFTER_LAST }; From a93fd4ea630c6dd3727978923327e03b281e568f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 10:46:06 -0800 Subject: [PATCH 02/74] src: Conform to existing naming convention. --- src/netlink_pm_upstream.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 0aef32e5..eb080998 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -42,10 +42,10 @@ #endif -static int upstream_cmd_announce(struct mptcpd_pm *pm, - struct sockaddr const *addr, - mptcpd_aid_t id, - mptcpd_token_t token) +static int upstream_announce(struct mptcpd_pm *pm, + struct sockaddr const *addr, + mptcpd_aid_t id, + mptcpd_token_t token) { (void) pm; (void) addr; @@ -55,9 +55,9 @@ static int upstream_cmd_announce(struct mptcpd_pm *pm, return ENOTSUP; } -static int upstream_cmd_remove(struct mptcpd_pm *pm, - mptcpd_aid_t address_id, - mptcpd_token_t token) +static int upstream_remove(struct mptcpd_pm *pm, + mptcpd_aid_t address_id, + mptcpd_token_t token) { (void) pm; (void) address_id; @@ -788,8 +788,8 @@ static int upstream_set_flags(struct mptcpd_pm *pm, static struct mptcpd_pm_cmd_ops const cmd_ops = { - .add_addr = upstream_cmd_announce, - .remove_addr = upstream_cmd_remove, + .add_addr = upstream_announce, + .remove_addr = upstream_remove, .add_subflow = upstream_add_subflow, .remove_subflow = upstream_remove_subflow, .set_backup = upstream_set_backup, From 2e3fe86ec8bb93ad7c6886e606174cf3ac244feb Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 10:51:47 -0800 Subject: [PATCH 03/74] src: Organize kernel/user space PM code. --- src/netlink_pm_upstream.c | 158 +++++++++++++++++++------------------- 1 file changed, 81 insertions(+), 77 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index eb080998..3709b0bf 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -42,6 +42,85 @@ #endif +// -------------------------------------------------------------- +// Common Utility Functions +// -------------------------------------------------------------- +/** + * @brief Initialize a @c struct @c mptcpd_addr_info instance. + * + * Initialize a @c addr_info instance with the provided IPv4 or + * IPv6 address. Only one is required and used. The @a port, @a id, + * @a flags, and @a index are optional and may be set to @c NULL if + * not used. + * + * @param[in] addr4 IPv4 internet address. + * @param[in] addr6 IPv6 internet address. + * @param[in] port IP port. + * @param[in] id Address ID. + * @param[in] flags MPTCP flags. + * @param[in] index Network interface index. + * @param[in,out] addr mptcpd network address information. + * + * @note This function is mostly meant for internal use. + * + * @return @c true on success. @c false otherwise. + */ +static bool mptcpd_addr_info_init(struct in_addr const *addr4, + struct in6_addr const *addr6, + in_port_t const *port, + uint8_t const *id, + uint32_t const *flags, + int32_t const *index, + struct mptcpd_addr_info *info) +{ + if (info == NULL + || !mptcpd_sockaddr_storage_init(addr4, + addr6, + port ? *port : 0, + &info->addr)) + return false; + + info->id = (id ? *id : 0); + info->flags = (flags ? *flags : 0); + info->index = (index ? *index : 0); + + return true; +} + + +static bool append_addr_attr(struct l_genl_msg *msg, + struct sockaddr const *addr) +{ + assert(mptcpd_is_inet_family(addr)); + + uint16_t type = 0; + uint16_t len = 0; + void const *data = NULL; + + if (addr->sa_family == AF_INET) { + type = MPTCP_PM_ADDR_ATTR_ADDR4; + + struct sockaddr_in const *const addr4 = + (struct sockaddr_in *) addr; + + data = &addr4->sin_addr; + len = sizeof(addr4->sin_addr); + } else { + type = MPTCP_PM_ADDR_ATTR_ADDR6; + + struct sockaddr_in6 const *const addr6 = + (struct sockaddr_in6 *) addr; + + data = &addr6->sin6_addr; + len = sizeof(addr6->sin6_addr); + } + + return l_genl_msg_append_attr(msg, type, len, data); +} + +// -------------------------------------------------------------- +// User Space Path Manager Related Functions +// -------------------------------------------------------------- static int upstream_announce(struct mptcpd_pm *pm, struct sockaddr const *addr, mptcpd_aid_t id, @@ -114,7 +193,8 @@ static int upstream_set_backup(struct mptcpd_pm *pm, } // -------------------------------------------------------------- - +// Kernel Path Manager Related Functions +// -------------------------------------------------------------- /** * @struct get_addr_user_callback * @@ -156,50 +236,6 @@ struct get_limits_user_callback void *data; }; -// ----------------------------------------------------------------------- - -/** - * @brief Initialize a @c struct @c mptcpd_addr_info instance. - * - * Initialize a @c addr_info instance with the provided IPv4 or - * IPv6 address. Only one is required and used. The @a port, @a id, - * @a flags, and @a index are optional and may be set to @c NULL if - * not used. - * - * @param[in] addr4 IPv4 internet address (network byte order). - * @param[in] addr6 IPv6 internet address. - * @param[in] port IP port (host byte order). - * @param[in] id Address ID. - * @param[in] flags MPTCP flags. - * @param[in] index Network interface index. - * @param[in,out] addr mptcpd network address information. - * - * @note This function is mostly meant for internal use. - * - * @return @c true on success. @c false otherwise. - */ -static bool mptcpd_addr_info_init(in_addr_t const *addr4, - struct in6_addr const *addr6, - in_port_t const *port, - uint8_t const *id, - uint32_t const *flags, - int32_t const *index, - struct mptcpd_addr_info *info) -{ - if (info == NULL - || !mptcpd_sockaddr_storage_init(addr4, - addr6, - port ? htons(*port) : 0, - &info->addr)) - return false; - - info->id = (id ? *id : 0); - info->flags = (flags ? *flags : 0); - info->index = (index ? *index : 0); - - return true; -} - static bool get_addr_callback_recurse(struct l_genl_attr *attr, struct mptcpd_addr_info *info) { @@ -330,38 +366,6 @@ static void get_addr_user_callback_free(void *data) l_free(cb); } -static bool append_addr_attr(struct l_genl_msg *msg, - struct sockaddr const *addr) -{ - assert(mptcpd_is_inet_family(addr)); - - uint16_t type = 0; - uint16_t len = 0; - void const *data = NULL; - - if (addr->sa_family == AF_INET) { - type = MPTCP_PM_ADDR_ATTR_ADDR4; - - struct sockaddr_in const *const addr4 = - (struct sockaddr_in *) addr; - - data = &addr4->sin_addr.s_addr; - len = sizeof(addr4->sin_addr.s_addr); - } else { - type = MPTCP_PM_ADDR_ATTR_ADDR6; - - struct sockaddr_in6 const *const addr6 = - (struct sockaddr_in6 *) addr; - - data = &addr6->sin6_addr; - len = sizeof(addr6->sin6_addr); - } - - return l_genl_msg_append_attr(msg, type, len, data); -} - -// -------------------------------------------------------------- - static uint16_t kernel_to_mptcpd_limit(uint16_t type) { // Translate from kernel to mptcpd MPTCP limit. From 7006b41c15a8f9e6951006f6da687e60c35de39f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 11:27:54 -0800 Subject: [PATCH 04/74] src: Add support for MPTCP_PM_CMD_ANNOUNCE. --- src/netlink_pm_upstream.c | 216 +++++++++++++++++++++++--------------- 1 file changed, 130 insertions(+), 86 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 3709b0bf..5e16f8a4 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -118,6 +118,110 @@ static bool append_addr_attr(struct l_genl_msg *msg, return l_genl_msg_append_attr(msg, type, len, data); } +struct add_addr_info +{ + uint8_t cmd; + char const *const cmd_name; + struct sockaddr const *const addr; + mptcpd_aid_t id; + mptcpd_token_t token; + uint32_t flags; + int32_t ifindex; +}; + +static int send_add_addr(struct mptcpd_pm *pm, + struct add_addr_info *info) +{ + assert(info->cmd == MPTCP_PM_CMD_ANNOUNCE + || info->cmd == MPTCP_PM_CMD_ADD_ADDR); + + /* + Payload (nested): + Local address family + Local address + Local port (optional) + Local address ID (optional) + Token (required for user space MPTCP_PM_CMD_ANNOUNCE) + Flags (optional) + Network inteface index (optional) + */ + + // Types chosen to match MPTCP genl API. + uint16_t const family = mptcpd_get_addr_family(info->addr); + uint16_t const port = mptcpd_get_port_number(info->addr); + + /* + The MPTCP_PM_ADDR_FLAG_SIGNAL flag is required when a port + is specified. Make sure it is set. + */ + if (port != 0) + info->flags |= MPTCP_PM_ADDR_FLAG_SIGNAL; + + size_t const payload_size = + MPTCPD_NLA_ALIGN(family) + + MPTCPD_NLA_ALIGN_ADDR(info->addr) + + MPTCPD_NLA_ALIGN_OPT(port) + + MPTCPD_NLA_ALIGN_OPT(info->id) + + MPTCPD_NLA_ALIGN_OPT(info->token) + + MPTCPD_NLA_ALIGN_OPT(info->flags) + + MPTCPD_NLA_ALIGN_OPT(info->ifindex); + + struct l_genl_msg *const msg = + l_genl_msg_new_sized(info->cmd, payload_size); + + bool const appended = + l_genl_msg_enter_nested(msg, + NLA_F_NESTED | MPTCP_PM_ATTR_ADDR) + && l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_FAMILY, + sizeof(family), // sizeof(uint16_t) + &family) + && append_addr_attr(msg, info->addr) + && (port == 0 || + l_genl_msg_append_attr(msg, + MPTCP_PM_ADDR_ATTR_PORT, + sizeof(port), // sizeof(uint16_t) + &port)) + && (info->id == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_ID, + sizeof(info->id), // sizeof(uint8_t) + &info->id)) + && (info->flags == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_FLAGS, + sizeof(info->flags), // sizeof(uint32_t) + &info->flags)) + && (info->ifindex == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_IF_IDX, + sizeof(info->ifindex), // sizeof(int32_t) + &info->ifindex)) + && l_genl_msg_leave_nested(msg) + && (info->token == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ATTR_TOKEN, + sizeof(info->token), // sizeof(uint32_t) + &info->token)); + + if (!appended) { + l_genl_msg_unref(msg); + + return ENOMEM; + } + + return l_genl_family_send(pm->family, + msg, + mptcpd_family_send_callback, + (void *) info->cmd_name, /* user data */ + NULL /* destroy */) == 0; +} + // -------------------------------------------------------------- // User Space Path Manager Related Functions // -------------------------------------------------------------- @@ -126,12 +230,21 @@ static int upstream_announce(struct mptcpd_pm *pm, mptcpd_aid_t id, mptcpd_token_t token) { - (void) pm; - (void) addr; - (void) id; - (void) token; - - return ENOTSUP; + /** + * @todo Add support for the optional network interface index + * attribute. + */ + struct add_addr_info info = { + .cmd = MPTCP_PM_CMD_ANNOUNCE, + .cmd_name = "announce", + .addr = addr, + .id = id, + .token = token, + .flags = MPTCP_PM_ADDR_FLAG_SIGNAL, + // .ifindex = ... + }; + + return send_add_addr(pm, &info); } static int upstream_remove(struct mptcpd_pm *pm, @@ -451,91 +564,22 @@ static void get_limits_callback(struct l_genl_msg *msg, void *user_data) l_free(limits); } -// -------------------------------------------------------------- - static int upstream_add_addr(struct mptcpd_pm *pm, struct sockaddr const *addr, - mptcpd_aid_t address_id, + mptcpd_aid_t id, uint32_t flags, int index) { - /* - Payload (nested): - Local address family - Local address (network byte order if IPv4 address) - Local port (host byte order) (optional) - Local address ID (optional) - Flags (optional) - Network inteface index (optional) - */ - - // Types chosen to match MPTCP genl API. - uint16_t const family = mptcpd_get_addr_family(addr); - uint16_t const port = mptcpd_get_port_number(addr); - - /* - The MPTCP_PM_ADDR_FLAG_SIGNAL flag is required when a port - is specified. Make sure it is set. - */ - if (port != 0) - flags |= MPTCP_PM_ADDR_FLAG_SIGNAL; - - size_t const payload_size = - MPTCPD_NLA_ALIGN(family) - + MPTCPD_NLA_ALIGN_ADDR(addr) - + MPTCPD_NLA_ALIGN_OPT(port) - + MPTCPD_NLA_ALIGN_OPT(address_id) - + MPTCPD_NLA_ALIGN_OPT(flags) - + MPTCPD_NLA_ALIGN_OPT(index); - - struct l_genl_msg *const msg = - l_genl_msg_new_sized(MPTCP_PM_CMD_ADD_ADDR, payload_size); - - bool const appended = - l_genl_msg_enter_nested(msg, - NLA_F_NESTED | MPTCP_PM_ATTR_ADDR) - && l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_FAMILY, - sizeof(family), // sizeof(uint16_t) - &family) - && append_addr_attr(msg, addr) - && (port == 0 || - l_genl_msg_append_attr(msg, - MPTCP_PM_ADDR_ATTR_PORT, - sizeof(port), // sizeof(uint16_t) - &port)) - && (address_id == 0 - || l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_ID, - sizeof(address_id), // sizeof(uint8_t) - &address_id)) - && (flags == 0 - || l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_FLAGS, - sizeof(flags), // sizeof(uint32_t) - &flags)) - && (index == 0 - || l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_IF_IDX, - sizeof(index), // sizeof(int32_t) - &index)) - && l_genl_msg_leave_nested(msg); - - if (!appended) { - l_genl_msg_unref(msg); - - return ENOMEM; - } - - return l_genl_family_send(pm->family, - msg, - mptcpd_family_send_callback, - "add_addr", /* user data */ - NULL /* destroy */) == 0; + struct add_addr_info info = { + .cmd = MPTCP_PM_CMD_ADD_ADDR, + .cmd_name = "add_addr", + .addr = addr, + .id = id, + .flags = flags, + .ifindex = index + }; + + return send_add_addr(pm, &info); } static int upstream_remove_addr(struct mptcpd_pm *pm, From 2983e45d00c2eb43e29432b890e90964b22b7fca Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 11:41:03 -0800 Subject: [PATCH 05/74] src: Add support for MPTCP_PM_CMD_REMOVE. --- src/netlink_pm_upstream.c | 47 ++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 5e16f8a4..be965cb5 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -248,14 +248,51 @@ static int upstream_announce(struct mptcpd_pm *pm, } static int upstream_remove(struct mptcpd_pm *pm, - mptcpd_aid_t address_id, + mptcpd_aid_t id, mptcpd_token_t token) { - (void) pm; - (void) address_id; - (void) token; + /** + * @todo Refactor upstream_remove() and + * mptcp_org_remove_addr() functions. They only differ + * by command and attribute types. + */ - return ENOTSUP; + /* + Payload: + Token + Local address ID + */ + + size_t const payload_size = + MPTCPD_NLA_ALIGN(token) + + MPTCPD_NLA_ALIGN(id); + + struct l_genl_msg *const msg = + l_genl_msg_new_sized(MPTCP_PM_CMD_REMOVE, payload_size); + + bool const appended = + l_genl_msg_append_attr(msg, + MPTCP_PM_ATTR_TOKEN, + sizeof(token), + &token) + && l_genl_msg_append_attr( + msg, + MPTCP_PM_ATTR_LOC_ID, + sizeof(id), + &id); + + if (!appended) { + l_genl_msg_unref(msg); + + return ENOMEM; + } + + return l_genl_family_send(pm->family, + msg, + mptcpd_family_send_callback, + "remove_addr", /* user data */ + NULL /* destroy */) + == 0; } static int upstream_add_subflow(struct mptcpd_pm *pm, From 026c1aeb041199403058df12eccd19a9b5d4a76c Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 23:12:24 -0800 Subject: [PATCH 06/74] src: Add support for MPTCP_PM_CMD_SUBFLOW_CREATE. --- src/netlink_pm_upstream.c | 253 ++++++++++++++++++++++++-------------- 1 file changed, 163 insertions(+), 90 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index be965cb5..677e469c 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -87,10 +87,20 @@ static bool mptcpd_addr_info_init(struct in_addr const *addr4, return true; } +struct addr_info +{ + uint8_t cmd; + char const *const cmd_name; + struct sockaddr const *const addr; + mptcpd_aid_t id; + uint32_t flags; + int32_t ifindex; +}; -static bool append_addr_attr(struct l_genl_msg *msg, - struct sockaddr const *addr) +static bool append_ip(struct l_genl_msg *msg, struct addr_info *info) { + struct sockaddr const *const addr = info->addr; + assert(mptcpd_is_inet_family(addr)); uint16_t type = 0; @@ -118,22 +128,75 @@ static bool append_addr_attr(struct l_genl_msg *msg, return l_genl_msg_append_attr(msg, type, len, data); } -struct add_addr_info +static bool append_addr_attr(struct l_genl_msg *msg, + struct addr_info *info, + uint16_t nested_type) { - uint8_t cmd; - char const *const cmd_name; - struct sockaddr const *const addr; - mptcpd_aid_t id; - mptcpd_token_t token; - uint32_t flags; - int32_t ifindex; -}; + assert(nested_type == MPTCP_PM_ATTR_ADDR + || nested_type == MPTCP_PM_ATTR_ADDR_REMOTE); + + // Types chosen to match MPTCP genl API. + uint16_t const family = mptcpd_get_addr_family(info->addr); + uint16_t const port = mptcpd_get_port_number(info->addr); + + return l_genl_msg_enter_nested(msg, + NLA_F_NESTED | nested_type) + && l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_FAMILY, + sizeof(family), // sizeof(uint16_t) + &family) + && append_ip(msg, info) + && (port == 0 || + l_genl_msg_append_attr(msg, + MPTCP_PM_ADDR_ATTR_PORT, + sizeof(port), // sizeof(uint16_t) + &port)) + && (info->id == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_ID, + sizeof(info->id), // sizeof(uint8_t) + &info->id)) + && (info->flags == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_FLAGS, + sizeof(info->flags), // sizeof(uint32_t) + &info->flags)) + && (info->ifindex == 0 + || l_genl_msg_append_attr( + msg, + MPTCP_PM_ADDR_ATTR_IF_IDX, + sizeof(info->ifindex), // sizeof(int32_t) + &info->ifindex)) + && l_genl_msg_leave_nested(msg); +} + +static bool append_local_addr_attr(struct l_genl_msg *msg, + struct addr_info *info) +{ + static uint16_t const nested_type = MPTCP_PM_ATTR_ADDR; + + return append_addr_attr(msg, info, nested_type); +} + +static bool append_remote_addr_attr(struct l_genl_msg *msg, + struct addr_info *info) +{ + static uint16_t const nested_type = MPTCP_PM_ATTR_ADDR_REMOTE; + + return append_addr_attr(msg, info, nested_type); +} static int send_add_addr(struct mptcpd_pm *pm, - struct add_addr_info *info) + uint8_t cmd, + char const *cmd_name, + struct addr_info *info, + mptcpd_token_t token) { - assert(info->cmd == MPTCP_PM_CMD_ANNOUNCE - || info->cmd == MPTCP_PM_CMD_ADD_ADDR); + assert(cmd == MPTCP_PM_CMD_ANNOUNCE + || cmd == MPTCP_PM_CMD_ADD_ADDR); /* Payload (nested): @@ -162,52 +225,21 @@ static int send_add_addr(struct mptcpd_pm *pm, + MPTCPD_NLA_ALIGN_ADDR(info->addr) + MPTCPD_NLA_ALIGN_OPT(port) + MPTCPD_NLA_ALIGN_OPT(info->id) - + MPTCPD_NLA_ALIGN_OPT(info->token) + MPTCPD_NLA_ALIGN_OPT(info->flags) - + MPTCPD_NLA_ALIGN_OPT(info->ifindex); + + MPTCPD_NLA_ALIGN_OPT(info->ifindex) + + MPTCPD_NLA_ALIGN_OPT(token); struct l_genl_msg *const msg = l_genl_msg_new_sized(info->cmd, payload_size); bool const appended = - l_genl_msg_enter_nested(msg, - NLA_F_NESTED | MPTCP_PM_ATTR_ADDR) - && l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_FAMILY, - sizeof(family), // sizeof(uint16_t) - &family) - && append_addr_attr(msg, info->addr) - && (port == 0 || - l_genl_msg_append_attr(msg, - MPTCP_PM_ADDR_ATTR_PORT, - sizeof(port), // sizeof(uint16_t) - &port)) - && (info->id == 0 - || l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_ID, - sizeof(info->id), // sizeof(uint8_t) - &info->id)) - && (info->flags == 0 - || l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_FLAGS, - sizeof(info->flags), // sizeof(uint32_t) - &info->flags)) - && (info->ifindex == 0 - || l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_IF_IDX, - sizeof(info->ifindex), // sizeof(int32_t) - &info->ifindex)) - && l_genl_msg_leave_nested(msg) - && (info->token == 0 + append_local_addr_attr(msg, info) + && (token == 0 || l_genl_msg_append_attr( msg, MPTCP_PM_ATTR_TOKEN, - sizeof(info->token), // sizeof(uint32_t) - &info->token)); + sizeof(token), // sizeof(uint32_t) + &token)); if (!appended) { l_genl_msg_unref(msg); @@ -218,7 +250,7 @@ static int send_add_addr(struct mptcpd_pm *pm, return l_genl_family_send(pm->family, msg, mptcpd_family_send_callback, - (void *) info->cmd_name, /* user data */ + (void *) cmd_name, /* user data */ NULL /* destroy */) == 0; } @@ -234,17 +266,18 @@ static int upstream_announce(struct mptcpd_pm *pm, * @todo Add support for the optional network interface index * attribute. */ - struct add_addr_info info = { - .cmd = MPTCP_PM_CMD_ANNOUNCE, - .cmd_name = "announce", + struct addr_info info = { .addr = addr, .id = id, - .token = token, .flags = MPTCP_PM_ADDR_FLAG_SIGNAL, // .ifindex = ... }; - return send_add_addr(pm, &info); + return send_add_addr(pm, + MPTCP_PM_CMD_ANNOUNCE, + "announce", + &info, + token); } static int upstream_remove(struct mptcpd_pm *pm, @@ -297,21 +330,70 @@ static int upstream_remove(struct mptcpd_pm *pm, static int upstream_add_subflow(struct mptcpd_pm *pm, mptcpd_token_t token, - mptcpd_aid_t local_address_id, - mptcpd_aid_t remote_address_id, + mptcpd_aid_t local_id, + mptcpd_aid_t remote_id, struct sockaddr const *local_addr, struct sockaddr const *remote_addr, bool backup) { - (void) pm; - (void) token; - (void) local_address_id; - (void) remote_address_id; - (void) local_addr; - (void) remote_addr; + (void) remote_id; (void) backup; - return ENOTSUP; + /* + Payload (nested): + Token + Local address ID + Local address family + Local address + Local port + Remote address family + Remote address + Remote port + + */ + struct addr_info local = { + .addr = local_addr, + .id = local_id + }; + + struct addr_info remote = { + .addr = remote_addr, + }; + + size_t const payload_size = + MPTCPD_NLA_ALIGN(token) + + MPTCPD_NLA_ALIGN(local_id) + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // local family + + MPTCPD_NLA_ALIGN_ADDR(local_addr) + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // local port + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // remote family + + MPTCPD_NLA_ALIGN_ADDR(remote_addr) + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)); // remote port + + struct l_genl_msg *const msg = + l_genl_msg_new_sized(MPTCP_PM_CMD_SUBFLOW_CREATE, + payload_size); + + bool const appended = + l_genl_msg_append_attr( + msg, + MPTCP_PM_ATTR_TOKEN, + sizeof(token), // sizeof(uint32_t) + &token) + && append_local_addr_attr(msg, &local) + && append_remote_addr_attr(msg, &remote); + + if (!appended) { + l_genl_msg_unref(msg); + + return ENOMEM; + } + + return l_genl_family_send(pm->family, + msg, + mptcpd_family_send_callback, + "add_subflow", /* user data */ + NULL /* destroy */) == 0; } static int upstream_remove_subflow(struct mptcpd_pm *pm, @@ -607,16 +689,20 @@ static int upstream_add_addr(struct mptcpd_pm *pm, uint32_t flags, int index) { - struct add_addr_info info = { - .cmd = MPTCP_PM_CMD_ADD_ADDR, - .cmd_name = "add_addr", + struct addr_info info = { .addr = addr, .id = id, .flags = flags, .ifindex = index }; - return send_add_addr(pm, &info); + static uint32_t const token = 0; // Unused + + return send_add_addr(pm, + MPTCP_PM_CMD_ADD_ADDR, + "add_addr", + &info, + token); } static int upstream_remove_addr(struct mptcpd_pm *pm, @@ -828,11 +914,14 @@ static int upstream_set_flags(struct mptcpd_pm *pm, Flags */ - // Types chosen to match MPTCP genl API. - uint16_t const family = mptcpd_get_addr_family(addr); + struct addr_info info = { + .addr = addr, + .flags = flags + }; + // Types chosen to match MPTCP genl API. size_t const payload_size = - MPTCPD_NLA_ALIGN(family) + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // family + MPTCPD_NLA_ALIGN_ADDR(addr) + MPTCPD_NLA_ALIGN(flags); @@ -840,23 +929,7 @@ static int upstream_set_flags(struct mptcpd_pm *pm, l_genl_msg_new_sized(MPTCP_PM_CMD_SET_FLAGS, payload_size); - bool const appended = - l_genl_msg_enter_nested(msg, - NLA_F_NESTED | MPTCP_PM_ATTR_ADDR) - && l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_FAMILY, - sizeof(family), // sizeof(uint16_t) - &family) - && append_addr_attr(msg, addr) - && l_genl_msg_append_attr( - msg, - MPTCP_PM_ADDR_ATTR_FLAGS, - sizeof(flags), // sizeof(uint32_t) - &flags) - && l_genl_msg_leave_nested(msg); - - if (!appended) { + if (!append_local_addr_attr(msg, &info)) { l_genl_msg_unref(msg); return ENOMEM; From d7629c727006c7903c96f62f3185b0a587b4e058 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 23:20:44 -0800 Subject: [PATCH 07/74] src: Add support for MPTCP_PM_CMD_SUBFLOW_DESTROY. --- src/netlink_pm_upstream.c | 68 +++++++++++++++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 7 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 677e469c..c065ee8f 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -345,12 +345,16 @@ static int upstream_add_subflow(struct mptcpd_pm *pm, Local address ID Local address family Local address - Local port Remote address family Remote address Remote port */ + + /** + * @todo The local port isn't used. Should we explicitly set it + * to zero, or at least issue a diagnostic if it isn't zero? + */ struct addr_info local = { .addr = local_addr, .id = local_id @@ -365,7 +369,6 @@ static int upstream_add_subflow(struct mptcpd_pm *pm, + MPTCPD_NLA_ALIGN(local_id) + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // local family + MPTCPD_NLA_ALIGN_ADDR(local_addr) - + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // local port + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // remote family + MPTCPD_NLA_ALIGN_ADDR(remote_addr) + MPTCPD_NLA_ALIGN(sizeof(uint16_t)); // remote port @@ -401,12 +404,63 @@ static int upstream_remove_subflow(struct mptcpd_pm *pm, struct sockaddr const *local_addr, struct sockaddr const *remote_addr) { - (void) pm; - (void) token; - (void) local_addr; - (void) remote_addr; + /* + Payload (nested): + Token + Local address family + Local address + Remote port + Remote address family + Remote address + Remote port - return ENOTSUP; + */ + + /** + * @todo The local port isn't used. Should we explicitly set it + * to zero, or at least issue a diagnostic if it isn't zero? + */ + struct addr_info local = { + .addr = local_addr, + }; + + struct addr_info remote = { + .addr = remote_addr, + }; + + size_t const payload_size = + MPTCPD_NLA_ALIGN(token) + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // local family + + MPTCPD_NLA_ALIGN_ADDR(local_addr) + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // local port + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)) // remote family + + MPTCPD_NLA_ALIGN_ADDR(remote_addr) + + MPTCPD_NLA_ALIGN(sizeof(uint16_t)); // remote port + + struct l_genl_msg *const msg = + l_genl_msg_new_sized(MPTCP_PM_CMD_SUBFLOW_DESTROY, + payload_size); + + bool const appended = + l_genl_msg_append_attr( + msg, + MPTCP_PM_ATTR_TOKEN, + sizeof(token), // sizeof(uint32_t) + &token) + && append_local_addr_attr(msg, &local) + && append_remote_addr_attr(msg, &remote); + + if (!appended) { + l_genl_msg_unref(msg); + + return ENOMEM; + } + + return l_genl_family_send(pm->family, + msg, + mptcpd_family_send_callback, + "remove_subflow", /* user data */ + NULL /* destroy */) == 0; } static int upstream_set_backup(struct mptcpd_pm *pm, From eb9018bf881b8f14aef215c330e919f4ef0eb20c Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 23:24:25 -0800 Subject: [PATCH 08/74] src: Remove unused private fields. --- src/netlink_pm_upstream.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index c065ee8f..5589fc01 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -89,8 +89,6 @@ static bool mptcpd_addr_info_init(struct in_addr const *addr4, struct addr_info { - uint8_t cmd; - char const *const cmd_name; struct sockaddr const *const addr; mptcpd_aid_t id; uint32_t flags; @@ -230,7 +228,7 @@ static int send_add_addr(struct mptcpd_pm *pm, + MPTCPD_NLA_ALIGN_OPT(token); struct l_genl_msg *const msg = - l_genl_msg_new_sized(info->cmd, payload_size); + l_genl_msg_new_sized(cmd, payload_size); bool const appended = append_local_addr_attr(msg, info) From f86445ede27fa4ee059a783e3d5fb1dacd5996d6 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 23:33:54 -0800 Subject: [PATCH 09/74] src: Correct code indentation. --- src/netlink_pm_upstream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 5589fc01..fea14171 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -168,7 +168,7 @@ static bool append_addr_attr(struct l_genl_msg *msg, MPTCP_PM_ADDR_ATTR_IF_IDX, sizeof(info->ifindex), // sizeof(int32_t) &info->ifindex)) - && l_genl_msg_leave_nested(msg); + && l_genl_msg_leave_nested(msg); } static bool append_local_addr_attr(struct l_genl_msg *msg, From 9fe1a3555734a58cfebe7042d7c08c1f964db4a3 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 14 Jan 2022 23:36:45 -0800 Subject: [PATCH 10/74] src: Correct comments. --- src/netlink_pm_upstream.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index fea14171..f3ebd61a 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -407,17 +407,13 @@ static int upstream_remove_subflow(struct mptcpd_pm *pm, Token Local address family Local address - Remote port + Local port Remote address family Remote address Remote port */ - /** - * @todo The local port isn't used. Should we explicitly set it - * to zero, or at least issue a diagnostic if it isn't zero? - */ struct addr_info local = { .addr = local_addr, }; From 2abcd4f2239d1cab9e933e5e8cc873ac47a1b038 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 12 May 2022 10:19:59 -0700 Subject: [PATCH 11/74] include: Update to latest . Update `mptcp_upstream.h' to the latest upstream version of . --- include/linux/mptcp_upstream.h | 56 +++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/include/linux/mptcp_upstream.h b/include/linux/mptcp_upstream.h index b20098e0..92196358 100644 --- a/include/linux/mptcp_upstream.h +++ b/include/linux/mptcp_upstream.h @@ -4,6 +4,13 @@ #include #include +#include /* for sockaddr_in */ +#include /* for sockaddr_in6 */ +#include /* for sockaddr_storage and sa_family */ + +#ifndef __KERNEL__ +#include /* for struct sockaddr */ +#endif #define MPTCP_SUBFLOW_FLAG_MCAP_REM _BITUL(0) #define MPTCP_SUBFLOW_FLAG_MCAP_LOC _BITUL(1) @@ -76,6 +83,8 @@ enum { #define MPTCP_PM_ADDR_FLAG_SIGNAL (1 << 0) #define MPTCP_PM_ADDR_FLAG_SUBFLOW (1 << 1) #define MPTCP_PM_ADDR_FLAG_BACKUP (1 << 2) +#define MPTCP_PM_ADDR_FLAG_FULLMESH (1 << 3) +#define MPTCP_PM_ADDR_FLAG_IMPLICIT (1 << 4) enum { MPTCP_PM_CMD_UNSPEC, @@ -135,19 +144,21 @@ struct mptcp_info { * MPTCP_EVENT_REMOVED: token, rem_id * An address has been lost by the peer. * - * MPTCP_EVENT_SUB_ESTABLISHED: token, family, saddr4 | saddr6, - * daddr4 | daddr6, sport, dport, backup, - * if_idx [, error] + * MPTCP_EVENT_SUB_ESTABLISHED: token, family, loc_id, rem_id, + * saddr4 | saddr6, daddr4 | daddr6, sport, + * dport, backup, if_idx [, error] * A new subflow has been established. 'error' should not be set. * - * MPTCP_EVENT_SUB_CLOSED: token, family, saddr4 | saddr6, daddr4 | daddr6, - * sport, dport, backup, if_idx [, error] + * MPTCP_EVENT_SUB_CLOSED: token, family, loc_id, rem_id, saddr4 | saddr6, + * daddr4 | daddr6, sport, dport, backup, if_idx + * [, error] * A subflow has been closed. An error (copy of sk_err) could be set if an * error has been detected for this subflow. * - * MPTCP_EVENT_SUB_PRIORITY: token, family, saddr4 | saddr6, daddr4 | daddr6, - * sport, dport, backup, if_idx [, error] - * The priority of a subflow has changed. 'error' should not be set. + * MPTCP_EVENT_SUB_PRIORITY: token, family, loc_id, rem_id, saddr4 | saddr6, + * daddr4 | daddr6, sport, dport, backup, if_idx + * [, error] + * The priority of a subflow has changed. 'error' should not be set. */ enum mptcp_event_type { MPTCP_EVENT_UNSPEC = 0, @@ -184,6 +195,7 @@ enum mptcp_event_attr { MPTCP_ATTR_IF_IDX, /* s32 */ MPTCP_ATTR_RESET_REASON,/* u32 */ MPTCP_ATTR_RESET_FLAGS, /* u32 */ + MPTCP_ATTR_SERVER_SIDE, /* u8 */ __MPTCP_ATTR_AFTER_LAST }; @@ -199,4 +211,32 @@ enum mptcp_event_attr { #define MPTCP_RST_EBADPERF 5 #define MPTCP_RST_EMIDDLEBOX 6 +struct mptcp_subflow_data { + __u32 size_subflow_data; /* size of this structure in userspace */ + __u32 num_subflows; /* must be 0, set by kernel */ + __u32 size_kernel; /* must be 0, set by kernel */ + __u32 size_user; /* size of one element in data[] */ +} __attribute__((aligned(8))); + +struct mptcp_subflow_addrs { + union { + __kernel_sa_family_t sa_family; + struct sockaddr sa_local; + struct sockaddr_in sin_local; + struct sockaddr_in6 sin6_local; + struct __kernel_sockaddr_storage ss_local; + }; + union { + struct sockaddr sa_remote; + struct sockaddr_in sin_remote; + struct sockaddr_in6 sin6_remote; + struct __kernel_sockaddr_storage ss_remote; + }; +}; + +/* MPTCP socket options */ +#define MPTCP_INFO 1 +#define MPTCP_TCPINFO 2 +#define MPTCP_SUBFLOW_ADDRS 3 + #endif /* _UAPI_MPTCP_H */ From 50543d74a422eb60f8aa5dc861568122a50655fa Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 12 May 2022 17:36:37 -0700 Subject: [PATCH 12/74] src: Reorder include directives to avoid conflict. In `src/netlink_pm_upstream.c', include before to avoid redefinition of the same symbols. The symbols in question, such as struct in_addr, are found in both and . The former is included in and the latter is included in a recent version of . Reording the include directives allows the C library compatibility logic in included by to detect a previous include of to avoid redefinition of symbols. --- src/netlink_pm_upstream.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index f3ebd61a..dc8a8c9a 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -22,13 +22,13 @@ #include #pragma GCC diagnostic pop -#include +#include +#include #include #include -#include -#include #include #include +#include #include "commands.h" #include "netlink_pm.h" From 1b3f0972c50ac7a77eac75cf39dd453c6ab20d60 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 12 May 2022 17:43:54 -0700 Subject: [PATCH 13/74] src: Display support kernel in "--help" output. Display the kernel supported by mptcpd in the "--help" command line option output, e.g.: Usage: mptcpd [OPTION...] Start the Multipath TCP daemon. ... -?, --help Give this help list --usage Give a short usage message -V, --version Print program version ... Supported Linux kernel: upstream ... --- src/configuration.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/configuration.c b/src/configuration.c index 36296663..ed5d6202 100644 --- a/src/configuration.c +++ b/src/configuration.c @@ -340,7 +340,15 @@ static void set_plugins_to_load(struct mptcpd_config *config, // --------------------------------------------------------------- // Command line options // --------------------------------------------------------------- -static char const doc[] = "Start the Multipath TCP daemon."; +#ifdef HAVE_UPSTREAM_KERNEL +# define MPTCPD_KERNEL "upstream" +#else +# define MPTCPD_KERNEL "multipath-tcp.org" +#endif +static char const doc[] = + "Start the Multipath TCP daemon." + "\v" + "Supported Linux kernel: " MPTCPD_KERNEL; /** * @name Command Line Option Key Values From c1396360f04763e02b8a5fe438f6cc4dec146778 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 14 Jun 2022 18:11:24 -0700 Subject: [PATCH 14/74] src: Fix build regression caused by rebase. --- src/netlink_pm_upstream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index dc8a8c9a..02ab8073 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -65,7 +65,7 @@ * * @return @c true on success. @c false otherwise. */ -static bool mptcpd_addr_info_init(struct in_addr const *addr4, +static bool mptcpd_addr_info_init(in_addr_t const *addr4, struct in6_addr const *addr6, in_port_t const *port, uint8_t const *id, From b47c81fcb694f331e2d80f05429a3e8cceb3de75 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 15 Jun 2022 22:31:51 -0700 Subject: [PATCH 15/74] src: Add a MPTCP listener manager. The MPTCP listener manager maps MPTCP local address IDs to a file descriptor for an open MPTCP listening socket. --- src/Makefile.am | 4 +- src/listener_manager.c | 138 +++++++++++++++++++++++++++++++++++++++++ src/listener_manager.h | 66 ++++++++++++++++++++ 3 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 src/listener_manager.c create mode 100644 src/listener_manager.h diff --git a/src/Makefile.am b/src/Makefile.am index eeb2f027..e9ad3ef1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,7 +20,9 @@ libpath_manager_la_SOURCES = \ netlink_pm.c \ netlink_pm.h \ path_manager.c \ - path_manager.h + path_manager.h \ + listener_manager.c \ + listener_manager.h if HAVE_UPSTREAM_KERNEL libpath_manager_la_SOURCES += netlink_pm_upstream.c diff --git a/src/listener_manager.c b/src/listener_manager.c new file mode 100644 index 00000000..2e9eb5f4 --- /dev/null +++ b/src/listener_manager.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: BSD-3-Clause +/** + * @file listener_manager.c + * + * @brief Map of MPTCP local address ID to listener. + * + * Copyright (c) 2022, Intel Corporation + */ + +#ifdef HAVE_CONFIG_H +# include +#endif + +#include +#include +#include +#include +#include +#include +#include + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +#include +#include +#include +#pragma GCC diagnostic pop + +#include "listener_manager.h" + + +// ---------------------------------------------------------------------- + +struct l_hashmap *mptcpd_lm_create(void) +{ + // Map of MPTCP local address ID to MPTCP listener file + // descriptor. + return l_hashmap_new(); +} + +static void close_listener(void *value) +{ + /* + We don't need to worry about overflow due to casting from + unsigned int since the file descriptor value stored in the + map will never be larger than INT_MAX by design. + */ + int const fd = L_PTR_TO_UINT(value); + + (void) close(fd); +} + + +void mptcpd_lm_destroy(struct l_hashmap *lm) +{ + if (lm == NULL) + return; + + l_hashmap_destroy(lm, close_listener); +} + +bool mptcpd_lm_listen(struct l_hashmap *lm, + mptcpd_aid_t id, + struct sockaddr const *sa) +{ + /** + * @todo Allow id == 0? + */ + if (lm == NULL || id == 0 || sa == NULL) + return false; + + if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) + return false; + +#ifndef IPPROTO_MPTCP +#define IPPROTO_MPTCP IPPROTO_TCP + 256 +#endif + + int const fd = socket(sa->sa_family, SOCK_STREAM, IPPROTO_MPTCP); + if (fd == -1) + l_error("Unable to open MPTCP listener: %s", + strerror(errno)); + + socklen_t const addr_size = + (sa->sa_family == AF_INET + ? sizeof(struct sockaddr_in) + : sizeof(struct sockaddr_in6)); + + if (bind(fd, sa, addr_size) == -1) { + l_error("Unable to bind MPTCP listener: %s", + strerror(errno)); + (void) close(fd); + return false; + } + + if (listen(fd, 0) == -1) { + l_error("Unable to listen on MPTCP socket: %s", + strerror(errno)); + (void) close(fd); + return false; + } + + if (!l_hashmap_insert(lm, L_UINT_TO_PTR(id), L_UINT_TO_PTR(fd))) { + l_error("Unable to map MPTCP address ID %u listener", id); + (void) close(fd); + return false; + } + + return true; +} + +bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id) +{ + /** + * @todo Allow id == 0? + */ + if (lm == NULL || id == 0) + return false; + + void const *const value = l_hashmap_remove(lm, L_UINT_TO_PTR(id)); + + if (value == NULL) { + l_error("No listener for MPTCP address id %u.", id); + return false; + } + + // Value will never exceed INT_MAX. + int const fd = L_PTR_TO_UINT(fd); + + return close(fd) == 0; +} + + +/* + Local Variables: + c-file-style: "linux" + End: +*/ diff --git a/src/listener_manager.h b/src/listener_manager.h new file mode 100644 index 00000000..4dac8286 --- /dev/null +++ b/src/listener_manager.h @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: BSD-3-Clause +/** + * @file listener_manager.h + * + * @brief Map of MPTCP local address ID to listener. + * + * Copyright (c) 2022, Intel Corporation + */ + +#ifndef MPTCPD_LISTENER_MANAGER_H +#define MPTCPD_LISTENER_MANAGER_H + +#include + +#include + + +struct l_hashmap; +struct sockaddr; + +/** + * @brief Create a MPTCP listener manager. + * + * @return Pointer to a MPTCP listener manager on success. @c NULL on + * failure. + */ +struct l_hashmap *mptcpd_lm_create(void); + +/** + * @brief Destroy MPTCP listener manager. + * + * @param[in,out] lm The mptcpd address listener manager object. + */ +void mptcpd_lm_destroy(struct l_hashmap *lm); + +/** + * @brief Listen on the given MPTCP local address. + * + * @param[in] lm The mptcpd address listener manager object. + * @param[in] id The MPTCP local address ID. + * @param[in] sa The MPTCP local address. + * + * @return @c true on success, and @c false on failure. + */ +bool mptcpd_lm_listen(struct l_hashmap *lm, + mptcpd_aid_t id, + struct sockaddr const *sa); + +/** + * @brief Stop listening on a MPTCP local address. + * + * @param[in] lm The mptcpd address listener manager object. + * @param[in] id The MPTCP local address ID. + * + * @return @c true on success, and @c false on failure. + */ +bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id); + +#endif /* MPTCPD_LISTENER_MANAGER_H */ + + +/* + Local Variables: + c-file-style: "linux" + End: +*/ From d45500a41aada3c93f3ec444e2e99332a00652b7 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 15 Jun 2022 22:36:15 -0700 Subject: [PATCH 16/74] src: Create/destroy mptcpd listener manager. --- include/mptcpd/private/path_manager.h | 21 +++++++++++++++++++-- src/path_manager.c | 11 +++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/include/mptcpd/private/path_manager.h b/include/mptcpd/private/path_manager.h index da2ae6e7..4bd8388b 100644 --- a/include/mptcpd/private/path_manager.h +++ b/include/mptcpd/private/path_manager.h @@ -4,7 +4,7 @@ * * @brief mptcpd path manager private interface. * - * Copyright (c) 2017-2021, Intel Corporation + * Copyright (c) 2017-2022, Intel Corporation */ #ifndef MPTCPD_PRIVATE_PATH_MANAGER_H @@ -23,6 +23,7 @@ struct l_genl; struct l_genl_family; struct l_queue; struct l_timeout; +struct l_hashmap; struct mptcpd_netlink_pm; struct mptcpd_addr_info; @@ -86,10 +87,26 @@ struct mptcpd_pm * @brief MPTCP address ID manager. * * Manager that maps IP addresses to MPTCP address IDs, and - * generated IDs as needed.. + * generated IDs as needed. */ struct mptcpd_idm *idm; + /** + * @brief MPTCP listener manager map. + * + * The underlying hash map used by the MPTCP listener manager + * to map a MPTCP local address ID to a MPTCP socket file + * descriptor. + * + * @todo A mptcpd path manager-wide MPTCP listener manager like + * this could be problematic if multiple client-oriented + * plugins attempt to advertise or stop advertising a + * different local addresses with the same MPTCP address ID + * through the mptcpd_pm_add_addr() and + * mptcpd_pm_remove_addr() functions. + */ + struct l_hashmap *lm; + /// List of @c pm_ops_info objects. struct l_queue *event_ops; }; diff --git a/src/path_manager.c b/src/path_manager.c index b0b1ca69..07626873 100644 --- a/src/path_manager.c +++ b/src/path_manager.c @@ -44,6 +44,7 @@ #include #include "path_manager.h" +#include "listener_manager.h" #include "netlink_pm.h" @@ -888,6 +889,15 @@ struct mptcpd_pm *mptcpd_pm_create(struct mptcpd_config const *config) return NULL; } + // Create mptcpd listener manager. + pm->lm = mptcpd_lm_create(); + + if (pm->lm == NULL) { + mptcpd_pm_destroy(pm); + l_error("Unable to create listener manager."); + return NULL; + } + pm->event_ops = l_queue_new(); return pm; @@ -906,6 +916,7 @@ void mptcpd_pm_destroy(struct mptcpd_pm *pm) mptcpd_plugin_unload(pm); l_queue_destroy(pm->event_ops, l_free); + mptcpd_lm_destroy(pm->lm); mptcpd_idm_destroy(pm->idm); mptcpd_nm_destroy(pm->nm); l_timeout_remove(pm->timeout); From 6273bbc6de3ce7d956a6663c92879a29fed00b5c Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 15 Jun 2022 22:36:41 -0700 Subject: [PATCH 17/74] src: Create MPTCP listening sockets as needed. --- src/netlink_pm_upstream.c | 55 ++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 02ab8073..4d1562b5 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -32,6 +32,7 @@ #include "commands.h" #include "netlink_pm.h" +#include "listener_manager.h" #include "path_manager.h" // Sanity check @@ -271,6 +272,14 @@ static int upstream_announce(struct mptcpd_pm *pm, // .ifindex = ... }; + /** + * Set up MPTCP listening socket. + * + * @todo This should be optional. + */ + if (!mptcpd_lm_listen(pm->lm, id, addr)) + return -1; + return send_add_addr(pm, MPTCP_PM_CMD_ANNOUNCE, "announce", @@ -278,6 +287,36 @@ static int upstream_announce(struct mptcpd_pm *pm, token); } +struct remove_info +{ + struct l_hashmap *const lm; + mptcpd_aid_t const id; +}; + +static void upstream_remove_callback(struct l_genl_msg *msg, void *user_data) +{ + static char const op[] = "remove_addr"; + + mptcpd_family_send_callback(msg, (void *) op); + + /** + * @todo The above @c mptcpd_family_send_callback() function + * also calls @c l_genl_msg_get_error(). We could + * refactor but that may not be worth the trouble since + * @c l_genl_msg_get_error() is not an expensive call. + */ + if (l_genl_msg_get_error(msg) == 0) { + struct remove_info *info = user_data; + + /** + * Stop listening on MPTCP socket. + * + * @todo This should be optional. + */ + (void) mptcpd_lm_close(info->lm, info->id); + } +} + static int upstream_remove(struct mptcpd_pm *pm, mptcpd_aid_t id, mptcpd_token_t token) @@ -318,12 +357,16 @@ static int upstream_remove(struct mptcpd_pm *pm, return ENOMEM; } - return l_genl_family_send(pm->family, - msg, - mptcpd_family_send_callback, - "remove_addr", /* user data */ - NULL /* destroy */) - == 0; + struct remove_info info = { .lm = pm->lm, .id = id }; + + bool const result = + l_genl_family_send(pm->family, + msg, + upstream_remove_callback, + &info, /* user data */ + NULL /* destroy */); + + return result == 0; } static int upstream_add_subflow(struct mptcpd_pm *pm, From 769b0bc12d4f4e2ff3d9028f264378e1942ba24f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 15 Jun 2022 22:38:18 -0700 Subject: [PATCH 18/74] tests: Add mptcpd listener manager unit test. --- .gitignore | 1 + tests/Makefile.am | 7 +++ tests/test-listener-manager.c | 86 +++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 tests/test-listener-manager.c diff --git a/.gitignore b/.gitignore index 05b79619..79dbd2a8 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,7 @@ tests/test-commands tests/test-configuration tests/test-cxx-build tests/test-id-manager +tests/test-listener-manager tests/test-sockaddr tests/test-addr-info tests/test-murmur-hash diff --git a/tests/Makefile.am b/tests/Makefile.am index 8557d37f..a0b4fa16 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -28,6 +28,7 @@ check_PROGRAMS = \ test-commands \ test-configuration \ test-id-manager \ + test-listener-manager \ test-sockaddr \ test-addr-info \ test-murmur-hash @@ -102,6 +103,12 @@ test_id_manager_LDADD = \ $(ELL_LIBS) \ $(CODE_COVERAGE_LIBS) +test_listener_manager_SOURCES = test-listener-manager.c +test_listener_manager_LDADD = \ + $(top_builddir)/src/libpath_manager.la \ + $(ELL_LIBS) \ + $(CODE_COVERAGE_LIBS) + test_sockaddr_SOURCES = test-sockaddr.c test_sockaddr_LDADD = \ $(top_builddir)/lib/libmptcpd.la \ diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c new file mode 100644 index 00000000..d9c57d94 --- /dev/null +++ b/tests/test-listener-manager.c @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: BSD-3-Clause +/** + * @file test-listener-manager.c + * + * @brief mptcpd listener manager test. + * + * Copyright (c) 2022, Intel Corporation + */ + + +#include + +#include +#include +#include + +#include "../src/listener_manager.h" + +#include "test-plugin.h" // For test sockaddrs + +#undef NDEBUG +#include + + +static struct l_hashmap *_lm; +static mptcpd_aid_t const _id = 245; + +static void test_create(void const *test_data) +{ + (void) test_data; + + _lm = mptcpd_lm_create(); + + assert(_lm != NULL); +} + +static void test_listen(void const *test_data) +{ + (void) test_data; + + struct sockaddr_in addr = { + .sin_family = AF_INET, + .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) } + }; + + struct sockaddr const *const sa = + (struct sockaddr const *) &addr; + + assert(mptcpd_lm_listen(_lm, _id, sa)); +} + +static void test_close(void const *test_data) +{ + (void) test_data; + + assert(mptcpd_lm_close(_lm, _id)); +} + + +static void test_destroy(void const *test_data) +{ + (void) test_data; + + mptcpd_lm_destroy(_lm); +} + +int main(int argc, char *argv[]) +{ + l_log_set_stderr(); + + l_test_init(&argc, &argv); + + l_test_add("create listener manager", test_create, NULL); + l_test_add("listen", test_listen, NULL); + l_test_add("close", test_close, NULL); + l_test_add("destroy listener manager", test_destroy, NULL); + + return l_test_run(); +} + + +/* + Local Variables: + c-file-style: "linux" + End: +*/ From 14f9e58d221958c17d8d5be0a4c1998164b49c4e Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 15 Jun 2022 22:42:37 -0700 Subject: [PATCH 19/74] tests: Clarify why the loopback address is used. --- tests/test-listener-manager.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index d9c57d94..fccadbdf 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -38,6 +38,11 @@ static void test_listen(void const *test_data) { (void) test_data; + /* + Listen on the loopback address since we need an address + backed by a network interface so that underlying bind() call + can succeed. + */ struct sockaddr_in addr = { .sin_family = AF_INET, .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) } From a7c97de78fd49293ad2a2d156ecc0e2fb231aecf Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 17 Jun 2022 10:56:33 -0700 Subject: [PATCH 20/74] include: Update mptcpd copies of . --- include/linux/mptcp_org.h | 18 ++++++++++-------- include/linux/mptcp_upstream.h | 9 +++++---- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/include/linux/mptcp_org.h b/include/linux/mptcp_org.h index 8696bf5c..f268e980 100644 --- a/include/linux/mptcp_org.h +++ b/include/linux/mptcp_org.h @@ -66,18 +66,20 @@ enum { * - MPTCP_EVENT_REMOVED: token, rem_id * An address has been lost by the peer. * - * - MPTCP_EVENT_SUB_ESTABLISHED: token, family, saddr4 | saddr6, - * daddr4 | daddr6, sport, dport, backup, - * if_idx [, error] + * - MPTCP_EVENT_SUB_ESTABLISHED: token, family, loc_id, rem_id, + * saddr4 | saddr6, daddr4 | daddr6, sport, + * dport, backup, if_idx [, error] * A new subflow has been established. 'error' should not be set. * - * - MPTCP_EVENT_SUB_CLOSED: token, family, saddr4 | saddr6, daddr4 | daddr6, - * sport, dport, backup, if_idx [, error] + * - MPTCP_EVENT_SUB_CLOSED: token, family, loc_id, rem_id, saddr4 | saddr6, + * daddr4 | daddr6, sport, dport, backup, if_idx + * [, error] * A subflow has been closed. An error (copy of sk_err) could be set if an * error has been detected for this subflow. * - * - MPTCP_EVENT_SUB_PRIORITY: token, family, saddr4 | saddr6, daddr4 | daddr6, - * sport, dport, backup, if_idx [, error] + * - MPTCP_EVENT_SUB_PRIORITY: token, family, loc_id, rem_id, saddr4 | saddr6, + * daddr4 | daddr6, sport, dport, backup, if_idx + * [, error] * The priority of a subflow has changed. 'error' should not be set. * * Commands for MPTCP: @@ -88,7 +90,7 @@ enum { * Announce that an address has been lost to the peer. * * - MPTCP_CMD_SUB_CREATE: token, family, loc_id, rem_id, daddr4 | daddr6, - * dport [,saddr4 | saddr6, sport, backup, if_idx] + * dport [, saddr4 | saddr6, sport, backup, if_idx] * Create a new subflow. * * - MPTCP_CMD_SUB_DESTROY: token, family, saddr4 | saddr6, daddr4 | daddr6, diff --git a/include/linux/mptcp_upstream.h b/include/linux/mptcp_upstream.h index 92196358..dfe19bf1 100644 --- a/include/linux/mptcp_upstream.h +++ b/include/linux/mptcp_upstream.h @@ -2,16 +2,17 @@ #ifndef _UAPI_MPTCP_H #define _UAPI_MPTCP_H +#ifndef __KERNEL__ +#include /* for sockaddr_in and sockaddr_in6 */ +#include /* for struct sockaddr */ +#endif + #include #include #include /* for sockaddr_in */ #include /* for sockaddr_in6 */ #include /* for sockaddr_storage and sa_family */ -#ifndef __KERNEL__ -#include /* for struct sockaddr */ -#endif - #define MPTCP_SUBFLOW_FLAG_MCAP_REM _BITUL(0) #define MPTCP_SUBFLOW_FLAG_MCAP_LOC _BITUL(1) #define MPTCP_SUBFLOW_FLAG_JOIN_REM _BITUL(2) From 40cb8a5443805ec00bf081b0b4a032908a0bc79e Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 10:50:40 -0700 Subject: [PATCH 21/74] m4: Update upstream detection. Mptcpd now requires the MPTCP_ATTR_SERVER_SIDE enumerator to exist in the upstream kernel header. Update the header detection logic accordingly. --- m4/mptcpd_kernel.m4 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/m4/mptcpd_kernel.m4 b/m4/mptcpd_kernel.m4 index b8ba3e92..dcf32597 100644 --- a/m4/mptcpd_kernel.m4 +++ b/m4/mptcpd_kernel.m4 @@ -39,16 +39,16 @@ AC_DEFUN([MPTCPD_CHECK_KERNEL_HEADER_UPSTREAM], [Define to 1 if you have the upstream kernel header.]) - AC_CACHE_CHECK([for MPTCP_PM_CMD_ANNOUNCE in linux/mptcp.h], + AC_CACHE_CHECK([for MPTCP_ATTR_SERVER_SIDE in linux/mptcp.h], [mptcpd_cv_header_upstream], [ - # Perform a compile-time test since MPTCP_PM_CMD_ANNOUNCE is an + # Perform a compile-time test since MPTCP_ATTR_SERVER_SIDE is an # enumerator, not a preprocessor symbol. AC_COMPILE_IFELSE([ AC_LANG_SOURCE([ #include -int announce_cmd(void) { return MPTCP_PM_CMD_ANNOUNCE; } +int get_mptcp_attr(void) { return (int) MPTCP_ATTR_SERVER_SIDE; } ]) ], [mptcpd_cv_header_upstream=yes], From cfa291f7b3bda8bc8cd63f66e0063c1245bac4cb Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 10:56:53 -0700 Subject: [PATCH 22/74] src: Parse MPTCP_ATTR_SERVER_SIDE event attribute. --- src/path_manager.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/path_manager.c b/src/path_manager.c index 07626873..501e28bf 100644 --- a/src/path_manager.c +++ b/src/path_manager.c @@ -127,7 +127,7 @@ struct pm_event_attrs /// Network interface index. int32_t const *index; - /// MPTCP subflow backup priority status. + /// MPTCP subflow backup priority status (boolean). uint8_t const *backup; /** @@ -137,6 +137,9 @@ struct pm_event_attrs * @c struct @c sock in the Linux kernel. */ uint8_t const *error; + + /// Server side connection event (boolean) + uint8_t const *server_side; }; /** @@ -197,6 +200,9 @@ static void parse_netlink_attributes(struct l_genl_msg *msg, case MPTCP_ATTR_IF_IDX: MPTCP_GET_NL_ATTR(data, len, attrs->index); break; + case MPTCP_ATTR_SERVER_SIDE: + MPTCP_GET_NL_ATTR(data, len, attrs->server_side); + break; case MPTCP_ATTR_FAMILY: case MPTCP_ATTR_FLAGS: case MPTCP_ATTR_TIMEOUT: @@ -221,7 +227,9 @@ static void handle_connection_created(struct pm_event_attrs const *attrs, Local port Remote address Remote port - Path management strategy (optional) + + Upsteam kernel event attributes: + Server side status */ if (!attrs->token || !(attrs->laddr4 || attrs->laddr6) @@ -250,11 +258,14 @@ static void handle_connection_created(struct pm_event_attrs const *attrs, } static char const *const pm_name = NULL; + bool const server_side = + (attrs->server_side != NULL ? *attrs->server_side : false); mptcpd_plugin_new_connection(pm_name, *attrs->token, (struct sockaddr *) &laddr, (struct sockaddr *) &raddr, + server_side, pm); } @@ -268,6 +279,9 @@ static void handle_connection_established(struct pm_event_attrs const *attrs, Local port Remote address Remote port + + Upsteam kernel event attributes: + Server side status */ if (!attrs->token || !(attrs->laddr4 || attrs->laddr6) @@ -295,9 +309,14 @@ static void handle_connection_established(struct pm_event_attrs const *attrs, return; } + // Assume server_side is false if event attribute is unavailable. + bool const server_side = + (attrs->server_side != NULL ? *attrs->server_side : false); + mptcpd_plugin_connection_established(*attrs->token, (struct sockaddr *) &laddr, (struct sockaddr *) &raddr, + server_side, pm); } From 81b615dd2f4379445febfbea1050717c976d20a2 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 10:58:15 -0700 Subject: [PATCH 23/74] plugin: Propagate server_side attribute to plugins Propagate the MPTCP_ATTR_SERVER_SIDE attribute value to plugin new_connection and connection_established operations. --- include/mptcpd/plugin.h | 26 +++++++++++++++----------- include/mptcpd/private/plugin.h | 24 ++++++++++++++---------- lib/plugin.c | 12 +++++++++--- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/include/mptcpd/plugin.h b/include/mptcpd/plugin.h index 25ff5308..d54c2b7c 100644 --- a/include/mptcpd/plugin.h +++ b/include/mptcpd/plugin.h @@ -4,7 +4,7 @@ * * @brief mptcpd user space path manager plugin header file. * - * Copyright (c) 2017-2020, Intel Corporation + * Copyright (c) 2017-2020, 2022, Intel Corporation */ #ifndef MPTCPD_PLUGIN_H @@ -139,29 +139,33 @@ struct mptcpd_plugin_ops * A new MPTCP connection has been created, and pending * completion. * - * @param[in] token MPTCP connection token. - * @param[in] laddr Local address information. - * @param[in] raddr Remote address information. - * @param[in] pm Opaque pointer to mptcpd path manager - * object. + * @param[in] token MPTCP connection token. + * @param[in] laddr Local address information. + * @param[in] raddr Remote address information. + * @param[in] server_side Server side connection flag. + * @param[in] pm Opaque pointer to mptcpd path + * manager object. */ void (*new_connection)(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm); /** * @brief New MPTCP-capable connection has been established. * - * @param[in] token MPTCP connection token. - * @param[in] laddr Local address information. - * @param[in] raddr Remote address information. - * @param[in] pm Opaque pointer to mptcpd path manager - * object. + * @param[in] token MPTCP connection token. + * @param[in] laddr Local address information. + * @param[in] raddr Remote address information. + * @param[in] server_side Server side connection flag. + * @param[in] pm Opaque pointer to mptcpd path + * manager object. */ void (*connection_established)(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm); /** diff --git a/include/mptcpd/private/plugin.h b/include/mptcpd/private/plugin.h index b8e00ba9..ad25ca85 100644 --- a/include/mptcpd/private/plugin.h +++ b/include/mptcpd/private/plugin.h @@ -4,7 +4,7 @@ * * @brief mptcpd private plugin interface. * - * Copyright (c) 2017-2021, Intel Corporation + * Copyright (c) 2017-2022, Intel Corporation */ #ifndef MPTCPD_PRIVATE_PLUGIN_H @@ -55,31 +55,35 @@ MPTCPD_API void mptcpd_plugin_unload(struct mptcpd_pm *pm); /** * @brief Notify plugin of new MPTCP connection pending completion. * - * @param[in] name Plugin name. - * @param[in] token MPTCP connection token. - * @param[in] laddr Local address information. - * @param[in] raddr Remote address information. - * @param[in] pm Opaque pointer to mptcpd path manager object. + * @param[in] name Plugin name. + * @param[in] token MPTCP connection token. + * @param[in] laddr Local address information. + * @param[in] raddr Remote address information. + * @param[in] server_side Server side connection flag. + * @param[in] pm Opaque pointer to mptcpd path manager object. */ MPTCPD_API void mptcpd_plugin_new_connection( char const *name, mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm); /** * @brief Notify plugin of MPTCP connection completion. * - * @param[in] token MPTCP connection token. - * @param[in] laddr Local address information. - * @param[in] raddr Remote address information. - * @param[in] pm Opaque pointer to mptcpd path manager object. + * @param[in] token MPTCP connection token. + * @param[in] laddr Local address information. + * @param[in] raddr Remote address information. + * @param[in] server_side Server side connection flag. + * @param[in] pm Opaque pointer to mptcpd path manager object. */ MPTCPD_API void mptcpd_plugin_connection_established( mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm); /** diff --git a/lib/plugin.c b/lib/plugin.c index afac121f..ecd44cfd 100644 --- a/lib/plugin.c +++ b/lib/plugin.c @@ -4,7 +4,7 @@ * * @brief Common path manager plugin functions. * - * Copyright (c) 2018-2021, Intel Corporation + * Copyright (c) 2018-2022, Intel Corporation */ #ifdef HAVE_CONFIG_H @@ -580,6 +580,7 @@ void mptcpd_plugin_new_connection(char const *name, mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { struct mptcpd_plugin_ops const *const ops = name_to_ops(name); @@ -591,18 +592,23 @@ void mptcpd_plugin_new_connection(char const *name, l_error("Unable to map connection to plugin."); if (ops && ops->new_connection) - ops->new_connection(token, laddr, raddr, pm); + ops->new_connection(token, laddr, raddr, server_side, pm); } void mptcpd_plugin_connection_established(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { struct mptcpd_plugin_ops const *const ops = token_to_ops(token); if (ops && ops->connection_established) - ops->connection_established(token, laddr, raddr, pm); + ops->connection_established(token, + laddr, + raddr, + server_side, + pm); } void mptcpd_plugin_connection_closed(mptcpd_token_t token, From 38dce3aa958d127856ab232eb51b88bc81a8cbcd Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 11:00:55 -0700 Subject: [PATCH 24/74] plugins: Update connection operations signatures. Mptcpd plugin new_connection and connection_established operations now accept a boolean "server_side" parameter. Update mptcpd plugin implementations accordingly. --- plugins/path_managers/sspi.c | 6 +++++- tests/plugins/noop/noop.c | 6 +++++- tests/plugins/priority/one.c | 6 +++++- tests/plugins/priority/two.c | 6 +++++- tests/plugins/security/four.c | 12 ++++++++---- tests/plugins/security/three.c | 6 +++++- 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/plugins/path_managers/sspi.c b/plugins/path_managers/sspi.c index f844cbad..a49c86cd 100644 --- a/plugins/path_managers/sspi.c +++ b/plugins/path_managers/sspi.c @@ -4,7 +4,7 @@ * * @brief MPTCP single-subflow-per-interface path manager plugin. * - * Copyright (c) 2018-2021, Intel Corporation + * Copyright (c) 2018-2022, Intel Corporation */ #ifdef HAVE_CONFIG_H @@ -543,9 +543,11 @@ static void sspi_send_addrs(struct mptcpd_interface const *i, void *data) static void sspi_new_connection(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) raddr; + (void) server_side; /** * @note Because we directly store connection tokens in a @@ -603,11 +605,13 @@ static void sspi_new_connection(mptcpd_token_t token, static void sspi_connection_established(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side, (void) pm; /** diff --git a/tests/plugins/noop/noop.c b/tests/plugins/noop/noop.c index 0a53a0a1..4b06ffa7 100644 --- a/tests/plugins/noop/noop.c +++ b/tests/plugins/noop/noop.c @@ -4,7 +4,7 @@ * * @brief MPTCP test plugin. * - * Copyright (c) 2019-2021, Intel Corporation + * Copyright (c) 2019-2022, Intel Corporation */ #pragma GCC diagnostic push @@ -23,11 +23,13 @@ static void plugin_noop_new_connection(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side; (void) pm; } @@ -35,11 +37,13 @@ static void plugin_noop_connection_established( mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side; (void) pm; } diff --git a/tests/plugins/priority/one.c b/tests/plugins/priority/one.c index 339b0862..8b668886 100644 --- a/tests/plugins/priority/one.c +++ b/tests/plugins/priority/one.c @@ -4,7 +4,7 @@ * * @brief MPTCP test plugin. * - * Copyright (c) 2019-2021, Intel Corporation + * Copyright (c) 2019-2022, Intel Corporation */ #pragma GCC diagnostic push @@ -40,6 +40,7 @@ static struct sockaddr const *const remote_addr = static void plugin_one_new_connection(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) pm; @@ -49,6 +50,7 @@ static void plugin_one_new_connection(mptcpd_token_t token, assert(!sockaddr_is_equal(laddr, raddr)); assert(sockaddr_is_equal(laddr, local_addr)); assert(sockaddr_is_equal(raddr, remote_addr)); + assert(server_side == test_server_side_1); ++call_count.new_connection; } @@ -57,6 +59,7 @@ static void plugin_one_connection_established( mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) pm; @@ -66,6 +69,7 @@ static void plugin_one_connection_established( assert(!sockaddr_is_equal(laddr, raddr)); assert(sockaddr_is_equal(laddr, local_addr)); assert(sockaddr_is_equal(raddr, remote_addr)); + assert(server_side == test_server_side_1); ++call_count.connection_established; } diff --git a/tests/plugins/priority/two.c b/tests/plugins/priority/two.c index f9cb9e0e..cc242a4a 100644 --- a/tests/plugins/priority/two.c +++ b/tests/plugins/priority/two.c @@ -4,7 +4,7 @@ * * @brief MPTCP test plugin. * - * Copyright (c) 2019-2021, Intel Corporation + * Copyright (c) 2019-2022, Intel Corporation */ #pragma GCC diagnostic push @@ -40,6 +40,7 @@ static struct sockaddr const *const remote_addr = static void plugin_two_new_connection(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) pm; @@ -49,6 +50,7 @@ static void plugin_two_new_connection(mptcpd_token_t token, assert(!sockaddr_is_equal(laddr, raddr)); assert(sockaddr_is_equal(laddr, local_addr)); assert(sockaddr_is_equal(raddr, remote_addr)); + assert(server_side == test_server_side_2); ++call_count.new_connection; } @@ -57,6 +59,7 @@ static void plugin_two_connection_established( mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) pm; @@ -66,6 +69,7 @@ static void plugin_two_connection_established( assert(!sockaddr_is_equal(laddr, raddr)); assert(sockaddr_is_equal(laddr, local_addr)); assert(sockaddr_is_equal(raddr, remote_addr)); + assert(server_side == test_server_side_2); ++call_count.connection_established; } diff --git a/tests/plugins/security/four.c b/tests/plugins/security/four.c index 9c4f13c7..f69ef800 100644 --- a/tests/plugins/security/four.c +++ b/tests/plugins/security/four.c @@ -4,7 +4,7 @@ * * @brief MPTCP test plugin. * - * Copyright (c) 2019-2021, Intel Corporation + * Copyright (c) 2019-2022, Intel Corporation */ #pragma GCC diagnostic push @@ -32,13 +32,15 @@ static struct plugin_call_count call_count; // ---------------------------------------------------------------- static void plugin_four_new_connection(mptcpd_token_t token, - struct sockaddr const *laddr, - struct sockaddr const *raddr, - struct mptcpd_pm *pm) + struct sockaddr const *laddr, + struct sockaddr const *raddr, + bool server_side, + struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side; (void) pm; ++call_count.new_connection; @@ -48,11 +50,13 @@ static void plugin_four_connection_established( mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side; (void) pm; ++call_count.connection_established; diff --git a/tests/plugins/security/three.c b/tests/plugins/security/three.c index fdec3da4..f1728f12 100644 --- a/tests/plugins/security/three.c +++ b/tests/plugins/security/three.c @@ -4,7 +4,7 @@ * * @brief MPTCP test plugin. * - * Copyright (c) 2019-2021, Intel Corporation + * Copyright (c) 2019-2022, Intel Corporation */ #pragma GCC diagnostic push @@ -34,11 +34,13 @@ static struct plugin_call_count call_count; static void plugin_three_new_connection(mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side; (void) pm; ++call_count.new_connection; @@ -48,11 +50,13 @@ static void plugin_three_connection_established( mptcpd_token_t token, struct sockaddr const *laddr, struct sockaddr const *raddr, + bool server_side, struct mptcpd_pm *pm) { (void) token; (void) laddr; (void) raddr; + (void) server_side; (void) pm; ++call_count.connection_established; From a878e0b0cbc7ccabc13d78f4c7d9dd5f3337df75 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 11:03:46 -0700 Subject: [PATCH 25/74] tests: Update test plugin infrastructure. Add the new "server_side" argument to all calls to test plugin new_connection and connection_established operations. --- tests/lib/call_plugin.c | 2 ++ tests/lib/test-plugin.h | 31 ++++++++++++++--------- tests/test-plugin.c | 55 +++++++++++++++++++++++------------------ 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/tests/lib/call_plugin.c b/tests/lib/call_plugin.c index 6e0067b7..fb9f374d 100644 --- a/tests/lib/call_plugin.c +++ b/tests/lib/call_plugin.c @@ -28,12 +28,14 @@ void call_plugin_ops(struct plugin_call_count const *count, args->token, args->laddr, args->raddr, + args->server_side, args->pm); for (int i = 0; i < count->connection_established; ++i) mptcpd_plugin_connection_established(args->token, args->laddr, args->raddr, + args->server_side, args->pm); for (int i = 0; i < count->new_address; ++i) diff --git a/tests/lib/test-plugin.h b/tests/lib/test-plugin.h index d55aee73..066e64f3 100644 --- a/tests/lib/test-plugin.h +++ b/tests/lib/test-plugin.h @@ -138,20 +138,24 @@ static struct plugin_call_count const test_count_4 = { * types of values they correspond to. */ ///@{ -static mptcpd_token_t const test_token_1 = 0x12345678; -static mptcpd_aid_t const test_laddr_id_1 = 0x34; -static mptcpd_aid_t const test_raddr_id_1 = 0x56; -static bool const test_backup_1 = true; +static mptcpd_token_t const test_token_1 = 0x12345678; +static mptcpd_aid_t const test_laddr_id_1 = 0x34; +static mptcpd_aid_t const test_raddr_id_1 = 0x56; +static bool const test_backup_1 = true; +static bool const test_server_side_1 = true; -static mptcpd_token_t const test_token_2 = 0x23456789; -static mptcpd_aid_t const test_laddr_id_2 = 0x23; -static mptcpd_aid_t const test_raddr_id_2 = 0x45; -static bool const test_backup_2 = false; -static mptcpd_token_t const test_token_4 = 0x34567890; -static mptcpd_aid_t const test_laddr_id_4 = 0x90; -static mptcpd_aid_t const test_raddr_id_4 = 0x01; -static bool const test_backup_4 = true; +static mptcpd_token_t const test_token_2 = 0x23456789; +static mptcpd_aid_t const test_laddr_id_2 = 0x23; +static mptcpd_aid_t const test_raddr_id_2 = 0x45; +static bool const test_backup_2 = false; +static bool const test_server_side_2 = true; + +static mptcpd_token_t const test_token_4 = 0x34567890; +static mptcpd_aid_t const test_laddr_id_4 = 0x90; +static mptcpd_aid_t const test_raddr_id_4 = 0x01; +static bool const test_backup_4 = true; +static bool const test_server_side_4 = false; // For verifying that a plugin will not be dispatched. static mptcpd_token_t const test_bad_token = 0xFFFFFFFF; @@ -291,6 +295,9 @@ struct plugin_call_args /// MPTCP backup priority. bool backup; + /// Server side connection flag. + bool server_side; + /// Mptcpd path manager object. struct mptcpd_pm *const pm; }; diff --git a/tests/test-plugin.c b/tests/test-plugin.c index ead79a0a..dcac54f6 100644 --- a/tests/test-plugin.c +++ b/tests/test-plugin.c @@ -47,11 +47,12 @@ static bool run_plugin_load(mode_t mode, struct l_queue const *queue) if (loaded) { static struct plugin_call_args const args = { - .token = test_token_4, - .raddr_id = test_raddr_id_4, - .laddr = (struct sockaddr const *) &test_laddr_4, - .raddr = (struct sockaddr const *) &test_raddr_4, - .backup = test_backup_4 + .token = test_token_4, + .raddr_id = test_raddr_id_4, + .laddr = (struct sockaddr const *) &test_laddr_4, + .raddr = (struct sockaddr const *) &test_raddr_4, + .backup = test_backup_4, + .server_side = test_server_side_4 }; call_plugin_ops(&test_count_4, &args); @@ -180,6 +181,7 @@ static void test_nonexistent_plugins(void const *test_data) 0, // token NULL, // laddr NULL, // raddr + false, // server_side NULL); // pm assert(!loaded); @@ -206,35 +208,38 @@ static void test_plugin_dispatch(void const *test_data) // Notice that we call plugin 1 twice. // Plugin 1 static struct plugin_call_args const args1 = { - .name = TEST_PLUGIN_ONE, - .token = test_token_1, - .raddr_id = test_raddr_id_1, - .laddr = (struct sockaddr const *) &test_laddr_1, - .raddr = (struct sockaddr const *) &test_raddr_1, - .backup = test_backup_1 + .name = TEST_PLUGIN_ONE, + .token = test_token_1, + .raddr_id = test_raddr_id_1, + .laddr = (struct sockaddr const *) &test_laddr_1, + .raddr = (struct sockaddr const *) &test_raddr_1, + .backup = test_backup_1, + .server_side = test_server_side_1 }; call_plugin_ops(&test_count_1, &args1); // Plugin 1 as default (no plugin name specified) static struct plugin_call_args const args1_default = { - .token = test_token_1, - .raddr_id = test_raddr_id_1, - .laddr = (struct sockaddr const *) &test_laddr_1, - .raddr = (struct sockaddr const *) &test_raddr_1, - .backup = test_backup_1 + .token = test_token_1, + .raddr_id = test_raddr_id_1, + .laddr = (struct sockaddr const *) &test_laddr_1, + .raddr = (struct sockaddr const *) &test_raddr_1, + .backup = test_backup_1, + .server_side = test_server_side_1 }; call_plugin_ops(&test_count_1, &args1_default); // Plugin 2 static struct plugin_call_args const args2 = { - .name = TEST_PLUGIN_TWO, - .token = test_token_2, - .raddr_id = test_raddr_id_2, - .laddr = (struct sockaddr const *) &test_laddr_2, - .raddr = (struct sockaddr const *) &test_raddr_2, - .backup = test_backup_2 + .name = TEST_PLUGIN_TWO, + .token = test_token_2, + .raddr_id = test_raddr_id_2, + .laddr = (struct sockaddr const *) &test_laddr_2, + .raddr = (struct sockaddr const *) &test_raddr_2, + .backup = test_backup_2, + .server_side = test_server_side_2 }; call_plugin_ops(&test_count_2, &args2); @@ -257,6 +262,7 @@ static void test_plugin_dispatch(void const *test_data) test_bad_token, (struct sockaddr const *) &test_laddr_2, (struct sockaddr const *) &test_raddr_2, + test_server_side_2, NULL); // Test assertions will be triggered during plugin unload. @@ -293,11 +299,12 @@ static void test_null_plugin_ops(void const *test_data) static struct sockaddr const *const laddr = NULL; static struct sockaddr const *const raddr = NULL; static bool backup = false; + static bool server_side = false; static struct mptcpd_interface const *const interface = NULL; // No dispatch should occur in the following calls. - mptcpd_plugin_new_connection(name, token, laddr, raddr, pm); - mptcpd_plugin_connection_established(token, laddr, raddr, pm); + mptcpd_plugin_new_connection(name, token, laddr, raddr, server_side, pm); + mptcpd_plugin_connection_established(token, laddr, raddr, server_side, pm); mptcpd_plugin_connection_closed(token, pm); mptcpd_plugin_new_address(token, id, raddr, pm); mptcpd_plugin_address_removed(token, id, pm); From 587581fa0abe4126c9d7b8a95e49e98ed7cdd3d6 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 11:08:13 -0700 Subject: [PATCH 26/74] tests: Clarify plugin call relationship. Make the relationship between two duplicate sets of test plugin call arguments by initializing the second set in terms of the first rather than simple cut-n-paste initialization. This minimizes potential for argument mismatches between the two by only requiring changes to the arguments in one place instead of two. It also improve code self-documentation. --- tests/test-plugin.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test-plugin.c b/tests/test-plugin.c index dcac54f6..87603971 100644 --- a/tests/test-plugin.c +++ b/tests/test-plugin.c @@ -221,12 +221,12 @@ static void test_plugin_dispatch(void const *test_data) // Plugin 1 as default (no plugin name specified) static struct plugin_call_args const args1_default = { - .token = test_token_1, - .raddr_id = test_raddr_id_1, - .laddr = (struct sockaddr const *) &test_laddr_1, - .raddr = (struct sockaddr const *) &test_raddr_1, - .backup = test_backup_1, - .server_side = test_server_side_1 + .token = args1.token, + .raddr_id = args1.raddr_id, + .laddr = args1.laddr, + .raddr = args1.raddr, + .backup = args1.backup, + .server_side = args1.server_side }; call_plugin_ops(&test_count_1, &args1_default); From 2c790013779d5bb9c87e3f54bbeffd0a9d7f0b22 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 11:17:19 -0700 Subject: [PATCH 27/74] listener_manager: Correct file descriptor cast. --- src/listener_manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/listener_manager.c b/src/listener_manager.c index 2e9eb5f4..4edef26a 100644 --- a/src/listener_manager.c +++ b/src/listener_manager.c @@ -125,7 +125,7 @@ bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id) } // Value will never exceed INT_MAX. - int const fd = L_PTR_TO_UINT(fd); + int const fd = L_PTR_TO_UINT(value); return close(fd) == 0; } From 4b0af5b655cacb209dd1ce0dbd11ab03f1780a29 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 8 Jul 2022 11:27:14 -0700 Subject: [PATCH 28/74] tests: Fix clang build regression. --- tests/test-plugin.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test-plugin.c b/tests/test-plugin.c index 87603971..e32fe301 100644 --- a/tests/test-plugin.c +++ b/tests/test-plugin.c @@ -207,7 +207,7 @@ static void test_plugin_dispatch(void const *test_data) // Notice that we call plugin 1 twice. // Plugin 1 - static struct plugin_call_args const args1 = { + struct plugin_call_args const args1 = { .name = TEST_PLUGIN_ONE, .token = test_token_1, .raddr_id = test_raddr_id_1, @@ -220,7 +220,7 @@ static void test_plugin_dispatch(void const *test_data) call_plugin_ops(&test_count_1, &args1); // Plugin 1 as default (no plugin name specified) - static struct plugin_call_args const args1_default = { + struct plugin_call_args const args1_default = { .token = args1.token, .raddr_id = args1.raddr_id, .laddr = args1.laddr, @@ -232,7 +232,7 @@ static void test_plugin_dispatch(void const *test_data) call_plugin_ops(&test_count_1, &args1_default); // Plugin 2 - static struct plugin_call_args const args2 = { + struct plugin_call_args const args2 = { .name = TEST_PLUGIN_TWO, .token = test_token_2, .raddr_id = test_raddr_id_2, From 4e477ae266684ae92e94d9b0283710c5c3cb94f2 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 12 Jul 2022 10:57:52 -0700 Subject: [PATCH 29/74] lib: Refactor sockaddr hash code. Move struct sockaddr hash code out of `id_manager.c' and in to a separate source and header file so that it may be used by other hash maps in libmptcpd. --- lib/Makefile.am | 4 +- lib/hash_sockaddr.c | 147 ++++++++++++++++++++++++++++++++++++++++++++ lib/hash_sockaddr.h | 95 ++++++++++++++++++++++++++++ lib/id_manager.c | 147 ++++++++------------------------------------ 4 files changed, 270 insertions(+), 123 deletions(-) create mode 100644 lib/hash_sockaddr.c create mode 100644 lib/hash_sockaddr.h diff --git a/lib/Makefile.am b/lib/Makefile.am index 201be91e..444cfcd6 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -25,7 +25,9 @@ libmptcpd_la_SOURCES = \ path_manager.c \ plugin.c \ sockaddr.c \ - murmur_hash.c + murmur_hash.c \ + hash_sockaddr.c \ + hash_sockaddr.h #if BUILDING_DLL #libmptcpd_la_SOURCES += debug.c diff --git a/lib/hash_sockaddr.c b/lib/hash_sockaddr.c new file mode 100644 index 00000000..e6f9b002 --- /dev/null +++ b/lib/hash_sockaddr.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: BSD-3-Clause +/** + * @file hash_sockaddr.c + * + * @brief @c struct @c sockaddr hash related functions. + * + * Copyright (c) 2022, Intel Corporation + */ + +#include + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +#include +#pragma GCC diagnostic pop + + +#include + +#include "hash_sockaddr.h" + + +static inline unsigned int +mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, + uint32_t seed) +{ + return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, + sizeof(sa->sin_addr.s_addr), + seed); +} + +static inline unsigned int +mptcpd_hash_sockaddr_in6(struct sockaddr_in6 const *sa, + uint32_t seed) +{ + return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, + sizeof(sa->sin6_addr.s6_addr), + seed); +} + +unsigned int mptcpd_hash_sockaddr(void const *p) +{ + struct mptcpd_hash_sockaddr_key const *const key = p; + struct sockaddr const *const sa = key->sa; + + if (sa->sa_family == AF_INET) { + struct sockaddr_in const *sa4 = + (struct sockaddr_in const *) sa; + + return mptcpd_hash_sockaddr_in(sa4, key->seed); + } else { + struct sockaddr_in6 const *sa6 = + (struct sockaddr_in6 const *) sa; + + return mptcpd_hash_sockaddr_in6(sa6, key->seed); + } +} + +int mptcpd_hash_sockaddr_compare(void const *a, void const *b) +{ + struct mptcpd_hash_sockaddr_key const *const lkey = a; + struct mptcpd_hash_sockaddr_key const *const rkey = b; + + struct sockaddr const *const lsa = lkey->sa; + struct sockaddr const *const rsa = rkey->sa; + + /** + * @todo Should we compare the other @c struct @c sockaddr_in + * and @c struct @c sockaddr_in6 fields as well? + */ + + if (lsa->sa_family == rsa->sa_family) { + if (lsa->sa_family == AF_INET) { + // IPv4 + struct sockaddr_in const *const lin = + (struct sockaddr_in const *) lsa; + + struct sockaddr_in const *const rin = + (struct sockaddr_in const *) rsa; + + uint32_t const lhs = lin->sin_addr.s_addr; + uint32_t const rhs = rin->sin_addr.s_addr; + + return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); + } else { + // IPv6 + struct sockaddr_in6 const *const lin = + (struct sockaddr_in6 const *) lsa; + + struct sockaddr_in6 const *const rin = + (struct sockaddr_in6 const *) rsa; + + uint8_t const *const lhs = lin->sin6_addr.s6_addr; + uint8_t const *const rhs = rin->sin6_addr.s6_addr; + + return memcmp(lhs, + rhs, + sizeof(lin->sin6_addr.s6_addr)); + } + } else if (lsa->sa_family == AF_INET) { + return 1; // IPv4 > IPv6 + } else { + return -1; // IPv6 < IPv4 + } +} + +void *mptcpd_hash_sockaddr_key_copy(void const *p) +{ + struct mptcpd_hash_sockaddr_key const *const src = p; + struct mptcpd_hash_sockaddr_key *const key = + l_new(struct mptcpd_hash_sockaddr_key, 1); + + struct sockaddr *sa = NULL; + + if (src->sa->sa_family == AF_INET) { + sa = (struct sockaddr *) l_new(struct sockaddr_in, 1); + + memcpy(sa, src->sa, sizeof(struct sockaddr_in)); + } else { + sa = (struct sockaddr *) l_new(struct sockaddr_in6, 1); + + memcpy(sa, src->sa, sizeof(struct sockaddr_in6)); + } + + key->sa = sa; + key->seed = src->seed; + + return key; +} + +void mptcpd_hash_sockaddr_key_free(void *p) +{ + if (p == NULL) + return; + + struct mptcpd_hash_sockaddr_key *const key = p; + + l_free((void *) key->sa); // Cast "const" away. + l_free(key); +} + + +/* + Local Variables: + c-file-style: "linux" + End: +*/ diff --git a/lib/hash_sockaddr.h b/lib/hash_sockaddr.h new file mode 100644 index 00000000..0d08e47b --- /dev/null +++ b/lib/hash_sockaddr.h @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: BSD-3-Clause +/** + * @file hash_sockaddr.h + * + * @brief @c struct @c sockaddr hash related functions. + * + * Copyright (c) 2022, Intel Corporation + */ + +#ifndef MPTCPD_HASH_SOCKADDR_H +#define MPTCPD_HASH_SOCKADDR_H + +#include + + +#ifdef __cplusplus +extern "C" { +#endif + +struct sockaddr; + +/** + * @name ELL Hash Functions For IP Addresses + * + * A set of types and functions for using an IP address through a + * @c struct @c sockaddr as the key for an ELL hashmap (@c struct + * @c l_hashmap). + * + * @note These functions are only used internally, and are not + * exported from libmptcpd. + */ +///@{ + +/** + * @brief Hash key information. + * + * Bundle the values needed to generate a hash value based on an IP + * address (@c struct @c sockaddr) using the MurmurHash3 algorithm. + */ +struct mptcpd_hash_sockaddr_key +{ + /// IP address to be hashed. + struct sockaddr const *sa; + + /// Hash algorithm (MurmurHash3) seed. + uint32_t seed; +}; + +/** + * @brief Generate a hash value based on a @c struct @c sockaddr. + * + * @param[in] p @c struct @c mptcpd_hash_sockaddr_key instance containing + * the IP address to be hashed. + * + * @return The hash value. + */ +unsigned int mptcpd_hash_sockaddr(void const *p); + + +/** + * @brief Compare hash map keys based on IP address. + * + * @param[in] a Pointer @c struct @c sockaddr (left hand side). + * @param[in] b Pointer @c struct @c sockaddr (right hand side). + * + * @return 0 if the IP addresses are equal, and -1 or 1 otherwise, + * depending on IP address family and comparisons of IP + * addresses of the same type. + */ +int mptcpd_hash_sockaddr_compare(void const *a, void const *b); + +/** + * @brief Deep copy the hash map key @a p. + * + * @return The dynamically allocated deep copy of the hash map key + * @a p. Deallocate with @c l_free(). + */ +void *mptcpd_hash_sockaddr_key_copy(void const *p); + +/// Deallocate @struct @c mptcpd_hash_sockaddr_key instance. +void mptcpd_hash_sockaddr_key_free(void *p); +///@} + +#ifdef __cplusplus +} +#endif + + +#endif // MPTCPD_HASH_SOCKADDR_H + +/* + Local Variables: + c-file-style: "linux" + End: +*/ diff --git a/lib/id_manager.c b/lib/id_manager.c index fb9bb8cb..f6ce355a 100644 --- a/lib/id_manager.c +++ b/lib/id_manager.c @@ -17,7 +17,6 @@ #include #include #include -#include #include #include @@ -34,6 +33,8 @@ #include #include +#include "hash_sockaddr.h" + /// Invalid MPTCP address ID. #define MPTCPD_INVALID_ID 0 @@ -43,27 +44,6 @@ /// Maximum MPTCP address ID. #define MPTCPD_MAX_ID UINT8_MAX - -/** - * @brief MurmurHash3 seed value. - * - * @note This could be tied to each @c mptcpd_idm instance so that the - * seed value wouldn't shared between multiple mptcpd_idm - * instances but the only way to pass the seed value to the - * mptcpd MurmurHash3 hash function through the ELL @c l_hashmap - * interface would be to make it a part of the key, such as - * through a convenience @c struct. That would double the size - * of the key on 64 bit platforms (sizeof(struct sockaddr*) + - * sizeof(uint32_t) + padding). That may not be a problem for - * the common case since most platforms would not have many - * local addresses, meaning the larger key size would not impact - * the run-time memory footprint by much. Furthermore, it is - * unlikely that such a global seed value shared between - * @c mptcpd_idm instances would be a problem since each - * @c mptcpd_idm insance would have its own @c l_hashmap. - */ -static uint32_t _idm_hash_seed = 0; - /** * @struct mptcpd_idm * @@ -84,99 +64,10 @@ struct mptcpd_idm * array should perform just as well. */ struct l_hashmap *map; -}; - -// ---------------------------------------------------------------------- - -static inline unsigned int -mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa) -{ - return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, - sizeof(sa->sin_addr.s_addr), - _idm_hash_seed); -} - -static inline unsigned int -mptcpd_hash_sockaddr_in6(struct sockaddr_in6 const *sa) -{ - return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, - sizeof(sa->sin6_addr.s6_addr), - _idm_hash_seed); -} - -static unsigned int mptcpd_hash_sockaddr(void const *p) -{ - struct sockaddr const *const sa = p; - if (sa->sa_family == AF_INET) { - struct sockaddr_in const *sa4 = - (struct sockaddr_in const *) sa; - - return mptcpd_hash_sockaddr_in(sa4); - } else { - struct sockaddr_in6 const *sa6 = - (struct sockaddr_in6 const *) sa; - return mptcpd_hash_sockaddr_in6(sa6); - } -} - -static int mptcpd_hashmap_compare(void const *a, void const *b) -{ - struct sockaddr const *const lsa = a; - struct sockaddr const *const rsa = b; - - if (lsa->sa_family == rsa->sa_family) { - if (lsa->sa_family == AF_INET) { - // IPv4 - struct sockaddr_in const *const lin = - (struct sockaddr_in const *) lsa; - - struct sockaddr_in const *const rin = - (struct sockaddr_in const *) rsa; - - uint32_t const lhs = lin->sin_addr.s_addr; - uint32_t const rhs = rin->sin_addr.s_addr; - - return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); - } else { - // IPv6 - struct sockaddr_in6 const *const lin = - (struct sockaddr_in6 const *) lsa; - - struct sockaddr_in6 const *const rin = - (struct sockaddr_in6 const *) rsa; - - uint8_t const *const lhs = lin->sin6_addr.s6_addr; - uint8_t const *const rhs = rin->sin6_addr.s6_addr; - - return memcmp(lhs, - rhs, - sizeof(lin->sin6_addr.s6_addr)); - } - } else if (lsa->sa_family == AF_INET) { - return 1; // IPv4 > IPv6 - } else { - return -1; // IPv6 < IPv4 - } -} - -static void *mptcpd_hashmap_key_copy(void const *p) -{ - struct sockaddr const *const sa = p; - struct sockaddr *key = NULL; - - if (sa->sa_family == AF_INET) { - key = (struct sockaddr *) l_new(struct sockaddr_in, 1); - - memcpy(key, sa, sizeof(struct sockaddr_in)); - } else { - key = (struct sockaddr *) l_new(struct sockaddr_in6, 1); - - memcpy(key, sa, sizeof(struct sockaddr_in6)); - } - - return key; -} + /// MurmurHash3 seed value. + uint32_t seed; +}; // ---------------------------------------------------------------------- @@ -211,16 +102,16 @@ struct mptcpd_idm *mptcpd_idm_create(void) if (!l_hashmap_set_hash_function(idm->map, mptcpd_hash_sockaddr) || !l_hashmap_set_compare_function(idm->map, - mptcpd_hashmap_compare) + mptcpd_hash_sockaddr_compare) || !l_hashmap_set_key_copy_function(idm->map, - mptcpd_hashmap_key_copy) - || !l_hashmap_set_key_free_function(idm->map, l_free)) { + mptcpd_hash_sockaddr_key_copy) + || !l_hashmap_set_key_free_function(idm->map, + mptcpd_hash_sockaddr_key_free)) { mptcpd_idm_destroy(idm); idm = NULL; } - if (_idm_hash_seed == 0) - _idm_hash_seed = l_getrandom_uint32(); + idm->seed = l_getrandom_uint32(); return idm; } @@ -250,8 +141,12 @@ bool mptcpd_idm_map_id(struct mptcpd_idm *idm, || !l_uintset_put(idm->ids, id)) return false; + struct mptcpd_hash_sockaddr_key const key = { + .sa = sa, .seed = idm->seed + }; + if (!mptcpd_hashmap_replace(idm->map, - sa, + &key, L_UINT_TO_PTR(id), NULL)) { (void) l_uintset_take(idm->ids, id); @@ -268,8 +163,12 @@ mptcpd_aid_t mptcpd_idm_get_id(struct mptcpd_idm *idm, if (idm == NULL || sa == NULL) return MPTCPD_INVALID_ID; + struct mptcpd_hash_sockaddr_key const key = { + .sa = sa, .seed = idm->seed + }; + // Check if an addr/ID mapping exists. - uint32_t id = L_PTR_TO_UINT(l_hashmap_lookup(idm->map, sa)); + uint32_t id = L_PTR_TO_UINT(l_hashmap_lookup(idm->map, &key)); if (id != MPTCPD_INVALID_ID) return (mptcpd_aid_t) id; @@ -292,8 +191,12 @@ mptcpd_aid_t mptcpd_idm_remove_id(struct mptcpd_idm *idm, if (idm == NULL || sa == NULL) return MPTCPD_INVALID_ID; + struct mptcpd_hash_sockaddr_key const key = { + .sa = sa, .seed = idm->seed + }; + mptcpd_aid_t const id = - L_PTR_TO_UINT(l_hashmap_remove(idm->map, sa)); + L_PTR_TO_UINT(l_hashmap_remove(idm->map, &key)); if (id == 0 || !l_uintset_take(idm->ids, id)) return MPTCPD_INVALID_ID; From 671ad53345529bc122bf6324f2543b4700c0a95f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 12 Jul 2022 17:42:37 -0700 Subject: [PATCH 30/74] tests: Add invalid MPTCP address ID test case. Verify that mptcpd_idm_map_id() correctly rejects an invalid (zero) MPTCP address ID. --- tests/test-id-manager.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test-id-manager.c b/tests/test-id-manager.c index 7c1b1266..8df83c54 100644 --- a/tests/test-id-manager.c +++ b/tests/test-id-manager.c @@ -53,6 +53,12 @@ static void test_map_id(void const *test_data) assert(mptcpd_idm_map_id(_idm, (struct sockaddr *) &test_laddr_4, _updated_id)); + + // Attempt to map invalid (0) ID. + mptcpd_aid_t const invalid_id = 0; + assert(!mptcpd_idm_map_id(_idm, + (struct sockaddr *) &test_laddr_4, + invalid_id)); } static void test_get_id(void const *test_data) From 16e6f1c49813a2355b50da7cc9e0935c2d6cdfa0 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 12 Jul 2022 17:52:42 -0700 Subject: [PATCH 31/74] lib: Export the mptcpd listener manager API. --- include/mptcpd/Makefile.am | 1 + {src => include/mptcpd}/listener_manager.h | 22 ++++++++++++++++------ lib/Makefile.am | 3 ++- {src => lib}/listener_manager.c | 2 +- src/Makefile.am | 4 +--- src/netlink_pm_upstream.c | 2 +- src/path_manager.c | 2 +- tests/test-listener-manager.c | 2 +- 8 files changed, 24 insertions(+), 14 deletions(-) rename {src => include/mptcpd}/listener_manager.h (72%) rename {src => lib}/listener_manager.c (98%) diff --git a/include/mptcpd/Makefile.am b/include/mptcpd/Makefile.am index 1bf6d02b..032b8aca 100644 --- a/include/mptcpd/Makefile.am +++ b/include/mptcpd/Makefile.am @@ -6,6 +6,7 @@ pkginclude_HEADERS = \ addr_info.h \ export.h \ id_manager.h \ + listener_manager.h \ network_monitor.h \ path_manager.h \ plugin.h \ diff --git a/src/listener_manager.h b/include/mptcpd/listener_manager.h similarity index 72% rename from src/listener_manager.h rename to include/mptcpd/listener_manager.h index 4dac8286..1c28dfa4 100644 --- a/src/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -12,9 +12,14 @@ #include +#include #include +#ifdef __cplusplus +extern "C" { +#endif + struct l_hashmap; struct sockaddr; @@ -24,14 +29,14 @@ struct sockaddr; * @return Pointer to a MPTCP listener manager on success. @c NULL on * failure. */ -struct l_hashmap *mptcpd_lm_create(void); +MPTCPD_API struct l_hashmap *mptcpd_lm_create(void); /** * @brief Destroy MPTCP listener manager. * * @param[in,out] lm The mptcpd address listener manager object. */ -void mptcpd_lm_destroy(struct l_hashmap *lm); +MPTCPD_API void mptcpd_lm_destroy(struct l_hashmap *lm); /** * @brief Listen on the given MPTCP local address. @@ -42,9 +47,9 @@ void mptcpd_lm_destroy(struct l_hashmap *lm); * * @return @c true on success, and @c false on failure. */ -bool mptcpd_lm_listen(struct l_hashmap *lm, - mptcpd_aid_t id, - struct sockaddr const *sa); +MPTCPD_API bool mptcpd_lm_listen(struct l_hashmap *lm, + mptcpd_aid_t id, + struct sockaddr const *sa); /** * @brief Stop listening on a MPTCP local address. @@ -54,7 +59,12 @@ bool mptcpd_lm_listen(struct l_hashmap *lm, * * @return @c true on success, and @c false on failure. */ -bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id); +MPTCPD_API bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id); + +#ifdef __cplusplus +} +#endif + #endif /* MPTCPD_LISTENER_MANAGER_H */ diff --git a/lib/Makefile.am b/lib/Makefile.am index 444cfcd6..c9b77ff6 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -21,6 +21,7 @@ libmptcpd_la_LDFLAGS = \ libmptcpd_la_SOURCES = \ addr_info.c \ id_manager.c \ + listener_manager.c \ network_monitor.c \ path_manager.c \ plugin.c \ @@ -29,7 +30,7 @@ libmptcpd_la_SOURCES = \ hash_sockaddr.c \ hash_sockaddr.h -#if BUILDING_DLL +#IF BUILDING_DLL #libmptcpd_la_SOURCES += debug.c #endif diff --git a/src/listener_manager.c b/lib/listener_manager.c similarity index 98% rename from src/listener_manager.c rename to lib/listener_manager.c index 4edef26a..b5ca7554 100644 --- a/src/listener_manager.c +++ b/lib/listener_manager.c @@ -26,7 +26,7 @@ #include #pragma GCC diagnostic pop -#include "listener_manager.h" +#include // ---------------------------------------------------------------------- diff --git a/src/Makefile.am b/src/Makefile.am index e9ad3ef1..eeb2f027 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,9 +20,7 @@ libpath_manager_la_SOURCES = \ netlink_pm.c \ netlink_pm.h \ path_manager.c \ - path_manager.h \ - listener_manager.c \ - listener_manager.h + path_manager.h if HAVE_UPSTREAM_KERNEL libpath_manager_la_SOURCES += netlink_pm_upstream.c diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 4d1562b5..7c60e9ef 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -23,6 +23,7 @@ #pragma GCC diagnostic pop #include +#include #include #include #include @@ -32,7 +33,6 @@ #include "commands.h" #include "netlink_pm.h" -#include "listener_manager.h" #include "path_manager.h" // Sanity check diff --git a/src/path_manager.c b/src/path_manager.c index 501e28bf..f73bcb84 100644 --- a/src/path_manager.c +++ b/src/path_manager.c @@ -39,12 +39,12 @@ #include #include #include +#include // For netlink events. Same API applies to multipath-tcp.org kernel. #include #include "path_manager.h" -#include "listener_manager.h" #include "netlink_pm.h" diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index fccadbdf..a3783e63 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -14,7 +14,7 @@ #include #include -#include "../src/listener_manager.h" +#include #include "test-plugin.h" // For test sockaddrs From 7154fb89824f69d02e8b21941c5a623529da3971 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 14 Jul 2022 08:14:49 -0700 Subject: [PATCH 32/74] listener_manager: Map sockaddr to file descriptor. Map sockaddrs to file descriptors instead of MPTCP address ID to file descriptor. Reference count listeners as well to allow sharing. --- include/mptcpd/listener_manager.h | 16 ++- lib/listener_manager.c | 192 ++++++++++++++++++++++-------- 2 files changed, 151 insertions(+), 57 deletions(-) diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index 1c28dfa4..7de4c1e2 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -13,15 +13,14 @@ #include #include -#include #ifdef __cplusplus extern "C" { #endif -struct l_hashmap; struct sockaddr; +struct mptcpd_lm; /** * @brief Create a MPTCP listener manager. @@ -29,37 +28,36 @@ struct sockaddr; * @return Pointer to a MPTCP listener manager on success. @c NULL on * failure. */ -MPTCPD_API struct l_hashmap *mptcpd_lm_create(void); +MPTCPD_API struct mptcpd_lm *mptcpd_lm_create(void); /** * @brief Destroy MPTCP listener manager. * * @param[in,out] lm The mptcpd address listener manager object. */ -MPTCPD_API void mptcpd_lm_destroy(struct l_hashmap *lm); +MPTCPD_API void mptcpd_lm_destroy(struct mptcpd_lm *lm); /** * @brief Listen on the given MPTCP local address. * * @param[in] lm The mptcpd address listener manager object. - * @param[in] id The MPTCP local address ID. * @param[in] sa The MPTCP local address. * * @return @c true on success, and @c false on failure. */ -MPTCPD_API bool mptcpd_lm_listen(struct l_hashmap *lm, - mptcpd_aid_t id, +MPTCPD_API bool mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr const *sa); /** * @brief Stop listening on a MPTCP local address. * * @param[in] lm The mptcpd address listener manager object. - * @param[in] id The MPTCP local address ID. + * @param[in] sa The MPTCP local IP address. * * @return @c true on success, and @c false on failure. */ -MPTCPD_API bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id); +MPTCPD_API bool mptcpd_lm_close(struct mptcpd_lm *lm, + struct sockaddr const *sa); #ifdef __cplusplus } diff --git a/lib/listener_manager.c b/lib/listener_manager.c index b5ca7554..2ab8a1c1 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -24,63 +24,66 @@ #include #include #include +#include #pragma GCC diagnostic pop #include +#include "hash_sockaddr.h" -// ---------------------------------------------------------------------- -struct l_hashmap *mptcpd_lm_create(void) -{ - // Map of MPTCP local address ID to MPTCP listener file - // descriptor. - return l_hashmap_new(); -} +// ---------------------------------------------------------------------- -static void close_listener(void *value) +/** + * @struct mptcpd_lm + * + * @brief Internal mptcpd listern manager data. + */ +struct mptcpd_lm { - /* - We don't need to worry about overflow due to casting from - unsigned int since the file descriptor value stored in the - map will never be larger than INT_MAX by design. - */ - int const fd = L_PTR_TO_UINT(value); + /// Map of @c struct @c sockaddr to listener file descriptor. + struct l_hashmap *map; - (void) close(fd); -} + /// MurmurHash3 seed value. + uint32_t seed; +}; +// ---------------------------------------------------------------------- -void mptcpd_lm_destroy(struct l_hashmap *lm) +/** + * @struct lm_value + * + * @brief mptcpd listener map entry value. + */ +struct lm_value { - if (lm == NULL) - return; + /// Listener file descriptor. + int fd; - l_hashmap_destroy(lm, close_listener); -} - -bool mptcpd_lm_listen(struct l_hashmap *lm, - mptcpd_aid_t id, - struct sockaddr const *sa) -{ /** - * @todo Allow id == 0? + * @brief Listener reference count. + * + * Mptcpd listeners are reference counted to allow sharing. */ - if (lm == NULL || id == 0 || sa == NULL) - return false; + int refcnt; +}; - if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) - return false; +// ---------------------------------------------------------------------- +static int open_listener(struct sockaddr const *sa) +{ #ifndef IPPROTO_MPTCP #define IPPROTO_MPTCP IPPROTO_TCP + 256 #endif int const fd = socket(sa->sa_family, SOCK_STREAM, IPPROTO_MPTCP); - if (fd == -1) + if (fd == -1) { l_error("Unable to open MPTCP listener: %s", strerror(errno)); + return -1; + } + socklen_t const addr_size = (sa->sa_family == AF_INET ? sizeof(struct sockaddr_in) @@ -90,44 +93,137 @@ bool mptcpd_lm_listen(struct l_hashmap *lm, l_error("Unable to bind MPTCP listener: %s", strerror(errno)); (void) close(fd); - return false; + return -1; } if (listen(fd, 0) == -1) { l_error("Unable to listen on MPTCP socket: %s", strerror(errno)); (void) close(fd); + return -1; + } + + return fd; +} + +static void close_listener(void *value) +{ + struct lm_value *const data = value; + + /** + * @todo Should care if the @c close() call fails? It + * shouldn't fail since we only get here if the listener + * socket was successfully opened earlier. + */ + (void) close(data->fd); + l_free(data); +} + +// ---------------------------------------------------------------------- + +struct mptcpd_lm *mptcpd_lm_create(void) +{ + struct mptcpd_lm *lm = l_new(struct mptcpd_lm, 1); + + // Map of IP address to MPTCP listener file descriptor. + lm->map = l_hashmap_new(); + lm->seed = l_getrandom_uint32(); + + if (!l_hashmap_set_hash_function(lm->map, mptcpd_hash_sockaddr) + || !l_hashmap_set_compare_function(lm->map, + mptcpd_hash_sockaddr_compare) + || !l_hashmap_set_key_copy_function(lm->map, + mptcpd_hash_sockaddr_key_copy) + || !l_hashmap_set_key_free_function(lm->map, + mptcpd_hash_sockaddr_key_free)) { + mptcpd_lm_destroy(lm); + lm = NULL; + } + + return lm; +} + +void mptcpd_lm_destroy(struct mptcpd_lm *lm) +{ + if (lm == NULL) + return; + + l_hashmap_destroy(lm->map, close_listener); + l_free(lm); +} + +bool mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr const *sa) +{ + if (lm == NULL || sa == NULL) + return false; + + if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) return false; + + struct mptcpd_hash_sockaddr_key const key = { + .sa = sa, .seed = lm->seed + }; + + struct lm_value *data = l_hashmap_lookup(lm->map, &key); + + /* + A listener already exists for the given IP address. + Increment the reference count. + */ + if (data != NULL) { + data->refcnt++; + + return true; } - if (!l_hashmap_insert(lm, L_UINT_TO_PTR(id), L_UINT_TO_PTR(fd))) { - l_error("Unable to map MPTCP address ID %u listener", id); - (void) close(fd); + data = l_new(struct lm_value, 1); + + data->fd = open_listener(sa); + if (data->fd == -1) { + l_free(data); + + return false; + } + + if (!l_hashmap_insert(lm->map, &key, data)) { + l_error("Unable to map MPTCP address to listener."); + + (void) close(data->fd); + l_free(data); + return false; } + data->refcnt = 1; + return true; } -bool mptcpd_lm_close(struct l_hashmap *lm, mptcpd_aid_t id) +bool mptcpd_lm_close(struct mptcpd_lm *lm, struct sockaddr const *sa) { - /** - * @todo Allow id == 0? - */ - if (lm == NULL || id == 0) + if (lm == NULL || sa == NULL) return false; - void const *const value = l_hashmap_remove(lm, L_UINT_TO_PTR(id)); + struct mptcpd_hash_sockaddr_key const key = { + .sa = sa, .seed = lm->seed + }; - if (value == NULL) { - l_error("No listener for MPTCP address id %u.", id); + struct lm_value *const data = l_hashmap_lookup(lm->map, &key); + + /* + A listener already exists for the given IP address. + Decrement the reference count. + */ + if (data == NULL) return false; - } - // Value will never exceed INT_MAX. - int const fd = L_PTR_TO_UINT(value); + if (--data->refcnt == 0) { + // No more listeners sharing the same address. + close_listener(data); + (void) l_hashmap_remove(lm->map, &key); + } - return close(fd) == 0; + return true; } From 6f2121f89e9fbec8429ce748223e2175b3c34f05 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 14 Jul 2022 08:23:43 -0700 Subject: [PATCH 33/74] Add sockaddr parameter to mptcpd_pm_remove_addr(). A sockaddr is now needed to close a listener. Add a sockaddr parameter to mptcpd_pm_remove_addr(), accordingly. --- include/mptcpd/path_manager.h | 6 ++++- include/mptcpd/private/path_manager.h | 33 ++++++++++++--------------- lib/path_manager.c | 5 ++-- src/netlink_pm_upstream.c | 11 +++++---- tests/test-commands.c | 1 + 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/include/mptcpd/path_manager.h b/include/mptcpd/path_manager.h index 048111d2..10b83b7b 100644 --- a/include/mptcpd/path_manager.h +++ b/include/mptcpd/path_manager.h @@ -4,7 +4,7 @@ * * @brief mptcpd generic netlink commands. * - * Copyright (c) 2017-2021, Intel Corporation + * Copyright (c) 2017-2022, Intel Corporation */ #ifndef MPTCPD_LIB_PATH_MANAGER_H @@ -126,6 +126,9 @@ MPTCPD_API int mptcpd_pm_add_addr(struct mptcpd_pm *pm, * @brief Stop advertising network address to peers. * * @param[in] pm The mptcpd path manager object. + * @param[in] addr Local IP address and port (zero if unused) + * that should no longer be advertised through + * MPTCP. * @param[in] address_id MPTCP local address ID to be sent in the * MPTCP protocol @c REMOVE_ADDR option * corresponding to the local address that will @@ -135,6 +138,7 @@ MPTCPD_API int mptcpd_pm_add_addr(struct mptcpd_pm *pm, * @return @c 0 if operation was successful. -1 or @c errno otherwise. */ MPTCPD_API int mptcpd_pm_remove_addr(struct mptcpd_pm *pm, + struct sockaddr const *addr, mptcpd_aid_t address_id, mptcpd_token_t token); diff --git a/include/mptcpd/private/path_manager.h b/include/mptcpd/private/path_manager.h index 4bd8388b..2498162d 100644 --- a/include/mptcpd/private/path_manager.h +++ b/include/mptcpd/private/path_manager.h @@ -23,13 +23,13 @@ struct l_genl; struct l_genl_family; struct l_queue; struct l_timeout; -struct l_hashmap; struct mptcpd_netlink_pm; struct mptcpd_addr_info; struct mptcpd_limit; struct mptcpd_nm; struct mptcpd_idm; +struct mptcpd_lm; /** * @struct mptcpd_pm path_manager.h @@ -92,20 +92,12 @@ struct mptcpd_pm struct mptcpd_idm *idm; /** - * @brief MPTCP listener manager map. - * - * The underlying hash map used by the MPTCP listener manager - * to map a MPTCP local address ID to a MPTCP socket file - * descriptor. - * - * @todo A mptcpd path manager-wide MPTCP listener manager like - * this could be problematic if multiple client-oriented - * plugins attempt to advertise or stop advertising a - * different local addresses with the same MPTCP address ID - * through the mptcpd_pm_add_addr() and - * mptcpd_pm_remove_addr() functions. + * @brief MPTCP listener manager. + * + * The MPTCP listener manager maps MPTCP local addresses to a + * listening socket file descriptors bound to those addresses. */ - struct l_hashmap *lm; + struct mptcpd_lm *lm; /// List of @c pm_ops_info objects. struct l_queue *event_ops; @@ -162,15 +154,20 @@ struct mptcpd_pm_cmd_ops /** * @brief Stop advertising network address to peers. - * @param[in] pm The mptcpd path manager object. - * @param[in] address_id MPTCP local address ID. - * @param[in] token MPTCP connection token. + * + * @param[in] pm The mptcpd path manager object. + * @param[in] addr Local IP address and port (zero if unused) + * that should no longer be advertised + * through MPTCP. + * @param[in] id MPTCP local address ID. + * @param[in] token MPTCP connection token. * * @return @c 0 if operation was successful. -1 or @c errno * otherwise. */ int (*remove_addr)(struct mptcpd_pm *pm, - mptcpd_aid_t address_id, + struct sockaddr const *addr, + mptcpd_aid_t id, mptcpd_token_t token); /** diff --git a/lib/path_manager.c b/lib/path_manager.c index 8116541c..6c939d76 100644 --- a/lib/path_manager.c +++ b/lib/path_manager.c @@ -4,7 +4,7 @@ * * @brief mptcpd generic netlink commands. * - * Copyright (c) 2017-2021, Intel Corporation + * Copyright (c) 2017-2022, Intel Corporation */ #ifdef HAVE_CONFIG_H @@ -262,6 +262,7 @@ int mptcpd_pm_add_addr(struct mptcpd_pm *pm, } int mptcpd_pm_remove_addr(struct mptcpd_pm *pm, + struct sockaddr const *addr, mptcpd_aid_t address_id, mptcpd_token_t token) { @@ -277,7 +278,7 @@ int mptcpd_pm_remove_addr(struct mptcpd_pm *pm, if (ops == NULL || ops->remove_addr == NULL) return ENOTSUP; - return ops->remove_addr(pm, address_id, token); + return ops->remove_addr(pm, addr, address_id, token); } int mptcpd_pm_add_subflow(struct mptcpd_pm *pm, diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 7c60e9ef..bcedd0e8 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -277,7 +277,7 @@ static int upstream_announce(struct mptcpd_pm *pm, * * @todo This should be optional. */ - if (!mptcpd_lm_listen(pm->lm, id, addr)) + if (!mptcpd_lm_listen(pm->lm, addr)) return -1; return send_add_addr(pm, @@ -289,8 +289,8 @@ static int upstream_announce(struct mptcpd_pm *pm, struct remove_info { - struct l_hashmap *const lm; - mptcpd_aid_t const id; + struct mptcpd_lm *const lm; + struct sockaddr const *const sa; }; static void upstream_remove_callback(struct l_genl_msg *msg, void *user_data) @@ -313,11 +313,12 @@ static void upstream_remove_callback(struct l_genl_msg *msg, void *user_data) * * @todo This should be optional. */ - (void) mptcpd_lm_close(info->lm, info->id); + (void) mptcpd_lm_close(info->lm, info->sa); } } static int upstream_remove(struct mptcpd_pm *pm, + struct sockaddr const *addr, mptcpd_aid_t id, mptcpd_token_t token) { @@ -357,7 +358,7 @@ static int upstream_remove(struct mptcpd_pm *pm, return ENOMEM; } - struct remove_info info = { .lm = pm->lm, .id = id }; + struct remove_info info = { .lm = pm->lm, .sa = addr }; bool const result = l_genl_family_send(pm->family, diff --git a/tests/test-commands.c b/tests/test-commands.c index 04d9ae99..d88887e4 100644 --- a/tests/test-commands.c +++ b/tests/test-commands.c @@ -259,6 +259,7 @@ static void test_remove_addr(void const *test_data) // Client-oriented path manager. int result = mptcpd_pm_remove_addr(pm, + laddr1, test_laddr_id_1, test_token_1); From af5eee06d9642b6e1a55ffcc1266d78be43ed070 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 14 Jul 2022 08:26:48 -0700 Subject: [PATCH 34/74] tests: Use new API in test-listener-manager. --- tests/Makefile.am | 2 +- tests/test-listener-manager.c | 62 ++++++++++++++++++++++------------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index a0b4fa16..a63412de 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -105,7 +105,7 @@ test_id_manager_LDADD = \ test_listener_manager_SOURCES = test-listener-manager.c test_listener_manager_LDADD = \ - $(top_builddir)/src/libpath_manager.la \ + $(top_builddir)/lib/libmptcpd.la \ $(ELL_LIBS) \ $(CODE_COVERAGE_LIBS) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index a3783e63..04d19260 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -22,8 +22,7 @@ #include -static struct l_hashmap *_lm; -static mptcpd_aid_t const _id = 245; +static struct mptcpd_lm *_lm; static void test_create(void const *test_data) { @@ -36,29 +35,16 @@ static void test_create(void const *test_data) static void test_listen(void const *test_data) { - (void) test_data; - - /* - Listen on the loopback address since we need an address - backed by a network interface so that underlying bind() call - can succeed. - */ - struct sockaddr_in addr = { - .sin_family = AF_INET, - .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) } - }; + struct sockaddr const *const sa = test_data; - struct sockaddr const *const sa = - (struct sockaddr const *) &addr; - - assert(mptcpd_lm_listen(_lm, _id, sa)); + assert(mptcpd_lm_listen(_lm, sa)); } static void test_close(void const *test_data) { - (void) test_data; + struct sockaddr const *const sa = test_data; - assert(mptcpd_lm_close(_lm, _id)); + assert(mptcpd_lm_close(_lm, sa)); } @@ -75,10 +61,40 @@ int main(int argc, char *argv[]) l_test_init(&argc, &argv); - l_test_add("create listener manager", test_create, NULL); - l_test_add("listen", test_listen, NULL); - l_test_add("close", test_close, NULL); - l_test_add("destroy listener manager", test_destroy, NULL); + /* + Listen on the loopback address since we need an address + backed by a network interface so that underlying bind() call + can succeed. + */ + struct sockaddr_in addr4 = { + .sin_family = AF_INET, + .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) } + }; + + struct sockaddr const *const sa4 = + (struct sockaddr const *) &addr4; + + /* + Listen on the loopback address since we need an address + backed by a network interface so that underlying bind() call + can succeed. + */ + struct sockaddr_in6 addr6 = { + .sin6_family = AF_INET6 + }; + + addr6.sin6_addr = in6addr_loopback; + + struct sockaddr const *const sa6 = + (struct sockaddr const *) &addr6; + + l_test_add("create listener manager", test_create, NULL); + l_test_add("listen - IPv4 - FIRST", test_listen, sa4); + l_test_add("listen - IPv6", test_listen, sa6); + l_test_add("listen - IPv4 - SECOND", test_listen, sa4); + l_test_add("close - IPv4", test_close, sa4); + l_test_add("close - IPv6", test_close, sa6); + l_test_add("destroy listener manager", test_destroy, NULL); return l_test_run(); } From 8e8fcca7af4ff3a6dbbb0b3dd98f45ed5f59024a Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 14 Jul 2022 08:27:47 -0700 Subject: [PATCH 35/74] hash_sockaddr: Add todo comment about port hash. --- lib/hash_sockaddr.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/hash_sockaddr.c b/lib/hash_sockaddr.c index e6f9b002..325edabd 100644 --- a/lib/hash_sockaddr.c +++ b/lib/hash_sockaddr.c @@ -24,6 +24,9 @@ static inline unsigned int mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, uint32_t seed) { + /** + * @todo The port should also be included in the hash. + */ return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, sizeof(sa->sin_addr.s_addr), seed); @@ -33,6 +36,9 @@ static inline unsigned int mptcpd_hash_sockaddr_in6(struct sockaddr_in6 const *sa, uint32_t seed) { + /** + * @todo The port should also be included in the hash. + */ return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, sizeof(sa->sin6_addr.s6_addr), seed); From f62e2124f1706a8412d861a4710fe59e05f4080f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 14 Jul 2022 08:28:25 -0700 Subject: [PATCH 36/74] id_manager: Fix potential NULL dereference. --- lib/id_manager.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/id_manager.c b/lib/id_manager.c index f6ce355a..7aba975d 100644 --- a/lib/id_manager.c +++ b/lib/id_manager.c @@ -97,8 +97,8 @@ struct mptcpd_idm *mptcpd_idm_create(void) assert(MPTCPD_MIN_ID != MPTCPD_INVALID_ID); idm->ids = l_uintset_new_from_range(MPTCPD_MIN_ID, MPTCPD_MAX_ID); - idm->map = l_hashmap_new(); + idm->seed = l_getrandom_uint32(); if (!l_hashmap_set_hash_function(idm->map, mptcpd_hash_sockaddr) || !l_hashmap_set_compare_function(idm->map, @@ -111,8 +111,6 @@ struct mptcpd_idm *mptcpd_idm_create(void) idm = NULL; } - idm->seed = l_getrandom_uint32(); - return idm; } From 72a25af099b9e974c79229d32a21fa1b5f22e947 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 14 Jul 2022 11:02:59 -0700 Subject: [PATCH 37/74] src: Add missing addr param for mptcp.org impl. --- src/netlink_pm_mptcp_org.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/netlink_pm_mptcp_org.c b/src/netlink_pm_mptcp_org.c index 277c4924..97f91124 100644 --- a/src/netlink_pm_mptcp_org.c +++ b/src/netlink_pm_mptcp_org.c @@ -222,9 +222,12 @@ static int mptcp_org_add_addr(struct mptcpd_pm *pm, } static int mptcp_org_remove_addr(struct mptcpd_pm *pm, + struct sockaddr const *addr, mptcpd_aid_t address_id, mptcpd_token_t token) { + (void) addr; + /* Payload: Token From 7dbc71c1350b8b32662406360d3c1b13f49dbe8a Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Sat, 16 Jul 2022 23:07:29 -0700 Subject: [PATCH 38/74] hash_sockaddr: Include IP port as part of the key. Hash the IP port as part of the key to ensure the resulting key hash is different between sockaddrs with the same IP address but different ports. This is necessary because the mptcpd listener manager would otherwise only create listeners for a given IP address once even if the port differs in subsequent mptcpd_lm_listen() calls. --- lib/hash_sockaddr.c | 128 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 115 insertions(+), 13 deletions(-) diff --git a/lib/hash_sockaddr.c b/lib/hash_sockaddr.c index 325edabd..51e5bdc6 100644 --- a/lib/hash_sockaddr.c +++ b/lib/hash_sockaddr.c @@ -7,6 +7,7 @@ * Copyright (c) 2022, Intel Corporation */ +#include #include #pragma GCC diagnostic push @@ -20,28 +21,129 @@ #include "hash_sockaddr.h" -static inline unsigned int -mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, +/** + * @brief Bundle IPv4 address and port as a hash key. + */ +struct endpoint_in +{ + in_addr_t addr; + in_port_t port; +}; + +/** + * @brief Bundle IPv6 address and port as a hash key. + */ +struct endpoint_in6 +{ + struct in6_addr addr; + in_port_t port; +}; + +/** + * @brief IPv4 address/port hash key buffer. + * + * This union ensures that padding bytes are initialized to zero in + * the @c endpoint member. For this to work the @c buffer member must + * initialized prior to the @c endpoint member, e.g. + * @code + * union key_in key = { .buffer = { 0 } }; + * + * key.endpoint.addr = sa->sin_addr.s_addr; + * key.endpoint.port = sa->sin_port; + * @endcode + * The goal is to avoid hashing uninitialized bytes in the hash key. + */ +union key_in +{ + struct endpoint_in endpoint; + uint8_t buffer[sizeof(struct endpoint_in)]; +}; + +/** + * @brief IPv6 address/port hash key buffer. + * + * This union ensures that padding bytes are initialized to zero in + * the @c endpoint member. For this to work the @c buffer member must + * initialized prior to the @c endpoint member, e.g. + * @code + * union key_in6 key = { .buffer = { 0 } }; + * + * memcpy(key.endpoint.addr.s6_addr, + * sa->sin6_addr.s6_addr, + * sizeof(key.endpoint.addr.s6_addr)); + * + * key.endpoint.port = sa->sin6_port; + * @endcode + * The goal is to avoid hashing uninitialized bytes in the hash key. + */ +union key_in6 +{ + struct endpoint_in6 endpoint; + uint8_t buffer[sizeof(struct endpoint_in6)]; +}; + + +static unsigned int mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, uint32_t seed) { - /** - * @todo The port should also be included in the hash. - */ - return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, - sizeof(sa->sin_addr.s_addr), - seed); + // No port set. Don't bother including it in the hash. + if (sa->sin_port == 0) + return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, + sizeof(sa->sin_addr.s_addr), + seed); + + // --------------------------------------- + + // Non-zero port. Include it in the hash. + + /* + Initialize the key buffer first to ensure padding bytes in + the endpoint union member are zero. This works since the + buffer union member also includes padding bytes found in the + endpoint member. + */ + union key_in key = { .buffer = { 0 } }; + + key.endpoint.addr = sa->sin_addr.s_addr; + key.endpoint.port = sa->sin_port; + + return mptcpd_murmur_hash3(key.buffer, sizeof(key.buffer), seed); } -static inline unsigned int +static unsigned int mptcpd_hash_sockaddr_in6(struct sockaddr_in6 const *sa, uint32_t seed) { /** - * @todo The port should also be included in the hash. + * @todo Should we include other sockaddr_in6 members, e.g. + * sin6_flowinfo and sin6_scope_id, as part of the key? */ - return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, - sizeof(sa->sin6_addr.s6_addr), - seed); + + // No port set. Don't bother including it in the hash. + if (sa->sin6_port == 0) + return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, + sizeof(sa->sin6_addr.s6_addr), + seed); + + // --------------------------------------- + + // Non-zero port. Include it in the hash. + + /* + Initialize the key buffer first to ensure padding bytes in + the endpoint union member are zero. This works since the + buffer union member also includes padding bytes found in the + endpoint member. + */ + union key_in6 key = { .buffer = { 0 } }; + + memcpy(key.endpoint.addr.s6_addr, + sa->sin6_addr.s6_addr, + sizeof(key.endpoint.addr.s6_addr)); + + key.endpoint.port = sa->sin6_port; + + return mptcpd_murmur_hash3(key.buffer, sizeof(key.buffer), seed); } unsigned int mptcpd_hash_sockaddr(void const *p) From b6679555fd75ac563aeb85caa14522283d022d6b Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Sun, 17 Jul 2022 08:18:01 -0700 Subject: [PATCH 39/74] hash_sockaddr: Simplify padding initialization. Add an implicitly initialized dummy padding member to the hash key structures, and drop the unions. This is simpler than the previous union based approach. --- lib/hash_sockaddr.c | 109 ++++++++++++++++---------------------------- 1 file changed, 39 insertions(+), 70 deletions(-) diff --git a/lib/hash_sockaddr.c b/lib/hash_sockaddr.c index 51e5bdc6..b4fc1943 100644 --- a/lib/hash_sockaddr.c +++ b/lib/hash_sockaddr.c @@ -7,6 +7,7 @@ * Copyright (c) 2022, Intel Corporation */ +#include #include #include @@ -23,67 +24,51 @@ /** * @brief Bundle IPv4 address and port as a hash key. + * + * A @c padding member is added to allow the padding bytes to be + * implicitly initialized to zero in a designated initializer, e.g.: + * @code + * struct endpoint_in endpoint = { + * .addr = 0xC0000202, + * .port = 0x4321 + * }; + * The goal is to avoid hashing uninitialized bytes in the hash key. + * @endcode */ -struct endpoint_in +struct key_in { in_addr_t addr; in_port_t port; + uint16_t padding; // To be implicitly initialized. }; /** * @brief Bundle IPv6 address and port as a hash key. - */ -struct endpoint_in6 -{ - struct in6_addr addr; - in_port_t port; -}; - -/** - * @brief IPv4 address/port hash key buffer. * - * This union ensures that padding bytes are initialized to zero in - * the @c endpoint member. For this to work the @c buffer member must - * initialized prior to the @c endpoint member, e.g. + * A @c padding member is added to allow the padding bytes to be + * implicitly initialized to zero in a designated initializer, e.g.: * @code - * union key_in key = { .buffer = { 0 } }; - * - * key.endpoint.addr = sa->sin_addr.s_addr; - * key.endpoint.port = sa->sin_port; - * @endcode + * struct endpoint_in6 endpoint = { + * .addr = { .s6_addr = { [0] = 0x20, + * [1] = 0x01, + * [2] = 0X0D, + * [3] = 0xB8, + * [14] = 0x01, + * [15] = 0x02 } }, + * .port = 0x4321 + * }; * The goal is to avoid hashing uninitialized bytes in the hash key. - */ -union key_in -{ - struct endpoint_in endpoint; - uint8_t buffer[sizeof(struct endpoint_in)]; -}; - -/** - * @brief IPv6 address/port hash key buffer. - * - * This union ensures that padding bytes are initialized to zero in - * the @c endpoint member. For this to work the @c buffer member must - * initialized prior to the @c endpoint member, e.g. - * @code - * union key_in6 key = { .buffer = { 0 } }; - * - * memcpy(key.endpoint.addr.s6_addr, - * sa->sin6_addr.s6_addr, - * sizeof(key.endpoint.addr.s6_addr)); - * - * key.endpoint.port = sa->sin6_port; * @endcode - * The goal is to avoid hashing uninitialized bytes in the hash key. */ -union key_in6 +struct key_in6 { - struct endpoint_in6 endpoint; - uint8_t buffer[sizeof(struct endpoint_in6)]; + struct in6_addr addr; + in_port_t port; + uint16_t padding; // To be implicitly initialized. }; - -static unsigned int mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, +static unsigned int +mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, uint32_t seed) { // No port set. Don't bother including it in the hash. @@ -95,19 +80,12 @@ static unsigned int mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, // --------------------------------------- // Non-zero port. Include it in the hash. + struct key_in const key = { + .addr = sa->sin_addr.s_addr, + .port = sa->sin_port + }; - /* - Initialize the key buffer first to ensure padding bytes in - the endpoint union member are zero. This works since the - buffer union member also includes padding bytes found in the - endpoint member. - */ - union key_in key = { .buffer = { 0 } }; - - key.endpoint.addr = sa->sin_addr.s_addr; - key.endpoint.port = sa->sin_port; - - return mptcpd_murmur_hash3(key.buffer, sizeof(key.buffer), seed); + return mptcpd_murmur_hash3(&key, sizeof(key), seed); } static unsigned int @@ -128,22 +106,13 @@ mptcpd_hash_sockaddr_in6(struct sockaddr_in6 const *sa, // --------------------------------------- // Non-zero port. Include it in the hash. + struct key_in6 key = { .port = sa->sin6_port }; - /* - Initialize the key buffer first to ensure padding bytes in - the endpoint union member are zero. This works since the - buffer union member also includes padding bytes found in the - endpoint member. - */ - union key_in6 key = { .buffer = { 0 } }; - - memcpy(key.endpoint.addr.s6_addr, + memcpy(key.addr.s6_addr, sa->sin6_addr.s6_addr, - sizeof(key.endpoint.addr.s6_addr)); - - key.endpoint.port = sa->sin6_port; + sizeof(key.addr.s6_addr)); - return mptcpd_murmur_hash3(key.buffer, sizeof(key.buffer), seed); + return mptcpd_murmur_hash3(&key, sizeof(key), seed); } unsigned int mptcpd_hash_sockaddr(void const *p) From 48274216956cdffa4a67bcde7c4415350e19de56 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Mon, 18 Jul 2022 14:43:37 -0700 Subject: [PATCH 40/74] hash_sockaddr: Include port in key comparison. --- lib/hash_sockaddr.c | 98 +++++++++++++++++++++++++++++---------------- 1 file changed, 64 insertions(+), 34 deletions(-) diff --git a/lib/hash_sockaddr.c b/lib/hash_sockaddr.c index b4fc1943..bafc1ae3 100644 --- a/lib/hash_sockaddr.c +++ b/lib/hash_sockaddr.c @@ -7,6 +7,11 @@ * Copyright (c) 2022, Intel Corporation */ +#ifdef HAVE_CONFIG_H +# include +#endif + +#include #include #include #include @@ -26,7 +31,7 @@ * @brief Bundle IPv4 address and port as a hash key. * * A @c padding member is added to allow the padding bytes to be - * implicitly initialized to zero in a designated initializer, e.g.: + * initialized to zero in a designated initializer, e.g.: * @code * struct endpoint_in endpoint = { * .addr = 0xC0000202, @@ -37,16 +42,16 @@ */ struct key_in { - in_addr_t addr; - in_port_t port; - uint16_t padding; // To be implicitly initialized. + in_addr_t const addr; + in_port_t const port; + uint16_t const padding; // Initialize to zero. }; /** * @brief Bundle IPv6 address and port as a hash key. * * A @c padding member is added to allow the padding bytes to be - * implicitly initialized to zero in a designated initializer, e.g.: + * initialized to zero in a designated initializer, e.g.: * @code * struct endpoint_in6 endpoint = { * .addr = { .s6_addr = { [0] = 0x20, @@ -63,8 +68,8 @@ struct key_in struct key_in6 { struct in6_addr addr; - in_port_t port; - uint16_t padding; // To be implicitly initialized. + in_port_t const port; + uint16_t const padding; // Initialize to zero. }; static unsigned int @@ -120,6 +125,8 @@ unsigned int mptcpd_hash_sockaddr(void const *p) struct mptcpd_hash_sockaddr_key const *const key = p; struct sockaddr const *const sa = key->sa; + assert(sa->sa_family == AF_INET || sa->sa_family == AF_INET6); + if (sa->sa_family == AF_INET) { struct sockaddr_in const *sa4 = (struct sockaddr_in const *) sa; @@ -133,6 +140,54 @@ unsigned int mptcpd_hash_sockaddr(void const *p) } } +static inline int compare_port(in_port_t lhs, in_port_t rhs) +{ + return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); +} + +static int compare_sockaddr_in(struct sockaddr const *lsa, + struct sockaddr const *rsa) +{ + struct sockaddr_in const *const lin = + (struct sockaddr_in const *) lsa; + + struct sockaddr_in const *const rin = + (struct sockaddr_in const *) rsa; + + in_addr_t const lhs = lin->sin_addr.s_addr; + in_addr_t const rhs = rin->sin_addr.s_addr; + + if (lhs == rhs) + return compare_port(lin->sin_port, rin->sin_port); + + return lhs < rhs ? -1 : 1; +} + +static int compare_sockaddr_in6(struct sockaddr const *lsa, + struct sockaddr const *rsa) +{ + /** + * @todo Should we compare the other @c struct @c sockaddr_in6 + * fields as well? + */ + + struct sockaddr_in6 const *const lin = + (struct sockaddr_in6 const *) lsa; + + struct sockaddr_in6 const *const rin = + (struct sockaddr_in6 const *) rsa; + + uint8_t const *const lhs = lin->sin6_addr.s6_addr; + uint8_t const *const rhs = rin->sin6_addr.s6_addr; + + int const cmp = memcmp(lhs, rhs, sizeof(lin->sin6_addr.s6_addr)); + + if (cmp == 0) + return compare_port(lin->sin6_port, rin->sin6_port); + + return cmp; +} + int mptcpd_hash_sockaddr_compare(void const *a, void const *b) { struct mptcpd_hash_sockaddr_key const *const lkey = a; @@ -141,38 +196,13 @@ int mptcpd_hash_sockaddr_compare(void const *a, void const *b) struct sockaddr const *const lsa = lkey->sa; struct sockaddr const *const rsa = rkey->sa; - /** - * @todo Should we compare the other @c struct @c sockaddr_in - * and @c struct @c sockaddr_in6 fields as well? - */ - if (lsa->sa_family == rsa->sa_family) { if (lsa->sa_family == AF_INET) { // IPv4 - struct sockaddr_in const *const lin = - (struct sockaddr_in const *) lsa; - - struct sockaddr_in const *const rin = - (struct sockaddr_in const *) rsa; - - uint32_t const lhs = lin->sin_addr.s_addr; - uint32_t const rhs = rin->sin_addr.s_addr; - - return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); + return compare_sockaddr_in(lsa, rsa); } else { // IPv6 - struct sockaddr_in6 const *const lin = - (struct sockaddr_in6 const *) lsa; - - struct sockaddr_in6 const *const rin = - (struct sockaddr_in6 const *) rsa; - - uint8_t const *const lhs = lin->sin6_addr.s6_addr; - uint8_t const *const rhs = rin->sin6_addr.s6_addr; - - return memcmp(lhs, - rhs, - sizeof(lin->sin6_addr.s6_addr)); + return compare_sockaddr_in6(lsa, rsa); } } else if (lsa->sa_family == AF_INET) { return 1; // IPv4 > IPv6 From 7d0d315a7dfff163524bd2f8eb370a901b6afe3f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Mon, 18 Jul 2022 14:46:38 -0700 Subject: [PATCH 41/74] tests: Expand mptcpd_lm test cases. Add test cases for non-zero IP ports to the test-listener-manager unit test. --- tests/test-listener-manager.c | 114 ++++++++++++++++++++++++++-------- 1 file changed, 87 insertions(+), 27 deletions(-) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index 04d19260..2c4b7b2c 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -9,7 +9,9 @@ #include +#include +#include #include #include #include @@ -22,6 +24,52 @@ #include +struct ipv4_listen_case +{ + struct sockaddr_in const addr; + char const *const desc; +}; + +struct ipv6_listen_case +{ + struct sockaddr_in6 const addr; + char const *const desc; +}; + +/** + * @brief Initialize test case to listed on IPv4 loopback address. + * + * @param[in] port IP port to listen on. + */ +#define INIT_IPV4_TEST_CASE(port) \ + { \ + .addr = { \ + .sin_family = AF_INET, \ + .sin_port = htons(port), \ + .sin_addr = { \ + .s_addr = htonl(INADDR_LOOPBACK) \ + } \ + }, \ + .desc = "listen - IPv4 on port " L_STRINGIFY(port) \ + } + +/** + * @brief Initialize test case to listed on IPv6 loopback address. + * + * @param[in] port IP port to listen on. + */ +#define INIT_IPV6_TEST_CASE(port) \ + { \ + .addr = { \ + .sin6_family = AF_INET6, \ + .sin6_port = htons(port), \ + .sin6_addr = { .s6_addr = { [15] = 0x01 } } \ + }, \ + .desc = "listen - IPv6 on port " L_STRINGIFY(port) \ + } + +// ----------------------------------------------------------------------- + static struct mptcpd_lm *_lm; static void test_create(void const *test_data) @@ -47,7 +95,6 @@ static void test_close(void const *test_data) assert(mptcpd_lm_close(_lm, sa)); } - static void test_destroy(void const *test_data) { (void) test_data; @@ -58,43 +105,56 @@ static void test_destroy(void const *test_data) int main(int argc, char *argv[]) { l_log_set_stderr(); + //l_debug_enable("*"); l_test_init(&argc, &argv); /* - Listen on the loopback address since we need an address - backed by a network interface so that underlying bind() call - can succeed. + Listen on the IPv4 and IPv6 loopback addresses since we need + an address backed by a network interface so that the + underlying bind() call can succeed. */ - struct sockaddr_in addr4 = { - .sin_family = AF_INET, - .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) } + struct ipv4_listen_case const ipv4_cases[] = { + INIT_IPV4_TEST_CASE(0x3456), + INIT_IPV4_TEST_CASE(0x3457), + INIT_IPV4_TEST_CASE(0x3456), // Same port as above. + INIT_IPV4_TEST_CASE(0) }; - struct sockaddr const *const sa4 = - (struct sockaddr const *) &addr4; - - /* - Listen on the loopback address since we need an address - backed by a network interface so that underlying bind() call - can succeed. - */ - struct sockaddr_in6 addr6 = { - .sin6_family = AF_INET6 + struct ipv6_listen_case const ipv6_cases[] = { + INIT_IPV6_TEST_CASE(0x4567), + INIT_IPV6_TEST_CASE(0x4578), + INIT_IPV6_TEST_CASE(0x4567), // Same port as above. + INIT_IPV6_TEST_CASE(0) }; - addr6.sin6_addr = in6addr_loopback; - struct sockaddr const *const sa6 = - (struct sockaddr const *) &addr6; + l_test_add("create lm", test_create, NULL); + + for (size_t i = 0; i < L_ARRAY_SIZE(ipv4_cases); ++i) { + char const *const desc = ipv4_cases[i].desc; + struct sockaddr const *const sa = + (struct sockaddr const *) &ipv4_cases[i].addr; + + l_test_add(desc, test_listen, sa); + } + + for (size_t i = 0; i < L_ARRAY_SIZE(ipv6_cases); ++i) { + char const *const desc = ipv6_cases[i].desc; + struct sockaddr const *const sa = + (struct sockaddr const *) &ipv6_cases[i].addr; + + l_test_add(desc, test_listen, sa); + } + + l_test_add("close - IPv4", + test_close, + (struct sockaddr const *) &ipv4_cases[0]); + l_test_add("close - IPv6", + test_close, + (struct sockaddr const *) &ipv6_cases[0]); - l_test_add("create listener manager", test_create, NULL); - l_test_add("listen - IPv4 - FIRST", test_listen, sa4); - l_test_add("listen - IPv6", test_listen, sa6); - l_test_add("listen - IPv4 - SECOND", test_listen, sa4); - l_test_add("close - IPv4", test_close, sa4); - l_test_add("close - IPv6", test_close, sa6); - l_test_add("destroy listener manager", test_destroy, NULL); + l_test_add("destroy lm", test_destroy, NULL); return l_test_run(); } From 89ae85c387a9a8a458efb605af36a9a909cbfffd Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Mon, 18 Jul 2022 14:49:03 -0700 Subject: [PATCH 42/74] tests: Fix spelling typo. --- tests/test-listener-manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index 2c4b7b2c..8c1ec208 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -37,7 +37,7 @@ struct ipv6_listen_case }; /** - * @brief Initialize test case to listed on IPv4 loopback address. + * @brief Initialize test case to listen on IPv4 loopback address. * * @param[in] port IP port to listen on. */ @@ -54,7 +54,7 @@ struct ipv6_listen_case } /** - * @brief Initialize test case to listed on IPv6 loopback address. + * @brief Initialize test case to listen on IPv6 loopback address. * * @param[in] port IP port to listen on. */ From ba87b88fc18040687df81b83e1878d92f26fd95d Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 19 Jul 2022 09:20:50 -0700 Subject: [PATCH 43/74] test-id-manager: Add different port test case. Verify that getting the MPTCP address ID for sockaddrs containing the same IP address but different ports results in the same ID. Only the IP address should be considered by the mptcpd ID manager. --- tests/test-id-manager.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test-id-manager.c b/tests/test-id-manager.c index 8df83c54..c81a3eea 100644 --- a/tests/test-id-manager.c +++ b/tests/test-id-manager.c @@ -69,6 +69,12 @@ static void test_get_id(void const *test_data) (struct sockaddr *) &test_laddr_1); assert(_id[0] != 0); + struct sockaddr_in laddr_1_diff_port = test_laddr_1; + laddr_1_diff_port.sin_port = test_laddr_1.sin_port + 1; + assert(mptcpd_idm_get_id(_idm, + (struct sockaddr *) &laddr_1_diff_port) + == _id[0]); + _id[1] = mptcpd_idm_get_id(_idm, (struct sockaddr *) &test_laddr_2); assert(_id[1] != 0 && _id[1] != _id[0]); From dbd8dbd3a8b11da56cc05af1f1991c7b0a2aa979 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 19 Jul 2022 09:24:29 -0700 Subject: [PATCH 44/74] lib: Refactor sockaddr hashing to IDM and LM. The mptcpd ID manager and listener manager each have specific sockaddr hashing requirements, where the former does not need the IP port and the latter does. Attempting to make a single hash implementation support both cases was clumsy. The mptcpd ID manager and listener manager will each implement their own hash functions. Common sockaddr hashing related functions still exist in hash_sockaddr.[hc]. --- lib/hash_sockaddr.c | 130 +-------------------------------- lib/hash_sockaddr.h | 15 +--- lib/id_manager.c | 55 +++++++++++++- lib/listener_manager.c | 158 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 215 insertions(+), 143 deletions(-) diff --git a/lib/hash_sockaddr.c b/lib/hash_sockaddr.c index bafc1ae3..e56a922f 100644 --- a/lib/hash_sockaddr.c +++ b/lib/hash_sockaddr.c @@ -27,124 +27,6 @@ #include "hash_sockaddr.h" -/** - * @brief Bundle IPv4 address and port as a hash key. - * - * A @c padding member is added to allow the padding bytes to be - * initialized to zero in a designated initializer, e.g.: - * @code - * struct endpoint_in endpoint = { - * .addr = 0xC0000202, - * .port = 0x4321 - * }; - * The goal is to avoid hashing uninitialized bytes in the hash key. - * @endcode - */ -struct key_in -{ - in_addr_t const addr; - in_port_t const port; - uint16_t const padding; // Initialize to zero. -}; - -/** - * @brief Bundle IPv6 address and port as a hash key. - * - * A @c padding member is added to allow the padding bytes to be - * initialized to zero in a designated initializer, e.g.: - * @code - * struct endpoint_in6 endpoint = { - * .addr = { .s6_addr = { [0] = 0x20, - * [1] = 0x01, - * [2] = 0X0D, - * [3] = 0xB8, - * [14] = 0x01, - * [15] = 0x02 } }, - * .port = 0x4321 - * }; - * The goal is to avoid hashing uninitialized bytes in the hash key. - * @endcode - */ -struct key_in6 -{ - struct in6_addr addr; - in_port_t const port; - uint16_t const padding; // Initialize to zero. -}; - -static unsigned int -mptcpd_hash_sockaddr_in(struct sockaddr_in const *sa, - uint32_t seed) -{ - // No port set. Don't bother including it in the hash. - if (sa->sin_port == 0) - return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, - sizeof(sa->sin_addr.s_addr), - seed); - - // --------------------------------------- - - // Non-zero port. Include it in the hash. - struct key_in const key = { - .addr = sa->sin_addr.s_addr, - .port = sa->sin_port - }; - - return mptcpd_murmur_hash3(&key, sizeof(key), seed); -} - -static unsigned int -mptcpd_hash_sockaddr_in6(struct sockaddr_in6 const *sa, - uint32_t seed) -{ - /** - * @todo Should we include other sockaddr_in6 members, e.g. - * sin6_flowinfo and sin6_scope_id, as part of the key? - */ - - // No port set. Don't bother including it in the hash. - if (sa->sin6_port == 0) - return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, - sizeof(sa->sin6_addr.s6_addr), - seed); - - // --------------------------------------- - - // Non-zero port. Include it in the hash. - struct key_in6 key = { .port = sa->sin6_port }; - - memcpy(key.addr.s6_addr, - sa->sin6_addr.s6_addr, - sizeof(key.addr.s6_addr)); - - return mptcpd_murmur_hash3(&key, sizeof(key), seed); -} - -unsigned int mptcpd_hash_sockaddr(void const *p) -{ - struct mptcpd_hash_sockaddr_key const *const key = p; - struct sockaddr const *const sa = key->sa; - - assert(sa->sa_family == AF_INET || sa->sa_family == AF_INET6); - - if (sa->sa_family == AF_INET) { - struct sockaddr_in const *sa4 = - (struct sockaddr_in const *) sa; - - return mptcpd_hash_sockaddr_in(sa4, key->seed); - } else { - struct sockaddr_in6 const *sa6 = - (struct sockaddr_in6 const *) sa; - - return mptcpd_hash_sockaddr_in6(sa6, key->seed); - } -} - -static inline int compare_port(in_port_t lhs, in_port_t rhs) -{ - return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); -} - static int compare_sockaddr_in(struct sockaddr const *lsa, struct sockaddr const *rsa) { @@ -157,10 +39,7 @@ static int compare_sockaddr_in(struct sockaddr const *lsa, in_addr_t const lhs = lin->sin_addr.s_addr; in_addr_t const rhs = rin->sin_addr.s_addr; - if (lhs == rhs) - return compare_port(lin->sin_port, rin->sin_port); - - return lhs < rhs ? -1 : 1; + return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); } static int compare_sockaddr_in6(struct sockaddr const *lsa, @@ -180,12 +59,7 @@ static int compare_sockaddr_in6(struct sockaddr const *lsa, uint8_t const *const lhs = lin->sin6_addr.s6_addr; uint8_t const *const rhs = rin->sin6_addr.s6_addr; - int const cmp = memcmp(lhs, rhs, sizeof(lin->sin6_addr.s6_addr)); - - if (cmp == 0) - return compare_port(lin->sin6_port, rin->sin6_port); - - return cmp; + return memcmp(lhs, rhs, sizeof(lin->sin6_addr.s6_addr)); } int mptcpd_hash_sockaddr_compare(void const *a, void const *b) diff --git a/lib/hash_sockaddr.h b/lib/hash_sockaddr.h index 0d08e47b..6b3e2512 100644 --- a/lib/hash_sockaddr.h +++ b/lib/hash_sockaddr.h @@ -47,18 +47,7 @@ struct mptcpd_hash_sockaddr_key }; /** - * @brief Generate a hash value based on a @c struct @c sockaddr. - * - * @param[in] p @c struct @c mptcpd_hash_sockaddr_key instance containing - * the IP address to be hashed. - * - * @return The hash value. - */ -unsigned int mptcpd_hash_sockaddr(void const *p); - - -/** - * @brief Compare hash map keys based on IP address. + * @brief Compare hash map keys based on IP address alone. * * @param[in] a Pointer @c struct @c sockaddr (left hand side). * @param[in] b Pointer @c struct @c sockaddr (right hand side). @@ -66,6 +55,8 @@ unsigned int mptcpd_hash_sockaddr(void const *p); * @return 0 if the IP addresses are equal, and -1 or 1 otherwise, * depending on IP address family and comparisons of IP * addresses of the same type. + * + * @note Ports are not compared. */ int mptcpd_hash_sockaddr_compare(void const *a, void const *b); diff --git a/lib/id_manager.c b/lib/id_manager.c index 7aba975d..ec4dbab6 100644 --- a/lib/id_manager.c +++ b/lib/id_manager.c @@ -90,6 +90,59 @@ static bool mptcpd_hashmap_replace(struct l_hashmap *map, // ---------------------------------------------------------------------- +static inline +unsigned int hash_sockaddr_in(struct sockaddr_in const *sa, + uint32_t seed) +{ + return mptcpd_murmur_hash3(&sa->sin_addr.s_addr, + sizeof(sa->sin_addr.s_addr), + seed); +} + +static inline +unsigned int hash_sockaddr_in6(struct sockaddr_in6 const *sa, + uint32_t seed) +{ + /** + * @todo Should we include other sockaddr_in6 members, e.g. + * sin6_flowinfo and sin6_scope_id, as part of the key? + */ + + return mptcpd_murmur_hash3(sa->sin6_addr.s6_addr, + sizeof(sa->sin6_addr.s6_addr), + seed); +} + +/** + * @brief Generate a hash value based on IP address alone. + * + * @param[in] p @c struct @c mptcpd_hash_sockaddr_key instance + * containing the IP address to be hashed. + * + * @return The hash value. + */ +static unsigned int hash_sockaddr(void const *p) +{ + struct mptcpd_hash_sockaddr_key const *const key = p; + struct sockaddr const *const sa = key->sa; + + assert(sa->sa_family == AF_INET || sa->sa_family == AF_INET6); + + if (sa->sa_family == AF_INET) { + struct sockaddr_in const *sa4 = + (struct sockaddr_in const *) sa; + + return hash_sockaddr_in(sa4, key->seed); + } else { + struct sockaddr_in6 const *sa6 = + (struct sockaddr_in6 const *) sa; + + return hash_sockaddr_in6(sa6, key->seed); + } +} + +// ---------------------------------------------------------------------- + struct mptcpd_idm *mptcpd_idm_create(void) { struct mptcpd_idm *idm = l_new(struct mptcpd_idm, 1); @@ -100,7 +153,7 @@ struct mptcpd_idm *mptcpd_idm_create(void) idm->map = l_hashmap_new(); idm->seed = l_getrandom_uint32(); - if (!l_hashmap_set_hash_function(idm->map, mptcpd_hash_sockaddr) + if (!l_hashmap_set_hash_function(idm->map, hash_sockaddr) || !l_hashmap_set_compare_function(idm->map, mptcpd_hash_sockaddr_compare) || !l_hashmap_set_key_copy_function(idm->map, diff --git a/lib/listener_manager.c b/lib/listener_manager.c index 2ab8a1c1..992dff28 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -27,6 +27,7 @@ #include #pragma GCC diagnostic pop +#include #include #include "hash_sockaddr.h" @@ -70,6 +71,159 @@ struct lm_value // ---------------------------------------------------------------------- +/** + * @brief Bundle IPv4 address and port as a hash key. + * + * A @c padding member is added to allow the padding bytes to be + * initialized to zero in a designated initializer, e.g.: + * @code + * struct endpoint_in endpoint = { + * .addr = 0xC0000202, + * .port = 0x4321 + * }; + * The goal is to avoid hashing uninitialized bytes in the hash key. + * @endcode + */ +struct key_in +{ + in_addr_t const addr; + in_port_t const port; + uint16_t const padding; // Initialize to zero. +}; + +/** + * @brief Bundle IPv6 address and port as a hash key. + * + * A @c padding member is added to allow the padding bytes to be + * initialized to zero in a designated initializer, e.g.: + * @code + * struct endpoint_in6 endpoint = { + * .addr = { .s6_addr = { [0] = 0x20, + * [1] = 0x01, + * [2] = 0X0D, + * [3] = 0xB8, + * [14] = 0x01, + * [15] = 0x02 } }, + * .port = 0x4321 + * }; + * The goal is to avoid hashing uninitialized bytes in the hash key. + * @endcode + */ +struct key_in6 +{ + struct in6_addr addr; + in_port_t const port; + uint16_t const padding; // Initialize to zero. +}; + +static unsigned int hash_sockaddr_in(struct sockaddr_in const *sa, + uint32_t seed) +{ + struct key_in const key = { + .addr = sa->sin_addr.s_addr, + .port = sa->sin_port + }; + + return mptcpd_murmur_hash3(&key, sizeof(key), seed); +} + +static unsigned int hash_sockaddr_in6(struct sockaddr_in6 const *sa, + uint32_t seed) +{ + /** + * @todo Should we include other sockaddr_in6 members, e.g. + * sin6_flowinfo and sin6_scope_id, as part of the key? + */ + struct key_in6 key = { .port = sa->sin6_port }; + + memcpy(key.addr.s6_addr, + sa->sin6_addr.s6_addr, + sizeof(key.addr.s6_addr)); + + return mptcpd_murmur_hash3(&key, sizeof(key), seed); +} + +/** + * @brief Generate a hash value based on IP address and port. + * + * @param[in] p @c struct @c mptcpd_hash_sockaddr_key instance + * containing the IP address and port to be hashed. + * + * @return The hash value. + */ +static unsigned int hash_sockaddr(void const *p) +{ + struct mptcpd_hash_sockaddr_key const *const key = p; + struct sockaddr const *const sa = key->sa; + + assert(sa->sa_family == AF_INET || sa->sa_family == AF_INET6); + + if (sa->sa_family == AF_INET) { + struct sockaddr_in const *sa4 = + (struct sockaddr_in const *) sa; + + return hash_sockaddr_in(sa4, key->seed); + } else { + struct sockaddr_in6 const *sa6 = + (struct sockaddr_in6 const *) sa; + + return hash_sockaddr_in6(sa6, key->seed); + } +} + +static inline int compare_port(in_port_t lhs, in_port_t rhs) +{ + return lhs < rhs ? -1 : (lhs > rhs ? 1 : 0); +} + +/** + * @brief Compare hash map keys based on IP address and port. + * + * @param[in] a Pointer @c struct @c sockaddr (left hand side). + * @param[in] b Pointer @c struct @c sockaddr (right hand side). + * + * @return 0 if the IP addresses and ports are equal, and -1 or 1 + * otherwise, depending on IP address family and comparisons + * of IP addresses and ports of the same type. + */ +static int hash_sockaddr_compare(void const *a, void const *b) +{ + int const cmp = mptcpd_hash_sockaddr_compare(a, b); + + if (cmp != 0) + return cmp; + + struct mptcpd_hash_sockaddr_key const *const lkey = a; + struct mptcpd_hash_sockaddr_key const *const rkey = b; + + struct sockaddr const *const lsa = lkey->sa; + struct sockaddr const *const rsa = rkey->sa; + + in_port_t lport, rport; + + if (lsa->sa_family == AF_INET) { + struct sockaddr_in const *const lin = + (struct sockaddr_in const *) lsa; + struct sockaddr_in const *const rin = + (struct sockaddr_in const *) rsa; + + lport = lin->sin_port; + rport = rin->sin_port; + } else { + struct sockaddr_in6 const *const lin = + (struct sockaddr_in6 const *) lsa; + struct sockaddr_in6 const *const rin = + (struct sockaddr_in6 const *) rsa; + + lport = lin->sin6_port; + rport = rin->sin6_port; + } + + return compare_port(lport, rport); +} + +// ---------------------------------------------------------------------- + static int open_listener(struct sockaddr const *sa) { #ifndef IPPROTO_MPTCP @@ -129,9 +283,9 @@ struct mptcpd_lm *mptcpd_lm_create(void) lm->map = l_hashmap_new(); lm->seed = l_getrandom_uint32(); - if (!l_hashmap_set_hash_function(lm->map, mptcpd_hash_sockaddr) + if (!l_hashmap_set_hash_function(lm->map, hash_sockaddr) || !l_hashmap_set_compare_function(lm->map, - mptcpd_hash_sockaddr_compare) + hash_sockaddr_compare) || !l_hashmap_set_key_copy_function(lm->map, mptcpd_hash_sockaddr_key_copy) || !l_hashmap_set_key_free_function(lm->map, From 9a0cf4925c0c4bed5d8b34ceed33a6f623e22d30 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 21 Jul 2022 22:46:02 -0700 Subject: [PATCH 45/74] listener_manager: Make factory functions private. Only one instance of a mptcpd listener manager should be created. That instance should be managed by the mptcpd core. Make the mptcpd_lm_create() and mptcpd_lm_destroy() part of the private mptcpd API. --- include/mptcpd/Makefile.am | 1 + include/mptcpd/listener_manager.h | 20 ++------- include/mptcpd/private/listener_manager.h | 50 +++++++++++++++++++++++ lib/listener_manager.c | 1 + src/path_manager.c | 2 +- tests/test-listener-manager.c | 1 + 6 files changed, 58 insertions(+), 17 deletions(-) create mode 100644 include/mptcpd/private/listener_manager.h diff --git a/include/mptcpd/Makefile.am b/include/mptcpd/Makefile.am index 032b8aca..086cbb72 100644 --- a/include/mptcpd/Makefile.am +++ b/include/mptcpd/Makefile.am @@ -17,6 +17,7 @@ noinst_HEADERS = \ private/config.h \ private/configuration.h \ private/id_manager.h \ + private/listener_manager.h \ private/mptcp_org.h \ private/mptcp_upstream.h \ private/murmur_hash.h \ diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index 7de4c1e2..af615c19 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -2,7 +2,7 @@ /** * @file listener_manager.h * - * @brief Map of MPTCP local address ID to listener. + * @brief Map of MPTCP local address to listener. * * Copyright (c) 2022, Intel Corporation */ @@ -22,24 +22,12 @@ extern "C" { struct sockaddr; struct mptcpd_lm; -/** - * @brief Create a MPTCP listener manager. - * - * @return Pointer to a MPTCP listener manager on success. @c NULL on - * failure. - */ -MPTCPD_API struct mptcpd_lm *mptcpd_lm_create(void); - -/** - * @brief Destroy MPTCP listener manager. - * - * @param[in,out] lm The mptcpd address listener manager object. - */ -MPTCPD_API void mptcpd_lm_destroy(struct mptcpd_lm *lm); - /** * @brief Listen on the given MPTCP local address. * + * Create a MPTCP listening socket for the given local address. This + * is needed to accept subflows, e.g. during a @c MP_JOIN operation. + * * @param[in] lm The mptcpd address listener manager object. * @param[in] sa The MPTCP local address. * diff --git a/include/mptcpd/private/listener_manager.h b/include/mptcpd/private/listener_manager.h new file mode 100644 index 00000000..c2dcdba5 --- /dev/null +++ b/include/mptcpd/private/listener_manager.h @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: BSD-3-Clause +/** + * @file listener_manager.h + * + * @brief Map of MPTCP local address to listener - private API. + * + * Copyright (c) 2022, Intel Corporation + */ + +#ifndef MPTCPD_PRIVATE_LISTENER_MANAGER_H +#define MPTCPD_PRIVATE_LISTENER_MANAGER_H + +#include + + +#ifdef __cplusplus +extern "C" { +#endif + +struct mptcpd_lm; + +/** + * @brief Create a MPTCP listener manager. + * + * @return Pointer to a MPTCP listener manager on success. @c NULL on + * failure. + */ +MPTCPD_API struct mptcpd_lm *mptcpd_lm_create(void); + +/** + * @brief Destroy MPTCP listener manager. + * + * @param[in,out] lm The mptcpd address listener manager object. + */ +MPTCPD_API void mptcpd_lm_destroy(struct mptcpd_lm *lm); + + +#ifdef __cplusplus +} +#endif + + +#endif /* MPTCPD_PRIVATE_LISTENER_MANAGER_H */ + + +/* + Local Variables: + c-file-style: "linux" + End: +*/ diff --git a/lib/listener_manager.c b/lib/listener_manager.c index 992dff28..15f01bd8 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -28,6 +28,7 @@ #pragma GCC diagnostic pop #include +#include #include #include "hash_sockaddr.h" diff --git a/src/path_manager.c b/src/path_manager.c index f73bcb84..be1b928a 100644 --- a/src/path_manager.c +++ b/src/path_manager.c @@ -39,7 +39,7 @@ #include #include #include -#include +#include // For netlink events. Same API applies to multipath-tcp.org kernel. #include diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index 8c1ec208..790af66b 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -16,6 +16,7 @@ #include #include +#include #include #include "test-plugin.h" // For test sockaddrs From 2a5ace109e518b1c1d0ed54dbbf45eec3b68e7b6 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 21 Jul 2022 22:49:19 -0700 Subject: [PATCH 46/74] path_manager: Add mptcpd_pm_get_lm() accessor. Allow the user to get access to the global mptcpd listener manager by adding a new mptcpd_pm_get_lm() function to the public API. --- include/mptcpd/path_manager.h | 13 +++++++++++++ lib/path_manager.c | 5 +++++ 2 files changed, 18 insertions(+) diff --git a/include/mptcpd/path_manager.h b/include/mptcpd/path_manager.h index 10b83b7b..e237e569 100644 --- a/include/mptcpd/path_manager.h +++ b/include/mptcpd/path_manager.h @@ -372,6 +372,19 @@ mptcpd_pm_get_nm(struct mptcpd_pm const *pm); MPTCPD_API struct mptcpd_idm * mptcpd_pm_get_idm(struct mptcpd_pm const *pm); +/** + * @brief Get pointer to the global MPTCP listener manager. + * + * @param[in] pm Mptcpd path manager data. + * + * @note The global MPTCP listener manager tracks MPTCP listening + * sockets associated with a local address. + * + * @return Global mptcpd MPTCP listener manager. + */ +MPTCPD_API struct mptcpd_lm * +mptcpd_pm_get_lm(struct mptcpd_pm const *pm); + #ifdef __cplusplus } #endif diff --git a/lib/path_manager.c b/lib/path_manager.c index 6c939d76..827e3c48 100644 --- a/lib/path_manager.c +++ b/lib/path_manager.c @@ -370,6 +370,11 @@ struct mptcpd_idm * mptcpd_pm_get_idm(struct mptcpd_pm const *pm) return pm->idm; } +struct mptcpd_lm * mptcpd_pm_get_lm(struct mptcpd_pm const *pm) +{ + return pm->lm; +} + /* Local Variables: From b40f91faf80a08e9216885002ab5fa6ad9743658 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 21 Jul 2022 22:51:01 -0700 Subject: [PATCH 47/74] tests: Expand internal mptcpd_pm state checks. --- tests/test-path-manager.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/test-path-manager.c b/tests/test-path-manager.c index 0241500c..558a8b20 100644 --- a/tests/test-path-manager.c +++ b/tests/test-path-manager.c @@ -90,17 +90,28 @@ static void test_pm_create(void const *test_data) { struct test_info *const info = (struct test_info *) test_data; - info->pm = mptcpd_pm_create(info->config); + struct mptcpd_pm *const pm = mptcpd_pm_create(info->config); - assert(info->pm != NULL); - assert(info->pm->genl != NULL); - assert(info->pm->nm != NULL); + assert(pm != NULL); + assert(pm->config != NULL); + assert(pm->genl != NULL); + assert(pm->timeout != NULL); + assert(pm->nm != NULL); + assert(pm->idm != NULL); + assert(pm->lm != NULL); + assert(pm->event_ops != NULL); + + assert(mptcpd_pm_get_nm(pm) == pm->nm); + assert(mptcpd_pm_get_idm(pm) == pm->idm); + assert(mptcpd_pm_get_lm(pm) == pm->lm); /* Other struct mptcpd_pm fields may not have been initialized yet since they depend on the existence of the "mptcp" generic netlink family. */ + + info->pm = pm; } static void test_pm_register_ops(void const *test_data) From 0208b79b2524caa726ea7521db8e26e31254b1cc Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 21 Jul 2022 22:51:39 -0700 Subject: [PATCH 48/74] include: Clarify the role of the listener manager. --- include/mptcpd/private/path_manager.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/mptcpd/private/path_manager.h b/include/mptcpd/private/path_manager.h index 2498162d..6f807fb1 100644 --- a/include/mptcpd/private/path_manager.h +++ b/include/mptcpd/private/path_manager.h @@ -95,7 +95,9 @@ struct mptcpd_pm * @brief MPTCP listener manager. * * The MPTCP listener manager maps MPTCP local addresses to a - * listening socket file descriptors bound to those addresses. + * listening socket file descriptors bound to those addresses + * to allow passive subflow connections (joins) to be + * accepted. */ struct mptcpd_lm *lm; From 56d292245a5190993c8a38f6f852adc958d99586 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 21 Jul 2022 22:52:43 -0700 Subject: [PATCH 49/74] include: Clarify 'server_side' plugin ops param. Co-authored-by: Mat Martineau --- include/mptcpd/plugin.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/mptcpd/plugin.h b/include/mptcpd/plugin.h index d54c2b7c..58683d08 100644 --- a/include/mptcpd/plugin.h +++ b/include/mptcpd/plugin.h @@ -142,7 +142,9 @@ struct mptcpd_plugin_ops * @param[in] token MPTCP connection token. * @param[in] laddr Local address information. * @param[in] raddr Remote address information. - * @param[in] server_side Server side connection flag. + * @param[in] server_side @c true if this peer was the + * listener (server), @c false if this + * peer initiated the connection. * @param[in] pm Opaque pointer to mptcpd path * manager object. */ @@ -158,7 +160,9 @@ struct mptcpd_plugin_ops * @param[in] token MPTCP connection token. * @param[in] laddr Local address information. * @param[in] raddr Remote address information. - * @param[in] server_side Server side connection flag. + * @param[in] server_side @c true if this peer was the + * listener (server), @c false if this + * peer initiated the connection. * @param[in] pm Opaque pointer to mptcpd path * manager object. */ From 96712bef6c4dbd4e78313915b4a1849a3ebb9c6d Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Mon, 25 Jul 2022 11:28:01 -0700 Subject: [PATCH 50/74] lib: Remove unused Makefile variable assignment. --- lib/Makefile.am | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/Makefile.am b/lib/Makefile.am index c9b77ff6..990efaca 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -30,10 +30,6 @@ libmptcpd_la_SOURCES = \ hash_sockaddr.c \ hash_sockaddr.h -#IF BUILDING_DLL -#libmptcpd_la_SOURCES += debug.c -#endif - pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = mptcpd.pc From f5eb494b2c92e4d783798476f222076e4358120d Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Mon, 25 Jul 2022 22:48:27 -0700 Subject: [PATCH 51/74] hash_sockaddr: Remove extra line. --- lib/hash_sockaddr.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/hash_sockaddr.h b/lib/hash_sockaddr.h index 6b3e2512..f8449955 100644 --- a/lib/hash_sockaddr.h +++ b/lib/hash_sockaddr.h @@ -30,7 +30,6 @@ struct sockaddr; * exported from libmptcpd. */ ///@{ - /** * @brief Hash key information. * From f915775449cc9d785598a140fb37ace9f5080ec7 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Mon, 25 Jul 2022 22:52:29 -0700 Subject: [PATCH 52/74] listener_manager: Track ephemeral ports. Track the kernel assigned ephemeral port if the user provided a sockaddr with a zero port to mptcpd_lm_listen(), and return that port to the user. The user is expected to pass a sockaddr with the port returned from mptcpd_lm_listen() in a subsequent call to mptcpd_lm_close(). Passing a sockaddr with a zero port to mptcpd_lm_close() will result in failure since the listener manager now only tracks listeners with non-zero addresses. --- include/mptcpd/listener_manager.h | 22 +++++-- lib/listener_manager.c | 105 ++++++++++++++++++++---------- src/netlink_pm_upstream.c | 2 +- tests/test-listener-manager.c | 40 ++++++++++-- 4 files changed, 125 insertions(+), 44 deletions(-) diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index af615c19..24c4a7c2 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -10,7 +10,7 @@ #ifndef MPTCPD_LISTENER_MANAGER_H #define MPTCPD_LISTENER_MANAGER_H -#include +#include #include @@ -19,7 +19,7 @@ extern "C" { #endif -struct sockaddr; + struct mptcpd_lm; /** @@ -31,16 +31,26 @@ struct mptcpd_lm; * @param[in] lm The mptcpd address listener manager object. * @param[in] sa The MPTCP local address. * - * @return @c true on success, and @c false on failure. + * @return Non-zero port in network byte order to which the listener + * was bound, and zero on failure. An ephemeral port will be + * returned if the port in the local address @a sa is zero. + * The @c sockaddr passed to subsequent calls to + * @c mptcpd_lm_close() should take this into account since + * the mptcpd listener manager will only keep track of + * addresses with non-zero ports, including addresses with + * ephemeral ports. */ -MPTCPD_API bool mptcpd_lm_listen(struct mptcpd_lm *lm, - struct sockaddr const *sa); +MPTCPD_API in_port_t mptcpd_lm_listen(struct mptcpd_lm *lm, + struct sockaddr const *sa); /** * @brief Stop listening on a MPTCP local address. * * @param[in] lm The mptcpd address listener manager object. - * @param[in] sa The MPTCP local IP address. + * @param[in] sa The MPTCP local address with the non-zero port + * returned from @c mptcpd_lm_listen(), i.e. the + * non-zero port provided by the user or the ephemeral + * port chosen by the kernel. * * @return @c true on success, and @c false on failure. */ diff --git a/lib/listener_manager.c b/lib/listener_manager.c index 15f01bd8..03920e57 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -225,6 +225,13 @@ static int hash_sockaddr_compare(void const *a, void const *b) // ---------------------------------------------------------------------- +static in_port_t get_port(struct sockaddr const *sa) +{ + return sa->sa_family == AF_INET + ? ((struct sockaddr_in const *) sa)->sin_port + : ((struct sockaddr_in6 const *) sa)->sin6_port; +} + static int open_listener(struct sockaddr const *sa) { #ifndef IPPROTO_MPTCP @@ -263,17 +270,65 @@ static int open_listener(struct sockaddr const *sa) static void close_listener(void *value) { + if (value == NULL) + return; + struct lm_value *const data = value; - /** - * @todo Should care if the @c close() call fails? It - * shouldn't fail since we only get here if the listener - * socket was successfully opened earlier. - */ + // This close() should not fail since the fd is valid. (void) close(data->fd); + l_free(data); } +static in_port_t make_listener(struct mptcpd_lm* lm, + struct sockaddr const *sa) +{ + int const fd = open_listener(sa); + if (fd == -1) + return 0; + + /* + The port in a sockaddr used for the key should be non-zero. + Retrieve the socket address to which the socket file + descriptor is bound in case an ephemeral port was chosen by + the kernel, as is the case when the port in the user + provided sockaddr passed to bind() is zero. + */ + struct sockaddr_storage ss; + socklen_t addrlen = sizeof(ss); + + struct sockaddr *const addr = (struct sockaddr *) &ss; + + int const r = getsockname(fd, addr, &addrlen); + + if (r == -1) { + l_error("Unable to retrieve listening socket name: %s", + strerror(errno)); + (void) close(fd); + return 0; + } + + struct mptcpd_hash_sockaddr_key const key = { + .sa = addr, .seed = lm->seed + }; + + struct lm_value *const data = l_new(struct lm_value, 1); + + if (!l_hashmap_insert(lm->map, &key, data)) { + l_error("Unable to map MPTCP address to listener."); + + close_listener(data); + + return 0; + } + + data->fd = fd; + data->refcnt = 1; + + return get_port(key.sa); +} + // ---------------------------------------------------------------------- struct mptcpd_lm *mptcpd_lm_create(void) @@ -307,51 +362,35 @@ void mptcpd_lm_destroy(struct mptcpd_lm *lm) l_free(lm); } -bool mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr const *sa) +in_port_t mptcpd_lm_listen(struct mptcpd_lm *lm, + struct sockaddr const *sa) { if (lm == NULL || sa == NULL) - return false; + return 0; if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) - return false; + return 0; struct mptcpd_hash_sockaddr_key const key = { .sa = sa, .seed = lm->seed }; - struct lm_value *data = l_hashmap_lookup(lm->map, &key); + struct lm_value *const data = l_hashmap_lookup(lm->map, &key); /* - A listener already exists for the given IP address. + A listener already exists for the given address. Increment the reference count. */ if (data != NULL) { data->refcnt++; - - return true; - } - - data = l_new(struct lm_value, 1); - - data->fd = open_listener(sa); - if (data->fd == -1) { - l_free(data); - - return false; - } - - if (!l_hashmap_insert(lm->map, &key, data)) { - l_error("Unable to map MPTCP address to listener."); - - (void) close(data->fd); - l_free(data); - - return false; + return get_port(sa); } - data->refcnt = 1; - - return true; + /* + The sockaddr doesn't exist in the map. Make a new + listener. + */ + return make_listener(lm, sa); } bool mptcpd_lm_close(struct mptcpd_lm *lm, struct sockaddr const *sa) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index bcedd0e8..b8fd41fa 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -277,7 +277,7 @@ static int upstream_announce(struct mptcpd_pm *pm, * * @todo This should be optional. */ - if (!mptcpd_lm_listen(pm->lm, addr)) + if (mptcpd_lm_listen(pm->lm, addr) == 0) return -1; return send_add_addr(pm, diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index 790af66b..c8b5afc2 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -40,7 +40,7 @@ struct ipv6_listen_case /** * @brief Initialize test case to listen on IPv4 loopback address. * - * @param[in] port IP port to listen on. + * @param[in] port TCP port to listen on - host byte order. */ #define INIT_IPV4_TEST_CASE(port) \ { \ @@ -57,7 +57,7 @@ struct ipv6_listen_case /** * @brief Initialize test case to listen on IPv6 loopback address. * - * @param[in] port IP port to listen on. + * @param[in] port TCP port to listen on - host byte order. */ #define INIT_IPV6_TEST_CASE(port) \ { \ @@ -73,6 +73,14 @@ struct ipv6_listen_case static struct mptcpd_lm *_lm; +static in_port_t get_port(struct sockaddr const *sa) +{ + return sa->sa_family == AF_INET + ? ((struct sockaddr_in const *) sa)->sin_port + : ((struct sockaddr_in6 const *) sa)->sin6_port; +} + + static void test_create(void const *test_data) { (void) test_data; @@ -86,14 +94,29 @@ static void test_listen(void const *test_data) { struct sockaddr const *const sa = test_data; - assert(mptcpd_lm_listen(_lm, sa)); + in_port_t const original_port = get_port(sa); + in_port_t port = mptcpd_lm_listen(_lm, sa); + + assert(port != 0); + + if (original_port != 0) + assert(original_port == port); + + port = ntohs(port); + + l_info("Listening on port 0x%x (%u)", port, port); } static void test_close(void const *test_data) { struct sockaddr const *const sa = test_data; - assert(mptcpd_lm_close(_lm, sa)); + in_port_t const port = get_port(sa); + + if (port == 0) + assert(!mptcpd_lm_close(_lm, sa)); + else + assert(mptcpd_lm_close(_lm, sa)); } static void test_destroy(void const *test_data) @@ -155,6 +178,15 @@ int main(int argc, char *argv[]) test_close, (struct sockaddr const *) &ipv6_cases[0]); + struct sockaddr_in const zero_port_case = { + .sin_family = AF_INET, + .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) } + }; + + l_test_add("close - IPv4 - zero port", + test_close, + (struct sockaddr const *) &zero_port_case); + l_test_add("destroy lm", test_destroy, NULL); return l_test_run(); From 8022d4c3bacb5f689042f0452f52b3ecfa33c753 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 26 Jul 2022 09:11:26 -0700 Subject: [PATCH 53/74] listener_manager: Reject unbound IP addresses. Reject unbound IP addresses such as INADDR_ANY, INADDR_BROADCAST, and in6addr_any. The mptcpd listener manager requires addresses to be bound to a network interface. --- lib/listener_manager.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/listener_manager.c b/lib/listener_manager.c index 03920e57..b25954cc 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -232,6 +232,33 @@ static in_port_t get_port(struct sockaddr const *sa) : ((struct sockaddr_in6 const *) sa)->sin6_port; } +/** + * @brief Is IP address not bound to a specific network interface? + * + * @param[in] sa IP address. + * + * @return @c true if @a sa corresponds to an IP address that isn't + * bound to a network interface, and @c false otherwise. + */ +static bool is_unbound_address(struct sockaddr const *sa) +{ + if (sa->sa_family == AF_INET) { + struct sockaddr_in const *const a = + (struct sockaddr_in const *) sa; + + in_addr_t const s_addr = a->sin_addr.s_addr; + + return s_addr == INADDR_ANY || s_addr == INADDR_BROADCAST; + } else { + struct sockaddr_in6 const *const a = + (struct sockaddr_in6 const *) sa; + + struct in6_addr const *const addr = &a->sin6_addr; + + return memcmp(addr, &in6addr_any, sizeof(*addr)) == 0; + } +} + static int open_listener(struct sockaddr const *sa) { #ifndef IPPROTO_MPTCP @@ -371,6 +398,9 @@ in_port_t mptcpd_lm_listen(struct mptcpd_lm *lm, if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) return 0; + if (is_unbound_address(sa)) + return 0; + struct mptcpd_hash_sockaddr_key const key = { .sa = sa, .seed = lm->seed }; From 9b82aff83f5c66c54d59ee66ed779a8c420aa9d1 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 26 Jul 2022 09:14:07 -0700 Subject: [PATCH 54/74] test-listener-manager: Add bad address test cases. Verify that the mptcpd listener manager rejects addresses that are not bound to a network interface, i.e. INADDR_ANY, INADDR_BROADCAST, and in6addr_any. --- tests/test-listener-manager.c | 60 +++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index c8b5afc2..50f55957 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -107,6 +107,15 @@ static void test_listen(void const *test_data) l_info("Listening on port 0x%x (%u)", port, port); } +static void test_listen_bad_address(void const *test_data) +{ + struct sockaddr const *const sa = test_data; + + in_port_t const port = mptcpd_lm_listen(_lm, sa); + + assert(port == 0); +} + static void test_close(void const *test_data) { struct sockaddr const *const sa = test_data; @@ -133,6 +142,8 @@ int main(int argc, char *argv[]) l_test_init(&argc, &argv); + l_test_add("create lm", test_create, NULL); + /* Listen on the IPv4 and IPv6 loopback addresses since we need an address backed by a network interface so that the @@ -152,31 +163,48 @@ int main(int argc, char *argv[]) INIT_IPV6_TEST_CASE(0) }; - - l_test_add("create lm", test_create, NULL); - for (size_t i = 0; i < L_ARRAY_SIZE(ipv4_cases); ++i) { char const *const desc = ipv4_cases[i].desc; - struct sockaddr const *const sa = - (struct sockaddr const *) &ipv4_cases[i].addr; - l_test_add(desc, test_listen, sa); + l_test_add(desc, test_listen, &ipv4_cases[i].addr); } for (size_t i = 0; i < L_ARRAY_SIZE(ipv6_cases); ++i) { char const *const desc = ipv6_cases[i].desc; - struct sockaddr const *const sa = - (struct sockaddr const *) &ipv6_cases[i].addr; - l_test_add(desc, test_listen, sa); + l_test_add(desc, test_listen, &ipv6_cases[i].addr); } - l_test_add("close - IPv4", - test_close, - (struct sockaddr const *) &ipv4_cases[0]); - l_test_add("close - IPv6", - test_close, - (struct sockaddr const *) &ipv6_cases[0]); + // Test listen failure with "bad" (unbound) addresses. + struct sockaddr_in const ipv4_bad_cases[] = { + { + .sin_family = AF_INET, + .sin_addr = { .s_addr = INADDR_ANY } + }, + { + .sin_family = AF_INET, + .sin_addr = { .s_addr = INADDR_BROADCAST } + } + }; + + for (size_t i = 0; i < L_ARRAY_SIZE(ipv4_bad_cases); ++i) { + l_test_add("listen - bad IPv4 address", + test_listen_bad_address, + &ipv4_bad_cases[i]); + } + + struct sockaddr_in6 const ipv6_bad_case = { + .sin6_family = AF_INET6, + .sin6_addr = IN6ADDR_ANY_INIT + }; + + l_test_add("listen - bad IPv6 address", + test_listen_bad_address, + &ipv6_bad_case); + + // Listener close test cases. + l_test_add("close - IPv4", test_close, &ipv4_cases[0]); + l_test_add("close - IPv6", test_close, &ipv6_cases[0]); struct sockaddr_in const zero_port_case = { .sin_family = AF_INET, @@ -185,7 +213,7 @@ int main(int argc, char *argv[]) l_test_add("close - IPv4 - zero port", test_close, - (struct sockaddr const *) &zero_port_case); + &zero_port_case); l_test_add("destroy lm", test_destroy, NULL); From f2b15a220a9aafaaccd90560d1ea7f3afc7fd848 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 26 Jul 2022 12:05:12 -0700 Subject: [PATCH 55/74] lib: Move IPPROTO_MPTCP def to global scope. --- lib/listener_manager.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/listener_manager.c b/lib/listener_manager.c index b25954cc..8bc9e996 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -33,6 +33,10 @@ #include "hash_sockaddr.h" +#ifndef IPPROTO_MPTCP +#define IPPROTO_MPTCP IPPROTO_TCP + 256 +#endif + // ---------------------------------------------------------------------- @@ -261,10 +265,6 @@ static bool is_unbound_address(struct sockaddr const *sa) static int open_listener(struct sockaddr const *sa) { -#ifndef IPPROTO_MPTCP -#define IPPROTO_MPTCP IPPROTO_TCP + 256 -#endif - int const fd = socket(sa->sa_family, SOCK_STREAM, IPPROTO_MPTCP); if (fd == -1) { l_error("Unable to open MPTCP listener: %s", From daa13d41cccaccae60b747b083fe83c67a3f01cd Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Tue, 26 Jul 2022 12:07:26 -0700 Subject: [PATCH 56/74] src: Improve genl command error logging. --- src/commands.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/commands.c b/src/commands.c index b4f9b949..ca303535 100644 --- a/src/commands.c +++ b/src/commands.c @@ -54,8 +54,7 @@ bool mptcpd_check_genl_error(struct l_genl_msg *msg, char const *fname) int const error = l_genl_msg_get_error(msg); if (error < 0) { - // Error during send. Likely insufficient perms. - + // Error during send. char const *const genl_errmsg = #ifdef HAVE_L_GENL_MSG_GET_EXTENDED_ERROR l_genl_msg_get_extended_error(msg); @@ -63,18 +62,13 @@ bool mptcpd_check_genl_error(struct l_genl_msg *msg, char const *fname) NULL; #endif - if (genl_errmsg != NULL) { - l_error("%s: %s", fname, genl_errmsg); - } else { - char errmsg[80]; - int const r = strerror_r(-error, - errmsg, - L_ARRAY_SIZE(errmsg)); - - l_error("%s error: %s", - fname, - r == 0 ? errmsg : ""); - } + char errmsg[80] = { 0 }; + (void) strerror_r(-error, errmsg, L_ARRAY_SIZE(errmsg)); + + if (genl_errmsg != NULL) + l_error("%s: %s: %s", fname, genl_errmsg, errmsg); + else + l_error("%s: %s", fname, errmsg); return false; } From 47bafcadca43ba5275ccd35031c6633ef2107ae4 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 3 Aug 2022 16:25:57 -0700 Subject: [PATCH 57/74] listener_manager: Update port in sockaddr arg. --- include/mptcpd/listener_manager.h | 27 +++++++++---------- lib/listener_manager.c | 44 +++++++++++++------------------ tests/test-listener-manager.c | 23 ++++++++-------- 3 files changed, 42 insertions(+), 52 deletions(-) diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index 24c4a7c2..0f4d7021 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -19,7 +19,6 @@ extern "C" { #endif - struct mptcpd_lm; /** @@ -28,27 +27,25 @@ struct mptcpd_lm; * Create a MPTCP listening socket for the given local address. This * is needed to accept subflows, e.g. during a @c MP_JOIN operation. * - * @param[in] lm The mptcpd address listener manager object. - * @param[in] sa The MPTCP local address. + * @param[in] lm The mptcpd address listener manager object. + * @param[in,out] sa The MPTCP local address. If the port is zero an + * ephemeral port will be chosen, and assigned to + * the appropriate underlying address + * family-specific port member, e.g. @c sin_port or + * @c sin6_port. The port will be in network byte + * order. * - * @return Non-zero port in network byte order to which the listener - * was bound, and zero on failure. An ephemeral port will be - * returned if the port in the local address @a sa is zero. - * The @c sockaddr passed to subsequent calls to - * @c mptcpd_lm_close() should take this into account since - * the mptcpd listener manager will only keep track of - * addresses with non-zero ports, including addresses with - * ephemeral ports. + * @return @c true on success, and @c false on failure. */ -MPTCPD_API in_port_t mptcpd_lm_listen(struct mptcpd_lm *lm, - struct sockaddr const *sa); +MPTCPD_API bool mptcpd_lm_listen(struct mptcpd_lm *lm, + struct sockaddr *sa); /** * @brief Stop listening on a MPTCP local address. * * @param[in] lm The mptcpd address listener manager object. - * @param[in] sa The MPTCP local address with the non-zero port - * returned from @c mptcpd_lm_listen(), i.e. the + * @param[in] sa The MPTCP local address with a non-zero port, such as + * the one assigned by @c mptcpd_lm_listen(), i.e. the * non-zero port provided by the user or the ephemeral * port chosen by the kernel. * diff --git a/lib/listener_manager.c b/lib/listener_manager.c index 8bc9e996..d3b176e0 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -229,11 +229,11 @@ static int hash_sockaddr_compare(void const *a, void const *b) // ---------------------------------------------------------------------- -static in_port_t get_port(struct sockaddr const *sa) +static socklen_t get_addr_size(struct sockaddr const *sa) { return sa->sa_family == AF_INET - ? ((struct sockaddr_in const *) sa)->sin_port - : ((struct sockaddr_in6 const *) sa)->sin6_port; + ? sizeof(struct sockaddr_in) + : sizeof(struct sockaddr_in6); } /** @@ -273,10 +273,7 @@ static int open_listener(struct sockaddr const *sa) return -1; } - socklen_t const addr_size = - (sa->sa_family == AF_INET - ? sizeof(struct sockaddr_in) - : sizeof(struct sockaddr_in6)); + socklen_t const addr_size = get_addr_size(sa); if (bind(fd, sa, addr_size) == -1) { l_error("Unable to bind MPTCP listener: %s", @@ -308,12 +305,11 @@ static void close_listener(void *value) l_free(data); } -static in_port_t make_listener(struct mptcpd_lm* lm, - struct sockaddr const *sa) +static int make_listener(struct mptcpd_lm* lm, struct sockaddr *sa) { int const fd = open_listener(sa); if (fd == -1) - return 0; + return -1; /* The port in a sockaddr used for the key should be non-zero. @@ -322,22 +318,19 @@ static in_port_t make_listener(struct mptcpd_lm* lm, the kernel, as is the case when the port in the user provided sockaddr passed to bind() is zero. */ - struct sockaddr_storage ss; - socklen_t addrlen = sizeof(ss); - - struct sockaddr *const addr = (struct sockaddr *) &ss; + socklen_t addrlen = get_addr_size(sa); - int const r = getsockname(fd, addr, &addrlen); + int const r = getsockname(fd, sa, &addrlen); if (r == -1) { l_error("Unable to retrieve listening socket name: %s", strerror(errno)); (void) close(fd); - return 0; + return -1; } struct mptcpd_hash_sockaddr_key const key = { - .sa = addr, .seed = lm->seed + .sa = sa, .seed = lm->seed }; struct lm_value *const data = l_new(struct lm_value, 1); @@ -347,13 +340,13 @@ static in_port_t make_listener(struct mptcpd_lm* lm, close_listener(data); - return 0; + return -1; } data->fd = fd; data->refcnt = 1; - return get_port(key.sa); + return 0; } // ---------------------------------------------------------------------- @@ -389,17 +382,16 @@ void mptcpd_lm_destroy(struct mptcpd_lm *lm) l_free(lm); } -in_port_t mptcpd_lm_listen(struct mptcpd_lm *lm, - struct sockaddr const *sa) +bool mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr *sa) { if (lm == NULL || sa == NULL) - return 0; + return false; if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) - return 0; + return false; if (is_unbound_address(sa)) - return 0; + return false; struct mptcpd_hash_sockaddr_key const key = { .sa = sa, .seed = lm->seed @@ -413,14 +405,14 @@ in_port_t mptcpd_lm_listen(struct mptcpd_lm *lm, */ if (data != NULL) { data->refcnt++; - return get_port(sa); + return true; } /* The sockaddr doesn't exist in the map. Make a new listener. */ - return make_listener(lm, sa); + return make_listener(lm, sa) == 0; } bool mptcpd_lm_close(struct mptcpd_lm *lm, struct sockaddr const *sa) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index 50f55957..f0669308 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -27,13 +27,13 @@ struct ipv4_listen_case { - struct sockaddr_in const addr; + struct sockaddr_in addr; char const *const desc; }; struct ipv6_listen_case { - struct sockaddr_in6 const addr; + struct sockaddr_in6 addr; char const *const desc; }; @@ -92,10 +92,13 @@ static void test_create(void const *test_data) static void test_listen(void const *test_data) { - struct sockaddr const *const sa = test_data; + struct sockaddr *const sa = (struct sockaddr *) test_data; in_port_t const original_port = get_port(sa); - in_port_t port = mptcpd_lm_listen(_lm, sa); + + assert(mptcpd_lm_listen(_lm, sa)); + + in_port_t port = get_port(sa); assert(port != 0); @@ -109,11 +112,9 @@ static void test_listen(void const *test_data) static void test_listen_bad_address(void const *test_data) { - struct sockaddr const *const sa = test_data; - - in_port_t const port = mptcpd_lm_listen(_lm, sa); + struct sockaddr *const sa = (struct sockaddr *) test_data; - assert(port == 0); + assert(!mptcpd_lm_listen(_lm, sa)); } static void test_close(void const *test_data) @@ -149,14 +150,14 @@ int main(int argc, char *argv[]) an address backed by a network interface so that the underlying bind() call can succeed. */ - struct ipv4_listen_case const ipv4_cases[] = { + struct ipv4_listen_case ipv4_cases[] = { INIT_IPV4_TEST_CASE(0x3456), INIT_IPV4_TEST_CASE(0x3457), INIT_IPV4_TEST_CASE(0x3456), // Same port as above. INIT_IPV4_TEST_CASE(0) }; - struct ipv6_listen_case const ipv6_cases[] = { + struct ipv6_listen_case ipv6_cases[] = { INIT_IPV6_TEST_CASE(0x4567), INIT_IPV6_TEST_CASE(0x4578), INIT_IPV6_TEST_CASE(0x4567), // Same port as above. @@ -176,7 +177,7 @@ int main(int argc, char *argv[]) } // Test listen failure with "bad" (unbound) addresses. - struct sockaddr_in const ipv4_bad_cases[] = { + struct sockaddr_in ipv4_bad_cases[] = { { .sin_family = AF_INET, .sin_addr = { .s_addr = INADDR_ANY } From eabb9231874ce90bedea64bb777a3f50b1bd71b2 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Wed, 3 Aug 2022 16:33:01 -0700 Subject: [PATCH 58/74] Use non-const sockaddr arg in mptcpd_pm_add_addr() --- include/mptcpd/path_manager.h | 25 ++++++++++++++----------- include/mptcpd/private/path_manager.h | 26 +++++++++++++++----------- lib/path_manager.c | 2 +- src/netlink_pm_mptcp_org.c | 2 +- src/netlink_pm_upstream.c | 21 ++++++++++++--------- 5 files changed, 43 insertions(+), 33 deletions(-) diff --git a/include/mptcpd/path_manager.h b/include/mptcpd/path_manager.h index e237e569..c604b6ff 100644 --- a/include/mptcpd/path_manager.h +++ b/include/mptcpd/path_manager.h @@ -107,18 +107,22 @@ MPTCPD_API bool mptcpd_pm_ready(struct mptcpd_pm const *pm); /** * @brief Advertise new network address to peers. * - * @param[in] pm The mptcpd path manager object. - * @param[in] addr Local IP address and port to be advertised - * through the MPTCP protocol @c ADD_ADDR - * option. The port is optional, and is - * ignored if it is zero. - * @param[in] id MPTCP local address ID. - * @param[in] token MPTCP connection token. + * @param[in] pm The mptcpd path manager object. + * @param[in,out] addr Local IP address and port to be advertised + * through the MPTCP protocol @c ADD_ADDR + * option. If the port is zero an ephemeral port + * will be chosen, and assigned to the + * appropriate underlying address family-specific + * port member, e.g. @c sin_port or + * @c sin6_port. The port will be in network + * byte order. + * @param[in] id MPTCP local address ID. + * @param[in] token MPTCP connection token. * * @return @c 0 if operation was successful. @c errno otherwise. */ MPTCPD_API int mptcpd_pm_add_addr(struct mptcpd_pm *pm, - struct sockaddr const *addr, + struct sockaddr *addr, mptcpd_aid_t id, mptcpd_token_t token); @@ -126,9 +130,8 @@ MPTCPD_API int mptcpd_pm_add_addr(struct mptcpd_pm *pm, * @brief Stop advertising network address to peers. * * @param[in] pm The mptcpd path manager object. - * @param[in] addr Local IP address and port (zero if unused) - * that should no longer be advertised through - * MPTCP. + * @param[in] addr Local IP address and port that should no longer + * be advertised through MPTCP. * @param[in] address_id MPTCP local address ID to be sent in the * MPTCP protocol @c REMOVE_ADDR option * corresponding to the local address that will diff --git a/include/mptcpd/private/path_manager.h b/include/mptcpd/private/path_manager.h index 6f807fb1..a560f13b 100644 --- a/include/mptcpd/private/path_manager.h +++ b/include/mptcpd/private/path_manager.h @@ -138,19 +138,24 @@ struct mptcpd_pm_cmd_ops /** * @brief Advertise new network address to peers. * - * @param[in] pm The mptcpd path manager object. - * @param[in] addr Local IP address and port to be advertised - * through the MPTCP protocol @c ADD_ADDR - * option. The port is optional, and is - * ignored if it is zero. - * @param[in] id MPTCP local address ID. - * @param[in] token MPTCP connection token. + * @param[in] pm The mptcpd path manager object. + * @param[in,out] addr Local IP address and port to be + * advertised through the MPTCP protocol + * @c ADD_ADDR option. If the port is + * zero an ephemeral port will be chosen, + * and assigned to the appropriate + * underlying address family-specific + * port member, e.g. @c sin_port or + * @c sin6_port. The port will be in + * network byte order. + * @param[in] id MPTCP local address ID. + * @param[in] token MPTCP connection token. * * @return @c 0 if operation was successful. -1 or @c errno * otherwise. */ int (*add_addr)(struct mptcpd_pm *pm, - struct sockaddr const *addr, + struct sockaddr *addr, mptcpd_aid_t id, mptcpd_token_t token); @@ -158,9 +163,8 @@ struct mptcpd_pm_cmd_ops * @brief Stop advertising network address to peers. * * @param[in] pm The mptcpd path manager object. - * @param[in] addr Local IP address and port (zero if unused) - * that should no longer be advertised - * through MPTCP. + * @param[in] addr Local IP address and port that should no + * longer be advertised through MPTCP. * @param[in] id MPTCP local address ID. * @param[in] token MPTCP connection token. * diff --git a/lib/path_manager.c b/lib/path_manager.c index 827e3c48..c4891a58 100644 --- a/lib/path_manager.c +++ b/lib/path_manager.c @@ -239,7 +239,7 @@ int mptcpd_kpm_set_flags(struct mptcpd_pm *pm, // ------------------------------------------------------------------- int mptcpd_pm_add_addr(struct mptcpd_pm *pm, - struct sockaddr const *addr, + struct sockaddr *addr, mptcpd_aid_t address_id, mptcpd_token_t token) { diff --git a/src/netlink_pm_mptcp_org.c b/src/netlink_pm_mptcp_org.c index 97f91124..4b518ac2 100644 --- a/src/netlink_pm_mptcp_org.c +++ b/src/netlink_pm_mptcp_org.c @@ -153,7 +153,7 @@ static bool append_remote_addr_attr(struct l_genl_msg *msg, // --------------------------------------------------------------------- static int mptcp_org_add_addr(struct mptcpd_pm *pm, - struct sockaddr const *addr, + struct sockaddr *addr, mptcpd_aid_t id, mptcpd_token_t token) { diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index b8fd41fa..49d5a044 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -257,10 +257,21 @@ static int send_add_addr(struct mptcpd_pm *pm, // User Space Path Manager Related Functions // -------------------------------------------------------------- static int upstream_announce(struct mptcpd_pm *pm, - struct sockaddr const *addr, + struct sockaddr *addr, mptcpd_aid_t id, mptcpd_token_t token) { + /** + * Set up MPTCP listening socket. + * + * @note An ephemeral port will be assigned to the port in + * @a addr if it is zero. + * + * @todo This should be optional. + */ + if (!mptcpd_lm_listen(pm->lm, addr)) + return -1; + /** * @todo Add support for the optional network interface index * attribute. @@ -272,14 +283,6 @@ static int upstream_announce(struct mptcpd_pm *pm, // .ifindex = ... }; - /** - * Set up MPTCP listening socket. - * - * @todo This should be optional. - */ - if (mptcpd_lm_listen(pm->lm, addr) == 0) - return -1; - return send_add_addr(pm, MPTCP_PM_CMD_ANNOUNCE, "announce", From ffcb6687779bd42c8fe85de7236af7d4b4cecaf3 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 10:02:38 -0700 Subject: [PATCH 59/74] Add mptcpd_sockaddr_copy() utility function. In some cases it is useful to copy the contents of a sockaddr based on the address family. Add a new mptcpd_sockaddr_copy() function that does so. Only AF_INET and AF_INET6 address families are supported. --- include/mptcpd/private/sockaddr.h | 16 +++++++ lib/sockaddr.c | 20 +++++++++ tests/test-sockaddr.c | 74 +++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) diff --git a/include/mptcpd/private/sockaddr.h b/include/mptcpd/private/sockaddr.h index 2c1ed414..f0653a98 100644 --- a/include/mptcpd/private/sockaddr.h +++ b/include/mptcpd/private/sockaddr.h @@ -64,6 +64,22 @@ mptcpd_sockaddr_storage_init(in_addr_t const *addr4, in_port_t port, struct sockaddr_storage *addr); +/** + * @brief Deep copy a @c sockaddr. + * + * Copy the address family-specific contents of a @c sockaddr. For an + * @c AF_INET address family, a @c struct @c sockaddr_in will be + * dynamically allocated and copied from @a sa. Similarly, @c struct + * @c sockaddr_in6 will be allocated and copied from @a sa for the + * @c AF_INET6 address family case. + * + * @return Dynamically allocated copy of @a sa if the @c sa_family + * member is @c AF_INET or @c AF_INET6, and @c NULL otherwise. + * Deallocate with @c l_free(). + */ +MPTCPD_API struct sockaddr * +mptcpd_sockaddr_copy(struct sockaddr const *sa); + #ifdef __cplusplus } #endif diff --git a/lib/sockaddr.c b/lib/sockaddr.c index d8045255..64722236 100644 --- a/lib/sockaddr.c +++ b/lib/sockaddr.c @@ -11,6 +11,11 @@ #include #include +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +#include +#pragma GCC diagnostic pop + #include @@ -41,6 +46,21 @@ bool mptcpd_sockaddr_storage_init(in_addr_t const *addr4, return true; } +struct sockaddr * +mptcpd_sockaddr_copy(struct sockaddr const *sa) +{ + if (sa == NULL) + return NULL; + + if (sa->sa_family == AF_INET) + return l_memdup(sa, sizeof(struct sockaddr_in)); + else if (sa->sa_family == AF_INET6) + return l_memdup(sa, sizeof(struct sockaddr_in6)); + + // Not an IPv4 or IPv6 address. + return NULL; +} + /* Local Variables: diff --git a/tests/test-sockaddr.c b/tests/test-sockaddr.c index fd859a19..f348b247 100644 --- a/tests/test-sockaddr.c +++ b/tests/test-sockaddr.c @@ -7,8 +7,11 @@ * Copyright (c) 2021, 2022, Intel Corporation */ +#include + #include #include +#include #include @@ -101,6 +104,73 @@ static void test_sockaddr_in6_init(void const *test_data) (struct sockaddr const *) &addr)); } +static void test_copy_null(void const *test_data) +{ + (void) test_data; + + assert(mptcpd_sockaddr_copy(NULL) == NULL); +} + +static bool test_copy_inet(struct sockaddr const *src) +{ + struct sockaddr *const dst = mptcpd_sockaddr_copy(src); + + bool const is_equal = sockaddr_is_equal(src, dst); + + l_free(dst); + + return is_equal; +} + +static void test_copy_af_inet(void const *test_data) +{ + (void) test_data; + + struct sockaddr_in const addr = { + .sin_family = AF_INET, + .sin_port = htons(0x1234), + .sin_addr = { + .s_addr = htonl(0xC0000201) // 192.0.2.1 + } + }; + + struct sockaddr const *const src = (struct sockaddr const *) &addr; + + assert(test_copy_inet(src)); +} + +static void test_copy_af_inet6(void const *test_data) +{ + (void) test_data; + + struct sockaddr_in6 const addr = { + .sin6_family = AF_INET6, + .sin6_port = htons(0x5678), + .sin6_addr = { + .s6_addr = { [0] = 0x20, + [1] = 0x01, + [2] = 0X0D, + [3] = 0xB8, + [14] = 0x01, + [15] = 0x02 } // 2001:DB8::102 + } + }; + + struct sockaddr const *const src = + (struct sockaddr const *) &addr; + + assert(test_copy_inet(src)); +} + +static void test_copy_non_inet(void const *test_data) +{ + (void) test_data; + + struct sockaddr const src = { .sa_family = AF_UNIX }; + + assert(mptcpd_sockaddr_copy(&src) == NULL); +} + int main(int argc, char *argv[]) { l_log_set_stderr(); @@ -111,6 +181,10 @@ int main(int argc, char *argv[]) l_test_add("bad sockaddr init", test_bad_sockaddr_init, NULL); l_test_add("sockaddr_in init", test_sockaddr_in_init, NULL); l_test_add("sockaddr_in6 init", test_sockaddr_in6_init, NULL); + l_test_add("copy - NULL", test_copy_null, NULL); + l_test_add("copy - AF_INET", test_copy_af_inet, NULL); + l_test_add("copy - AF_INET6", test_copy_af_inet6, NULL); + l_test_add("copy - non-INET", test_copy_non_inet, NULL); return l_test_run(); } From 0d3194ec28d8ccc0b935f8168c1beb8223029fad Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 10:05:19 -0700 Subject: [PATCH 60/74] sspi: Pass non-const addr to mptcpd_pm_add_addr(). The mptcpd_pm_add_addr() function now expects a pointer to a non-const struct sockaddr. Update the sspi plugin accordingly. --- plugins/path_managers/sspi.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/plugins/path_managers/sspi.c b/plugins/path_managers/sspi.c index a49c86cd..ff076d10 100644 --- a/plugins/path_managers/sspi.c +++ b/plugins/path_managers/sspi.c @@ -27,6 +27,7 @@ #include #include #include +#include /** * @brief Local address to interface mapping failure value. @@ -459,8 +460,9 @@ static bool sspi_remove_token(void *data, void *user_data) /** * @brief Inform kernel of local address available for subflows. * - * @param[in] i Network interface information. - * @param[in] data User supplied data, the path manager in this case. + * @param[in] data @c struct @c sockaddr containing address to + * advertise. + * @param[in] user_data New connection information. */ static void sspi_send_addr(void *data, void *user_data) { @@ -475,24 +477,25 @@ static void sspi_send_addr(void *data, void *user_data) */ mptcpd_aid_t address_id = 0; - /** - * @note The port is an optional field of the MPTCP - * @c ADD_ADDR option. Setting it to zero causes it to - * be ignored when sending the address information to - * the kernel. - */ /* - in_port_t const port = 0; - if (addr->sa_family == AF_INET) - ((struct sockaddr_in const *) addr)->sin_port = port; - else - ((struct sockaddr_in6 const *) addr)->sin6_port = port; - */ + mptcpd_pm_add_addr() will modify the sockaddr passed to it + if the port is zero. Make a copy to avoid modifying the + original. + */ + struct sockaddr *const sa = mptcpd_sockaddr_copy(addr); mptcpd_pm_add_addr(info->pm, - addr, + sa, address_id, info->token); + + /** + * @todo The sspi plugin currently doesn't stop advertising IP + * addresses. The address will need to be stored for later + * use in mptcpd_pm_remove_addr() once the need for that + * occurs. + */ + l_free(sa); } /** From 0690d9f1ae868c3c752f96c0d6e8c3a2a2f5411a Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 10:58:49 -0700 Subject: [PATCH 61/74] Make hexadecimal capitalization consistent. --- lib/listener_manager.c | 2 +- tests/lib/test-plugin.h | 4 ++-- tests/test-murmur-hash.c | 2 +- tests/test-sockaddr.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/listener_manager.c b/lib/listener_manager.c index d3b176e0..8f0c3d94 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -105,7 +105,7 @@ struct key_in * struct endpoint_in6 endpoint = { * .addr = { .s6_addr = { [0] = 0x20, * [1] = 0x01, - * [2] = 0X0D, + * [2] = 0x0D, * [3] = 0xB8, * [14] = 0x01, * [15] = 0x02 } }, diff --git a/tests/lib/test-plugin.h b/tests/lib/test-plugin.h index 066e64f3..1a2658cf 100644 --- a/tests/lib/test-plugin.h +++ b/tests/lib/test-plugin.h @@ -196,7 +196,7 @@ static struct sockaddr_in6 const test_laddr_2 = { .sin6_port = MPTCPD_CONSTANT_HTONS(0x5678), .sin6_addr = { .s6_addr = { [0] = 0x20, [1] = 0x01, - [2] = 0X0D, + [2] = 0x0D, [3] = 0xB8, [14] = 0x01, [15] = 0x02 } // 2001:DB8::102 @@ -224,7 +224,7 @@ static struct sockaddr_in6 const test_raddr_1 = { .sin6_port = MPTCPD_CONSTANT_HTONS(0x3456), .sin6_addr = { .s6_addr = { [0] = 0x20, [1] = 0x01, - [2] = 0X0D, + [2] = 0x0D, [3] = 0xB8, [14] = 0x02, [15] = 0x01 } // 2001:DB8::201 diff --git a/tests/test-murmur-hash.c b/tests/test-murmur-hash.c index 4cebfa92..03479f45 100644 --- a/tests/test-murmur-hash.c +++ b/tests/test-murmur-hash.c @@ -40,7 +40,7 @@ static void test_hash_32(void const *test_data) uint8_t const k3[16] = { [0] = 0x20, [1] = 0x01, - [2] = 0X0D, + [2] = 0x0D, [3] = 0xB8, [14] = 0x01, [15] = 0x02 diff --git a/tests/test-sockaddr.c b/tests/test-sockaddr.c index f348b247..f3e2727f 100644 --- a/tests/test-sockaddr.c +++ b/tests/test-sockaddr.c @@ -149,7 +149,7 @@ static void test_copy_af_inet6(void const *test_data) .sin6_addr = { .s6_addr = { [0] = 0x20, [1] = 0x01, - [2] = 0X0D, + [2] = 0x0D, [3] = 0xB8, [14] = 0x01, [15] = 0x02 } // 2001:DB8::102 From a99543f02d2d1ee21448e11593dc68c121e8943c Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 15:15:38 -0700 Subject: [PATCH 62/74] listener_manager: Forward declare struct sockaddr. --- include/mptcpd/listener_manager.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index 0f4d7021..cc1de187 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -10,8 +10,6 @@ #ifndef MPTCPD_LISTENER_MANAGER_H #define MPTCPD_LISTENER_MANAGER_H -#include - #include @@ -20,6 +18,7 @@ extern "C" { #endif struct mptcpd_lm; +struct sockaddr; /** * @brief Listen on the given MPTCP local address. From 0d4210cbcf93f56031f9fa25a314d3a848f226ab Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 15:55:35 -0700 Subject: [PATCH 63/74] listener_manager: Add missing include. --- include/mptcpd/listener_manager.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index cc1de187..dae252eb 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -10,6 +10,8 @@ #ifndef MPTCPD_LISTENER_MANAGER_H #define MPTCPD_LISTENER_MANAGER_H +#include + #include From 8b9cc84c06b35804f03fb802e553c599f21225fe Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 15:56:05 -0700 Subject: [PATCH 64/74] listener_manager: Disambiguate filename in docs. --- include/mptcpd/private/listener_manager.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/mptcpd/private/listener_manager.h b/include/mptcpd/private/listener_manager.h index c2dcdba5..f804bd2a 100644 --- a/include/mptcpd/private/listener_manager.h +++ b/include/mptcpd/private/listener_manager.h @@ -1,6 +1,6 @@ // SPDX-License-Identifier: BSD-3-Clause /** - * @file listener_manager.h + * @file private/listener_manager.h * * @brief Map of MPTCP local address to listener - private API. * @@ -34,7 +34,6 @@ MPTCPD_API struct mptcpd_lm *mptcpd_lm_create(void); */ MPTCPD_API void mptcpd_lm_destroy(struct mptcpd_lm *lm); - #ifdef __cplusplus } #endif From 73facae4746f172e3307552ad886731b6af749ae Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 23:36:39 -0700 Subject: [PATCH 65/74] tests: Overhaul test-commands unit test. Overhaul the test-commands unit test with the following notable changes: * Use different addresses for user and kernel space address advertising related commands. This is needed to prevent listening on the same TCP address and port. * Use the same local and remote addresses for all user space subflow related commands. * Log addresses obtained from mptcpd_kpm_get_addr(). --- tests/test-commands.c | 335 ++++++++++++++++++++++++++++++------------ 1 file changed, 240 insertions(+), 95 deletions(-) diff --git a/tests/test-commands.c b/tests/test-commands.c index d88887e4..330202d5 100644 --- a/tests/test-commands.c +++ b/tests/test-commands.c @@ -44,21 +44,46 @@ // ------------------------------------------------------------------- -struct test_info +struct test_addr_info { - struct l_netlink *const rtnl; - - // Address used for kernel add_addr and dump_addr calls. - struct sockaddr const *const addr; + // Test address. + struct sockaddr *const addr; // Network interface on which to bind the test address. int const ifindex; - // CIDR prefix - uint8_t const prefix; + // CIDR prefix length. + uint8_t const prefix_len; + + /** + * @brief String form of the addr. + * + * @note Long enough for both IPv4 and IPv6 addresses. + */ + char ip[INET6_ADDRSTRLEN]; + + /// MPTCP connection used in user space PM calls. + mptcpd_token_t token; + + // MPTCP address ID used for add_addr and dump_addr calls. + mptcpd_aid_t id; +}; + +struct test_info +{ + struct l_netlink *const rtnl; + + /* + Address information for user space add_addr and remove_addr + calls. + */ + struct test_addr_info u_addr; - // String form of the addr. - char const *const ip; + /* + Address information for kernel add_addr and dump_addr + calls. + */ + struct test_addr_info k_addr; // Mptcpd configuration. struct mptcpd_config *config; @@ -66,9 +91,6 @@ struct test_info // Mptcpd path manager object. struct mptcpd_pm *pm; - // ID used for kernel add_addr and dump_addr calls. - mptcpd_aid_t id; - // Number of times dump_addrs call was completed. int dump_addrs_complete_count; @@ -77,16 +99,22 @@ struct test_info }; // ------------------------------------------------------------------- +/* + Number of addresses to set up for test purposes (2, user space and + kernel space). +*/ +static int const addrs_to_setup_count = 2; -static struct sockaddr const *const laddr1 = - (struct sockaddr const *) &test_laddr_1; +// Number of addresses set up for test purposes. +static int addr_setup_count; + +// ------------------------------------------------------------------- +// Addresses used for user space PM subflow command tests. +// ------------------------------------------------------------------- static struct sockaddr const *const laddr2 = (struct sockaddr const *) &test_laddr_2; -static struct sockaddr const *const raddr1 = - (struct sockaddr const *) &test_raddr_1; - static struct sockaddr const *const raddr2 = (struct sockaddr const *) &test_raddr_2; @@ -108,10 +136,48 @@ static struct mptcpd_limit const _limits[] = { // ------------------------------------------------------------------- +static void const *get_in_addr(struct sockaddr const *sa) +{ + if (sa->sa_family == AF_INET) { + struct sockaddr_in const *const addr = + (struct sockaddr_in const *) sa; + + return &addr->sin_addr; + } else if (sa->sa_family == AF_INET6) { + struct sockaddr_in6 const *const addr = + (struct sockaddr_in6 const *) sa; + + return &addr->sin6_addr; + } + + return NULL; // Not an internet address. Unlikely. +} + +static void dump_addr(char const *description, struct sockaddr const *a) +{ + assert(a != NULL); + + void const *src = get_in_addr(a); + + char addrstr[INET6_ADDRSTRLEN]; // Long enough for both IPv4 + // and IPv6 addresses. + + assert(inet_ntop(a->sa_family, src, addrstr, sizeof(addrstr))); + + in_port_t const port = mptcpd_get_port_number(a); + + l_info("%s: %s:<0x%x (%u)>", + description, + addrstr, + port, + port); +} + static void get_addr_callback(struct mptcpd_addr_info const *info, void *user_data) { - struct test_info *const tinfo = (struct test_info *) user_data; + struct test_info const *const tinfo = user_data; + struct test_addr_info const *const k_addr = &tinfo->k_addr; /** * @bug We could have a resource leak in the kernel here if @@ -121,16 +187,23 @@ static void get_addr_callback(struct mptcpd_addr_info const *info, */ assert(info != NULL); - assert(mptcpd_addr_info_get_id(info) == tinfo->id); - assert(mptcpd_addr_info_get_index(info) == tinfo->ifindex); - assert(sockaddr_is_equal(tinfo->addr, + l_info("======================="); + dump_addr("Address to mptcpd_kpm_add_addr()", + k_addr->addr); + dump_addr("Address from mptcpd_kpm_get_addr()", + mptcpd_addr_info_get_addr(info)); + l_info("======================="); + + assert(mptcpd_addr_info_get_id(info) == k_addr->id); + assert(mptcpd_addr_info_get_index(info) == k_addr->ifindex); + assert(sockaddr_is_equal(k_addr->addr, mptcpd_addr_info_get_addr(info))); } static void dump_addrs_callback(struct mptcpd_addr_info const *info, void *user_data) { - struct test_info *const tinfo = (struct test_info *) user_data; + struct test_info const *const tinfo = user_data; /** * @bug We could have a resource leak in the kernel here if @@ -140,18 +213,20 @@ static void dump_addrs_callback(struct mptcpd_addr_info const *info, */ assert(info != NULL); + struct test_addr_info const *const k_addr = &tinfo->k_addr; + // Other IDs unrelated to this test could already exist. - if (mptcpd_addr_info_get_id(info) != tinfo->id) + if (mptcpd_addr_info_get_id(info) != k_addr->id) return; - assert(mptcpd_addr_info_get_index(info) == tinfo->ifindex); - assert(sockaddr_is_equal(tinfo->addr, + assert(mptcpd_addr_info_get_index(info) == k_addr->ifindex); + assert(sockaddr_is_equal(k_addr->addr, mptcpd_addr_info_get_addr(info))); } static void dump_addrs_complete(void *user_data) { - struct test_info *const info = (struct test_info *) user_data; + struct test_info *const info = user_data; info->dump_addrs_complete_count++; } @@ -231,23 +306,27 @@ static void test_add_addr(void const *test_data) struct mptcpd_idm *const idm = mptcpd_pm_get_idm(pm); // Client-oriented path manager. + struct test_addr_info const *const u_addr = &info->u_addr; + int result = mptcpd_pm_add_addr(pm, - laddr1, - test_laddr_id_1, - test_token_1); + u_addr->addr, + u_addr->id, + u_addr->token); assert(result == 0 || result == ENOTSUP); // In-kernel (server-oriented) path manager. - info->id = mptcpd_idm_get_id(idm, info->addr); + struct test_addr_info *const k_addr = &info->k_addr; + + k_addr->id = mptcpd_idm_get_id(idm, k_addr->addr); uint32_t flags = 0; result = mptcpd_kpm_add_addr(pm, - info->addr, - info->id, + k_addr->addr, + k_addr->id, flags, - info->ifindex); + k_addr->ifindex); assert(result == 0 || result == ENOTSUP); } @@ -258,15 +337,19 @@ static void test_remove_addr(void const *test_data) struct mptcpd_pm *const pm = info->pm; // Client-oriented path manager. + struct test_addr_info const *const u_addr = &info->u_addr; + int result = mptcpd_pm_remove_addr(pm, - laddr1, - test_laddr_id_1, - test_token_1); + u_addr->addr, + u_addr->id, + u_addr->token); assert(result == 0 || result == ENOTSUP); // In-kernel (server-oriented) path manager. - result = mptcpd_kpm_remove_addr(pm, info->id); + struct test_addr_info const *const k_addr = &info->k_addr; + + result = mptcpd_kpm_remove_addr(pm, k_addr->id); assert(result == 0 || result == ENOTSUP); } @@ -278,7 +361,7 @@ static void test_get_addr(void const *test_data) int const result = mptcpd_kpm_get_addr(pm, - info->id, + info->k_addr.id, get_addr_callback, info, NULL); @@ -353,7 +436,8 @@ static void test_set_flags(void const *test_data) static mptcpd_flags_t const flags = MPTCPD_ADDR_FLAG_BACKUP; - int const result = mptcpd_kpm_set_flags(pm, info->addr, flags); + int const result = + mptcpd_kpm_set_flags(pm, info->k_addr.addr, flags); assert(result == 0 || result == ENOTSUP); } @@ -380,10 +464,10 @@ void test_set_backup(void const *test_data) struct mptcpd_pm *const pm = info->pm; int const result = mptcpd_pm_set_backup(pm, - test_token_1, - laddr1, - raddr1, - test_backup_1); + test_token_2, + laddr2, + raddr2, + !test_backup_2); assert(result == 0 || result == ENOTSUP); } @@ -394,9 +478,9 @@ void test_remove_subflow(void const *test_data) struct mptcpd_pm *const pm = info->pm; int const result = mptcpd_pm_remove_subflow(pm, - test_token_1, - laddr1, - raddr1); + test_token_2, + laddr2, + raddr2); assert(result == 0 || result == ENOTSUP); } @@ -425,17 +509,15 @@ static void handle_rtm_newaddr(int errnum, assert(data == NULL); assert(len == 0); + (void) user_data; // Pointer to struct test_info. + if (errnum != 0) { static int const status = 0; // Do not exit on error. - struct test_info *const info = user_data; - error(status, errnum, - "bind of test address %s to interface %d failed", - info->ip, - info->ifindex); + "bind of test address to interface failed"); } } @@ -456,7 +538,7 @@ static void handle_rtm_deladdr(int errnum, if (errnum != 0) { static int const status = 0; // Do not exit on error. - struct test_info *const info = user_data; + struct test_addr_info *const info = user_data; error(status, errnum, @@ -525,44 +607,97 @@ static void setup_tests (void *user_data) static void complete_address_setup(void *user_data) { - // Run tests after address setup is complete. - bool const result = l_idle_oneshot(setup_tests, user_data, NULL); - assert(result); + if (++addr_setup_count == addrs_to_setup_count) { + // Run tests after address setup is complete. + bool const result = + l_idle_oneshot(setup_tests, user_data, NULL); + + assert(result); + } } static void complete_address_teardown(void *user_data) { (void) user_data; - l_main_quit(); + if (--addr_setup_count == 0) + l_main_quit(); +} + +static void setup_test_address(struct test_info *data, + struct test_addr_info *info) +{ + sa_family_t const sa_family = info->addr->sa_family; + + int id = 0; + + if (sa_family == AF_INET) { + id = l_rtnl_ifaddr4_add(data->rtnl, + info->ifindex, + info->prefix_len, + info->ip, + NULL, // broadcast + handle_rtm_newaddr, + data, + complete_address_setup); + } else if (sa_family == AF_INET6) { + id = l_rtnl_ifaddr6_add(data->rtnl, + info->ifindex, + info->prefix_len, + info->ip, + handle_rtm_newaddr, + data, + complete_address_setup); + } + + assert(id != 0); } static void setup_test_addresses(struct test_info *info) { - int const id = l_rtnl_ifaddr4_add(info->rtnl, - info->ifindex, - info->prefix, - info->ip, - NULL, // broadcast - handle_rtm_newaddr, - info, - complete_address_setup); + // Address used for user space PM advertising tests. + setup_test_address(info, &info->u_addr); + + // Address used for kernel space PM tests. + setup_test_address(info, &info->k_addr); +} + +static void teardown_test_address(struct l_netlink *rtnl, + struct test_addr_info *info) +{ + sa_family_t const sa_family = info->addr->sa_family; + + int id = 0; + + if (sa_family == AF_INET) { + id = l_rtnl_ifaddr4_delete(rtnl, + info->ifindex, + info->prefix_len, + info->ip, + NULL, // broadcast + handle_rtm_deladdr, + info, + complete_address_teardown); + } else if (sa_family == AF_INET6) { + id = l_rtnl_ifaddr6_delete(rtnl, + info->ifindex, + info->prefix_len, + info->ip, + handle_rtm_deladdr, + info, + complete_address_teardown); + } assert(id != 0); } static void teardown_test_addresses(struct test_info *info) { - int const id = l_rtnl_ifaddr4_delete(info->rtnl, - info->ifindex, - info->prefix, - info->ip, - NULL, // broadcast - handle_rtm_deladdr, - info, - complete_address_teardown); + // Address used for user space PM advertising tests. + teardown_test_address(info->rtnl, &info->u_addr); - assert(id != 0); + // Address used for kernel space PM tests. + teardown_test_address(info->rtnl, &info->k_addr); } // ------------------------------------------------------------------- @@ -596,6 +731,12 @@ static void idle_callback(struct l_idle *idle, void *user_data) teardown_test_addresses(user_data); // Done running tests. } +static uint8_t get_prefix_len(struct sockaddr const *sa) +{ + // One IP address + return sa->sa_family == AF_INET ? 32 : 128; +} + // ------------------------------------------------------------------- int main(void) @@ -612,35 +753,39 @@ int main(void) struct l_netlink *const rtnl = l_netlink_new(NETLINK_ROUTE); assert(rtnl != NULL); - static struct sockaddr const *const sa = laddr1; - - // Bind test IP address to loopback interface. + // Bind test IP addresses to loopback interface. static char const loopback[] = "lo"; - char ip[INET6_ADDRSTRLEN] = { 0 }; - - uint8_t prefix = 0; - void const *src = NULL; - - if (sa->sa_family == AF_INET) { - prefix = 32; // One IPv4 address - src = &((struct sockaddr_in const *) sa)->sin_addr; - } else { - prefix = 128; // One IPv6 address - src = &((struct sockaddr_in6 const *) sa)->sin6_addr; - } + // Mutable sockaddr copies. + struct sockaddr_in laddr1 = test_laddr_1; + struct sockaddr_in laddr4 = test_laddr_4; struct test_info info = { .rtnl = rtnl, - .addr = sa, - .ifindex = if_nametoindex(loopback), - .prefix = prefix, - .ip = inet_ntop(sa->sa_family, - src, - ip, - sizeof(ip)) + .u_addr = { + .addr = (struct sockaddr *) &laddr4, + .ifindex = if_nametoindex(loopback), + .prefix_len = get_prefix_len(info.u_addr.addr), + .token = test_token_4, + .id = test_laddr_id_4 + }, + .k_addr = { + .addr = (struct sockaddr *) &laddr1, + .ifindex = if_nametoindex(loopback), + .prefix_len = get_prefix_len(info.k_addr.addr) + } }; + inet_ntop(info.u_addr.addr->sa_family, + get_in_addr(info.u_addr.addr), + info.u_addr.ip, + sizeof(info.u_addr.ip)); + + inet_ntop(info.k_addr.addr->sa_family, + get_in_addr(info.k_addr.addr), + info.k_addr.ip, + sizeof(info.k_addr.ip)); + setup_test_addresses(&info); struct l_idle *const idle = From 087cb4aeaeeb60308c87b58af5b9f7fe6caa59cf Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Thu, 4 Aug 2022 23:53:18 -0700 Subject: [PATCH 66/74] netlink_pm_upstream: Clarify TODO comment. --- src/netlink_pm_upstream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index c57bcbff..1fc4ceed 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -286,7 +286,8 @@ static int upstream_remove(struct mptcpd_pm *pm, /** * @todo Refactor upstream_remove() and * mptcp_org_remove_addr() functions. They only differ - * by command and attribute types. + * by command and attribute types, and callback + * function. */ /* From dc40045a1442930e599783cf1aae42825b4ccd18 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 09:40:01 -0700 Subject: [PATCH 67/74] test-commands: Split kernel and user space cases. Split kernel and user space test cases to make it easier to determine causes of failure. The code is also cleaner with the split. --- tests/test-commands.c | 137 +++++++++++++++++++++++++++--------------- 1 file changed, 87 insertions(+), 50 deletions(-) diff --git a/tests/test-commands.c b/tests/test-commands.c index 330202d5..4868fc7e 100644 --- a/tests/test-commands.c +++ b/tests/test-commands.c @@ -299,57 +299,38 @@ static void test_get_port(void const *test_data) assert(hport == ntohs(nport)); } -static void test_add_addr(void const *test_data) +// ------------------------------------------------------------------- +// In-kernel (server-oriented) path manager commands. +// ------------------------------------------------------------------- +static void test_add_addr_kernel(void const *test_data) { struct test_info *const info = (struct test_info *) test_data; struct mptcpd_pm *const pm = info->pm; struct mptcpd_idm *const idm = mptcpd_pm_get_idm(pm); - // Client-oriented path manager. - struct test_addr_info const *const u_addr = &info->u_addr; - - int result = mptcpd_pm_add_addr(pm, - u_addr->addr, - u_addr->id, - u_addr->token); - - assert(result == 0 || result == ENOTSUP); - - // In-kernel (server-oriented) path manager. struct test_addr_info *const k_addr = &info->k_addr; k_addr->id = mptcpd_idm_get_id(idm, k_addr->addr); uint32_t flags = 0; - result = mptcpd_kpm_add_addr(pm, - k_addr->addr, - k_addr->id, - flags, - k_addr->ifindex); + int const result = mptcpd_kpm_add_addr(pm, + k_addr->addr, + k_addr->id, + flags, + k_addr->ifindex); assert(result == 0 || result == ENOTSUP); } -static void test_remove_addr(void const *test_data) +static void test_remove_addr_kernel(void const *test_data) { struct test_info *const info = (struct test_info *) test_data; struct mptcpd_pm *const pm = info->pm; - // Client-oriented path manager. - struct test_addr_info const *const u_addr = &info->u_addr; - - int result = mptcpd_pm_remove_addr(pm, - u_addr->addr, - u_addr->id, - u_addr->token); - - assert(result == 0 || result == ENOTSUP); - - // In-kernel (server-oriented) path manager. struct test_addr_info const *const k_addr = &info->k_addr; - result = mptcpd_kpm_remove_addr(pm, k_addr->id); + int const result = mptcpd_kpm_remove_addr(pm, k_addr->id); assert(result == 0 || result == ENOTSUP); } @@ -442,13 +423,48 @@ static void test_set_flags(void const *test_data) assert(result == 0 || result == ENOTSUP); } +// ------------------------------------------------------------------- +// User space (client-oriented) path manager commands. +// ------------------------------------------------------------------- +static void test_add_addr_user(void const *test_data) +{ + struct test_info *const info = (struct test_info *) test_data; + struct mptcpd_pm *const pm = info->pm; + + struct test_addr_info const *const u_addr = &info->u_addr; + + int const result = mptcpd_pm_add_addr(pm, + u_addr->addr, + u_addr->id, + u_addr->token); + + assert(result == 0 || result == ENOTSUP); +} + +static void test_remove_addr_user(void const *test_data) +{ + struct test_info *const info = (struct test_info *) test_data; + struct mptcpd_pm *const pm = info->pm; + + struct test_addr_info const *const u_addr = &info->u_addr; + + int const result = mptcpd_pm_remove_addr(pm, + u_addr->addr, + u_addr->id, + u_addr->token); + + assert(result == 0 || result == ENOTSUP); +} + static void test_add_subflow(void const *test_data) { struct test_info *const info = (struct test_info *) test_data; struct mptcpd_pm *const pm = info->pm; + struct test_addr_info const *const u_addr = &info->u_addr; + int const result = mptcpd_pm_add_subflow(pm, - test_token_2, + u_addr->token, test_laddr_id_2, test_raddr_id_2, laddr2, @@ -463,8 +479,10 @@ void test_set_backup(void const *test_data) struct test_info *const info = (struct test_info *) test_data; struct mptcpd_pm *const pm = info->pm; + struct test_addr_info const *const u_addr = &info->u_addr; + int const result = mptcpd_pm_set_backup(pm, - test_token_2, + u_addr->token, laddr2, raddr2, !test_backup_2); @@ -477,14 +495,19 @@ void test_remove_subflow(void const *test_data) struct test_info *const info = (struct test_info *) test_data; struct mptcpd_pm *const pm = info->pm; + struct test_addr_info const *const u_addr = &info->u_addr; + int const result = mptcpd_pm_remove_subflow(pm, - test_token_2, + u_addr->token, laddr2, raddr2); assert(result == 0 || result == ENOTSUP); } +// ------------------------------------------------------------------- + + void test_get_nm(void const *test_data) { struct test_info *const info = (struct test_info *) test_data; @@ -590,19 +613,26 @@ static void setup_tests (void *user_data) l_test_init(&argc, &args); - l_test_add("get_port", test_get_port, NULL); - l_test_add("add_addr", test_add_addr, info); - l_test_add("get_addr", test_get_addr, info); - l_test_add("dump_addrs", test_dump_addrs, info); - l_test_add("remove_addr", test_remove_addr, info); - l_test_add("set_limits", test_set_limits, info); - l_test_add("get_limits", test_get_limits, info); - l_test_add("set_flags", test_set_flags, info); - l_test_add("flush_addrs", test_flush_addrs, info); - l_test_add("add_subflow", test_add_subflow, info); - l_test_add("set_backup", test_set_backup, info); - l_test_add("remove_subflow", test_remove_subflow, info); - l_test_add("get_nm", test_get_nm, info); + // Non-command tests. + l_test_add("get_port", test_get_port, NULL); + l_test_add("get_nm", test_get_nm, info); + + // In-kernel path manager tests. + l_test_add("add_addr - kernel", test_add_addr_kernel, info); + l_test_add("get_addr", test_get_addr, info); + l_test_add("dump_addrs", test_dump_addrs, info); + l_test_add("remove_addr - kernel", test_remove_addr_kernel, info); + l_test_add("set_limits", test_set_limits, info); + l_test_add("get_limits", test_get_limits, info); + l_test_add("set_flags", test_set_flags, info); + l_test_add("flush_addrs", test_flush_addrs, info); + + // User space path manager tests. + l_test_add("add_addr - user", test_add_addr_user, info); + l_test_add("add_subflow", test_add_subflow, info); + l_test_add("set_backup", test_set_backup, info); + l_test_add("remove_subflow", test_remove_subflow, info); + l_test_add("remove_addr - user", test_remove_addr_user, info); } static void complete_address_setup(void *user_data) @@ -757,20 +787,27 @@ int main(void) static char const loopback[] = "lo"; // Mutable sockaddr copies. - struct sockaddr_in laddr1 = test_laddr_1; - struct sockaddr_in laddr4 = test_laddr_4; + struct sockaddr_in user_addr = test_laddr_4; + struct sockaddr_in kernel_addr = test_laddr_1; + + /* + Set the test port to zero make the kernel choose a random + (ephemeral) unused port to prevent potential reuse of an + existing address. + */ + user_addr.sin_port = 0; struct test_info info = { .rtnl = rtnl, .u_addr = { - .addr = (struct sockaddr *) &laddr4, + .addr = (struct sockaddr *) &user_addr, .ifindex = if_nametoindex(loopback), .prefix_len = get_prefix_len(info.u_addr.addr), .token = test_token_4, .id = test_laddr_id_4 }, .k_addr = { - .addr = (struct sockaddr *) &laddr1, + .addr = (struct sockaddr *) &kernel_addr, .ifindex = if_nametoindex(loopback), .prefix_len = get_prefix_len(info.k_addr.addr) } From e80db71bc8e13fef6fcee09d81f10a147f24b071 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 11:24:36 -0700 Subject: [PATCH 68/74] listener_manager: Return 0 or errno, not bool. Propagate errors to callers by returning 0 on success, and -1 or errno on error. This allows for better error handling and diagnostics. --- include/mptcpd/listener_manager.h | 12 +++--- lib/listener_manager.c | 62 ++++++++++++++++++++----------- tests/test-listener-manager.c | 12 +++--- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/include/mptcpd/listener_manager.h b/include/mptcpd/listener_manager.h index dae252eb..e814bb7d 100644 --- a/include/mptcpd/listener_manager.h +++ b/include/mptcpd/listener_manager.h @@ -36,10 +36,10 @@ struct sockaddr; * @c sin6_port. The port will be in network byte * order. * - * @return @c true on success, and @c false on failure. + * @return @c 0 if operation was successful. -1 or @c errno otherwise. */ -MPTCPD_API bool mptcpd_lm_listen(struct mptcpd_lm *lm, - struct sockaddr *sa); +MPTCPD_API int mptcpd_lm_listen(struct mptcpd_lm *lm, + struct sockaddr *sa); /** * @brief Stop listening on a MPTCP local address. @@ -50,10 +50,10 @@ MPTCPD_API bool mptcpd_lm_listen(struct mptcpd_lm *lm, * non-zero port provided by the user or the ephemeral * port chosen by the kernel. * - * @return @c true on success, and @c false on failure. + * @return @c 0 if operation was successful. -1 or @c errno otherwise. */ -MPTCPD_API bool mptcpd_lm_close(struct mptcpd_lm *lm, - struct sockaddr const *sa); +MPTCPD_API int mptcpd_lm_close(struct mptcpd_lm *lm, + struct sockaddr const *sa); #ifdef __cplusplus } diff --git a/lib/listener_manager.c b/lib/listener_manager.c index 8f0c3d94..aa3e3011 100644 --- a/lib/listener_manager.c +++ b/lib/listener_manager.c @@ -263,30 +263,44 @@ static bool is_unbound_address(struct sockaddr const *sa) } } +/** + * @brief Create a listening socket with the address @a sa. + * + * @return The listening socket file descriptor on success, or -errno + * on failure. The errno is made negative since a valid file + * descriptor will be positive. + */ static int open_listener(struct sockaddr const *sa) { int const fd = socket(sa->sa_family, SOCK_STREAM, IPPROTO_MPTCP); + + int error = 0; // In case errno is clobbered store here. + if (fd == -1) { + error = errno; + l_error("Unable to open MPTCP listener: %s", - strerror(errno)); + strerror(error)); - return -1; + return -error; } socklen_t const addr_size = get_addr_size(sa); if (bind(fd, sa, addr_size) == -1) { + error = errno; l_error("Unable to bind MPTCP listener: %s", - strerror(errno)); + strerror(error)); (void) close(fd); - return -1; + return -error; } if (listen(fd, 0) == -1) { + error = errno; l_error("Unable to listen on MPTCP socket: %s", - strerror(errno)); + strerror(error)); (void) close(fd); - return -1; + return -error; } return fd; @@ -308,8 +322,8 @@ static void close_listener(void *value) static int make_listener(struct mptcpd_lm* lm, struct sockaddr *sa) { int const fd = open_listener(sa); - if (fd == -1) - return -1; + if (fd < 0) + return -fd; // -(-errno) /* The port in a sockaddr used for the key should be non-zero. @@ -322,11 +336,14 @@ static int make_listener(struct mptcpd_lm* lm, struct sockaddr *sa) int const r = getsockname(fd, sa, &addrlen); + int error = 0; // In case errno is clobbered store here. + if (r == -1) { + error = errno; l_error("Unable to retrieve listening socket name: %s", - strerror(errno)); + strerror(error)); (void) close(fd); - return -1; + return error; } struct mptcpd_hash_sockaddr_key const key = { @@ -382,16 +399,16 @@ void mptcpd_lm_destroy(struct mptcpd_lm *lm) l_free(lm); } -bool mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr *sa) +int mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr *sa) { if (lm == NULL || sa == NULL) - return false; + return EINVAL; if (sa->sa_family != AF_INET && sa->sa_family != AF_INET6) - return false; + return EINVAL; if (is_unbound_address(sa)) - return false; + return EINVAL; struct mptcpd_hash_sockaddr_key const key = { .sa = sa, .seed = lm->seed @@ -405,20 +422,20 @@ bool mptcpd_lm_listen(struct mptcpd_lm *lm, struct sockaddr *sa) */ if (data != NULL) { data->refcnt++; - return true; + return 0; } /* The sockaddr doesn't exist in the map. Make a new listener. */ - return make_listener(lm, sa) == 0; + return make_listener(lm, sa); } -bool mptcpd_lm_close(struct mptcpd_lm *lm, struct sockaddr const *sa) +int mptcpd_lm_close(struct mptcpd_lm *lm, struct sockaddr const *sa) { if (lm == NULL || sa == NULL) - return false; + return EINVAL; struct mptcpd_hash_sockaddr_key const key = { .sa = sa, .seed = lm->seed @@ -426,20 +443,21 @@ bool mptcpd_lm_close(struct mptcpd_lm *lm, struct sockaddr const *sa) struct lm_value *const data = l_hashmap_lookup(lm->map, &key); + // No listener associated with the given address. + if (data == NULL) + return EINVAL; + /* A listener already exists for the given IP address. Decrement the reference count. */ - if (data == NULL) - return false; - if (--data->refcnt == 0) { // No more listeners sharing the same address. close_listener(data); (void) l_hashmap_remove(lm->map, &key); } - return true; + return 0; } diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index f0669308..3b6110ad 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -7,14 +7,16 @@ * Copyright (c) 2022, Intel Corporation */ - #include #include +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" #include #include #include #include +#pragma GCC diagnostic pop #include #include @@ -96,7 +98,7 @@ static void test_listen(void const *test_data) in_port_t const original_port = get_port(sa); - assert(mptcpd_lm_listen(_lm, sa)); + assert(mptcpd_lm_listen(_lm, sa) == 0); in_port_t port = get_port(sa); @@ -114,7 +116,7 @@ static void test_listen_bad_address(void const *test_data) { struct sockaddr *const sa = (struct sockaddr *) test_data; - assert(!mptcpd_lm_listen(_lm, sa)); + assert(mptcpd_lm_listen(_lm, sa) != 0); } static void test_close(void const *test_data) @@ -124,9 +126,9 @@ static void test_close(void const *test_data) in_port_t const port = get_port(sa); if (port == 0) - assert(!mptcpd_lm_close(_lm, sa)); + assert(mptcpd_lm_close(_lm, sa) != 0); else - assert(mptcpd_lm_close(_lm, sa)); + assert(mptcpd_lm_close(_lm, sa) == 0); } static void test_destroy(void const *test_data) From 3e94d232a66fa428748662abeb4dd1380bb6cb6f Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 11:34:12 -0700 Subject: [PATCH 69/74] Return listen error to mptcpd_pm_add_addr() caller --- src/netlink_pm_upstream.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/netlink_pm_upstream.c b/src/netlink_pm_upstream.c index 1fc4ceed..61335901 100644 --- a/src/netlink_pm_upstream.c +++ b/src/netlink_pm_upstream.c @@ -227,8 +227,10 @@ static int upstream_announce(struct mptcpd_pm *pm, * * @todo This should be optional. */ - if (!mptcpd_lm_listen(pm->lm, addr)) - return -1; + int const r = mptcpd_lm_listen(pm->lm, addr); + + if (r != 0) + return r; /** * @todo Add support for the optional network interface index From a7a9b90118ec85d88d353ff1f7ee8553760049f8 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 11:35:14 -0700 Subject: [PATCH 70/74] include: Clarify return values. --- include/mptcpd/path_manager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mptcpd/path_manager.h b/include/mptcpd/path_manager.h index c604b6ff..f6c77c54 100644 --- a/include/mptcpd/path_manager.h +++ b/include/mptcpd/path_manager.h @@ -119,7 +119,7 @@ MPTCPD_API bool mptcpd_pm_ready(struct mptcpd_pm const *pm); * @param[in] id MPTCP local address ID. * @param[in] token MPTCP connection token. * - * @return @c 0 if operation was successful. @c errno otherwise. + * @return @c 0 if operation was successful. -1 or @c errno otherwise. */ MPTCPD_API int mptcpd_pm_add_addr(struct mptcpd_pm *pm, struct sockaddr *addr, From 3d64db80cbde152f3692096d8ee67304ea28d4c3 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 11:36:06 -0700 Subject: [PATCH 71/74] tests: Add listener manager negative test cases. --- tests/test-listener-manager.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index 3b6110ad..ecaccd89 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -9,6 +9,7 @@ #include #include +#include #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wpedantic" @@ -96,6 +97,11 @@ static void test_listen(void const *test_data) { struct sockaddr *const sa = (struct sockaddr *) test_data; + if (sa == NULL) { + assert(mptcpd_lm_listen(_lm, sa) != 0); + return; + } + in_port_t const original_port = get_port(sa); assert(mptcpd_lm_listen(_lm, sa) == 0); @@ -123,6 +129,11 @@ static void test_close(void const *test_data) { struct sockaddr const *const sa = test_data; + if (sa == NULL) { + assert(mptcpd_lm_close(_lm, sa) != 0); + return; + } + in_port_t const port = get_port(sa); if (port == 0) @@ -136,6 +147,7 @@ static void test_destroy(void const *test_data) (void) test_data; mptcpd_lm_destroy(_lm); + mptcpd_lm_destroy(NULL); } int main(int argc, char *argv[]) @@ -205,6 +217,15 @@ int main(int argc, char *argv[]) test_listen_bad_address, &ipv6_bad_case); + struct sockaddr const unix_sa = { .sa_family = AF_UNIX }; + l_test_add("listen - bad non-INET address", + test_listen_bad_address, + &unix_sa); + + l_test_add("listen - bad NULL address", + test_listen_bad_address, + NULL); + // Listener close test cases. l_test_add("close - IPv4", test_close, &ipv4_cases[0]); l_test_add("close - IPv6", test_close, &ipv6_cases[0]); @@ -218,6 +239,8 @@ int main(int argc, char *argv[]) test_close, &zero_port_case); + l_test_add("close - NULL", test_close, NULL); + l_test_add("destroy lm", test_destroy, NULL); return l_test_run(); From c1883e43f2ad2acd5c9011596c0899505afef413 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 11:36:37 -0700 Subject: [PATCH 72/74] test-commands: Ignore EADDRNOTAVAIL error. The EADDRNOTAVAIL error will generally occur during a call to mptcpd_pm_add_addr() if the test is run without sufficient capabilities (CAP_NET_ADMIN) to set up the test addresses by associating them with a network interface. Ignore the failure for now. --- tests/test-commands.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test-commands.c b/tests/test-commands.c index 4868fc7e..9d07acb4 100644 --- a/tests/test-commands.c +++ b/tests/test-commands.c @@ -438,7 +438,15 @@ static void test_add_addr_user(void const *test_data) u_addr->id, u_addr->token); - assert(result == 0 || result == ENOTSUP); + /* + EADDRNOTAVAIL error will generally occur if the test is run + without sufficient permissions to set up the test addresses + by associating them with a network interface. + + */ + assert(result == 0 + || result == ENOTSUP + || result == EADDRNOTAVAIL); } static void test_remove_addr_user(void const *test_data) From bde46817ea6aa6ce6db6c3247962c7590100737c Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 13:42:12 -0700 Subject: [PATCH 73/74] tests: Remove unnecessary include. --- tests/test-listener-manager.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test-listener-manager.c b/tests/test-listener-manager.c index ecaccd89..0762cdf8 100644 --- a/tests/test-listener-manager.c +++ b/tests/test-listener-manager.c @@ -16,7 +16,6 @@ #include #include #include -#include #pragma GCC diagnostic pop #include From 9fed337f36c580cda90abaa28f22029f173c56b0 Mon Sep 17 00:00:00 2001 From: Ossama Othman Date: Fri, 5 Aug 2022 13:44:03 -0700 Subject: [PATCH 74/74] tests: Fix compatibility with ELL 0.45 and later. --- tests/test-sockaddr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test-sockaddr.c b/tests/test-sockaddr.c index f3e2727f..fc3ba46c 100644 --- a/tests/test-sockaddr.c +++ b/tests/test-sockaddr.c @@ -9,9 +9,12 @@ #include +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" #include #include #include +#pragma GCC diagnostic pop #include