Hi Geliang,
On 08/10/2023 12:27, Geliang Tang wrote:
> This patch adds the ability to send RM_ADDR for local ID 0. Check
> whether id 0 address is removed, if not, put id 0 into a removing
> list, pass it to mptcp_pm_remove_addr() to remove id 0 address.
>
> There is no reason not to allow the userspace to remove the initial
> address (ID 0). This special case was not taken into account not
> letting the userspace to delete all addresses as announced.
Thank you for the next version!
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/379
> Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE")
(This should target mptcp-net)
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/pm_userspace.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 6b8083650bc1..ed9db6a4e228 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -211,6 +211,37 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
> return err;
> }
>
> +static int mptcp_userspace_remove_id_zero_address(struct mptcp_sock *msk,
> + struct genl_info *info)
> +{
> + struct mptcp_rm_list list = { .nr = 0 };
> + struct mptcp_subflow_context *subflow;
> + struct sock *sk = (struct sock *)msk;
> + bool has_id_0 = false;
> + int err = -EINVAL;
> +
> + lock_sock(sk);
> + mptcp_for_each_subflow(msk, subflow) {
> + if (subflow->local_id == 0) {
> + has_id_0 = true;
> + break;
> + }
> + }
> + if (!has_id_0) {
> + GENL_SET_ERR_MSG(info, "address with id 0 not found");
> + goto out;
(here it can be 'goto remove_err')
> + }
> +
> + list.ids[list.nr++] = 0;
detail: I think it is best to add a new line before spin_lock_bh() and
after spin_unlock_bh(), to clearly show the block with the "dangerous"
section with lock/unlock.
> + spin_lock_bh(&msk->pm.lock);
> + mptcp_pm_remove_addr(msk, &list);
> + spin_unlock_bh(&msk->pm.lock);
> + err = 0;
(and a new line here too)
> +out:
> + release_sock(sk);
> + return err;
> +}
> +
> int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
> {
> struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
> @@ -245,6 +276,11 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
> goto remove_err;
> }
>
> + if (id_val == 0) {
> + err = mptcp_userspace_remove_id_zero_address(msk, info);
> + goto remove_err;
Here it is best to use 'goto out' I think because we expect not to have
errors most of the time, no?
> + }
> +
> lock_sock(sk);
>
> list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net