mptcp: add pm listener events

[RFC mptcp-next] mptcp: add pm listener events

Posted by Geliang Tang 1 year, 6 months ago

This patch adds MPTCP netlink events for PM listening socket create and
close.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/uapi/linux/mptcp.h |  9 ++++++
 net/mptcp/pm_netlink.c     | 66 +++++++++++++++++++++++++++++++++++++-
 net/mptcp/protocol.h       |  3 ++
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index dfe19bf13f4c..4307807ba8d4 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: loc_id, family, sport, saddr4 | saddr6
+ * A new PM listener is created.
+ *
+ * MPTCP_EVENT_LISTENER_CLOSED: loc_id, 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 9813ed0fde9b..85cdd957bb11 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -894,8 +894,13 @@ static bool address_use_port(struct mptcp_pm_addr_entry *entry)
 /* caller must ensure the RCU grace period is already elapsed */
 static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
 {
-	if (entry->lsk)
+	if (entry->lsk) {
+		struct mptcp_sock *msk = mptcp_sk(entry->lsk->sk);
+
 		sock_release(entry->lsk);
+		mptcp_event_pm_listener(msk, &entry->addr,
+					MPTCP_EVENT_LISTENER_CLOSED);
+	}
 	kfree(entry);
 }
 
@@ -1008,6 +1013,9 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
 		goto out;
 	}
 
+	mptcp_event_pm_listener(msk, &entry->addr,
+				MPTCP_EVENT_LISTENER_CREATED);
+
 	ssock = __mptcp_nmpc_socket(msk);
 	if (!ssock) {
 		err = -EINVAL;
@@ -2159,6 +2167,58 @@ void mptcp_event_addr_announced(const struct sock *ssk,
 	kfree_skb(skb);
 }
 
+void mptcp_event_pm_listener(const struct mptcp_sock *msk,
+			     const struct mptcp_addr_info *info,
+			     enum mptcp_event_type event)
+{
+	struct net *net = sock_net((struct sock *)msk);
+	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);
+	if (!skb)
+		return;
+
+	nlh = genlmsg_put(skb, 0, 0, &mptcp_genl_family, 0, event);
+	if (!nlh)
+		goto nla_put_failure;
+
+	if (nla_put_u8(skb, MPTCP_ATTR_LOC_ID, info->id))
+		goto nla_put_failure;
+
+	if (nla_put_u16(skb, MPTCP_ATTR_FAMILY, info->family))
+		goto nla_put_failure;
+
+	if (nla_put_be16(skb, MPTCP_ATTR_SPORT, info->port))
+		goto nla_put_failure;
+
+	switch (info->family) {
+	case AF_INET:
+		if (nla_put_in_addr(skb, MPTCP_ATTR_SADDR4, info->addr.s_addr))
+			goto nla_put_failure;
+		break;
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+	case AF_INET6:
+		if (nla_put_in6_addr(skb, MPTCP_ATTR_SADDR6, &info->addr6))
+			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);
+	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)
 {
@@ -2204,6 +2264,10 @@ 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:
+		break;
+	case MPTCP_EVENT_LISTENER_CLOSED:
+		break;
 	}
 
 	genlmsg_end(skb, nlh);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 8f48f881adf8..be1e8f434b1c 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -839,6 +839,9 @@ 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 mptcp_sock *msk,
+			     const struct mptcp_addr_info *info,
+			     enum mptcp_event_type event);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
-- 
2.35.3

Re: [RFC mptcp-next] mptcp: add pm listener events

Posted by Mat Martineau 1 year, 6 months ago

On Tue, 25 Oct 2022, Geliang Tang wrote:

> This patch adds MPTCP netlink events for PM listening socket create and
> close.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> include/uapi/linux/mptcp.h |  9 ++++++
> net/mptcp/pm_netlink.c     | 66 +++++++++++++++++++++++++++++++++++++-
> net/mptcp/protocol.h       |  3 ++
> 3 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> index dfe19bf13f4c..4307807ba8d4 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: loc_id, family, sport, saddr4 | saddr6
> + * A new PM listener is created.
> + *
> + * MPTCP_EVENT_LISTENER_CLOSED: loc_id, 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 9813ed0fde9b..85cdd957bb11 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -894,8 +894,13 @@ static bool address_use_port(struct mptcp_pm_addr_entry *entry)
> /* caller must ensure the RCU grace period is already elapsed */
> static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
> {
> -	if (entry->lsk)
> +	if (entry->lsk) {
> +		struct mptcp_sock *msk = mptcp_sk(entry->lsk->sk);
> +
> 		sock_release(entry->lsk);
> +		mptcp_event_pm_listener(msk, &entry->addr,
> +					MPTCP_EVENT_LISTENER_CLOSED);
> +	}

It would be better to send the event before the sock_release() to ensure 
the msk pointer is valid.

> 	kfree(entry);
> }
>
> @@ -1008,6 +1013,9 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
> 		goto out;
> 	}
>
> +	mptcp_event_pm_listener(msk, &entry->addr,
> +				MPTCP_EVENT_LISTENER_CREATED);
> +

Hi Geliang -

I think my github issue did not have enough detail :)

I was thinking the events should be triggered for any MPTCP listening 
socket. So, MPTCP_EVENT_LISTENER_CREATED would be sent from 
mptcp_listen(), and then send the _CLOSED event when any MPTCP listener is 
destroyed.

This would allow a userspace PM to act when any process in the net 
namespace creates a listening socket.

- Mat


> 	ssock = __mptcp_nmpc_socket(msk);
> 	if (!ssock) {
> 		err = -EINVAL;
> @@ -2159,6 +2167,58 @@ void mptcp_event_addr_announced(const struct sock *ssk,
> 	kfree_skb(skb);
> }
>
> +void mptcp_event_pm_listener(const struct mptcp_sock *msk,
> +			     const struct mptcp_addr_info *info,
> +			     enum mptcp_event_type event)
> +{
> +	struct net *net = sock_net((struct sock *)msk);
> +	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);
> +	if (!skb)
> +		return;
> +
> +	nlh = genlmsg_put(skb, 0, 0, &mptcp_genl_family, 0, event);
> +	if (!nlh)
> +		goto nla_put_failure;
> +
> +	if (nla_put_u8(skb, MPTCP_ATTR_LOC_ID, info->id))
> +		goto nla_put_failure;
> +
> +	if (nla_put_u16(skb, MPTCP_ATTR_FAMILY, info->family))
> +		goto nla_put_failure;
> +
> +	if (nla_put_be16(skb, MPTCP_ATTR_SPORT, info->port))
> +		goto nla_put_failure;
> +
> +	switch (info->family) {
> +	case AF_INET:
> +		if (nla_put_in_addr(skb, MPTCP_ATTR_SADDR4, info->addr.s_addr))
> +			goto nla_put_failure;
> +		break;
> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> +	case AF_INET6:
> +		if (nla_put_in6_addr(skb, MPTCP_ATTR_SADDR6, &info->addr6))
> +			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);
> +	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)
> {
> @@ -2204,6 +2264,10 @@ 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:
> +		break;
> +	case MPTCP_EVENT_LISTENER_CLOSED:
> +		break;
> 	}
>
> 	genlmsg_end(skb, nlh);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 8f48f881adf8..be1e8f434b1c 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -839,6 +839,9 @@ 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 mptcp_sock *msk,
> +			     const struct mptcp_addr_info *info,
> +			     enum mptcp_event_type event);
> bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
>
> static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel

Re: [RFC mptcp-next] mptcp: add pm listener events

Posted by Mat Martineau 1 year, 6 months ago

On Tue, 25 Oct 2022, Mat Martineau wrote:

> On Tue, 25 Oct 2022, Geliang Tang wrote:
>
>> This patch adds MPTCP netlink events for PM listening socket create and
>> close.
>> 
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>> ---
>> include/uapi/linux/mptcp.h |  9 ++++++
>> net/mptcp/pm_netlink.c     | 66 +++++++++++++++++++++++++++++++++++++-
>> net/mptcp/protocol.h       |  3 ++
>> 3 files changed, 77 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
>> index dfe19bf13f4c..4307807ba8d4 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: loc_id, family, sport, saddr4 | saddr6
>> + * A new PM listener is created.
>> + *
>> + * MPTCP_EVENT_LISTENER_CLOSED: loc_id, 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 9813ed0fde9b..85cdd957bb11 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -894,8 +894,13 @@ static bool address_use_port(struct 
>> mptcp_pm_addr_entry *entry)
>> /* caller must ensure the RCU grace period is already elapsed */
>> static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry 
>> *entry)
>> {
>> -	if (entry->lsk)
>> +	if (entry->lsk) {
>> +		struct mptcp_sock *msk = mptcp_sk(entry->lsk->sk);
>> +
>> 		sock_release(entry->lsk);
>> +		mptcp_event_pm_listener(msk, &entry->addr,
>> +					MPTCP_EVENT_LISTENER_CLOSED);
>> +	}
>
> It would be better to send the event before the sock_release() to ensure the 
> msk pointer is valid.
>

Whoops, I meant to credit Ossama with catching this!

>> 	kfree(entry);
>> }
>> 
>> @@ -1008,6 +1013,9 @@ static int mptcp_pm_nl_create_listen_socket(struct 
>> sock *sk,
>> 		goto out;
>> 	}
>> 
>> +	mptcp_event_pm_listener(msk, &entry->addr,
>> +				MPTCP_EVENT_LISTENER_CREATED);
>> +
>
> Hi Geliang -
>
> I think my github issue did not have enough detail :)
>
> I was thinking the events should be triggered for any MPTCP listening socket. 
> So, MPTCP_EVENT_LISTENER_CREATED would be sent from mptcp_listen(), and then 
> send the _CLOSED event when any MPTCP listener is destroyed.
>
> This would allow a userspace PM to act when any process in the net namespace 
> creates a listening socket.
>
> - Mat
>
>
>> 	ssock = __mptcp_nmpc_socket(msk);
>> 	if (!ssock) {
>> 		err = -EINVAL;
>> @@ -2159,6 +2167,58 @@ void mptcp_event_addr_announced(const struct sock 
>> *ssk,
>> 	kfree_skb(skb);
>> }
>> 
>> +void mptcp_event_pm_listener(const struct mptcp_sock *msk,
>> +			     const struct mptcp_addr_info *info,
>> +			     enum mptcp_event_type event)
>> +{
>> +	struct net *net = sock_net((struct sock *)msk);
>> +	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);
>> +	if (!skb)
>> +		return;
>> +
>> +	nlh = genlmsg_put(skb, 0, 0, &mptcp_genl_family, 0, event);
>> +	if (!nlh)
>> +		goto nla_put_failure;
>> +
>> +	if (nla_put_u8(skb, MPTCP_ATTR_LOC_ID, info->id))
>> +		goto nla_put_failure;
>> +
>> +	if (nla_put_u16(skb, MPTCP_ATTR_FAMILY, info->family))
>> +		goto nla_put_failure;
>> +
>> +	if (nla_put_be16(skb, MPTCP_ATTR_SPORT, info->port))
>> +		goto nla_put_failure;
>> +
>> +	switch (info->family) {
>> +	case AF_INET:
>> +		if (nla_put_in_addr(skb, MPTCP_ATTR_SADDR4, 
>> info->addr.s_addr))
>> +			goto nla_put_failure;
>> +		break;
>> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
>> +	case AF_INET6:
>> +		if (nla_put_in6_addr(skb, MPTCP_ATTR_SADDR6, &info->addr6))
>> +			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);
>> +	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)
>> {
>> @@ -2204,6 +2264,10 @@ 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:
>> +		break;
>> +	case MPTCP_EVENT_LISTENER_CLOSED:
>> +		break;
>> 	}
>>
>> 	genlmsg_end(skb, nlh);
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index 8f48f881adf8..be1e8f434b1c 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -839,6 +839,9 @@ 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 mptcp_sock *msk,
>> +			     const struct mptcp_addr_info *info,
>> +			     enum mptcp_event_type event);
>> bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
>> 
>> static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
>> -- 
>> 2.35.3
>> 
>> 
>> 
>
> --
> Mat Martineau
> Intel
>

--
Mat Martineau
Intel

Re: mptcp: add pm listener events: Tests Results

Posted by MPTCP CI 1 year, 6 months ago

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/6369359325560832
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6369359325560832/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 3 failed test(s): packetdrill_add_addr packetdrill_sockopts selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/4961984442007552
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4961984442007552/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e850ac34e772


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)