From owner-svn-src-projects@freebsd.org Sat May 9 01:18:43 2020 Return-Path: Delivered-To: svn-src-projects@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 50EAB2D27E1 for ; Sat, 9 May 2020 01:18:43 +0000 (UTC) (envelope-from rmacklem@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49Jq8g1Wqlz3yrV; Sat, 9 May 2020 01:18:43 +0000 (UTC) (envelope-from rmacklem@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 2FA1DB8D7; Sat, 9 May 2020 01:18:43 +0000 (UTC) (envelope-from rmacklem@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0491IhSd099558; Sat, 9 May 2020 01:18:43 GMT (envelope-from rmacklem@FreeBSD.org) Received: (from rmacklem@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0491Ih2W099557; Sat, 9 May 2020 01:18:43 GMT (envelope-from rmacklem@FreeBSD.org) Message-Id: <202005090118.0491Ih2W099557@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: rmacklem set sender to rmacklem@FreeBSD.org using -f From: Rick Macklem Date: Sat, 9 May 2020 01:18:43 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r360830 - projects/nfs-over-tls/sys/rpc X-SVN-Group: projects X-SVN-Commit-Author: rmacklem X-SVN-Commit-Paths: projects/nfs-over-tls/sys/rpc X-SVN-Commit-Revision: 360830 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.32 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 May 2020 01:18:43 -0000 Author: rmacklem Date: Sat May 9 01:18:42 2020 New Revision: 360830 URL: https://svnweb.freebsd.org/changeset/base/360830 Log: Rewrite clnt_vc_soupcall() so that it does soreceive() for an entire TLS record (or all data available for TCP without TLS), similar to what soreceive() in svc_vc.c does. This should avoid problems if/when an NFS RPC message record mark straddles TLS records and reduces the number of soreceive() calls done by the client. It also makes adding code to handle the TLS control records simpler. This is next on my todo list. Modified: projects/nfs-over-tls/sys/rpc/clnt_vc.c Modified: projects/nfs-over-tls/sys/rpc/clnt_vc.c ============================================================================== --- projects/nfs-over-tls/sys/rpc/clnt_vc.c Fri May 8 23:00:02 2020 (r360829) +++ projects/nfs-over-tls/sys/rpc/clnt_vc.c Sat May 9 01:18:42 2020 (r360830) @@ -274,6 +274,7 @@ clnt_vc_create( soupcall_set(ct->ct_socket, SO_RCV, clnt_vc_soupcall, ct); SOCKBUF_UNLOCK(&ct->ct_socket->so_rcv); + ct->ct_raw = NULL; ct->ct_record = NULL; ct->ct_record_resid = 0; TAILQ_INIT(&ct->ct_pending); @@ -921,9 +922,9 @@ clnt_vc_soupcall(struct socket *so, void *arg, int wai struct ct_request *cr; int error, rcvflag, foundreq; uint32_t xid_plus_direction[2], header; - bool_t do_read; SVCXPRT *xprt; struct cf_conn *cd; + u_int rawlen; CTASSERT(sizeof(xid_plus_direction) == 2 * sizeof(uint32_t)); @@ -935,117 +936,125 @@ clnt_vc_soupcall(struct socket *so, void *arg, int wai } mtx_unlock(&ct->ct_lock); + /* + * If another thread is already here, it must be in + * soreceive(), so just return. + * ct_upcallrefs is protected by the SOCKBUF_LOCK(), + * which is held in this function, except when + * soreceive() is called. + */ + if (ct->ct_upcallrefs > 0) + return (SU_OK); ct->ct_upcallrefs++; - uio.uio_td = curthread; - do { - /* - * If ct_record_resid is zero, we are waiting for a - * record mark. - */ - if (ct->ct_record_resid == 0) { + /* + * Read as much as possible off the socket and link it + * onto ct_raw. + */ + for (;;) { + uio.uio_resid = 1000000000; + uio.uio_td = curthread; + m = NULL; + rcvflag = MSG_DONTWAIT | MSG_SOCALLBCK; + SOCKBUF_UNLOCK(&so->so_rcv); + error = soreceive(so, NULL, &uio, &m, NULL, &rcvflag); + SOCKBUF_LOCK(&so->so_rcv); + + if (error == EWOULDBLOCK) { /* - * Make sure there is either a whole record - * mark in the buffer or there is some other - * error condition + * We must re-test for readability after + * taking the lock to protect us in the case + * where a new packet arrives on the socket + * after our call to soreceive fails with + * EWOULDBLOCK. */ - do_read = FALSE; - if (sbavail(&so->so_rcv) >= sizeof(uint32_t) - || (so->so_rcv.sb_state & SBS_CANTRCVMORE) - || so->so_error) - do_read = TRUE; - - if (!do_read) + if (!soreadable(so)) break; - - SOCKBUF_UNLOCK(&so->so_rcv); - uio.uio_resid = sizeof(uint32_t); - m = NULL; - rcvflag = MSG_DONTWAIT | MSG_SOCALLBCK; - error = soreceive(so, NULL, &uio, &m, NULL, &rcvflag); - SOCKBUF_LOCK(&so->so_rcv); - - if (error == EWOULDBLOCK) - break; - + continue; + } + if (error == 0 && m == NULL) { /* - * If there was an error, wake up all pending - * requests. + * We must have got EOF trying + * to read from the stream. */ - if (error || uio.uio_resid > 0) { - wakeup_all: - mtx_lock(&ct->ct_lock); - if (!error) { - /* - * We must have got EOF trying - * to read from the stream. - */ - error = ECONNRESET; - } - ct->ct_error.re_status = RPC_CANTRECV; - ct->ct_error.re_errno = error; - TAILQ_FOREACH(cr, &ct->ct_pending, cr_link) { - cr->cr_error = error; - wakeup(cr); - } - mtx_unlock(&ct->ct_lock); - break; + error = ECONNRESET; + } + if (error != 0) { + mtx_lock(&ct->ct_lock); + ct->ct_error.re_status = RPC_CANTRECV; + ct->ct_error.re_errno = error; + TAILQ_FOREACH(cr, &ct->ct_pending, cr_link) { + cr->cr_error = error; + wakeup(cr); } - m_copydata(m, 0, sizeof(uint32_t), (char *)&header); + mtx_unlock(&ct->ct_lock); + goto out; + } + + if (ct->ct_raw != NULL) + m_last(ct->ct_raw)->m_next = m; + else + ct->ct_raw = m; + } + rawlen = m_length(ct->ct_raw, NULL); + + /* Now, process as much of ct_raw as possible. */ + for (;;) { + /* + * If ct_record_resid is zero, we are waiting for a + * record mark. + */ + if (ct->ct_record_resid == 0) { + if (rawlen < sizeof(uint32_t)) + break; + m_copydata(ct->ct_raw, 0, sizeof(uint32_t), + (char *)&header); header = ntohl(header); ct->ct_record = NULL; ct->ct_record_resid = header & 0x7fffffff; ct->ct_record_eor = ((header & 0x80000000) != 0); - m_freem(m); +if (ct->ct_record_resid < 20 || ct->ct_record_resid > 70000 || !ct->ct_record_eor) +printf("EEK!! recres=%zd eor=%d\n", ct->ct_record_resid, ct->ct_record_eor); + m_adj(ct->ct_raw, sizeof(uint32_t)); + rawlen -= sizeof(uint32_t); } else { /* - * Wait until the socket has the whole record - * buffered. + * Move as much of the record as possible to + * ct_record. */ - do_read = FALSE; - if (sbavail(&so->so_rcv) >= ct->ct_record_resid - || (so->so_rcv.sb_state & SBS_CANTRCVMORE) - || so->so_error) - do_read = TRUE; - - if (!do_read) + if (rawlen == 0) break; - - /* - * We have the record mark. Read as much as - * the socket has buffered up to the end of - * this record. - */ - SOCKBUF_UNLOCK(&so->so_rcv); - uio.uio_resid = ct->ct_record_resid; - m = NULL; - rcvflag = MSG_DONTWAIT | MSG_SOCALLBCK; - error = soreceive(so, NULL, &uio, &m, NULL, &rcvflag); - SOCKBUF_LOCK(&so->so_rcv); - - if (error == EWOULDBLOCK) + if (rawlen <= ct->ct_record_resid) { + if (ct->ct_record != NULL) + m_last(ct->ct_record)->m_next = + ct->ct_raw; + else + ct->ct_record = ct->ct_raw; + ct->ct_raw = NULL; + ct->ct_record_resid -= rawlen; + rawlen = 0; + } else { + m = m_split(ct->ct_raw, ct->ct_record_resid, + M_NOWAIT); + if (m == NULL) + break; + if (ct->ct_record != NULL) + m_last(ct->ct_record)->m_next = + ct->ct_raw; + else + ct->ct_record = ct->ct_raw; + rawlen -= ct->ct_record_resid; + ct->ct_record_resid = 0; + ct->ct_raw = m; + } + if (ct->ct_record_resid > 0) break; - if (error || uio.uio_resid == ct->ct_record_resid) - goto wakeup_all; - /* - * If we have part of the record already, - * chain this bit onto the end. - */ - if (ct->ct_record) - m_last(ct->ct_record)->m_next = m; - else - ct->ct_record = m; - - ct->ct_record_resid = uio.uio_resid; - - /* * If we have the entire record, see if we can * match it to a request. */ - if (ct->ct_record_resid == 0 - && ct->ct_record_eor) { + if (ct->ct_record_eor) { /* * The XID is in the first uint32_t of * the reply and the message direction @@ -1067,11 +1076,9 @@ clnt_vc_soupcall(struct socket *so, void *arg, int wai ntohl(xid_plus_direction[1]); /* Check message direction. */ if (xid_plus_direction[1] == CALL) { -printf("Got backchannel callback\n"); /* This is a backchannel request. */ mtx_lock(&ct->ct_lock); xprt = ct->ct_backchannelxprt; -printf("backxprt=%p\n", xprt); if (xprt == NULL) { mtx_unlock(&ct->ct_lock); /* Just throw it away. */ @@ -1135,7 +1142,8 @@ printf("backxprt=%p\n", xprt); } } } - } while (m); + } +out: ct->ct_upcallrefs--; if (ct->ct_upcallrefs < 0) panic("rpcvc upcall refcnt");