From: Mat Martineau <mathew.j.martineau@linux.intel.com>
To: Kishen Maloor <kishen.maloor@intel.com>
Cc: mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-net v3 1/2] mptcp: netlink: issue MP_PRIO signals from userspace PMs
Date: Tue, 28 Jun 2022 15:57:52 -0700 (PDT) [thread overview]
Message-ID: <cd551eb2-fff8-718d-3c3f-382cbf22a020@linux.intel.com> (raw)
In-Reply-To: <20220628212918.417515-2-kishen.maloor@intel.com>
On Tue, 28 Jun 2022, Kishen Maloor wrote:
> This change updates MPTCP_PM_CMD_SET_FLAGS to allow userspace PMs
> to issue MP_PRIO signals over a specific subflow selected by
> the connection token, local and remote address+port.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/286
> Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
> ---
> v3:
> -use local and remote address+port (instead of address ID) alongwith the
> connection token to select a subflow.
> ---
> net/mptcp/pm_netlink.c | 30 +++++++++++++++++++++++++-----
> net/mptcp/pm_userspace.c | 30 ++++++++++++++++++++++++++++++
> net/mptcp/protocol.h | 8 +++++++-
> 3 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 05c6a95e9c28..602892f2f921 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -717,9 +717,10 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk)
> }
> }
>
> -static int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
> - struct mptcp_addr_info *addr,
> - u8 bkup)
> +int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
> + struct mptcp_addr_info *addr,
> + struct mptcp_addr_info *rem,
> + u8 bkup)
> {
> struct mptcp_subflow_context *subflow;
>
> @@ -728,7 +729,7 @@ static int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
> mptcp_for_each_subflow(msk, subflow) {
> struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> struct sock *sk = (struct sock *)msk;
> - struct mptcp_addr_info local;
> + struct mptcp_addr_info local, remote;
> bool slow;
>
> local_address((struct sock_common *)ssk, &local);
> @@ -736,6 +737,12 @@ static int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
> continue;
>
> slow = lock_sock_fast(ssk);
> + if (rem && rem->family != AF_UNSPEC) {
> + remote_address((struct sock_common *)ssk, &remote);
> + if (!mptcp_addresses_equal(&remote, rem, rem->port))
> + continue;
> + }
Hi Kishen -
Thanks for the v3!
The CI build failed because the ssk lock is not released before this
'continue' statement. The sparse error message was pretty vague, though.
remote_address() is only checking sock_common fields that don't change, I
don't think the ssk lock is required (it's used this way elsewhere in the
code). So this new hunk of code should be before the lock_sock_fast()
call.
- Mat
> +
> if (subflow->backup != bkup)
> msk->last_snd = NULL;
> subflow->backup = bkup;
> @@ -1839,7 +1846,7 @@ static int mptcp_nl_set_flags(struct net *net,
>
> lock_sock(sk);
> if (changed & MPTCP_PM_ADDR_FLAG_BACKUP)
> - ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup);
> + ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, NULL, bkup);
> if (changed & MPTCP_PM_ADDR_FLAG_FULLMESH)
> mptcp_pm_nl_fullmesh(msk, addr);
> release_sock(sk);
> @@ -1855,6 +1862,9 @@ static int mptcp_nl_set_flags(struct net *net,
> 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;
> + struct mptcp_pm_addr_entry remote = { .addr = { .family = AF_UNSPEC }, };
> + struct nlattr *attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
> + struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
> struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
> struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
> u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
> @@ -1867,6 +1877,12 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> if (ret < 0)
> return ret;
>
> + if (attr_rem) {
> + ret = mptcp_pm_parse_entry(attr_rem, info, false, &remote);
> + if (ret < 0)
> + return ret;
> + }
> +
> if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> bkup = 1;
> if (addr.addr.family == AF_UNSPEC) {
> @@ -1875,6 +1891,10 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> return -EOPNOTSUPP;
> }
>
> + if (token)
> + return mptcp_userspace_pm_set_flags(sock_net(skb->sk),
> + token, &addr, &remote, bkup);
> +
> spin_lock_bh(&pernet->lock);
> entry = __lookup_addr(pernet, &addr.addr, lookup_by_id);
> if (!entry) {
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 26212bebc5ed..51e2f066d54f 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -420,3 +420,33 @@ int mptcp_nl_cmd_sf_destroy(struct sk_buff *skb, struct genl_info *info)
> sock_put((struct sock *)msk);
> return err;
> }
> +
> +int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token,
> + struct mptcp_pm_addr_entry *loc,
> + struct mptcp_pm_addr_entry *rem, u8 bkup)
> +{
> + struct mptcp_sock *msk;
> + int ret = -EINVAL;
> + u32 token_val;
> +
> + token_val = nla_get_u32(token);
> +
> + msk = mptcp_token_get_sock(net, token_val);
> + if (!msk)
> + return ret;
> +
> + if (!mptcp_pm_is_userspace(msk))
> + goto set_flags_err;
> +
> + if (loc->addr.family == AF_UNSPEC ||
> + rem->addr.family == AF_UNSPEC)
> + goto set_flags_err;
> +
> + lock_sock((struct sock *)msk);
> + ret = mptcp_pm_nl_mp_prio_send_ack(msk, &loc->addr, &rem->addr, bkup);
> + release_sock((struct sock *)msk);
> +
> +set_flags_err:
> + sock_put((struct sock *)msk);
> + return ret;
> +}
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 033c995772dc..480c5320b86e 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -772,6 +772,10 @@ void mptcp_pm_rm_addr_received(struct mptcp_sock *msk,
> const struct mptcp_rm_list *rm_list);
> void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup);
> void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq);
> +int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
> + struct mptcp_addr_info *addr,
> + struct mptcp_addr_info *rem,
> + u8 bkup);
> bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
> const struct mptcp_pm_addr_entry *entry);
> void mptcp_pm_free_anno_list(struct mptcp_sock *msk);
> @@ -788,7 +792,9 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
> int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
> unsigned int id,
> u8 *flags, int *ifindex);
> -
> +int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token,
> + struct mptcp_pm_addr_entry *loc,
> + struct mptcp_pm_addr_entry *rem, u8 bkup);
> int mptcp_pm_announce_addr(struct mptcp_sock *msk,
> const struct mptcp_addr_info *addr,
> bool echo);
> --
> 2.31.1
>
>
>
--
Mat Martineau
Intel
next prev parent reply other threads:[~2022-06-28 22:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-28 21:29 [PATCH mptcp-net v3 0/2] mptcp: support MP_PRIO signals with userspace PMs Kishen Maloor
2022-06-28 21:29 ` [PATCH mptcp-net v3 1/2] mptcp: netlink: issue MP_PRIO signals from " Kishen Maloor
2022-06-28 22:57 ` Mat Martineau [this message]
2022-06-28 23:53 ` Kishen Maloor
2022-06-28 21:29 ` [PATCH mptcp-net v3 2/2] selftests: mptcp: userspace PM support for MP_PRIO signals Kishen Maloor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cd551eb2-fff8-718d-3c3f-382cbf22a020@linux.intel.com \
--to=mathew.j.martineau@linux.intel.com \
--cc=kishen.maloor@intel.com \
--cc=mptcp@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).