From nobody Sat Aug 19 10:47:25 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RSb6Q06b9z4qXyF; Sat, 19 Aug 2023 10:47:26 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RSb6P6YbLz3NYW; Sat, 19 Aug 2023 10:47:25 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1692442045; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=WZA1TPevvS8iJsE5z0dgG4HDz6QxJRfE91eqaNR+WDc=; b=rk/bPwJe9m5RaqDlGRqpr3EwROIUA4CKPo/sH2vIw9mjvk6ZLYpfhykaJjDHehneJEUZMu v/IgREn0i5YlVe2w1yTbE7Tn4zBdc5QGozhyuz1pdBCIo+699/NY8QyItV2MEjF7pUXWOM IEOJTN1UEnourGEvFXpdLyM7zyNu2W8ySiZQn21m8Xtnzq3ZCHkm4xTAL1XVpDGg8PXoO0 6evtyr54kp97U47Sd+fokn58RRhZ0lgF/DJqNhs0juE5wS/rWXT+ka9Fsz74tn6PQU+JAf QGwazOI7lYkHuFQiXkFFxt7PBXTjhmeN82aN1d/3h5mzdjrpGlc5011y1ETC8w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1692442045; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=WZA1TPevvS8iJsE5z0dgG4HDz6QxJRfE91eqaNR+WDc=; b=s4PcANVxxMGP480ghiF7msgr1rtj0dLBGxd8+pKhMykqDUPKXXzxmD+2g5e8rPxZRDdCyN bHJUNjsZilXa9ZNzlJXp/xBIApc2bcN8/3G8Nj70k8LmS///rOyISNNd+BwdPeCvCGRZ5r ETohaReH5o9+NOrBUsaVfCSnrOMfJRofO1YjzwzFNwcWgNti2Z3e3Aj9dc6URzrCI4dAF4 042bk2+mWgdIDORKDxouf+IEyk0a0RxkSKURGluzxp0HLAMCh93dpXivcGR7fcu4rwF0Dk 4SgNRCaMybUwLldY7qY0MiQ8z25D6nO3oTQexI8XdxqpcasIWEoAvUVfmkvE+w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1692442045; a=rsa-sha256; cv=none; b=Dkq5dxhSGh9qkpuw8TyMXnCnGsUiESYCONHdNtpVT50vF+51jMHfzVVd4AyYuTgnNaYLeN tbtggO6+YFobigggxG3XCinbSbFCBAOI5sgR92CvFGiSRGK5OH4kzVjJoNX11yKcPgC9Qv THHMd/8IMIsaEYikNEXIqLSqHLuemx8fcfFrhcQnbFMLO7l/uXVfa9vUje7wgSFlw/nkuM +zWp7q+ySEk4SOcS4MKmpK8oH+3KQ7LkNjrbGBxyoicu/GhM971EfBpGuMvK33vMGqSUpx A8YkaXZLS7tTp7RNHt1scyZK8AukLX4JEhxZ9sxO4+Ge3Rz042WdIUxK/YgACg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4RSb6P5MMbzvMG; Sat, 19 Aug 2023 10:47:25 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 37JAlPE8069696; Sat, 19 Aug 2023 10:47:25 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 37JAlPN4069693; Sat, 19 Aug 2023 10:47:25 GMT (envelope-from git) Date: Sat, 19 Aug 2023 10:47:25 GMT Message-Id: <202308191047.37JAlPN4069693@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Michael Tuexen Subject: git: 4f14d4b6b7f0 - main - sctp: cleanup handling of graceful shutdown of the peer List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: tuexen X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 4f14d4b6b7f0ca49b14379e48117121af3ed2669 Auto-Submitted: auto-generated The branch main has been updated by tuexen: URL: https://cgit.FreeBSD.org/src/commit/?id=4f14d4b6b7f0ca49b14379e48117121af3ed2669 commit 4f14d4b6b7f0ca49b14379e48117121af3ed2669 Author: Michael Tuexen AuthorDate: 2023-08-19 10:35:49 +0000 Commit: Michael Tuexen CommitDate: 2023-08-19 10:35:49 +0000 sctp: cleanup handling of graceful shutdown of the peer Don't handle a graceful shutdown of the peer as an implicit signal that all partial messages are complete. First, this is not implemented correctly and second this should not be done by the peer. It is more appropriate to handle this as a protocol violation. Remove the incorrect code and leave detecting the protocol violation and its handling in a followup commit. MFC after: 1 week --- sys/netinet/sctp_input.c | 63 +++++++++++----------------------------------- sys/netinet/sctp_pcb.c | 11 +------- sys/netinet/sctp_structs.h | 9 ------- 3 files changed, 16 insertions(+), 67 deletions(-) diff --git a/sys/netinet/sctp_input.c b/sys/netinet/sctp_input.c index 81b011b7e78a..059c6ded2e5e 100644 --- a/sys/netinet/sctp_input.c +++ b/sys/netinet/sctp_input.c @@ -837,8 +837,7 @@ sctp_handle_shutdown(struct sctp_shutdown_chunk *cp, int some_on_streamwheel; int old_state; - SCTPDBG(SCTP_DEBUG_INPUT2, - "sctp_handle_shutdown: handling SHUTDOWN\n"); + SCTPDBG(SCTP_DEBUG_INPUT2, "sctp_handle_shutdown: handling SHUTDOWN\n"); if (stcb == NULL) return; asoc = &stcb->asoc; @@ -855,40 +854,12 @@ sctp_handle_shutdown(struct sctp_shutdown_chunk *cp, if (*abort_flag) { return; } - if (asoc->control_pdapi) { - /* - * With a normal shutdown we assume the end of last record. - */ - SCTP_INP_READ_LOCK(stcb->sctp_ep); - if (asoc->control_pdapi->on_strm_q) { - struct sctp_stream_in *strm; - - strm = &asoc->strmin[asoc->control_pdapi->sinfo_stream]; - if (asoc->control_pdapi->on_strm_q == SCTP_ON_UNORDERED) { - /* Unordered */ - TAILQ_REMOVE(&strm->uno_inqueue, asoc->control_pdapi, next_instrm); - asoc->control_pdapi->on_strm_q = 0; - } else if (asoc->control_pdapi->on_strm_q == SCTP_ON_ORDERED) { - /* Ordered */ - TAILQ_REMOVE(&strm->inqueue, asoc->control_pdapi, next_instrm); - asoc->control_pdapi->on_strm_q = 0; -#ifdef INVARIANTS - } else { - panic("Unknown state on ctrl:%p on_strm_q:%d", - asoc->control_pdapi, - asoc->control_pdapi->on_strm_q); -#endif - } - } - asoc->control_pdapi->end_added = 1; - asoc->control_pdapi->pdapi_aborted = 1; - asoc->control_pdapi = NULL; - SCTP_INP_READ_UNLOCK(stcb->sctp_ep); - if (stcb->sctp_socket) { - sctp_sorwakeup(stcb->sctp_ep, stcb->sctp_socket); - } - } - /* goto SHUTDOWN_RECEIVED state to block new requests */ + /* + * FIXME MT: Handle the case where there are still incomplete + * received user messages or known missing user messages from the + * peer. One way to handle this is to abort the associations in this + * case. + */ if (stcb->sctp_socket) { if ((SCTP_GET_STATE(stcb) != SCTP_STATE_SHUTDOWN_RECEIVED) && (SCTP_GET_STATE(stcb) != SCTP_STATE_SHUTDOWN_ACK_SENT) && @@ -949,8 +920,9 @@ sctp_handle_shutdown_ack(struct sctp_shutdown_ack_chunk *cp SCTP_UNUSED, SCTPDBG(SCTP_DEBUG_INPUT2, "sctp_handle_shutdown_ack: handling SHUTDOWN ACK\n"); - if (stcb == NULL) + if (stcb == NULL) { return; + } asoc = &stcb->asoc; /* process according to association state */ @@ -967,17 +939,12 @@ sctp_handle_shutdown_ack(struct sctp_shutdown_ack_chunk *cp SCTP_UNUSED, SCTP_TCB_UNLOCK(stcb); return; } - if (asoc->control_pdapi) { - /* - * With a normal shutdown we assume the end of last record. - */ - SCTP_INP_READ_LOCK(stcb->sctp_ep); - asoc->control_pdapi->end_added = 1; - asoc->control_pdapi->pdapi_aborted = 1; - asoc->control_pdapi = NULL; - SCTP_INP_READ_UNLOCK(stcb->sctp_ep); - sctp_sorwakeup(stcb->sctp_ep, stcb->sctp_socket); - } + /* + * FIXME MT: Handle the case where there are still incomplete + * received user messages or known missing user messages from the + * peer. One way to handle this is to abort the associations in this + * case. + */ #ifdef INVARIANTS if (!TAILQ_EMPTY(&asoc->send_queue) || !TAILQ_EMPTY(&asoc->sent_queue) || diff --git a/sys/netinet/sctp_pcb.c b/sys/netinet/sctp_pcb.c index ac47b6aa1bfc..16cde18c4c1d 100644 --- a/sys/netinet/sctp_pcb.c +++ b/sys/netinet/sctp_pcb.c @@ -3405,7 +3405,6 @@ sctp_inpcb_free(struct sctp_inpcb *inp, int immediate, int from) continue; } if ((stcb->asoc.size_on_reasm_queue > 0) || - (stcb->asoc.control_pdapi) || (stcb->asoc.size_on_all_streams > 0) || ((so != NULL) && (SCTP_SBAVAIL(&so->so_rcv) > 0))) { /* Left with Data unread */ @@ -4762,18 +4761,10 @@ sctp_free_assoc(struct sctp_inpcb *inp, struct sctp_tcb *stcb, int from_inpcbfre * now. */ if (sq->end_added == 0) { - /* Held for PD-API clear that. */ + /* Held for PD-API, clear that. */ sq->pdapi_aborted = 1; sq->held_length = 0; if (sctp_stcb_is_feature_on(inp, stcb, SCTP_PCB_FLAGS_PDAPIEVNT) && (so != NULL)) { - /* - * Need to add a PD-API - * aborted indication. - * Setting the control_pdapi - * assures that it will be - * added right after this - * msg. - */ sctp_ulp_notify(SCTP_NOTIFY_PARTIAL_DELVIERY_INDICATION, stcb, SCTP_PARTIAL_DELIVERY_ABORTED, diff --git a/sys/netinet/sctp_structs.h b/sys/netinet/sctp_structs.h index 278afb2cc554..53d9bfd4f445 100644 --- a/sys/netinet/sctp_structs.h +++ b/sys/netinet/sctp_structs.h @@ -956,15 +956,6 @@ struct sctp_association { uint32_t fast_recovery_tsn; uint32_t sat_t3_recovery_tsn; uint32_t tsn_last_delivered; - /* - * For the pd-api we should re-write this a bit more efficient. We - * could have multiple sctp_queued_to_read's that we are building at - * once. Now we only do this when we get ready to deliver to the - * socket buffer. Note that we depend on the fact that the struct is - * "stuck" on the read queue until we finish all the pd-api. - */ - struct sctp_queued_to_read *control_pdapi; - uint32_t tsn_of_pdapi_last_delivered; uint32_t pdapi_ppid; uint32_t context;