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

[Geliang Tang <geliang.tang@suse.com>]

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