1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v2 0/2] add pm listener events
  4. View patch

[PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly

Geliang Tang posted 2 patches 3 years, 3 months ago
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
There is a newer version of this series
[PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly
Posted by Geliang Tang 3 years, 3 months ago 
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
Re: [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly
Posted by Mat Martineau 3 years, 3 months ago 
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
Re: [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly
Posted by Geliang Tang 3 years, 3 months ago 
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
Re: [PATCH mptcp-next v2 1/2] mptcp: use entry->lsk directly
Posted by Matthieu Baerts 3 years, 3 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.