Re: [PATCH mptcp-next v4 1/4] mptcp: add pm listener events

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Wed, 16 Nov 2022, Geliang Tang wrote:

> This patch adds MPTCP netlink events for PM listening socket create and
> close.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>

Hi Geliang -

The functionality here looks good, thanks for adding the tests. I have 
some questions below and on patch 4.

> ---
> include/uapi/linux/mptcp.h |  9 ++++++
> net/mptcp/pm_netlink.c     | 57 ++++++++++++++++++++++++++++++++++++++
> net/mptcp/protocol.c       |  3 ++
> net/mptcp/protocol.h       |  2 ++
> 4 files changed, 71 insertions(+)
>
> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> index dfe19bf13f4c..32af2d278cb4 100644
> --- a/include/uapi/linux/mptcp.h
> +++ b/include/uapi/linux/mptcp.h
> @@ -160,6 +160,12 @@ struct mptcp_info {
>  *                           daddr4 | daddr6, sport, dport, backup, if_idx
>  *                           [, error]
>  * The priority of a subflow has changed. 'error' should not be set.
> + *
> + * MPTCP_EVENT_LISTENER_CREATED: family, sport, saddr4 | saddr6
> + * A new PM listener is created.
> + *
> + * MPTCP_EVENT_LISTENER_CLOSED: family, sport, saddr4 | saddr6
> + * A PM listener is closed.
>  */
> enum mptcp_event_type {
> 	MPTCP_EVENT_UNSPEC = 0,
> @@ -174,6 +180,9 @@ enum mptcp_event_type {
> 	MPTCP_EVENT_SUB_CLOSED = 11,
>
> 	MPTCP_EVENT_SUB_PRIORITY = 13,
> +
> +	MPTCP_EVENT_LISTENER_CREATED = 15,
> +	MPTCP_EVENT_LISTENER_CLOSED = 16,
> };
>
> enum mptcp_event_attr {
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 08806f97c8fb..685240f6bc80 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1026,6 +1026,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
> 	if (err)
> 		return err;
>
> +	mptcp_event_pm_listener(ssock->sk, MPTCP_EVENT_LISTENER_CREATED);
> +
> 	return 0;
> }
>
> @@ -2149,6 +2151,58 @@ void mptcp_event_addr_announced(const struct sock *ssk,
> 	kfree_skb(skb);
> }
>
> +void mptcp_event_pm_listener(const struct sock *ssk,
> +			     enum mptcp_event_type event)
> +{
> +	const struct inet_sock *issk = inet_sk(ssk);
> +	struct net *net = sock_net(ssk);
> +	struct nlmsghdr *nlh;
> +	struct sk_buff *skb;
> +
> +	if (!genl_has_listeners(&mptcp_genl_family, net, MPTCP_PM_EV_GRP_OFFSET))
> +		return;
> +
> +	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);

This doesn't appear to run in atomic context, so GFP_KERNEL would be 
better.

> +	if (!skb)
> +		return;
> +
> +	nlh = genlmsg_put(skb, 0, 0, &mptcp_genl_family, 0, event);
> +	if (!nlh)
> +		goto nla_put_failure;
> +
> +	if (nla_put_u16(skb, MPTCP_ATTR_FAMILY, ssk->sk_family))
> +		goto nla_put_failure;
> +
> +	if (nla_put_be16(skb, MPTCP_ATTR_SPORT, issk->inet_sport))
> +		goto nla_put_failure;
> +
> +	switch (ssk->sk_family) {
> +	case AF_INET:
> +		if (nla_put_in_addr(skb, MPTCP_ATTR_SADDR4, issk->inet_saddr))
> +			goto nla_put_failure;
> +		break;
> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> +	case AF_INET6: {
> +		const struct ipv6_pinfo *np = inet6_sk(ssk);
> +
> +		if (nla_put_in6_addr(skb, MPTCP_ATTR_SADDR6, &np->saddr))
> +			goto nla_put_failure;
> +		break;
> +	}
> +#endif
> +	default:
> +		WARN_ON_ONCE(1);
> +		goto nla_put_failure;
> +	}
> +
> +	genlmsg_end(skb, nlh);
> +	mptcp_nl_mcast_send(net, skb, GFP_ATOMIC);

GFP_KERNEL here too.

- Mat

> +	return;
> +
> +nla_put_failure:
> +	kfree_skb(skb);
> +}
> +
> void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
> 		 const struct sock *ssk, gfp_t gfp)
> {
> @@ -2194,6 +2248,9 @@ void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
> 		if (mptcp_event_sub_closed(skb, msk, ssk) < 0)
> 			goto nla_put_failure;
> 		break;
> +	case MPTCP_EVENT_LISTENER_CREATED:
> +	case MPTCP_EVENT_LISTENER_CLOSED:
> +		break;
> 	}
>
> 	genlmsg_end(skb, nlh);
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 1f9b72b62998..c2bb4255969e 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2366,6 +2366,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> 			tcp_set_state(ssk, TCP_CLOSE);
> 			mptcp_subflow_queue_clean(ssk);
> 			inet_csk_listen_stop(ssk);
> +			mptcp_event_pm_listener(ssk, MPTCP_EVENT_LISTENER_CLOSED);
> 		}
> 		__tcp_close(ssk, 0);
>
> @@ -3682,6 +3683,8 @@ static int mptcp_listen(struct socket *sock, int backlog)
> 	if (!err)
> 		mptcp_copy_inaddrs(sock->sk, ssock->sk);
>
> +	mptcp_event_pm_listener(ssock->sk, MPTCP_EVENT_LISTENER_CREATED);
> +
> unlock:
> 	release_sock(sock->sk);
> 	return err;
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index ffecd103cc50..bae216bff6e4 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -846,6 +846,8 @@ void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
> 		 const struct sock *ssk, gfp_t gfp);
> void mptcp_event_addr_announced(const struct sock *ssk, const struct mptcp_addr_info *info);
> void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
> +void mptcp_event_pm_listener(const struct sock *ssk,
> +			     enum mptcp_event_type event);
> bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
>
> void mptcp_fastopen_gen_msk_ackseq(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel