Date: Fri, 29 Apr 2022 23:12:23 GMT From: John Baldwin <jhb@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 1d290d36d82f - stable/13 - iscsi: Handle large Text responses. Message-ID: <202204292312.23TNCNRf044594@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=1d290d36d82fbc937c153c533fa6151a0f836f13 commit 1d290d36d82fbc937c153c533fa6151a0f836f13 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2021-12-29 22:36:04 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-04-29 21:17:43 +0000 iscsi: Handle large Text responses. Text requests and responses can span multiple PDUs. In that case, the sender sets the Continue bit in non-final PDUs and the Final bit in the last PDU. The receiver responds to non-final PDUs with an empty text PDU. To support this, add a more abstract API in libiscsi which accepts and receives key sets rather than PDUs. These routines internally send or receive one or more PDUs. Use these new functions to replace the handling of TextRequest and TextResponse PDUs in discovery sessions in both ctld and iscsid. Note that there is not currently a use case for large Text requests and those are still always sent as a single PDU. However, discovery sessions can return a text response listing targets that spans multiple PDUs, so the new API supports sending and receiving multi-PDU responses. Reported by: Jithesh Arakkan @ Chelsio Reviewed by: mav Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D33548 (cherry picked from commit b406897911ea6c18c401907a076c4642dac46127) --- lib/libiscsiutil/Makefile | 2 +- lib/libiscsiutil/libiscsiutil.h | 8 + lib/libiscsiutil/text.c | 333 ++++++++++++++++++++++++++++++++++++++++ usr.sbin/ctld/discovery.c | 76 +-------- usr.sbin/iscsid/discovery.c | 63 +------- 5 files changed, 348 insertions(+), 134 deletions(-) diff --git a/lib/libiscsiutil/Makefile b/lib/libiscsiutil/Makefile index 9ec625970eae..c99fb6366536 100644 --- a/lib/libiscsiutil/Makefile +++ b/lib/libiscsiutil/Makefile @@ -4,7 +4,7 @@ PACKAGE= iscsi INCS= libiscsiutil.h -SRCS= chap.c connection.c keys.c log.c pdu.c utils.c +SRCS= chap.c connection.c keys.c log.c pdu.c text.c utils.c CFLAGS+= -I${SRCTOP}/sys/dev/iscsi .include <bsd.lib.mk> diff --git a/lib/libiscsiutil/libiscsiutil.h b/lib/libiscsiutil/libiscsiutil.h index 59682a7f6c74..97d699c6c4b9 100644 --- a/lib/libiscsiutil/libiscsiutil.h +++ b/lib/libiscsiutil/libiscsiutil.h @@ -141,6 +141,14 @@ void pdu_receive(struct pdu *request); void pdu_send(struct pdu *response); void pdu_delete(struct pdu *ip); +void text_send_request(struct connection *conn, + struct keys *request_keys); +struct keys * text_read_response(struct connection *conn); +struct keys * text_read_request(struct connection *conn, + struct pdu **requestp); +void text_send_response(struct pdu *request, + struct keys *response_keys); + void connection_init(struct connection *conn, const struct connection_ops *ops, bool use_proxy); diff --git a/lib/libiscsiutil/text.c b/lib/libiscsiutil/text.c new file mode 100644 index 000000000000..f8b88e4f70be --- /dev/null +++ b/lib/libiscsiutil/text.c @@ -0,0 +1,333 @@ +/*- + * SPDX-License-Identifier: BSD-2-Clause-FreeBSD + * + * Copyright (c) 2012 The FreeBSD Foundation + * + * This software was developed by Edward Tomasz Napierala under sponsorship + * from the FreeBSD Foundation. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <sys/types.h> +#include <netinet/in.h> + +#include <stdlib.h> +#include <string.h> + +#include <iscsi_proto.h> +#include "libiscsiutil.h" + +/* Construct a new TextRequest PDU. */ +static struct pdu * +text_new_request(struct connection *conn, uint32_t ttt) +{ + struct pdu *request; + struct iscsi_bhs_text_request *bhstr; + + request = pdu_new(conn); + bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; + bhstr->bhstr_opcode = ISCSI_BHS_OPCODE_TEXT_REQUEST | + ISCSI_BHS_OPCODE_IMMEDIATE; + bhstr->bhstr_flags = BHSTR_FLAGS_FINAL; + bhstr->bhstr_initiator_task_tag = 0; + bhstr->bhstr_target_transfer_tag = ttt; + + bhstr->bhstr_cmdsn = conn->conn_cmdsn; + bhstr->bhstr_expstatsn = htonl(conn->conn_statsn + 1); + + return (request); +} + +/* Receive a TextRequest PDU from a connection. */ +static struct pdu * +text_receive_request(struct connection *conn) +{ + struct pdu *request; + struct iscsi_bhs_text_request *bhstr; + + request = pdu_new(conn); + pdu_receive(request); + if ((request->pdu_bhs->bhs_opcode & ~ISCSI_BHS_OPCODE_IMMEDIATE) != + ISCSI_BHS_OPCODE_TEXT_REQUEST) + log_errx(1, "protocol error: received invalid opcode 0x%x", + request->pdu_bhs->bhs_opcode); + bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; + + /* + * XXX: Implement the C flag some day. + */ + if ((bhstr->bhstr_flags & (BHSTR_FLAGS_FINAL | BHSTR_FLAGS_CONTINUE)) != + BHSTR_FLAGS_FINAL) + log_errx(1, "received TextRequest PDU with invalid " + "flags: %u", bhstr->bhstr_flags); + if (ISCSI_SNLT(ntohl(bhstr->bhstr_cmdsn), conn->conn_cmdsn)) { + log_errx(1, "received TextRequest PDU with decreasing CmdSN: " + "was %u, is %u", conn->conn_cmdsn, ntohl(bhstr->bhstr_cmdsn)); + } + conn->conn_cmdsn = ntohl(bhstr->bhstr_cmdsn); + if ((bhstr->bhstr_opcode & ISCSI_BHS_OPCODE_IMMEDIATE) == 0) + conn->conn_cmdsn++; + + return (request); +} + +/* Construct a new TextResponse PDU in reply to a request. */ +static struct pdu * +text_new_response(struct pdu *request, uint32_t ttt, bool final) +{ + struct pdu *response; + struct connection *conn; + struct iscsi_bhs_text_request *bhstr; + struct iscsi_bhs_text_response *bhstr2; + + bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; + conn = request->pdu_connection; + + response = pdu_new_response(request); + bhstr2 = (struct iscsi_bhs_text_response *)response->pdu_bhs; + bhstr2->bhstr_opcode = ISCSI_BHS_OPCODE_TEXT_RESPONSE; + if (final) + bhstr2->bhstr_flags = BHSTR_FLAGS_FINAL; + else + bhstr2->bhstr_flags = BHSTR_FLAGS_CONTINUE; + bhstr2->bhstr_lun = bhstr->bhstr_lun; + bhstr2->bhstr_initiator_task_tag = bhstr->bhstr_initiator_task_tag; + bhstr2->bhstr_target_transfer_tag = ttt; + bhstr2->bhstr_statsn = htonl(conn->conn_statsn++); + bhstr2->bhstr_expcmdsn = htonl(conn->conn_cmdsn); + bhstr2->bhstr_maxcmdsn = htonl(conn->conn_cmdsn); + + return (response); +} + +/* Receive a TextResponse PDU from a connection. */ +static struct pdu * +text_receive_response(struct connection *conn) +{ + struct pdu *response; + struct iscsi_bhs_text_response *bhstr; + uint8_t flags; + + response = pdu_new(conn); + pdu_receive(response); + if (response->pdu_bhs->bhs_opcode != ISCSI_BHS_OPCODE_TEXT_RESPONSE) + log_errx(1, "protocol error: received invalid opcode 0x%x", + response->pdu_bhs->bhs_opcode); + bhstr = (struct iscsi_bhs_text_response *)response->pdu_bhs; + flags = bhstr->bhstr_flags & (BHSTR_FLAGS_FINAL | BHSTR_FLAGS_CONTINUE); + switch (flags) { + case BHSTR_FLAGS_CONTINUE: + if (bhstr->bhstr_target_transfer_tag == 0xffffffff) + log_errx(1, "received continue TextResponse PDU with " + "invalid TTT 0x%x", + bhstr->bhstr_target_transfer_tag); + break; + case BHSTR_FLAGS_FINAL: + if (bhstr->bhstr_target_transfer_tag != 0xffffffff) + log_errx(1, "received final TextResponse PDU with " + "invalid TTT 0x%x", + bhstr->bhstr_target_transfer_tag); + break; + default: + log_errx(1, "received TextResponse PDU with invalid " + "flags: %u", bhstr->bhstr_flags); + } + if (ntohl(bhstr->bhstr_statsn) != conn->conn_statsn + 1) { + log_errx(1, "received TextResponse PDU with wrong StatSN: " + "is %u, should be %u", ntohl(bhstr->bhstr_statsn), + conn->conn_statsn + 1); + } + conn->conn_statsn = ntohl(bhstr->bhstr_statsn); + + return (response); +} + +/* + * Send a list of keys from the initiator to the target in a + * TextRequest PDU. + */ +void +text_send_request(struct connection *conn, struct keys *request_keys) +{ + struct pdu *request; + + request = text_new_request(conn, 0xffffffff); + keys_save_pdu(request_keys, request); + if (request->pdu_data_len == 0) + log_errx(1, "No keys to send in a TextRequest"); + if (request->pdu_data_len > + (size_t)conn->conn_max_send_data_segment_length) + log_errx(1, "Keys to send in TextRequest are too long"); + + pdu_send(request); + pdu_delete(request); +} + +/* + * Read a list of keys from the target in a series of TextResponse + * PDUs. + */ +struct keys * +text_read_response(struct connection *conn) +{ + struct keys *response_keys; + char *keys_data; + size_t keys_len; + uint32_t ttt; + + keys_data = NULL; + keys_len = 0; + ttt = 0xffffffff; + for (;;) { + struct pdu *request, *response; + struct iscsi_bhs_text_response *bhstr; + + response = text_receive_response(conn); + bhstr = (struct iscsi_bhs_text_response *)response->pdu_bhs; + if (keys_data == NULL) { + ttt = bhstr->bhstr_target_transfer_tag; + keys_data = response->pdu_data; + keys_len = response->pdu_data_len; + response->pdu_data = NULL; + } else { + keys_data = realloc(keys_data, + keys_len + response->pdu_data_len); + if (keys_data == NULL) + log_err(1, "failed to grow keys block"); + memcpy(keys_data + keys_len, response->pdu_data, + response->pdu_data_len); + keys_len += response->pdu_data_len; + } + if ((bhstr->bhstr_flags & BHSTR_FLAGS_FINAL) != 0) { + pdu_delete(response); + break; + } + if (bhstr->bhstr_target_transfer_tag != ttt) + log_errx(1, "received non-final TextRequest PDU with " + "invalid TTT 0x%x", + bhstr->bhstr_target_transfer_tag); + pdu_delete(response); + + /* Send an empty request. */ + request = text_new_request(conn, ttt); + pdu_send(request); + pdu_delete(request); + } + + response_keys = keys_new(); + keys_load(response_keys, keys_data, keys_len); + free(keys_data); + return (response_keys); +} + +/* + * Read a list of keys from the initiator in a TextRequest PDU. + */ +struct keys * +text_read_request(struct connection *conn, struct pdu **requestp) +{ + struct iscsi_bhs_text_request *bhstr; + struct pdu *request; + struct keys *request_keys; + + request = text_receive_request(conn); + bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; + if (bhstr->bhstr_target_transfer_tag != 0xffffffff) + log_errx(1, "received TextRequest PDU with invalid TTT 0x%x", + bhstr->bhstr_target_transfer_tag); + if (ntohl(bhstr->bhstr_expstatsn) != conn->conn_statsn) { + log_errx(1, "received TextRequest PDU with wrong ExpStatSN: " + "is %u, should be %u", ntohl(bhstr->bhstr_expstatsn), + conn->conn_statsn); + } + + request_keys = keys_new(); + keys_load_pdu(request_keys, request); + *requestp = request; + return (request_keys); +} + +/* + * Send a response back to the initiator as a series of TextResponse + * PDUs. + */ +void +text_send_response(struct pdu *request, struct keys *response_keys) +{ + struct connection *conn = request->pdu_connection; + char *keys_data; + size_t keys_len; + size_t keys_offset; + uint32_t ttt; + + keys_save(response_keys, &keys_data, &keys_len); + keys_offset = 0; + ttt = keys_len; + for (;;) { + struct pdu *request2, *response; + struct iscsi_bhs_text_request *bhstr; + size_t todo; + bool final; + + todo = keys_len - keys_offset; + if (todo > (size_t)conn->conn_max_send_data_segment_length) { + final = false; + todo = conn->conn_max_send_data_segment_length; + } else { + final = true; + ttt = 0xffffffff; + } + + response = text_new_response(request, ttt, final); + response->pdu_data = keys_data + keys_offset; + response->pdu_data_len = todo; + keys_offset += todo; + + pdu_send(response); + response->pdu_data = NULL; + pdu_delete(response); + + if (final) + break; + + /* + * Wait for an empty request. + * + * XXX: Linux's Open-iSCSI initiator doesn't update + * ExpStatSN when receiving a TextResponse PDU. + */ + request2 = text_receive_request(conn); + bhstr = (struct iscsi_bhs_text_request *)request2->pdu_bhs; + if ((bhstr->bhstr_flags & BHSTR_FLAGS_FINAL) == 0) + log_errx(1, "received continuation TextRequest PDU " + "without F set"); + if (pdu_data_segment_length(request2) != 0) + log_errx(1, "received non-empty continuation " + "TextRequest PDU"); + if (bhstr->bhstr_target_transfer_tag != ttt) + log_errx(1, "received TextRequest PDU with invalid " + "TTT 0x%x", bhstr->bhstr_target_transfer_tag); + pdu_delete(request2); + } + free(keys_data); +} diff --git a/usr.sbin/ctld/discovery.c b/usr.sbin/ctld/discovery.c index 244b08a7f63c..3a713c1afac0 100644 --- a/usr.sbin/ctld/discovery.c +++ b/usr.sbin/ctld/discovery.c @@ -43,69 +43,6 @@ __FBSDID("$FreeBSD$"); #include "ctld.h" #include "iscsi_proto.h" -static struct pdu * -text_receive(struct connection *conn) -{ - struct pdu *request; - struct iscsi_bhs_text_request *bhstr; - - request = pdu_new(conn); - pdu_receive(request); - if ((request->pdu_bhs->bhs_opcode & ~ISCSI_BHS_OPCODE_IMMEDIATE) != - ISCSI_BHS_OPCODE_TEXT_REQUEST) - log_errx(1, "protocol error: received invalid opcode 0x%x", - request->pdu_bhs->bhs_opcode); - bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; -#if 0 - if ((bhstr->bhstr_flags & ISCSI_BHSTR_FLAGS_FINAL) == 0) - log_errx(1, "received Text PDU without the \"F\" flag"); -#endif - /* - * XXX: Implement the C flag some day. - */ - if ((bhstr->bhstr_flags & BHSTR_FLAGS_CONTINUE) != 0) - log_errx(1, "received Text PDU with unsupported \"C\" flag"); - if (ISCSI_SNLT(ntohl(bhstr->bhstr_cmdsn), conn->conn_cmdsn)) { - log_errx(1, "received Text PDU with decreasing CmdSN: " - "was %u, is %u", conn->conn_cmdsn, ntohl(bhstr->bhstr_cmdsn)); - } - if (ntohl(bhstr->bhstr_expstatsn) != conn->conn_statsn) { - log_errx(1, "received Text PDU with wrong ExpStatSN: " - "is %u, should be %u", ntohl(bhstr->bhstr_expstatsn), - conn->conn_statsn); - } - conn->conn_cmdsn = ntohl(bhstr->bhstr_cmdsn); - if ((bhstr->bhstr_opcode & ISCSI_BHS_OPCODE_IMMEDIATE) == 0) - conn->conn_cmdsn++; - - return (request); -} - -static struct pdu * -text_new_response(struct pdu *request) -{ - struct pdu *response; - struct connection *conn; - struct iscsi_bhs_text_request *bhstr; - struct iscsi_bhs_text_response *bhstr2; - - bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; - conn = request->pdu_connection; - - response = pdu_new_response(request); - bhstr2 = (struct iscsi_bhs_text_response *)response->pdu_bhs; - bhstr2->bhstr_opcode = ISCSI_BHS_OPCODE_TEXT_RESPONSE; - bhstr2->bhstr_flags = BHSTR_FLAGS_FINAL; - bhstr2->bhstr_lun = bhstr->bhstr_lun; - bhstr2->bhstr_initiator_task_tag = bhstr->bhstr_initiator_task_tag; - bhstr2->bhstr_target_transfer_tag = bhstr->bhstr_target_transfer_tag; - bhstr2->bhstr_statsn = htonl(conn->conn_statsn++); - bhstr2->bhstr_expcmdsn = htonl(conn->conn_cmdsn); - bhstr2->bhstr_maxcmdsn = htonl(conn->conn_cmdsn); - - return (response); -} - static struct pdu * logout_receive(struct connection *conn) { @@ -284,16 +221,13 @@ discovery(struct ctld_connection *conn) pg = conn->conn_portal->p_portal_group; - log_debugx("beginning discovery session; waiting for Text PDU"); - request = text_receive(&conn->conn); - request_keys = keys_new(); - keys_load_pdu(request_keys, request); + log_debugx("beginning discovery session; waiting for TextRequest PDU"); + request_keys = text_read_request(&conn->conn, &request); send_targets = keys_find(request_keys, "SendTargets"); if (send_targets == NULL) - log_errx(1, "received Text PDU without SendTargets"); + log_errx(1, "received TextRequest PDU without SendTargets"); - response = text_new_response(request); response_keys = keys_new(); if (strcmp(send_targets, "All") == 0) { @@ -317,10 +251,8 @@ discovery(struct ctld_connection *conn) } } } - keys_save_pdu(response_keys, response); - pdu_send(response); - pdu_delete(response); + text_send_response(request, response_keys); keys_delete(response_keys); pdu_delete(request); keys_delete(request_keys); diff --git a/usr.sbin/iscsid/discovery.c b/usr.sbin/iscsid/discovery.c index 60aff28c1b03..942d4a4bc408 100644 --- a/usr.sbin/iscsid/discovery.c +++ b/usr.sbin/iscsid/discovery.c @@ -41,58 +41,6 @@ __FBSDID("$FreeBSD$"); #include "iscsid.h" #include "iscsi_proto.h" -static struct pdu * -text_receive(struct connection *conn) -{ - struct pdu *response; - struct iscsi_bhs_text_response *bhstr; - - response = pdu_new(conn); - pdu_receive(response); - if (response->pdu_bhs->bhs_opcode != ISCSI_BHS_OPCODE_TEXT_RESPONSE) - log_errx(1, "protocol error: received invalid opcode 0x%x", - response->pdu_bhs->bhs_opcode); - bhstr = (struct iscsi_bhs_text_response *)response->pdu_bhs; -#if 0 - if ((bhstr->bhstr_flags & BHSTR_FLAGS_FINAL) == 0) - log_errx(1, "received Text PDU without the \"F\" flag"); -#endif - /* - * XXX: Implement the C flag some day. - */ - if ((bhstr->bhstr_flags & BHSTR_FLAGS_CONTINUE) != 0) - log_errx(1, "received Text PDU with unsupported \"C\" flag"); - if (ntohl(bhstr->bhstr_statsn) != conn->conn_statsn + 1) { - log_errx(1, "received Text PDU with wrong StatSN: " - "is %u, should be %u", ntohl(bhstr->bhstr_statsn), - conn->conn_statsn + 1); - } - conn->conn_statsn = ntohl(bhstr->bhstr_statsn); - - return (response); -} - -static struct pdu * -text_new_request(struct connection *conn) -{ - struct pdu *request; - struct iscsi_bhs_text_request *bhstr; - - request = pdu_new(conn); - bhstr = (struct iscsi_bhs_text_request *)request->pdu_bhs; - bhstr->bhstr_opcode = ISCSI_BHS_OPCODE_TEXT_REQUEST | - ISCSI_BHS_OPCODE_IMMEDIATE; - bhstr->bhstr_flags = BHSTR_FLAGS_FINAL; - bhstr->bhstr_initiator_task_tag = 0; - bhstr->bhstr_target_transfer_tag = 0xffffffff; - - bhstr->bhstr_initiator_task_tag = 0; /* XXX */ - bhstr->bhstr_cmdsn = 0; /* XXX */ - bhstr->bhstr_expstatsn = htonl(conn->conn_statsn + 1); - - return (request); -} - static struct pdu * logout_receive(struct connection *conn) { @@ -174,20 +122,14 @@ discovery(struct iscsid_connection *conn) int i; log_debugx("beginning discovery session"); - request = text_new_request(&conn->conn); request_keys = keys_new(); keys_add(request_keys, "SendTargets", "All"); - keys_save_pdu(request_keys, request); + text_send_request(&conn->conn, request_keys); keys_delete(request_keys); request_keys = NULL; - pdu_send(request); - pdu_delete(request); - request = NULL; log_debugx("waiting for Text Response"); - response = text_receive(&conn->conn); - response_keys = keys_new(); - keys_load_pdu(response_keys, response); + response_keys = text_read_response(&conn->conn); for (i = 0; i < KEYS_MAX; i++) { if (response_keys->keys_names[i] == NULL) break; @@ -202,7 +144,6 @@ discovery(struct iscsid_connection *conn) kernel_add(conn, response_keys->keys_values[i]); } keys_delete(response_keys); - pdu_delete(response); log_debugx("removing temporary discovery session"); kernel_remove(conn);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202204292312.23TNCNRf044594>