[PATCH mptcp-iproute2] mptcp: add new listener events

Matthieu Baerts posted 1 patch 1 year, 3 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
ip/ipmptcp.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH mptcp-iproute2] mptcp: add new listener events
Posted by Matthieu Baerts 1 year, 3 months ago
These new events have been added in kernel commit f8c9dfbd875b ("mptcp:
add pm listener events") by Geliang Tang.

Two new MPTCP Netlink event types for PM listening socket creation and
closure have been recently added. They will be available in the future
v6.2 kernel.

They have been added because MPTCP for Linux, when not using the
in-kernel PM, depends on the userspace PM to create extra listening
sockets -- called "PM listeners" -- before announcing addresses and
ports. With the existing MPTCP Netlink events, a userspace PM can create
PM listeners at startup time, or in response to an incoming connection.
Creating sockets in response to connections is not optimal: ADD_ADDRs
can't be sent until the sockets are created and listen()ed, and if all
connections are closed then it may not be clear to the userspace PM
daemon that PM listener sockets should be cleaned up. Hence these new
events: PM listening sockets can be managed based on application
activity.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/313
Cc: Geliang Tang <geliang.tang@suse.com>
Cc: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---

Notes:
    No objection for the names?

    We are currently limiting the name to 14 chars. If we want to have
    "LISTENER_xxx" instead of "LISTEN_xxx", we need to increase this
    limit. I'm fine with the current names or with +ER.

 ip/ipmptcp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index ce62ab9a..2338ba53 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -454,6 +454,8 @@ static const char * const event_to_str[] = {
 	[MPTCP_EVENT_SUB_ESTABLISHED] = "SF_ESTABLISHED",
 	[MPTCP_EVENT_SUB_CLOSED] = "SF_CLOSED",
 	[MPTCP_EVENT_SUB_PRIORITY] = "SF_PRIO",
+	[MPTCP_EVENT_LISTENER_CREATED] = "LISTEN_CREATED",
+	[MPTCP_EVENT_LISTENER_CLOSED] = "LISTEN_CLOSED",
 };

 static void print_addr(const char *key, int af, struct rtattr *value)

base-commit: 1e994cf69cfd55fefd000565914fd85e3ec150af
--
2.37.2
Re: [PATCH mptcp-iproute2] mptcp: add new listener events
Posted by Mat Martineau 1 year, 2 months ago
On Mon, 26 Dec 2022, Matthieu Baerts wrote:

> These new events have been added in kernel commit f8c9dfbd875b ("mptcp:
> add pm listener events") by Geliang Tang.
>
> Two new MPTCP Netlink event types for PM listening socket creation and
> closure have been recently added. They will be available in the future
> v6.2 kernel.
>
> They have been added because MPTCP for Linux, when not using the
> in-kernel PM, depends on the userspace PM to create extra listening
> sockets -- called "PM listeners" -- before announcing addresses and
> ports. With the existing MPTCP Netlink events, a userspace PM can create
> PM listeners at startup time, or in response to an incoming connection.
> Creating sockets in response to connections is not optimal: ADD_ADDRs
> can't be sent until the sockets are created and listen()ed, and if all
> connections are closed then it may not be clear to the userspace PM
> daemon that PM listener sockets should be cleaned up. Hence these new
> events: PM listening sockets can be managed based on application
> activity.
>
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/313
> Cc: Geliang Tang <geliang.tang@suse.com>
> Cc: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>
> Notes:
>    No objection for the names?
>
>    We are currently limiting the name to 14 chars. If we want to have
>    "LISTENER_xxx" instead of "LISTEN_xxx", we need to increase this
>    limit. I'm fine with the current names or with +ER.

I have a slight preference to go with the LISTENER_* names and increase 
the fixed event length to 16.

Also had to add "if (tb[MPTCP_ATTR_TOKEN])" before the token printf() in 
mptcp_monitor_msg() (line 502) to avoid a segfault, since the listener 
events don't have token attributes.

The patch only applied to iproute2, not iproute2-next. Not sure which git 
tree they expect to reply to.

- Mat

>
> ip/ipmptcp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> index ce62ab9a..2338ba53 100644
> --- a/ip/ipmptcp.c
> +++ b/ip/ipmptcp.c
> @@ -454,6 +454,8 @@ static const char * const event_to_str[] = {
> 	[MPTCP_EVENT_SUB_ESTABLISHED] = "SF_ESTABLISHED",
> 	[MPTCP_EVENT_SUB_CLOSED] = "SF_CLOSED",
> 	[MPTCP_EVENT_SUB_PRIORITY] = "SF_PRIO",
> +	[MPTCP_EVENT_LISTENER_CREATED] = "LISTEN_CREATED",
> +	[MPTCP_EVENT_LISTENER_CLOSED] = "LISTEN_CLOSED",
> };
>
> static void print_addr(const char *key, int af, struct rtattr *value)
>
> base-commit: 1e994cf69cfd55fefd000565914fd85e3ec150af
> --
> 2.37.2
>
>

--
Mat Martineau
Intel
Re: [PATCH mptcp-iproute2] mptcp: add new listener events
Posted by Matthieu Baerts 1 year, 2 months ago
Hi Mat,

On 04/01/2023 02:26, Mat Martineau wrote:
> On Mon, 26 Dec 2022, Matthieu Baerts wrote:
> 
>> These new events have been added in kernel commit f8c9dfbd875b ("mptcp:
>> add pm listener events") by Geliang Tang.
>>
>> Two new MPTCP Netlink event types for PM listening socket creation and
>> closure have been recently added. They will be available in the future
>> v6.2 kernel.
>>
>> They have been added because MPTCP for Linux, when not using the
>> in-kernel PM, depends on the userspace PM to create extra listening
>> sockets -- called "PM listeners" -- before announcing addresses and
>> ports. With the existing MPTCP Netlink events, a userspace PM can create
>> PM listeners at startup time, or in response to an incoming connection.
>> Creating sockets in response to connections is not optimal: ADD_ADDRs
>> can't be sent until the sockets are created and listen()ed, and if all
>> connections are closed then it may not be clear to the userspace PM
>> daemon that PM listener sockets should be cleaned up. Hence these new
>> events: PM listening sockets can be managed based on application
>> activity.
>>
>> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/313
>> Cc: Geliang Tang <geliang.tang@suse.com>
>> Cc: Mat Martineau <mathew.j.martineau@linux.intel.com>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>
>> Notes:
>>    No objection for the names?
>>
>>    We are currently limiting the name to 14 chars. If we want to have
>>    "LISTENER_xxx" instead of "LISTEN_xxx", we need to increase this
>>    limit. I'm fine with the current names or with +ER.
> 
> I have a slight preference to go with the LISTENER_* names and increase
> the fixed event length to 16.

Fine for me, I can do that in the v2.

> Also had to add "if (tb[MPTCP_ATTR_TOKEN])" before the token printf() in
> mptcp_monitor_msg() (line 502) to avoid a segfault, since the listener
> events don't have token attributes.

Oops, sorry for that, I was too quick in the testing and just realised
the selftests are not using 'ip monitor' :-/

The v2 will contain the fix that I just properly validated on my side.

> The patch only applied to iproute2, not iproute2-next. Not sure which
> git tree they expect to reply to.

I'm targeting iproute2 because Geliang's patches are in netdev's net tree.
I can probably use 'mptcp-iproute2-net' tag in the subject.

Thank you for the nice review!

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net