* [PATCH mptcp-next v2 0/2] add pm listener events @ 2022-10-26 10:45 Geliang Tang 2022-10-26 10:45 ` [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly Geliang Tang 2022-10-26 10:45 ` [PATCH mptcp-next v2 2/2] mptcp: add pm listener events Geliang Tang 0 siblings, 2 replies; 7+ messages in thread From: Geliang Tang @ 2022-10-26 10:45 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang v2: - send created event from mptcp_listen. - add a new patch, use entry->lsk directly. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313 Geliang Tang (2): mptcp: use entry->lsk directly mptcp: add pm listener events include/uapi/linux/mptcp.h | 9 +++++ net/mptcp/pm_netlink.c | 73 +++++++++++++++++++++++++++++--------- net/mptcp/protocol.c | 3 ++ net/mptcp/protocol.h | 2 ++ 4 files changed, 71 insertions(+), 16 deletions(-) -- 2.35.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly 2022-10-26 10:45 [PATCH mptcp-next v2 0/2] add pm listener events Geliang Tang @ 2022-10-26 10:45 ` Geliang Tang 2022-10-27 19:50 ` Mat Martineau 2022-10-26 10:45 ` [PATCH mptcp-next v2 2/2] mptcp: add pm listener events Geliang Tang 1 sibling, 1 reply; 7+ messages in thread From: Geliang Tang @ 2022-10-26 10:45 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang This patch drops msk and ssock in mptcp_pm_nl_create_listen_socket(), use entry->lsk directly instead of ssock. Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- net/mptcp/pm_netlink.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index 9813ed0fde9b..52a5847b9d74 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -992,8 +992,6 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, { int addrlen = sizeof(struct sockaddr_in); struct sockaddr_storage addr; - struct mptcp_sock *msk; - struct socket *ssock; int backlog = 1024; int err; @@ -1002,30 +1000,18 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, if (err) return err; - msk = mptcp_sk(entry->lsk->sk); - if (!msk) { - err = -EINVAL; - goto out; - } - - ssock = __mptcp_nmpc_socket(msk); - if (!ssock) { - err = -EINVAL; - goto out; - } - mptcp_info2sockaddr(&entry->addr, &addr, entry->addr.family); #if IS_ENABLED(CONFIG_MPTCP_IPV6) if (entry->addr.family == AF_INET6) addrlen = sizeof(struct sockaddr_in6); #endif - err = kernel_bind(ssock, (struct sockaddr *)&addr, addrlen); + err = kernel_bind(entry->lsk, (struct sockaddr *)&addr, addrlen); if (err) { pr_warn("kernel_bind error, err=%d", err); goto out; } - err = kernel_listen(ssock, backlog); + err = kernel_listen(entry->lsk, backlog); if (err) { pr_warn("kernel_listen error, err=%d", err); goto out; -- 2.35.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly 2022-10-26 10:45 ` [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly Geliang Tang @ 2022-10-27 19:50 ` Mat Martineau 2022-10-28 11:12 ` Geliang Tang 0 siblings, 1 reply; 7+ messages in thread From: Mat Martineau @ 2022-10-27 19:50 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp On Wed, 26 Oct 2022, Geliang Tang wrote: > This patch drops msk and ssock in mptcp_pm_nl_create_listen_socket(), > use entry->lsk directly instead of ssock. > Hi Geliang - The change looks good, but the old code looks buggy. Seems like it would be trying to re-bind the nmpc socket each time a listener socket was created. Is this a fix for mptcp-net in your opinion? Thanks, Mat > Signed-off-by: Geliang Tang <geliang.tang@suse.com> > --- > net/mptcp/pm_netlink.c | 18 ++---------------- > 1 file changed, 2 insertions(+), 16 deletions(-) > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > index 9813ed0fde9b..52a5847b9d74 100644 > --- a/net/mptcp/pm_netlink.c > +++ b/net/mptcp/pm_netlink.c > @@ -992,8 +992,6 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, > { > int addrlen = sizeof(struct sockaddr_in); > struct sockaddr_storage addr; > - struct mptcp_sock *msk; > - struct socket *ssock; > int backlog = 1024; > int err; > > @@ -1002,30 +1000,18 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, > if (err) > return err; > > - msk = mptcp_sk(entry->lsk->sk); > - if (!msk) { > - err = -EINVAL; > - goto out; > - } > - > - ssock = __mptcp_nmpc_socket(msk); > - if (!ssock) { > - err = -EINVAL; > - goto out; > - } > - > mptcp_info2sockaddr(&entry->addr, &addr, entry->addr.family); > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > if (entry->addr.family == AF_INET6) > addrlen = sizeof(struct sockaddr_in6); > #endif > - err = kernel_bind(ssock, (struct sockaddr *)&addr, addrlen); > + err = kernel_bind(entry->lsk, (struct sockaddr *)&addr, addrlen); > if (err) { > pr_warn("kernel_bind error, err=%d", err); > goto out; > } > > - err = kernel_listen(ssock, backlog); > + err = kernel_listen(entry->lsk, backlog); > if (err) { > pr_warn("kernel_listen error, err=%d", err); > goto out; > -- > 2.35.3 > > > -- Mat Martineau Intel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly 2022-10-27 19:50 ` Mat Martineau @ 2022-10-28 11:12 ` Geliang Tang 2022-10-31 16:02 ` Matthieu Baerts 0 siblings, 1 reply; 7+ messages in thread From: Geliang Tang @ 2022-10-28 11:12 UTC (permalink / raw) To: Mat Martineau; +Cc: mptcp On Thu, Oct 27, 2022 at 12:50:03PM -0700, Mat Martineau wrote: > On Wed, 26 Oct 2022, Geliang Tang wrote: > > > This patch drops msk and ssock in mptcp_pm_nl_create_listen_socket(), > > use entry->lsk directly instead of ssock. > > > > Hi Geliang - > > The change looks good, but the old code looks buggy. Seems like it would be > trying to re-bind the nmpc socket each time a listener socket was created. > Is this a fix for mptcp-net in your opinion? Yes, this should be a fix. Please add this into the -net section when merging it. Thanks, -Geliang > > > Thanks, > > Mat > > > Signed-off-by: Geliang Tang <geliang.tang@suse.com> > > --- > > net/mptcp/pm_netlink.c | 18 ++---------------- > > 1 file changed, 2 insertions(+), 16 deletions(-) > > > > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c > > index 9813ed0fde9b..52a5847b9d74 100644 > > --- a/net/mptcp/pm_netlink.c > > +++ b/net/mptcp/pm_netlink.c > > @@ -992,8 +992,6 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, > > { > > int addrlen = sizeof(struct sockaddr_in); > > struct sockaddr_storage addr; > > - struct mptcp_sock *msk; > > - struct socket *ssock; > > int backlog = 1024; > > int err; > > > > @@ -1002,30 +1000,18 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, > > if (err) > > return err; > > > > - msk = mptcp_sk(entry->lsk->sk); > > - if (!msk) { > > - err = -EINVAL; > > - goto out; > > - } > > - > > - ssock = __mptcp_nmpc_socket(msk); > > - if (!ssock) { > > - err = -EINVAL; > > - goto out; > > - } > > - > > mptcp_info2sockaddr(&entry->addr, &addr, entry->addr.family); > > #if IS_ENABLED(CONFIG_MPTCP_IPV6) > > if (entry->addr.family == AF_INET6) > > addrlen = sizeof(struct sockaddr_in6); > > #endif > > - err = kernel_bind(ssock, (struct sockaddr *)&addr, addrlen); > > + err = kernel_bind(entry->lsk, (struct sockaddr *)&addr, addrlen); > > if (err) { > > pr_warn("kernel_bind error, err=%d", err); > > goto out; > > } > > > > - err = kernel_listen(ssock, backlog); > > + err = kernel_listen(entry->lsk, backlog); > > if (err) { > > pr_warn("kernel_listen error, err=%d", err); > > goto out; > > -- > > 2.35.3 > > > > > > > > -- > Mat Martineau > Intel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly 2022-10-28 11:12 ` Geliang Tang @ 2022-10-31 16:02 ` Matthieu Baerts 0 siblings, 0 replies; 7+ messages in thread From: Matthieu Baerts @ 2022-10-31 16:02 UTC (permalink / raw) To: Geliang Tang, Mat Martineau; +Cc: mptcp Hi Geliang, Mat, On 28/10/2022 13:12, Geliang Tang wrote: > On Thu, Oct 27, 2022 at 12:50:03PM -0700, Mat Martineau wrote: >> On Wed, 26 Oct 2022, Geliang Tang wrote: >> >>> This patch drops msk and ssock in mptcp_pm_nl_create_listen_socket(), >>> use entry->lsk directly instead of ssock. >>> >> >> Hi Geliang - >> >> The change looks good, but the old code looks buggy. Seems like it would be >> trying to re-bind the nmpc socket each time a listener socket was created. >> Is this a fix for mptcp-net in your opinion? > > Yes, this should be a fix. Please add this into the -net section when > merging it. Only this patch 1/2 should go to -net I suppose and with this tag, right? Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port") Also, I think the commit message should be improved to describe the issue that is being fixed. Currently, it sounds like it is a simple refactoring to me :) Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH mptcp-next v2 2/2] mptcp: add pm listener events 2022-10-26 10:45 [PATCH mptcp-next v2 0/2] add pm listener events Geliang Tang 2022-10-26 10:45 ` [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly Geliang Tang @ 2022-10-26 10:45 ` Geliang Tang 2022-10-26 12:20 ` mptcp: add pm listener events: Tests Results MPTCP CI 1 sibling, 1 reply; 7+ messages in thread From: Geliang Tang @ 2022-10-26 10:45 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang This patch adds MPTCP netlink events for PM listening socket create and close. Signed-off-by: Geliang Tang <geliang.tang@suse.com> --- include/uapi/linux/mptcp.h | 9 +++++++ net/mptcp/pm_netlink.c | 55 ++++++++++++++++++++++++++++++++++++++ net/mptcp/protocol.c | 3 +++ net/mptcp/protocol.h | 2 ++ 4 files changed, 69 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 52a5847b9d74..02469d603261 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -2145,6 +2145,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); + 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); + 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) { @@ -2190,6 +2242,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 dceeaaf3bab4..a1f9dddbdf2c 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2358,6 +2358,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); @@ -3674,6 +3675,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 8f48f881adf8..d2c34360a3e3 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -839,6 +839,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); static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk) -- 2.35.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: mptcp: add pm listener events: Tests Results 2022-10-26 10:45 ` [PATCH mptcp-next v2 2/2] mptcp: add pm listener events Geliang Tang @ 2022-10-26 12:20 ` MPTCP CI 0 siblings, 0 replies; 7+ messages in thread From: MPTCP CI @ 2022-10-26 12:20 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp Hi Geliang, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: - Unstable: 2 failed test(s): selftest_mptcp_join selftest_userspace_pm 🔴: - Task: https://cirrus-ci.com/task/5474410546069504 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5474410546069504/summary/summary.txt - KVM Validation: debug: - Unstable: 3 failed test(s): packetdrill_add_addr selftest_mptcp_join selftest_userspace_pm - Critical: 1 Call Trace(s) ❌: - Task: https://cirrus-ci.com/task/6600310452912128 - Summary: https://api.cirrus-ci.com/v1/artifact/task/6600310452912128/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4458805fd0e0 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) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-31 16:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-26 10:45 [PATCH mptcp-next v2 0/2] add pm listener events Geliang Tang 2022-10-26 10:45 ` [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly Geliang Tang 2022-10-27 19:50 ` Mat Martineau 2022-10-28 11:12 ` Geliang Tang 2022-10-31 16:02 ` Matthieu Baerts 2022-10-26 10:45 ` [PATCH mptcp-next v2 2/2] mptcp: add pm listener events Geliang Tang 2022-10-26 12:20 ` mptcp: add pm listener events: Tests Results MPTCP CI
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.