diff mbox series

[mptcp-next,v2,13/21] mptcp: netlink: Add MPTCP_PM_CMD_ANNOUNCE

Message ID 20220112221523.1829397-14-kishen.maloor@intel.com (mailing list archive)
State Superseded, archived
Headers show
Series mptcp: support userspace path management | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 143 lines checked
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal warning Unstable: 2 failed test(s): packetdrill_add_addr selftest_mptcp_join

Commit Message

Kishen Maloor Jan. 12, 2022, 10:15 p.m. UTC 
This change adds a MPTCP netlink interface for issuing
ADD_ADDR advertisements over the chosen MPTCP connection from a
userspace path manager.

The command requires the following parameters:
{ token, { loc_id, family, daddr4 | daddr6 [, dport] } [, if_idx],
flags/signal }.

Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 include/uapi/linux/mptcp.h |   2 +
 net/mptcp/pm_netlink.c     | 111 +++++++++++++++++++++++++++++++++++++
 2 files changed, 113 insertions(+)

Comments

Paolo Abeni Jan. 14, 2022, 5:04 p.m. UTC | #1 
On Wed, 2022-01-12 at 17:15 -0500, Kishen Maloor wrote:
> This change adds a MPTCP netlink interface for issuing
> ADD_ADDR advertisements over the chosen MPTCP connection from a
> userspace path manager.
> 
> The command requires the following parameters:
> { token, { loc_id, family, daddr4 | daddr6 [, dport] } [, if_idx],
> flags/signal }.
> 
> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
> ---
>  include/uapi/linux/mptcp.h |   2 +
>  net/mptcp/pm_netlink.c     | 111 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 113 insertions(+)
> 
> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> index f106a3941cdf..40380be396c8 100644
> --- a/include/uapi/linux/mptcp.h
> +++ b/include/uapi/linux/mptcp.h
> @@ -55,6 +55,7 @@ enum {
>  	MPTCP_PM_ATTR_ADDR,				/* nested address */
>  	MPTCP_PM_ATTR_RCV_ADD_ADDRS,			/* u32 */
>  	MPTCP_PM_ATTR_SUBFLOWS,				/* u32 */
> +	MPTCP_PM_ATTR_TOKEN,				/* u32 */
>  
>  	__MPTCP_PM_ATTR_MAX
>  };
> @@ -92,6 +93,7 @@ enum {
>  	MPTCP_PM_CMD_SET_LIMITS,
>  	MPTCP_PM_CMD_GET_LIMITS,
>  	MPTCP_PM_CMD_SET_FLAGS,
> +	MPTCP_PM_CMD_ANNOUNCE,
>  
>  	__MPTCP_PM_CMD_AFTER_LAST
>  };
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 07536abab3e5..18228b2dfa2d 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1268,6 +1268,7 @@ static const struct nla_policy mptcp_pm_policy[MPTCP_PM_ATTR_MAX + 1] = {
>  					NLA_POLICY_NESTED(mptcp_pm_addr_policy),
>  	[MPTCP_PM_ATTR_RCV_ADD_ADDRS]	= { .type	= NLA_U32,	},
>  	[MPTCP_PM_ATTR_SUBFLOWS]	= { .type	= NLA_U32,	},
> +	[MPTCP_PM_ATTR_TOKEN]		= { .type	= NLA_U32,	},
>  };
>  
>  void mptcp_pm_nl_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
> @@ -2028,6 +2029,111 @@ static int mptcp_nl_addr_backup(struct net *net,
>  	return ret;
>  }
>  
> +static int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
> +	struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
> +	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
> +	struct mptcp_pm_addr_entry addr_val;
> +	struct mptcp_local_lsk *lsk_ref;
> +	bool reuse_port = false;
> +	struct mptcp_sock *msk;
> +	struct socket *lsk;
> +	u32 token_val;
> +	int err;
> +
> +	if (!addr || !token) {
> +		GENL_SET_ERR_MSG(info, "missing required inputs");
> +		return -EINVAL;
> +	}
> +
> +	token_val = nla_get_u32(token);
> +
> +	msk = mptcp_token_get_sock(sock_net(skb->sk), token_val);
> +	if (!msk) {
> +		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
> +		return -EINVAL;
> +	}
> +
> +	if (READ_ONCE(msk->pm.pm_type) != MPTCP_PM_TYPE_USERSPACE) {
> +		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
> +		return -EINVAL;
> +	}
> +
> +	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
> +	if (err < 0) {
> +		GENL_SET_ERR_MSG(info, "error parsing local address");
> +		return err;
> +	}
> +
> +	if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> +		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
> +		return -EINVAL;
> +	}
> +
> +	if (!addr_val.addr.port) {
> +		addr_val.addr.port =
> +			((struct inet_sock *)inet_sk
> +			 ((struct sock *)msk))->inet_sport;
> +
> +		reuse_port = true;
> +	}
> +
> +	lsk_ref = lsk_list_find(pernet, &addr_val.addr);
> +
> +	if (!lsk_ref) {
> +		err = mptcp_pm_nl_create_listen_socket(skb->sk, &addr_val, &lsk);
> +		if ((err && !reuse_port) || (err && (err != -EADDRINUSE) && reuse_port)) {
> +			GENL_SET_ERR_MSG(info, "error creating listen socket");
> +			return err;
> +		}
> +
> +		if (lsk) {
> +			lsk_ref = lsk_list_add(pernet, &addr_val.addr, lsk);
> +			if (!lsk_ref) {
> +				GENL_SET_ERR_MSG(info, "can't allocate lsk ref");
> +				sock_release(lsk);
> +				return -ENOMEM;
> +			}
> +		}
> +	}

It looks like that the above chunk of code is sort of recurring. it's
likely worthy to factor it in some helper alike
"lsk_list_find_or_create()"

Cheers,

Paolo
Kishen Maloor Jan. 19, 2022, 1:21 a.m. UTC | #2 
On 1/14/22 9:04 AM, Paolo Abeni wrote:
> On Wed, 2022-01-12 at 17:15 -0500, Kishen Maloor wrote:
>> This change adds a MPTCP netlink interface for issuing
>> ADD_ADDR advertisements over the chosen MPTCP connection from a
>> userspace path manager.
>>
>> The command requires the following parameters:
>> { token, { loc_id, family, daddr4 | daddr6 [, dport] } [, if_idx],
>> flags/signal }.
>>
>> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
>> ---
>>  include/uapi/linux/mptcp.h |   2 +
>>  net/mptcp/pm_netlink.c     | 111 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 113 insertions(+)
>>
>> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
>> index f106a3941cdf..40380be396c8 100644
>> --- a/include/uapi/linux/mptcp.h
>> +++ b/include/uapi/linux/mptcp.h
>> @@ -55,6 +55,7 @@ enum {
>>  	MPTCP_PM_ATTR_ADDR,				/* nested address */
>>  	MPTCP_PM_ATTR_RCV_ADD_ADDRS,			/* u32 */
>>  	MPTCP_PM_ATTR_SUBFLOWS,				/* u32 */
>> +	MPTCP_PM_ATTR_TOKEN,				/* u32 */
>>  
>>  	__MPTCP_PM_ATTR_MAX
>>  };
>> @@ -92,6 +93,7 @@ enum {
>>  	MPTCP_PM_CMD_SET_LIMITS,
>>  	MPTCP_PM_CMD_GET_LIMITS,
>>  	MPTCP_PM_CMD_SET_FLAGS,
>> +	MPTCP_PM_CMD_ANNOUNCE,
>>  
>>  	__MPTCP_PM_CMD_AFTER_LAST
>>  };
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index 07536abab3e5..18228b2dfa2d 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -1268,6 +1268,7 @@ static const struct nla_policy mptcp_pm_policy[MPTCP_PM_ATTR_MAX + 1] = {
>>  					NLA_POLICY_NESTED(mptcp_pm_addr_policy),
>>  	[MPTCP_PM_ATTR_RCV_ADD_ADDRS]	= { .type	= NLA_U32,	},
>>  	[MPTCP_PM_ATTR_SUBFLOWS]	= { .type	= NLA_U32,	},
>> +	[MPTCP_PM_ATTR_TOKEN]		= { .type	= NLA_U32,	},
>>  };
>>  
>>  void mptcp_pm_nl_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
>> @@ -2028,6 +2029,111 @@ static int mptcp_nl_addr_backup(struct net *net,
>>  	return ret;
>>  }
>>  
>> +static int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info)
>> +{
>> +	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
>> +	struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
>> +	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
>> +	struct mptcp_pm_addr_entry addr_val;
>> +	struct mptcp_local_lsk *lsk_ref;
>> +	bool reuse_port = false;
>> +	struct mptcp_sock *msk;
>> +	struct socket *lsk;
>> +	u32 token_val;
>> +	int err;
>> +
>> +	if (!addr || !token) {
>> +		GENL_SET_ERR_MSG(info, "missing required inputs");
>> +		return -EINVAL;
>> +	}
>> +
>> +	token_val = nla_get_u32(token);
>> +
>> +	msk = mptcp_token_get_sock(sock_net(skb->sk), token_val);
>> +	if (!msk) {
>> +		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (READ_ONCE(msk->pm.pm_type) != MPTCP_PM_TYPE_USERSPACE) {
>> +		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
>> +		return -EINVAL;
>> +	}
>> +
>> +	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
>> +	if (err < 0) {
>> +		GENL_SET_ERR_MSG(info, "error parsing local address");
>> +		return err;
>> +	}
>> +
>> +	if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>> +		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (!addr_val.addr.port) {
>> +		addr_val.addr.port =
>> +			((struct inet_sock *)inet_sk
>> +			 ((struct sock *)msk))->inet_sport;
>> +
>> +		reuse_port = true;
>> +	}
>> +
>> +	lsk_ref = lsk_list_find(pernet, &addr_val.addr);
>> +
>> +	if (!lsk_ref) {
>> +		err = mptcp_pm_nl_create_listen_socket(skb->sk, &addr_val, &lsk);
>> +		if ((err && !reuse_port) || (err && (err != -EADDRINUSE) && reuse_port)) {
>> +			GENL_SET_ERR_MSG(info, "error creating listen socket");
>> +			return err;
>> +		}
>> +
>> +		if (lsk) {
>> +			lsk_ref = lsk_list_add(pernet, &addr_val.addr, lsk);
>> +			if (!lsk_ref) {
>> +				GENL_SET_ERR_MSG(info, "can't allocate lsk ref");
>> +				sock_release(lsk);
>> +				return -ENOMEM;
>> +			}
>> +		}
>> +	}
> 
> It looks like that the above chunk of code is sort of recurring. it's
> likely worthy to factor it in some helper alike
> "lsk_list_find_or_create()"

The find->create_listen_sock sequence is run in 3 places, but there are deviations in
the logic that follows a failed create_listen_sock. Hence, I did not over optimize.
But I will check if it makes sense to refactor.

> 
> Cheers,
> 
> Paolo
>
diff mbox series

Patch

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index f106a3941cdf..40380be396c8 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -55,6 +55,7 @@  enum {
 	MPTCP_PM_ATTR_ADDR,				/* nested address */
 	MPTCP_PM_ATTR_RCV_ADD_ADDRS,			/* u32 */
 	MPTCP_PM_ATTR_SUBFLOWS,				/* u32 */
+	MPTCP_PM_ATTR_TOKEN,				/* u32 */
 
 	__MPTCP_PM_ATTR_MAX
 };
@@ -92,6 +93,7 @@  enum {
 	MPTCP_PM_CMD_SET_LIMITS,
 	MPTCP_PM_CMD_GET_LIMITS,
 	MPTCP_PM_CMD_SET_FLAGS,
+	MPTCP_PM_CMD_ANNOUNCE,
 
 	__MPTCP_PM_CMD_AFTER_LAST
 };
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 07536abab3e5..18228b2dfa2d 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1268,6 +1268,7 @@  static const struct nla_policy mptcp_pm_policy[MPTCP_PM_ATTR_MAX + 1] = {
 					NLA_POLICY_NESTED(mptcp_pm_addr_policy),
 	[MPTCP_PM_ATTR_RCV_ADD_ADDRS]	= { .type	= NLA_U32,	},
 	[MPTCP_PM_ATTR_SUBFLOWS]	= { .type	= NLA_U32,	},
+	[MPTCP_PM_ATTR_TOKEN]		= { .type	= NLA_U32,	},
 };
 
 void mptcp_pm_nl_subflow_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
@@ -2028,6 +2029,111 @@  static int mptcp_nl_addr_backup(struct net *net,
 	return ret;
 }
 
+static int mptcp_nl_cmd_announce(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
+	struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
+	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
+	struct mptcp_pm_addr_entry addr_val;
+	struct mptcp_local_lsk *lsk_ref;
+	bool reuse_port = false;
+	struct mptcp_sock *msk;
+	struct socket *lsk;
+	u32 token_val;
+	int err;
+
+	if (!addr || !token) {
+		GENL_SET_ERR_MSG(info, "missing required inputs");
+		return -EINVAL;
+	}
+
+	token_val = nla_get_u32(token);
+
+	msk = mptcp_token_get_sock(sock_net(skb->sk), token_val);
+	if (!msk) {
+		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+		return -EINVAL;
+	}
+
+	if (READ_ONCE(msk->pm.pm_type) != MPTCP_PM_TYPE_USERSPACE) {
+		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
+		return -EINVAL;
+	}
+
+	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
+	if (err < 0) {
+		GENL_SET_ERR_MSG(info, "error parsing local address");
+		return err;
+	}
+
+	if (addr_val.addr.id == 0 || !(addr_val.flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+		GENL_SET_ERR_MSG(info, "invalid addr id or flags");
+		return -EINVAL;
+	}
+
+	if (!addr_val.addr.port) {
+		addr_val.addr.port =
+			((struct inet_sock *)inet_sk
+			 ((struct sock *)msk))->inet_sport;
+
+		reuse_port = true;
+	}
+
+	lsk_ref = lsk_list_find(pernet, &addr_val.addr);
+
+	if (!lsk_ref) {
+		err = mptcp_pm_nl_create_listen_socket(skb->sk, &addr_val, &lsk);
+		if ((err && !reuse_port) || (err && (err != -EADDRINUSE) && reuse_port)) {
+			GENL_SET_ERR_MSG(info, "error creating listen socket");
+			return err;
+		}
+
+		if (lsk) {
+			lsk_ref = lsk_list_add(pernet, &addr_val.addr, lsk);
+			if (!lsk_ref) {
+				GENL_SET_ERR_MSG(info, "can't allocate lsk ref");
+				sock_release(lsk);
+				return -ENOMEM;
+			}
+		}
+	}
+
+	if (!reuse_port) {
+		addr_val.lsk_ref = lsk_ref;
+		lsk_ref = NULL;
+	} else {
+		addr_val.addr.port = 0;
+	}
+
+	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val);
+	if (err < 0) {
+		if (addr_val.lsk_ref)
+			lsk_list_release(pernet, addr_val.lsk_ref);
+		else if (lsk_ref)
+			lsk_list_release(pernet, lsk_ref);
+		GENL_SET_ERR_MSG(info, "did not match address and id");
+		return err;
+	}
+
+	lock_sock((struct sock *)msk);
+	spin_lock_bh(&msk->pm.lock);
+
+	if (mptcp_pm_alloc_anno_list(msk, &addr_val, lsk_ref)) {
+		mptcp_pm_announce_addr(msk, &addr_val.addr, false);
+		mptcp_pm_nl_addr_send_ack(msk);
+	}
+
+	spin_unlock_bh(&msk->pm.lock);
+	release_sock((struct sock *)msk);
+
+	if (addr_val.lsk_ref)
+		lsk_list_release(pernet, addr_val.lsk_ref);
+	else if (lsk_ref)
+		lsk_list_release(pernet, lsk_ref);
+
+	return 0;
+}
+
 static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }, *entry;
@@ -2370,6 +2476,11 @@  static const struct genl_small_ops mptcp_pm_ops[] = {
 		.doit   = mptcp_nl_cmd_set_flags,
 		.flags  = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd    = MPTCP_PM_CMD_ANNOUNCE,
+		.doit   = mptcp_nl_cmd_announce,
+		.flags  = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family mptcp_genl_family __ro_after_init = {