* [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen
@ 2023-03-06 4:08 D. Wythe
2023-03-06 12:24 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: D. Wythe @ 2023-03-06 4:08 UTC (permalink / raw)
To: kgraul, wenjia, jaka; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
From: "D. Wythe" <alibuda@linux.alibaba.com>
Before determining whether the msg has unsupported options, it has been
prematurely terminated by the wrong status check.
For the application, the general method of MSG_FASTOPEN likes
fd = socket(...)
/* rather than connect */
sendto(fd, data, len, MSG_FASTOPEN)
Hence, We need to check the flag before state check, because the sock state
here is always SMC_INIT when applications tries MSG_FASTOPEN. Once we
found unsupported options, fallback it to TCP.
Fixes: ee9dfbef02d1 ("net/smc: handle sockopts forcing fallback")
Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
---
net/smc/af_smc.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index b233c94..fd80879 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -2662,24 +2662,30 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
int rc = -EPIPE;
smc = smc_sk(sk);
- lock_sock(sk);
- if ((sk->sk_state != SMC_ACTIVE) &&
- (sk->sk_state != SMC_APPCLOSEWAIT1) &&
- (sk->sk_state != SMC_INIT))
- goto out;
+ /* SMC do not support connect with fastopen */
if (msg->msg_flags & MSG_FASTOPEN) {
+ rc = -EINVAL;
+ lock_sock(sk);
+ /* not perform connect yet, fallback it */
if (sk->sk_state == SMC_INIT && !smc->connect_nonblock) {
rc = smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP);
- if (rc)
- goto out;
- } else {
- rc = -EINVAL;
- goto out;
+ /* fallback success */
+ if (rc == 0)
+ goto fallback; /* with sock lock hold */
}
+ release_sock(sk);
+ return rc;
}
+ lock_sock(sk);
+ if (sk->sk_state != SMC_ACTIVE &&
+ sk->sk_state != SMC_APPCLOSEWAIT1 &&
+ sk->sk_state != SMC_INIT)
+ goto out;
+
if (smc->use_fallback) {
+fallback:
rc = smc->clcsock->ops->sendmsg(smc->clcsock, msg, len);
} else {
rc = smc_tx_sendmsg(smc, msg, len);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen 2023-03-06 4:08 [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen D. Wythe @ 2023-03-06 12:24 ` Simon Horman 2023-03-06 16:01 ` D. Wythe 0 siblings, 1 reply; 5+ messages in thread From: Simon Horman @ 2023-03-06 12:24 UTC (permalink / raw) To: D. Wythe Cc: kgraul, wenjia, jaka, kuba, davem, netdev, linux-s390, linux-rdma On Mon, Mar 06, 2023 at 12:08:48PM +0800, D. Wythe wrote: > From: "D. Wythe" <alibuda@linux.alibaba.com> > > Before determining whether the msg has unsupported options, it has been > prematurely terminated by the wrong status check. > > For the application, the general method of MSG_FASTOPEN likes > > fd = socket(...) > /* rather than connect */ > sendto(fd, data, len, MSG_FASTOPEN) > > Hence, We need to check the flag before state check, because the sock state > here is always SMC_INIT when applications tries MSG_FASTOPEN. Once we > found unsupported options, fallback it to TCP. > > Fixes: ee9dfbef02d1 ("net/smc: handle sockopts forcing fallback") > Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> > --- > net/smc/af_smc.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index b233c94..fd80879 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -2662,24 +2662,30 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) > int rc = -EPIPE; > > smc = smc_sk(sk); > - lock_sock(sk); > - if ((sk->sk_state != SMC_ACTIVE) && > - (sk->sk_state != SMC_APPCLOSEWAIT1) && > - (sk->sk_state != SMC_INIT)) > - goto out; > > + /* SMC do not support connect with fastopen */ > if (msg->msg_flags & MSG_FASTOPEN) { > + rc = -EINVAL; > + lock_sock(sk); > + /* not perform connect yet, fallback it */ > if (sk->sk_state == SMC_INIT && !smc->connect_nonblock) { > rc = smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP); > - if (rc) > - goto out; > - } else { > - rc = -EINVAL; > - goto out; > + /* fallback success */ > + if (rc == 0) > + goto fallback; /* with sock lock hold */ > } > + release_sock(sk); > + return rc; > } > > + lock_sock(sk); > + if (sk->sk_state != SMC_ACTIVE && > + sk->sk_state != SMC_APPCLOSEWAIT1 && > + sk->sk_state != SMC_INIT) > + goto out; > + > if (smc->use_fallback) { > +fallback: > rc = smc->clcsock->ops->sendmsg(smc->clcsock, msg, len); > } else { > rc = smc_tx_sendmsg(smc, msg, len); > -- > 1.8.3.1 Probably I messed something this, as this is *compile tested only*. But as the code at the out label looks like this: out: release_sock(sk); return rc; And smc_switch_to_fallback sets smc->use_fallback, I wonder if the following is a bit nicer: diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c index a4cccdfdc00a..5d5c19e53b77 100644 --- a/net/smc/af_smc.c +++ b/net/smc/af_smc.c @@ -2657,16 +2657,14 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) { struct sock *sk = sock->sk; struct smc_sock *smc; - int rc = -EPIPE; + int rc; smc = smc_sk(sk); lock_sock(sk); - if ((sk->sk_state != SMC_ACTIVE) && - (sk->sk_state != SMC_APPCLOSEWAIT1) && - (sk->sk_state != SMC_INIT)) - goto out; + /* SMC does not support connect with fastopen */ if (msg->msg_flags & MSG_FASTOPEN) { + /* not connected yet, fallback */ if (sk->sk_state == SMC_INIT && !smc->connect_nonblock) { rc = smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP); if (rc) @@ -2675,6 +2673,11 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) rc = -EINVAL; goto out; } + } else if (sk->sk_state != SMC_ACTIVE && + sk->sk_state != SMC_APPCLOSEWAIT1 && + sk->sk_state != SMC_INIT) { + rc = -EPIPE; + goto out; } if (smc->use_fallback) { ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen 2023-03-06 12:24 ` Simon Horman @ 2023-03-06 16:01 ` D. Wythe 2023-03-06 16:02 ` Simon Horman 0 siblings, 1 reply; 5+ messages in thread From: D. Wythe @ 2023-03-06 16:01 UTC (permalink / raw) To: Simon Horman Cc: kgraul, wenjia, jaka, kuba, davem, netdev, linux-s390, linux-rdma Hi Simon, Thank you for your suggestion. Your writing style is more elegant. I will modify it according to your plan. Can I add your name as a co-developer? Best wishes. D. Wythe On 3/6/23 8:24 PM, Simon Horman wrote: > On Mon, Mar 06, 2023 at 12:08:48PM +0800, D. Wythe wrote: >> From: "D. Wythe" <alibuda@linux.alibaba.com> >> >> Before determining whether the msg has unsupported options, it has been >> prematurely terminated by the wrong status check. >> >> For the application, the general method of MSG_FASTOPEN likes >> >> fd = socket(...) >> /* rather than connect */ >> sendto(fd, data, len, MSG_FASTOPEN) >> >> Hence, We need to check the flag before state check, because the sock state >> here is always SMC_INIT when applications tries MSG_FASTOPEN. Once we >> found unsupported options, fallback it to TCP. >> >> Fixes: ee9dfbef02d1 ("net/smc: handle sockopts forcing fallback") >> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> >> --- >> net/smc/af_smc.c | 26 ++++++++++++++++---------- >> 1 file changed, 16 insertions(+), 10 deletions(-) >> >> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c >> index b233c94..fd80879 100644 >> --- a/net/smc/af_smc.c >> +++ b/net/smc/af_smc.c >> @@ -2662,24 +2662,30 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) >> int rc = -EPIPE; >> >> smc = smc_sk(sk); >> - lock_sock(sk); >> - if ((sk->sk_state != SMC_ACTIVE) && >> - (sk->sk_state != SMC_APPCLOSEWAIT1) && >> - (sk->sk_state != SMC_INIT)) >> - goto out; >> >> + /* SMC do not support connect with fastopen */ >> if (msg->msg_flags & MSG_FASTOPEN) { >> + rc = -EINVAL; >> + lock_sock(sk); >> + /* not perform connect yet, fallback it */ >> if (sk->sk_state == SMC_INIT && !smc->connect_nonblock) { >> rc = smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP); >> - if (rc) >> - goto out; >> - } else { >> - rc = -EINVAL; >> - goto out; >> + /* fallback success */ >> + if (rc == 0) >> + goto fallback; /* with sock lock hold */ >> } >> + release_sock(sk); >> + return rc; >> } >> >> + lock_sock(sk); >> + if (sk->sk_state != SMC_ACTIVE && >> + sk->sk_state != SMC_APPCLOSEWAIT1 && >> + sk->sk_state != SMC_INIT) >> + goto out; >> + >> if (smc->use_fallback) { >> +fallback: >> rc = smc->clcsock->ops->sendmsg(smc->clcsock, msg, len); >> } else { >> rc = smc_tx_sendmsg(smc, msg, len); >> -- >> 1.8.3.1 > Probably I messed something this, as this is *compile tested only*. > > But as the code at the out label looks like this: > > out: > release_sock(sk); > return rc; > > And smc_switch_to_fallback sets smc->use_fallback, > I wonder if the following is a bit nicer: > > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c > index a4cccdfdc00a..5d5c19e53b77 100644 > --- a/net/smc/af_smc.c > +++ b/net/smc/af_smc.c > @@ -2657,16 +2657,14 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) > { > struct sock *sk = sock->sk; > struct smc_sock *smc; > - int rc = -EPIPE; > + int rc; > > smc = smc_sk(sk); > lock_sock(sk); > - if ((sk->sk_state != SMC_ACTIVE) && > - (sk->sk_state != SMC_APPCLOSEWAIT1) && > - (sk->sk_state != SMC_INIT)) > - goto out; > > + /* SMC does not support connect with fastopen */ > if (msg->msg_flags & MSG_FASTOPEN) { > + /* not connected yet, fallback */ > if (sk->sk_state == SMC_INIT && !smc->connect_nonblock) { > rc = smc_switch_to_fallback(smc, SMC_CLC_DECL_OPTUNSUPP); > if (rc) > @@ -2675,6 +2673,11 @@ static int smc_sendmsg(struct socket *sock, struct msghdr *msg, size_t len) > rc = -EINVAL; > goto out; > } > + } else if (sk->sk_state != SMC_ACTIVE && > + sk->sk_state != SMC_APPCLOSEWAIT1 && > + sk->sk_state != SMC_INIT) { > + rc = -EPIPE; > + goto out; > } > > if (smc->use_fallback) { > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen 2023-03-06 16:01 ` D. Wythe @ 2023-03-06 16:02 ` Simon Horman 2023-03-06 16:09 ` D. Wythe 0 siblings, 1 reply; 5+ messages in thread From: Simon Horman @ 2023-03-06 16:02 UTC (permalink / raw) To: D. Wythe Cc: kgraul, wenjia, jaka, kuba, davem, netdev, linux-s390, linux-rdma On Tue, Mar 07, 2023 at 12:01:01AM +0800, D. Wythe wrote: > Hi Simon, > > Thank you for your suggestion. Your writing style is more elegant. > > I will modify it according to your plan. Can I add your name as a > co-developer? Sure, thanks! If you need it, my code can be: Signed-off-by: Simon Horman <simon.horman@corigine.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen 2023-03-06 16:02 ` Simon Horman @ 2023-03-06 16:09 ` D. Wythe 0 siblings, 0 replies; 5+ messages in thread From: D. Wythe @ 2023-03-06 16:09 UTC (permalink / raw) To: Simon Horman Cc: kgraul, wenjia, jaka, kuba, davem, netdev, linux-s390, linux-rdma Roger that. Thanks for you information. I will issue the next version after the community has no more comments. On 3/7/23 12:02 AM, Simon Horman wrote: > On Tue, Mar 07, 2023 at 12:01:01AM +0800, D. Wythe wrote: >> Hi Simon, >> >> Thank you for your suggestion. Your writing style is more elegant. >> >> I will modify it according to your plan. Can I add your name as a >> co-developer? > Sure, thanks! > > If you need it, my code can be: > > Signed-off-by: Simon Horman <simon.horman@corigine.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-06 16:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-06 4:08 [PATCH net] net/smc: fix fallback failed while sendmsg with fastopen D. Wythe 2023-03-06 12:24 ` Simon Horman 2023-03-06 16:01 ` D. Wythe 2023-03-06 16:02 ` Simon Horman 2023-03-06 16:09 ` D. Wythe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).