svn commit: r313731 - head/sys/dev/iscsi

[Alexander Motin]

Author: mav
Date: Tue Feb 14 16:33:42 2017
New Revision: 313731
URL: https://svnweb.freebsd.org/changeset/base/313731

Log:
  Do not rely on data alignment after m_pullup().
  
  In general case m_pullup() does not really guarantee any data alignment.
  Instead of depenting on side effects caused by data being always copied
  out of mbuf cluster (which is probably a bug by itself), always allocate
  aligned BHS buffer and read data there directly from socket.
  
  While there, reuse new icl_conn_receive_buf() function to read digests.
  The code could probably be even more optimized to aggregate those reads,
  but until that done, this is still easier then the way it was before.
  
  MFC after:	2 weeks

Modified:
  head/sys/dev/iscsi/icl_soft.c

Modified: head/sys/dev/iscsi/icl_soft.c
==============================================================================
--- head/sys/dev/iscsi/icl_soft.c	Tue Feb 14 13:54:05 2017	(r313730)
+++ head/sys/dev/iscsi/icl_soft.c	Tue Feb 14 16:33:42 2017	(r313731)
@@ -169,6 +169,40 @@ icl_conn_receive(struct icl_conn *ic, si
 	return (m);
 }
 
+static int
+icl_conn_receive_buf(struct icl_conn *ic, void *buf, size_t len)
+{
+	struct iovec iov[1];
+	struct uio uio;
+	struct socket *so;
+	int error, flags;
+
+	so = ic->ic_socket;
+
+	memset(&uio, 0, sizeof(uio));
+	iov[0].iov_base = buf;
+	iov[0].iov_len = len;
+	uio.uio_iov = iov;
+	uio.uio_iovcnt = 1;
+	uio.uio_offset = 0;
+	uio.uio_resid = len;
+	uio.uio_segflg = UIO_SYSSPACE;
+	uio.uio_rw = UIO_READ;
+
+	flags = MSG_DONTWAIT;
+	error = soreceive(so, NULL, &uio, NULL, NULL, &flags);
+	if (error != 0) {
+		ICL_DEBUG("soreceive error %d", error);
+		return (-1);
+	}
+	if (uio.uio_resid != 0) {
+		ICL_DEBUG("short read");
+		return (-1);
+	}
+
+	return (0);
+}
+
 static struct icl_pdu *
 icl_pdu_new_empty(struct icl_conn *ic, int flags)
 {
@@ -229,7 +263,7 @@ icl_soft_conn_new_pdu(struct icl_conn *i
 	ip->ip_bhs_mbuf = m_getm2(NULL, sizeof(struct iscsi_bhs),
 	    flags, MT_DATA, M_PKTHDR);
 	if (ip->ip_bhs_mbuf == NULL) {
-		ICL_WARN("failed to allocate %zd bytes", sizeof(*ip));
+		ICL_WARN("failed to allocate BHS mbuf");
 		icl_pdu_free(ip);
 		return (NULL);
 	}
@@ -308,28 +342,13 @@ icl_pdu_size(const struct icl_pdu *respo
 static int
 icl_pdu_receive_bhs(struct icl_pdu *request, size_t *availablep)
 {
-	struct mbuf *m;
 
-	m = icl_conn_receive(request->ip_conn, sizeof(struct iscsi_bhs));
-	if (m == NULL) {
+	if (icl_conn_receive_buf(request->ip_conn,
+	    request->ip_bhs, sizeof(struct iscsi_bhs))) {
 		ICL_DEBUG("failed to receive BHS");
 		return (-1);
 	}
 
-	request->ip_bhs_mbuf = m_pullup(m, sizeof(struct iscsi_bhs));
-	if (request->ip_bhs_mbuf == NULL) {
-		ICL_WARN("m_pullup failed");
-		return (-1);
-	}
-	request->ip_bhs = mtod(request->ip_bhs_mbuf, struct iscsi_bhs *);
-
-	/*
-	 * XXX: For architectures with strict alignment requirements
-	 * 	we may need to allocate ip_bhs and copy the data into it.
-	 * 	For some reason, though, not doing this doesn't seem
-	 * 	to cause problems; tested on sparc64.
-	 */
-
 	*availablep -= sizeof(struct iscsi_bhs);
 	return (0);
 }
@@ -371,22 +390,17 @@ icl_mbuf_to_crc32c(const struct mbuf *m0
 static int
 icl_pdu_check_header_digest(struct icl_pdu *request, size_t *availablep)
 {
-	struct mbuf *m;
 	uint32_t received_digest, valid_digest;
 
 	if (request->ip_conn->ic_header_crc32c == false)
 		return (0);
 
-	m = icl_conn_receive(request->ip_conn, ISCSI_HEADER_DIGEST_SIZE);
-	if (m == NULL) {
+	CTASSERT(sizeof(received_digest) == ISCSI_HEADER_DIGEST_SIZE);
+	if (icl_conn_receive_buf(request->ip_conn,
+	    &received_digest, ISCSI_HEADER_DIGEST_SIZE)) {
 		ICL_DEBUG("failed to receive header digest");
 		return (-1);
 	}
-
-	CTASSERT(sizeof(received_digest) == ISCSI_HEADER_DIGEST_SIZE);
-	m_copydata(m, 0, ISCSI_HEADER_DIGEST_SIZE, (void *)&received_digest);
-	m_freem(m);
-
 	*availablep -= ISCSI_HEADER_DIGEST_SIZE;
 
 	/*
@@ -526,7 +540,6 @@ icl_pdu_receive_data_segment(struct icl_
 static int
 icl_pdu_check_data_digest(struct icl_pdu *request, size_t *availablep)
 {
-	struct mbuf *m;
 	uint32_t received_digest, valid_digest;
 
 	if (request->ip_conn->ic_data_crc32c == false)
@@ -535,16 +548,12 @@ icl_pdu_check_data_digest(struct icl_pdu
 	if (request->ip_data_len == 0)
 		return (0);
 
-	m = icl_conn_receive(request->ip_conn, ISCSI_DATA_DIGEST_SIZE);
-	if (m == NULL) {
+	CTASSERT(sizeof(received_digest) == ISCSI_DATA_DIGEST_SIZE);
+	if (icl_conn_receive_buf(request->ip_conn,
+	    &received_digest, ISCSI_DATA_DIGEST_SIZE)) {
 		ICL_DEBUG("failed to receive data digest");
 		return (-1);
 	}
-
-	CTASSERT(sizeof(received_digest) == ISCSI_DATA_DIGEST_SIZE);
-	m_copydata(m, 0, ISCSI_DATA_DIGEST_SIZE, (void *)&received_digest);
-	m_freem(m);
-
 	*availablep -= ISCSI_DATA_DIGEST_SIZE;
 
 	/*
@@ -580,7 +589,7 @@ icl_conn_receive_pdu(struct icl_conn *ic
 	if (ic->ic_receive_state == ICL_CONN_STATE_BHS) {
 		KASSERT(ic->ic_receive_pdu == NULL,
 		    ("ic->ic_receive_pdu != NULL"));
-		request = icl_pdu_new_empty(ic, M_NOWAIT);
+		request = icl_soft_conn_new_pdu(ic, M_NOWAIT);
 		if (request == NULL) {
 			ICL_DEBUG("failed to allocate PDU; "
 			    "dropping connection");