* [PATCH net 0/3] bugfixs for smc
@ 2023-11-01 3:42 D. Wythe
2023-11-01 3:42 ` [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT D. Wythe
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: D. Wythe @ 2023-11-01 3:42 UTC (permalink / raw)
To: kgraul, wenjia, jaka, wintera; +Cc: kuba, davem, netdev, linux-s390, linux-rdma
From: "D. Wythe" <alibuda@linux.alibaba.com>
This patches includes bugfix following:
1. hung state
2. sock leak
3. potential panic
We have been testing these patches for some time, but
if you have any questions, please let us know.
Thanks,
D. Wythe
D. Wythe (3):
net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT
net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc
net/smc: put sk reference if close work was canceled
net/smc/af_smc.c | 4 ++--
net/smc/smc.h | 5 +++++
net/smc/smc_cdc.c | 11 +++++------
net/smc/smc_close.c | 5 +++--
4 files changed, 15 insertions(+), 10 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT 2023-11-01 3:42 [PATCH net 0/3] bugfixs for smc D. Wythe @ 2023-11-01 3:42 ` D. Wythe 2023-11-01 8:13 ` Dust Li 2023-11-01 8:14 ` Dust Li 2023-11-01 3:42 ` [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc D. Wythe 2023-11-01 3:42 ` [PATCH net 3/3] net/smc: put sk reference if close work was canceled D. Wythe 2 siblings, 2 replies; 8+ messages in thread From: D. Wythe @ 2023-11-01 3:42 UTC (permalink / raw) To: kgraul, wenjia, jaka, wintera; +Cc: kuba, davem, netdev, linux-s390, linux-rdma From: "D. Wythe" <alibuda@linux.alibaba.com> Considering scenario: smc_cdc_rx_handler_rwwi __smc_release sock_set_flag smc_close_active() sock_set_flag __set_bit(DEAD) __set_bit(DONE) Dues to __set_bit is not atomic, the DEAD or DONE might be lost. if the DEAD flag lost, the state SMC_CLOSED will be never be reached in smc_close_passive_work: if (sock_flag(sk, SOCK_DEAD) && smc_close_sent_any_close(conn)) { sk->sk_state = SMC_CLOSED; } else { /* just shutdown, but not yet closed locally */ sk->sk_state = SMC_APPFINCLOSEWAIT; } Replace sock_set_flags or __set_bit to set_bit will fix this problem. Since set_bit is atomic. Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> --- net/smc/af_smc.c | 4 ++-- net/smc/smc.h | 5 +++++ net/smc/smc_cdc.c | 2 +- net/smc/smc_close.c | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c index abd2667..da97f94 100644 --- a/net/smc/af_smc.c +++ b/net/smc/af_smc.c @@ -275,7 +275,7 @@ static int __smc_release(struct smc_sock *smc) if (!smc->use_fallback) { rc = smc_close_active(smc); - sock_set_flag(sk, SOCK_DEAD); + smc_sock_set_flag(sk, SOCK_DEAD); sk->sk_shutdown |= SHUTDOWN_MASK; } else { if (sk->sk_state != SMC_CLOSED) { @@ -1743,7 +1743,7 @@ static int smc_clcsock_accept(struct smc_sock *lsmc, struct smc_sock **new_smc) if (new_clcsock) sock_release(new_clcsock); new_sk->sk_state = SMC_CLOSED; - sock_set_flag(new_sk, SOCK_DEAD); + smc_sock_set_flag(new_sk, SOCK_DEAD); sock_put(new_sk); /* final */ *new_smc = NULL; goto out; diff --git a/net/smc/smc.h b/net/smc/smc.h index 24745fd..e377980 100644 --- a/net/smc/smc.h +++ b/net/smc/smc.h @@ -377,4 +377,9 @@ void smc_fill_gid_list(struct smc_link_group *lgr, int smc_nl_enable_hs_limitation(struct sk_buff *skb, struct genl_info *info); int smc_nl_disable_hs_limitation(struct sk_buff *skb, struct genl_info *info); +static inline void smc_sock_set_flag(struct sock *sk, enum sock_flags flag) +{ + set_bit(flag, &sk->sk_flags); +} + #endif /* __SMC_H */ diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c index 89105e9..01bdb79 100644 --- a/net/smc/smc_cdc.c +++ b/net/smc/smc_cdc.c @@ -385,7 +385,7 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc, smc->sk.sk_shutdown |= RCV_SHUTDOWN; if (smc->clcsock && smc->clcsock->sk) smc->clcsock->sk->sk_shutdown |= RCV_SHUTDOWN; - sock_set_flag(&smc->sk, SOCK_DONE); + smc_sock_set_flag(&smc->sk, SOCK_DONE); sock_hold(&smc->sk); /* sock_put in close_work */ if (!queue_work(smc_close_wq, &conn->close_work)) sock_put(&smc->sk); diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c index dbdf03e..449ef45 100644 --- a/net/smc/smc_close.c +++ b/net/smc/smc_close.c @@ -173,7 +173,7 @@ void smc_close_active_abort(struct smc_sock *smc) break; } - sock_set_flag(sk, SOCK_DEAD); + smc_sock_set_flag(sk, SOCK_DEAD); sk->sk_state_change(sk); if (release_clcsock) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT 2023-11-01 3:42 ` [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT D. Wythe @ 2023-11-01 8:13 ` Dust Li 2023-11-01 8:14 ` Dust Li 1 sibling, 0 replies; 8+ messages in thread From: Dust Li @ 2023-11-01 8:13 UTC (permalink / raw) To: D. Wythe, kgraul, wenjia, jaka, wintera Cc: kuba, davem, netdev, linux-s390, linux-rdma On Wed, Nov 01, 2023 at 11:42:55AM +0800, D. Wythe wrote: >From: "D. Wythe" <alibuda@linux.alibaba.com> > >Considering scenario: > > smc_cdc_rx_handler_rwwi >__smc_release > sock_set_flag >smc_close_active() >sock_set_flag > >__set_bit(DEAD) __set_bit(DONE) > >Dues to __set_bit is not atomic, the DEAD or DONE might be lost. >if the DEAD flag lost, the state SMC_CLOSED will be never be reached >in smc_close_passive_work: > >if (sock_flag(sk, SOCK_DEAD) && > smc_close_sent_any_close(conn)) { > sk->sk_state = SMC_CLOSED; >} else { > /* just shutdown, but not yet closed locally */ > sk->sk_state = SMC_APPFINCLOSEWAIT; >} > >Replace sock_set_flags or __set_bit to set_bit will fix this problem. >Since set_bit is atomic. > >Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> >Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> Reviewed-by: Dust Li <dust.li@linux.alibaba.com> >--- > net/smc/af_smc.c | 4 ++-- > net/smc/smc.h | 5 +++++ > net/smc/smc_cdc.c | 2 +- > net/smc/smc_close.c | 2 +- > 4 files changed, 9 insertions(+), 4 deletions(-) > >diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c >index abd2667..da97f94 100644 >--- a/net/smc/af_smc.c >+++ b/net/smc/af_smc.c >@@ -275,7 +275,7 @@ static int __smc_release(struct smc_sock *smc) > > if (!smc->use_fallback) { > rc = smc_close_active(smc); >- sock_set_flag(sk, SOCK_DEAD); >+ smc_sock_set_flag(sk, SOCK_DEAD); > sk->sk_shutdown |= SHUTDOWN_MASK; > } else { > if (sk->sk_state != SMC_CLOSED) { >@@ -1743,7 +1743,7 @@ static int smc_clcsock_accept(struct smc_sock *lsmc, struct smc_sock **new_smc) > if (new_clcsock) > sock_release(new_clcsock); > new_sk->sk_state = SMC_CLOSED; >- sock_set_flag(new_sk, SOCK_DEAD); >+ smc_sock_set_flag(new_sk, SOCK_DEAD); > sock_put(new_sk); /* final */ > *new_smc = NULL; > goto out; >diff --git a/net/smc/smc.h b/net/smc/smc.h >index 24745fd..e377980 100644 >--- a/net/smc/smc.h >+++ b/net/smc/smc.h >@@ -377,4 +377,9 @@ void smc_fill_gid_list(struct smc_link_group *lgr, > int smc_nl_enable_hs_limitation(struct sk_buff *skb, struct genl_info *info); > int smc_nl_disable_hs_limitation(struct sk_buff *skb, struct genl_info *info); > >+static inline void smc_sock_set_flag(struct sock *sk, enum sock_flags flag) >+{ >+ set_bit(flag, &sk->sk_flags); >+} >+ > #endif /* __SMC_H */ >diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c >index 89105e9..01bdb79 100644 >--- a/net/smc/smc_cdc.c >+++ b/net/smc/smc_cdc.c >@@ -385,7 +385,7 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc, > smc->sk.sk_shutdown |= RCV_SHUTDOWN; > if (smc->clcsock && smc->clcsock->sk) > smc->clcsock->sk->sk_shutdown |= RCV_SHUTDOWN; >- sock_set_flag(&smc->sk, SOCK_DONE); >+ smc_sock_set_flag(&smc->sk, SOCK_DONE); > sock_hold(&smc->sk); /* sock_put in close_work */ > if (!queue_work(smc_close_wq, &conn->close_work)) > sock_put(&smc->sk); >diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c >index dbdf03e..449ef45 100644 >--- a/net/smc/smc_close.c >+++ b/net/smc/smc_close.c >@@ -173,7 +173,7 @@ void smc_close_active_abort(struct smc_sock *smc) > break; > } > >- sock_set_flag(sk, SOCK_DEAD); >+ smc_sock_set_flag(sk, SOCK_DEAD); > sk->sk_state_change(sk); > > if (release_clcsock) { >-- >1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT 2023-11-01 3:42 ` [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT D. Wythe 2023-11-01 8:13 ` Dust Li @ 2023-11-01 8:14 ` Dust Li 1 sibling, 0 replies; 8+ messages in thread From: Dust Li @ 2023-11-01 8:14 UTC (permalink / raw) To: D. Wythe, kgraul, wenjia, jaka, wintera Cc: kuba, davem, netdev, linux-s390, linux-rdma On Wed, Nov 01, 2023 at 11:42:55AM +0800, D. Wythe wrote: >From: "D. Wythe" <alibuda@linux.alibaba.com> > >Considering scenario: > > smc_cdc_rx_handler_rwwi Nit, smc_cdc_rx_handler_rwwi should be smc_cdc_rx_handler() >__smc_release > sock_set_flag >smc_close_active() >sock_set_flag > >__set_bit(DEAD) __set_bit(DONE) > >Dues to __set_bit is not atomic, the DEAD or DONE might be lost. >if the DEAD flag lost, the state SMC_CLOSED will be never be reached >in smc_close_passive_work: > >if (sock_flag(sk, SOCK_DEAD) && > smc_close_sent_any_close(conn)) { > sk->sk_state = SMC_CLOSED; >} else { > /* just shutdown, but not yet closed locally */ > sk->sk_state = SMC_APPFINCLOSEWAIT; >} > >Replace sock_set_flags or __set_bit to set_bit will fix this problem. >Since set_bit is atomic. > >Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> >Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> >--- > net/smc/af_smc.c | 4 ++-- > net/smc/smc.h | 5 +++++ > net/smc/smc_cdc.c | 2 +- > net/smc/smc_close.c | 2 +- > 4 files changed, 9 insertions(+), 4 deletions(-) > >diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c >index abd2667..da97f94 100644 >--- a/net/smc/af_smc.c >+++ b/net/smc/af_smc.c >@@ -275,7 +275,7 @@ static int __smc_release(struct smc_sock *smc) > > if (!smc->use_fallback) { > rc = smc_close_active(smc); >- sock_set_flag(sk, SOCK_DEAD); >+ smc_sock_set_flag(sk, SOCK_DEAD); > sk->sk_shutdown |= SHUTDOWN_MASK; > } else { > if (sk->sk_state != SMC_CLOSED) { >@@ -1743,7 +1743,7 @@ static int smc_clcsock_accept(struct smc_sock *lsmc, struct smc_sock **new_smc) > if (new_clcsock) > sock_release(new_clcsock); > new_sk->sk_state = SMC_CLOSED; >- sock_set_flag(new_sk, SOCK_DEAD); >+ smc_sock_set_flag(new_sk, SOCK_DEAD); > sock_put(new_sk); /* final */ > *new_smc = NULL; > goto out; >diff --git a/net/smc/smc.h b/net/smc/smc.h >index 24745fd..e377980 100644 >--- a/net/smc/smc.h >+++ b/net/smc/smc.h >@@ -377,4 +377,9 @@ void smc_fill_gid_list(struct smc_link_group *lgr, > int smc_nl_enable_hs_limitation(struct sk_buff *skb, struct genl_info *info); > int smc_nl_disable_hs_limitation(struct sk_buff *skb, struct genl_info *info); > >+static inline void smc_sock_set_flag(struct sock *sk, enum sock_flags flag) >+{ >+ set_bit(flag, &sk->sk_flags); >+} >+ > #endif /* __SMC_H */ >diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c >index 89105e9..01bdb79 100644 >--- a/net/smc/smc_cdc.c >+++ b/net/smc/smc_cdc.c >@@ -385,7 +385,7 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc, > smc->sk.sk_shutdown |= RCV_SHUTDOWN; > if (smc->clcsock && smc->clcsock->sk) > smc->clcsock->sk->sk_shutdown |= RCV_SHUTDOWN; >- sock_set_flag(&smc->sk, SOCK_DONE); >+ smc_sock_set_flag(&smc->sk, SOCK_DONE); > sock_hold(&smc->sk); /* sock_put in close_work */ > if (!queue_work(smc_close_wq, &conn->close_work)) > sock_put(&smc->sk); >diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c >index dbdf03e..449ef45 100644 >--- a/net/smc/smc_close.c >+++ b/net/smc/smc_close.c >@@ -173,7 +173,7 @@ void smc_close_active_abort(struct smc_sock *smc) > break; > } > >- sock_set_flag(sk, SOCK_DEAD); >+ smc_sock_set_flag(sk, SOCK_DEAD); > sk->sk_state_change(sk); > > if (release_clcsock) { >-- >1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc 2023-11-01 3:42 [PATCH net 0/3] bugfixs for smc D. Wythe 2023-11-01 3:42 ` [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT D. Wythe @ 2023-11-01 3:42 ` D. Wythe 2023-11-01 8:19 ` Dust Li 2023-11-01 3:42 ` [PATCH net 3/3] net/smc: put sk reference if close work was canceled D. Wythe 2 siblings, 1 reply; 8+ messages in thread From: D. Wythe @ 2023-11-01 3:42 UTC (permalink / raw) To: kgraul, wenjia, jaka, wintera; +Cc: kuba, davem, netdev, linux-s390, linux-rdma From: "D. Wythe" <alibuda@linux.alibaba.com> This patch re-fix the issues memtianed by commit 22a825c541d7 ("net/smc: fix NULL sndbuf_desc in smc_cdc_tx_handler()"). Blocking sending message do solve the issues though, but it also prevents the peer to receive the final message. Besides, in logic, whether the sndbuf_desc is NULL or not have no impact on the processing of cdc message sending. Hence that, this patch allow the cdc message sending but to check the sndbuf_desc with care in smc_cdc_tx_handler(). Fixes: 22a825c541d7 ("net/smc: fix NULL sndbuf_desc in smc_cdc_tx_handler()") Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> --- net/smc/smc_cdc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c index 01bdb79..3c06625 100644 --- a/net/smc/smc_cdc.c +++ b/net/smc/smc_cdc.c @@ -28,13 +28,15 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd, { struct smc_cdc_tx_pend *cdcpend = (struct smc_cdc_tx_pend *)pnd_snd; struct smc_connection *conn = cdcpend->conn; + struct smc_buf_desc *sndbuf_desc; struct smc_sock *smc; int diff; + sndbuf_desc = conn->sndbuf_desc; smc = container_of(conn, struct smc_sock, conn); bh_lock_sock(&smc->sk); - if (!wc_status) { - diff = smc_curs_diff(cdcpend->conn->sndbuf_desc->len, + if (!wc_status && sndbuf_desc) { + diff = smc_curs_diff(sndbuf_desc->len, &cdcpend->conn->tx_curs_fin, &cdcpend->cursor); /* sndbuf_space is decreased in smc_sendmsg */ @@ -114,9 +116,6 @@ int smc_cdc_msg_send(struct smc_connection *conn, union smc_host_cursor cfed; int rc; - if (unlikely(!READ_ONCE(conn->sndbuf_desc))) - return -ENOBUFS; - smc_cdc_add_pending_send(conn, pend); conn->tx_cdc_seq++; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc 2023-11-01 3:42 ` [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc D. Wythe @ 2023-11-01 8:19 ` Dust Li 2023-11-01 8:36 ` D. Wythe 0 siblings, 1 reply; 8+ messages in thread From: Dust Li @ 2023-11-01 8:19 UTC (permalink / raw) To: D. Wythe, kgraul, wenjia, jaka, wintera Cc: kuba, davem, netdev, linux-s390, linux-rdma On Wed, Nov 01, 2023 at 11:42:56AM +0800, D. Wythe wrote: >From: "D. Wythe" <alibuda@linux.alibaba.com> > >This patch re-fix the issues memtianed by commit 22a825c541d7 memtianed -> mentioned ? >("net/smc: fix NULL sndbuf_desc in smc_cdc_tx_handler()"). > >Blocking sending message do solve the issues though, but it also >prevents the peer to receive the final message. Besides, in logic, >whether the sndbuf_desc is NULL or not have no impact on the processing >of cdc message sending. > >Hence that, this patch allow the cdc message sending but to check the allows >sndbuf_desc with care in smc_cdc_tx_handler(). > >Fixes: 22a825c541d7 ("net/smc: fix NULL sndbuf_desc in smc_cdc_tx_handler()") >Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> >Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> Reviewed-by: Dust Li <dust.li@linux.alibaba.com> >--- > net/smc/smc_cdc.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c >index 01bdb79..3c06625 100644 >--- a/net/smc/smc_cdc.c >+++ b/net/smc/smc_cdc.c >@@ -28,13 +28,15 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd, > { > struct smc_cdc_tx_pend *cdcpend = (struct smc_cdc_tx_pend *)pnd_snd; > struct smc_connection *conn = cdcpend->conn; >+ struct smc_buf_desc *sndbuf_desc; > struct smc_sock *smc; > int diff; > >+ sndbuf_desc = conn->sndbuf_desc; > smc = container_of(conn, struct smc_sock, conn); > bh_lock_sock(&smc->sk); >- if (!wc_status) { >- diff = smc_curs_diff(cdcpend->conn->sndbuf_desc->len, >+ if (!wc_status && sndbuf_desc) { >+ diff = smc_curs_diff(sndbuf_desc->len, > &cdcpend->conn->tx_curs_fin, > &cdcpend->cursor); > /* sndbuf_space is decreased in smc_sendmsg */ >@@ -114,9 +116,6 @@ int smc_cdc_msg_send(struct smc_connection *conn, > union smc_host_cursor cfed; > int rc; > >- if (unlikely(!READ_ONCE(conn->sndbuf_desc))) >- return -ENOBUFS; >- > smc_cdc_add_pending_send(conn, pend); > > conn->tx_cdc_seq++; >-- >1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc 2023-11-01 8:19 ` Dust Li @ 2023-11-01 8:36 ` D. Wythe 0 siblings, 0 replies; 8+ messages in thread From: D. Wythe @ 2023-11-01 8:36 UTC (permalink / raw) To: dust.li, kgraul, wenjia, jaka, wintera Cc: kuba, davem, netdev, linux-s390, linux-rdma On 11/1/23 4:19 PM, Dust Li wrote: > On Wed, Nov 01, 2023 at 11:42:56AM +0800, D. Wythe wrote: >> From: "D. Wythe" <alibuda@linux.alibaba.com> >> >> This patch re-fix the issues memtianed by commit 22a825c541d7 > memtianed -> mentioned ? > >> ("net/smc: fix NULL sndbuf_desc in smc_cdc_tx_handler()"). >> >> Blocking sending message do solve the issues though, but it also >> prevents the peer to receive the final message. Besides, in logic, >> whether the sndbuf_desc is NULL or not have no impact on the processing >> of cdc message sending. >> >> Hence that, this patch allow the cdc message sending but to check the > allows > >> sndbuf_desc with care in smc_cdc_tx_handler(). >> >> Fixes: 22a825c541d7 ("net/smc: fix NULL sndbuf_desc in smc_cdc_tx_handler()") >> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> >> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> Thanks for that. I will fix them in next version. > Reviewed-by: Dust Li <dust.li@linux.alibaba.com> >> --- >> net/smc/smc_cdc.c | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c >> index 01bdb79..3c06625 100644 >> --- a/net/smc/smc_cdc.c >> +++ b/net/smc/smc_cdc.c >> @@ -28,13 +28,15 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd, >> { >> struct smc_cdc_tx_pend *cdcpend = (struct smc_cdc_tx_pend *)pnd_snd; >> struct smc_connection *conn = cdcpend->conn; >> + struct smc_buf_desc *sndbuf_desc; >> struct smc_sock *smc; >> int diff; >> >> + sndbuf_desc = conn->sndbuf_desc; >> smc = container_of(conn, struct smc_sock, conn); >> bh_lock_sock(&smc->sk); >> - if (!wc_status) { >> - diff = smc_curs_diff(cdcpend->conn->sndbuf_desc->len, >> + if (!wc_status && sndbuf_desc) { >> + diff = smc_curs_diff(sndbuf_desc->len, >> &cdcpend->conn->tx_curs_fin, >> &cdcpend->cursor); >> /* sndbuf_space is decreased in smc_sendmsg */ >> @@ -114,9 +116,6 @@ int smc_cdc_msg_send(struct smc_connection *conn, >> union smc_host_cursor cfed; >> int rc; >> >> - if (unlikely(!READ_ONCE(conn->sndbuf_desc))) >> - return -ENOBUFS; >> - >> smc_cdc_add_pending_send(conn, pend); >> >> conn->tx_cdc_seq++; >> -- >> 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 3/3] net/smc: put sk reference if close work was canceled 2023-11-01 3:42 [PATCH net 0/3] bugfixs for smc D. Wythe 2023-11-01 3:42 ` [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT D. Wythe 2023-11-01 3:42 ` [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc D. Wythe @ 2023-11-01 3:42 ` D. Wythe 2 siblings, 0 replies; 8+ messages in thread From: D. Wythe @ 2023-11-01 3:42 UTC (permalink / raw) To: kgraul, wenjia, jaka, wintera; +Cc: kuba, davem, netdev, linux-s390, linux-rdma From: "D. Wythe" <alibuda@linux.alibaba.com> Note that we always hold a reference to sock when attempting to submit close_work. Therefore, if we have successfully canceled close_work from pending, we MUST release that reference to avoid potential leaks. Fixes: 42bfba9eaa33 ("net/smc: immediate termination for SMCD link groups") Signed-off-by: D. Wythe <alibuda@linux.alibaba.com> Reviewed-by: Dust Li <dust.li@linux.alibaba.com> Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com> --- net/smc/smc_close.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c index 449ef45..10219f5 100644 --- a/net/smc/smc_close.c +++ b/net/smc/smc_close.c @@ -116,7 +116,8 @@ static void smc_close_cancel_work(struct smc_sock *smc) struct sock *sk = &smc->sk; release_sock(sk); - cancel_work_sync(&smc->conn.close_work); + if (cancel_work_sync(&smc->conn.close_work)) + sock_put(sk); cancel_delayed_work_sync(&smc->conn.tx_work); lock_sock(sk); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-11-01 8:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-01 3:42 [PATCH net 0/3] bugfixs for smc D. Wythe 2023-11-01 3:42 ` [PATCH net 1/3] net/smc: fix dangling sock under state SMC_APPFINCLOSEWAIT D. Wythe 2023-11-01 8:13 ` Dust Li 2023-11-01 8:14 ` Dust Li 2023-11-01 3:42 ` [PATCH net 2/3] net/smc: allow cdc msg send rather than drop it with NULL sndbuf_desc D. Wythe 2023-11-01 8:19 ` Dust Li 2023-11-01 8:36 ` D. Wythe 2023-11-01 3:42 ` [PATCH net 3/3] net/smc: put sk reference if close work was canceled 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).