Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 May 2021 16:59:40 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 0cc7d64a2a37 - main - iscsi: Move the maximum data segment limits into 'struct icl_conn'.
Message-ID:  <202105201659.14KGxeHr056475@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=0cc7d64a2a37533afe03d2b640dc107be41b5f56

commit 0cc7d64a2a37533afe03d2b640dc107be41b5f56
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2021-05-20 16:59:11 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2021-05-20 16:59:11 +0000

    iscsi: Move the maximum data segment limits into 'struct icl_conn'.
    
    This fixes a few bugs in iSCSI backends where the backends were using
    the limits they advertised initially during the login phase as the
    final values instead of the values negotiated with the other end.
    
    Reported by:    Jithesh Arakkan @ Chelsio
    Reviewed by:    mav
    Differential Revision:  https://reviews.freebsd.org/D30271
---
 sys/cam/ctl/ctl_frontend_iscsi.c  | 23 ++++++++++++--------
 sys/cam/ctl/ctl_frontend_iscsi.h  |  2 --
 sys/dev/cxgbe/cxgbei/icl_cxgbei.c | 23 +++-----------------
 sys/dev/iscsi/icl.h               |  3 ++-
 sys/dev/iscsi/icl_soft.c          |  9 +++++---
 sys/dev/iscsi/iscsi.c             | 44 +++++++++++++++++++--------------------
 sys/dev/iscsi/iscsi.h             |  2 --
 7 files changed, 47 insertions(+), 59 deletions(-)

diff --git a/sys/cam/ctl/ctl_frontend_iscsi.c b/sys/cam/ctl/ctl_frontend_iscsi.c
index a5a80848c763..b3cd8ab79d76 100644
--- a/sys/cam/ctl/ctl_frontend_iscsi.c
+++ b/sys/cam/ctl/ctl_frontend_iscsi.c
@@ -1568,8 +1568,10 @@ cfiscsi_ioctl_handoff(struct ctl_iscsi *ci)
 	 */
 	cs->cs_cmdsn = cihp->cmdsn;
 	cs->cs_statsn = cihp->statsn;
-	cs->cs_max_recv_data_segment_length = cihp->max_recv_data_segment_length;
-	cs->cs_max_send_data_segment_length = cihp->max_send_data_segment_length;
+	cs->cs_conn->ic_max_recv_data_segment_length =
+	    cihp->max_recv_data_segment_length;
+	cs->cs_conn->ic_max_send_data_segment_length =
+	    cihp->max_send_data_segment_length;
 	cs->cs_max_burst_length = cihp->max_burst_length;
 	cs->cs_first_burst_length = cihp->first_burst_length;
 	cs->cs_immediate_data = !!cihp->immediate_data;
@@ -1734,8 +1736,8 @@ cfiscsi_ioctl_list(struct ctl_iscsi *ci)
 		    cs->cs_target->ct_tag,
 		    cs->cs_conn->ic_header_crc32c ? "CRC32C" : "None",
 		    cs->cs_conn->ic_data_crc32c ? "CRC32C" : "None",
-		    cs->cs_max_recv_data_segment_length,
-		    cs->cs_max_send_data_segment_length,
+		    cs->cs_conn->ic_max_recv_data_segment_length,
+		    cs->cs_conn->ic_max_send_data_segment_length,
 		    cs->cs_max_burst_length,
 		    cs->cs_first_burst_length,
 		    cs->cs_immediate_data,
@@ -2534,12 +2536,14 @@ cfiscsi_datamove_in(union ctl_io *io)
 		/*
 		 * Truncate to maximum data segment length.
 		 */
-		KASSERT(response->ip_data_len < cs->cs_max_send_data_segment_length,
+		KASSERT(response->ip_data_len <
+		    cs->cs_conn->ic_max_send_data_segment_length,
 		    ("ip_data_len %zd >= max_send_data_segment_length %d",
-		    response->ip_data_len, cs->cs_max_send_data_segment_length));
+		    response->ip_data_len,
+		    cs->cs_conn->ic_max_send_data_segment_length));
 		if (response->ip_data_len + len >
-		    cs->cs_max_send_data_segment_length) {
-			len = cs->cs_max_send_data_segment_length -
+		    cs->cs_conn->ic_max_send_data_segment_length) {
+			len = cs->cs_conn->ic_max_send_data_segment_length -
 			    response->ip_data_len;
 			KASSERT(len <= sg_len, ("len %zd > sg_len %zd",
 			    len, sg_len));
@@ -2599,7 +2603,8 @@ cfiscsi_datamove_in(union ctl_io *io)
 			i++;
 		}
 
-		if (response->ip_data_len == cs->cs_max_send_data_segment_length) {
+		if (response->ip_data_len ==
+		    cs->cs_conn->ic_max_send_data_segment_length) {
 			/*
 			 * Can't stuff more data into the current PDU;
 			 * queue it.  Note that's not enough to check
diff --git a/sys/cam/ctl/ctl_frontend_iscsi.h b/sys/cam/ctl/ctl_frontend_iscsi.h
index a1c857231428..7c7f422a8d1f 100644
--- a/sys/cam/ctl/ctl_frontend_iscsi.h
+++ b/sys/cam/ctl/ctl_frontend_iscsi.h
@@ -86,8 +86,6 @@ struct cfiscsi_session {
 	bool				cs_terminating;
 	bool				cs_handoff_in_progress;
 	bool				cs_tasks_aborted;
-	int				cs_max_recv_data_segment_length;
-	int				cs_max_send_data_segment_length;
 	int				cs_max_burst_length;
 	int				cs_first_burst_length;
 	bool				cs_immediate_data;
diff --git a/sys/dev/cxgbe/cxgbei/icl_cxgbei.c b/sys/dev/cxgbe/cxgbei/icl_cxgbei.c
index fce593b54032..17d5685f1c1a 100644
--- a/sys/dev/cxgbe/cxgbei/icl_cxgbei.c
+++ b/sys/dev/cxgbe/cxgbei/icl_cxgbei.c
@@ -469,7 +469,7 @@ icl_cxgbei_conn_pdu_append_data(struct icl_conn *ic, struct icl_pdu *ip,
 		}
 		MPASS(len == 0);
 	}
-	MPASS(ip->ip_data_len <= ic->ic_max_data_segment_length);
+	MPASS(ip->ip_data_len <= ic->ic_max_send_data_segment_length);
 
 	return (0);
 }
@@ -565,8 +565,6 @@ icl_cxgbei_new_conn(const char *name, struct mtx *lock)
 #ifdef DIAGNOSTIC
 	refcount_init(&ic->ic_outstanding_pdus, 0);
 #endif
-	/* This is a stop-gap value that will be corrected during handoff. */
-	ic->ic_max_data_segment_length = 16384;
 	ic->ic_name = name;
 	ic->ic_offload = "cxgbei";
 	ic->ic_unmapped = false;
@@ -806,26 +804,11 @@ icl_cxgbei_conn_handoff(struct icl_conn *ic, int fd)
 		icc->toep = toep;
 		icc->cwt = cxgbei_select_worker_thread(icc);
 
-		/*
-		 * We maintain the _send_ DSL in this field just to have a
-		 * convenient way to assert that the kernel never sends
-		 * oversized PDUs.  This field is otherwise unused in the driver
-		 * or the kernel.
-		 */
-		ic->ic_max_data_segment_length = ci->max_tx_pdu_len -
-		    ISCSI_BHS_SIZE;
-
 		icc->ulp_submode = 0;
-		if (ic->ic_header_crc32c) {
+		if (ic->ic_header_crc32c)
 			icc->ulp_submode |= ULP_CRC_HEADER;
-			ic->ic_max_data_segment_length -=
-			    ISCSI_HEADER_DIGEST_SIZE;
-		}
-		if (ic->ic_data_crc32c) {
+		if (ic->ic_data_crc32c)
 			icc->ulp_submode |= ULP_CRC_DATA;
-			ic->ic_max_data_segment_length -=
-			    ISCSI_DATA_DIGEST_SIZE;
-		}
 		so->so_options |= SO_NO_DDP;
 		toep->params.ulp_mode = ULP_MODE_ISCSI;
 		toep->ulpcb = icc;
diff --git a/sys/dev/iscsi/icl.h b/sys/dev/iscsi/icl.h
index 0b897a50302a..94600c0edad1 100644
--- a/sys/dev/iscsi/icl.h
+++ b/sys/dev/iscsi/icl.h
@@ -109,7 +109,8 @@ struct icl_conn {
 	bool			ic_data_crc32c;
 	bool			ic_send_running;
 	bool			ic_receive_running;
-	size_t			ic_max_data_segment_length;
+	uint32_t		ic_max_recv_data_segment_length;
+	uint32_t		ic_max_send_data_segment_length;
 	size_t			ic_maxtags;
 	bool			ic_disconnecting;
 	bool			ic_iser;
diff --git a/sys/dev/iscsi/icl_soft.c b/sys/dev/iscsi/icl_soft.c
index 9cede6b44311..c15afbb59a68 100644
--- a/sys/dev/iscsi/icl_soft.c
+++ b/sys/dev/iscsi/icl_soft.c
@@ -573,7 +573,7 @@ icl_conn_receive_pdu(struct icl_conn *ic, struct mbuf **r, size_t *rs)
 		 */
 
 		len = icl_pdu_data_segment_length(request);
-		if (len > ic->ic_max_data_segment_length) {
+		if (len > ic->ic_max_recv_data_segment_length) {
 			ICL_WARN("received data segment "
 			    "length %zd is larger than negotiated; "
 			    "dropping connection", len);
@@ -1154,7 +1154,6 @@ icl_soft_new_conn(const char *name, struct mtx *lock)
 #ifdef DIAGNOSTIC
 	refcount_init(&ic->ic_outstanding_pdus, 0);
 #endif
-	ic->ic_max_data_segment_length = max_data_segment_length;
 	ic->ic_name = name;
 	ic->ic_offload = "None";
 	ic->ic_unmapped = false;
@@ -1205,13 +1204,17 @@ icl_conn_start(struct icl_conn *ic)
 	 * send a PDU in pieces; thus, the minimum buffer size is equal
 	 * to the maximum PDU size.  "+4" is to account for possible padding.
 	 */
-	minspace = sizeof(struct iscsi_bhs) + ic->ic_max_data_segment_length +
+	minspace = sizeof(struct iscsi_bhs) +
+	    ic->ic_max_send_data_segment_length +
 	    ISCSI_HEADER_DIGEST_SIZE + ISCSI_DATA_DIGEST_SIZE + 4;
 	if (sendspace < minspace) {
 		ICL_WARN("kern.icl.sendspace too low; must be at least %zd",
 		    minspace);
 		sendspace = minspace;
 	}
+	minspace = sizeof(struct iscsi_bhs) +
+	    ic->ic_max_recv_data_segment_length +
+	    ISCSI_HEADER_DIGEST_SIZE + ISCSI_DATA_DIGEST_SIZE + 4;
 	if (recvspace < minspace) {
 		ICL_WARN("kern.icl.recvspace too low; must be at least %zd",
 		    minspace);
diff --git a/sys/dev/iscsi/iscsi.c b/sys/dev/iscsi/iscsi.c
index 13a35c371c40..7ddb5a9ce1ec 100644
--- a/sys/dev/iscsi/iscsi.c
+++ b/sys/dev/iscsi/iscsi.c
@@ -1206,8 +1206,8 @@ iscsi_pdu_handle_r2t(struct icl_pdu *response)
 	for (;;) {
 		len = total_len;
 
-		if (len > is->is_max_send_data_segment_length)
-			len = is->is_max_send_data_segment_length;
+		if (len > is->is_conn->ic_max_send_data_segment_length)
+			len = is->is_conn->ic_max_send_data_segment_length;
 
 		if (off + len > csio->dxfer_len) {
 			ISCSI_SESSION_WARN(is, "target requested invalid "
@@ -1433,9 +1433,9 @@ iscsi_ioctl_daemon_handoff(struct iscsi_softc *sc,
 	is->is_initial_r2t = handoff->idh_initial_r2t;
 	is->is_immediate_data = handoff->idh_immediate_data;
 
-	is->is_max_recv_data_segment_length =
+	ic->ic_max_recv_data_segment_length =
 	    handoff->idh_max_recv_data_segment_length;
-	is->is_max_send_data_segment_length =
+	ic->ic_max_send_data_segment_length =
 	    handoff->idh_max_send_data_segment_length;
 	is->is_max_burst_length = handoff->idh_max_burst_length;
 	is->is_first_burst_length = handoff->idh_first_burst_length;
@@ -1648,7 +1648,7 @@ iscsi_ioctl_daemon_send(struct iscsi_softc *sc,
 		return (EIO);
 
 	datalen = ids->ids_data_segment_len;
-	if (datalen > is->is_max_send_data_segment_length)
+	if (datalen > is->is_conn->ic_max_send_data_segment_length)
 		return (EINVAL);
 	if (datalen > 0) {
 		data = malloc(datalen, M_ISCSI, M_WAITOK);
@@ -1793,18 +1793,6 @@ iscsi_ioctl_session_add(struct iscsi_softc *sc, struct iscsi_session_add *isa)
 	is = malloc(sizeof(*is), M_ISCSI, M_ZERO | M_WAITOK);
 	memcpy(&is->is_conf, &isa->isa_conf, sizeof(is->is_conf));
 
-	/*
-	 * Set some default values, from RFC 3720, section 12.
-	 *
-	 * These values are updated by the handoff IOCTL, but are
-	 * needed prior to the handoff to support sending the ISER
-	 * login PDU.
-	 */
-	is->is_max_recv_data_segment_length = 8192;
-	is->is_max_send_data_segment_length = 8192;
-	is->is_max_burst_length = 262144;
-	is->is_first_burst_length = 65536;
-
 	sx_xlock(&sc->sc_lock);
 
 	/*
@@ -1847,6 +1835,18 @@ iscsi_ioctl_session_add(struct iscsi_softc *sc, struct iscsi_session_add *isa)
 	cv_init(&is->is_login_cv, "iscsi_login");
 #endif
 
+	/*
+	 * Set some default values, from RFC 3720, section 12.
+	 *
+	 * These values are updated by the handoff IOCTL, but are
+	 * needed prior to the handoff to support sending the ISER
+	 * login PDU.
+	 */
+	is->is_conn->ic_max_recv_data_segment_length = 8192;
+	is->is_conn->ic_max_send_data_segment_length = 8192;
+	is->is_max_burst_length = 262144;
+	is->is_first_burst_length = 65536;
+
 	is->is_softc = sc;
 	sc->sc_last_session_id++;
 	is->is_id = sc->sc_last_session_id;
@@ -1960,9 +1960,9 @@ iscsi_ioctl_session_list(struct iscsi_softc *sc, struct iscsi_session_list *isl)
 			iss.iss_data_digest = ISCSI_DIGEST_NONE;
 
 		iss.iss_max_send_data_segment_length =
-		    is->is_max_send_data_segment_length;
+		    is->is_conn->ic_max_send_data_segment_length;
 		iss.iss_max_recv_data_segment_length =
-		    is->is_max_recv_data_segment_length;
+		    is->is_conn->ic_max_recv_data_segment_length;
 		iss.iss_max_burst_length = is->is_max_burst_length;
 		iss.iss_first_burst_length = is->is_first_burst_length;
 		iss.iss_immediate_data = is->is_immediate_data;
@@ -2330,10 +2330,10 @@ iscsi_action_scsiio(struct iscsi_session *is, union ccb *ccb)
 			ISCSI_SESSION_DEBUG(is, "len %zd -> %d", len, is->is_first_burst_length);
 			len = is->is_first_burst_length;
 		}
-		if (len > is->is_max_send_data_segment_length) {
+		if (len > is->is_conn->ic_max_send_data_segment_length) {
 			ISCSI_SESSION_DEBUG(is, "len %zd -> %d", len,
-			    is->is_max_send_data_segment_length);
-			len = is->is_max_send_data_segment_length;
+			    is->is_conn->ic_max_send_data_segment_length);
+			len = is->is_conn->ic_max_send_data_segment_length;
 		}
 
 		error = icl_pdu_append_data(request, csio->data_ptr, len, M_NOWAIT);
diff --git a/sys/dev/iscsi/iscsi.h b/sys/dev/iscsi/iscsi.h
index 793b7529c7c0..fe1cc64f88db 100644
--- a/sys/dev/iscsi/iscsi.h
+++ b/sys/dev/iscsi/iscsi.h
@@ -67,8 +67,6 @@ struct iscsi_session {
 	uint8_t				is_isid[6];
 	uint16_t			is_tsih;
 	bool				is_immediate_data;
-	int				is_max_recv_data_segment_length;
-	int				is_max_send_data_segment_length;
 	char				is_target_alias[ISCSI_ALIAS_LEN];
 
 	TAILQ_HEAD(, iscsi_outstanding)	is_outstanding;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105201659.14KGxeHr056475>