From 0cc7d64a2a37533afe03d2b640dc107be41b5f56 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Thu, 20 May 2021 09:59:11 -0700 Subject: [PATCH] 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;