mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-09-11 05:43:16 -04:00
net/rds: tcp: don't force RDS_CONN_RESETTING over a concurrent shutdown
rds_tcp_reset_callbacks() resolves a duelling SYN by storing RDS_CONN_RESETTING into cp_state unconditionally. Nothing serializes that store against the shutdown path: rds_tcp_accept_one() checks for RDS_CONN_CONNECTING or RDS_CONN_ERROR under t_conn_path_lock, but neither rds_conn_path_drop(), which forces RDS_CONN_ERROR, nor rds_conn_shutdown(), which moves the path to RDS_CONN_DISCONNECTING under cp_cm_lock, takes that lock. The store can therefore land on top of a shutdown that is already in progress, or that gets queued right after the accept-side check. When it does, the shutdown worker's final DISCONNECTING -> DOWN transition fails and the path goes through rds_conn_path_error() and a second drop/shutdown cycle instead of a clean reconnect, tearing down the socket the accept path has just installed. Before commitad22d24be6("net/rds: No shortcut out of RDS_CONN_ERROR") a path found in RDS_CONN_RESETTING even made rds_conn_shutdown() bail out altogether. Make the transition conditional: move CONNECTING -> RESETTING (or stay in RESETTING from an earlier duel), and drop the path in any other state. The drop has side effects of its own: it replaces the shutdown's RDS_CONN_DISCONNECTING (or RDS_CONN_ERROR) with RDS_CONN_ERROR and queues one more cp_down_w run. The difference is that rds_conn_shutdown() accepts RDS_CONN_ERROR in its final transition to RDS_CONN_DOWN, so the shutdown in flight completes normally instead of through rds_conn_path_error(); the extra down-work pass then finds the path already down and falls through to the reconnect check, or catches a reconnect that has already started and restarts it. The accept path still installs the new socket, rds_connect_path_complete() then fails its RESETTING -> UP transition and drops it: the raced socket ends up torn down as it does today. The comment at that call site, which promised that rds_connect_path_complete() marks the path RDS_CONN_UP, is updated to name this outcome as well. The state can change again between the failed transitions and the drop. That is inherent to rds_conn_path_drop(), which the socket state-change callbacks also call unconditionally, and costs at most one extra drop/reconnect cycle. Based on Oracle UEK commit "net/rds: Don't force state RDS_CONN_RESETTING" by Gerd Rausch. Fixes:9c79440e2c("RDS: TCP: fix race windows in send-path quiescence by rds_tcp_accept_one()") Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com> [achender: port to net-next: use the two-argument rds_conn_path_transition()/rds_conn_path_drop() and rewrite the changelog for the upstream shutdown path] Signed-off-by: Allison Henderson <achender@kernel.org> Link: https://patch.msgid.link/20260828223921.202913-5-achender@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
committed by
Jakub Kicinski
parent
103c4b13c4
commit
e8e60d74fe
@@ -150,9 +150,22 @@ void rds_tcp_reset_callbacks(struct socket *sock,
|
||||
* end up deadlocking with tcp_sendmsg(), and the RDS_IN_XMIT
|
||||
* would not get set. As a result, we set c_state to
|
||||
* RDS_CONN_RESETTTING, to ensure that rds_tcp_state_change
|
||||
* cannot mark rds_conn_path_up() in the window before lock_sock()
|
||||
* cannot mark rds_conn_path_up() in the window before lock_sock().
|
||||
*
|
||||
* Only make that transition if the path is still connecting
|
||||
* (or already resetting from an earlier duel). A path in any
|
||||
* other state - typically RDS_CONN_DISCONNECTING or
|
||||
* RDS_CONN_ERROR with a shutdown in flight - is dropped
|
||||
* instead. That still replaces its state, with RDS_CONN_ERROR,
|
||||
* and queues one more shutdown pass, but rds_conn_shutdown()
|
||||
* accepts RDS_CONN_ERROR in its final transition to
|
||||
* RDS_CONN_DOWN, so the shutdown in flight completes normally.
|
||||
*/
|
||||
atomic_set(&cp->cp_state, RDS_CONN_RESETTING);
|
||||
if (!rds_conn_path_transition(cp, RDS_CONN_CONNECTING,
|
||||
RDS_CONN_RESETTING) &&
|
||||
!rds_conn_path_transition(cp, RDS_CONN_RESETTING,
|
||||
RDS_CONN_RESETTING))
|
||||
rds_conn_path_drop(cp, 0);
|
||||
wait_event(cp->cp_waitq, !test_bit(RDS_IN_XMIT, &cp->cp_flags));
|
||||
/* reset receive side state for rds_tcp_data_recv() for osock */
|
||||
cancel_delayed_work_sync(&cp->cp_send_w);
|
||||
|
||||
@@ -295,7 +295,11 @@ int rds_tcp_accept_one(struct rds_tcp_net *rtn)
|
||||
if (rs_tcp->t_sock) {
|
||||
/* Duelling SYN has been handled in rds_tcp_accept_one() */
|
||||
rds_tcp_reset_callbacks(new_sock, cp);
|
||||
/* rds_connect_path_complete() marks RDS_CONN_UP */
|
||||
/* rds_connect_path_complete() marks RDS_CONN_UP, or,
|
||||
* if a concurrent shutdown won the duel, drops the
|
||||
* path again and the pass that drop queues reaps the
|
||||
* socket installed above.
|
||||
*/
|
||||
rds_connect_path_complete(cp, RDS_CONN_RESETTING);
|
||||
} else {
|
||||
rds_tcp_set_callbacks(new_sock, cp);
|
||||
|
||||
Reference in New Issue
Block a user