From owner-freebsd-bugs@FreeBSD.ORG Sat Mar 14 11:40:01 2009 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1E9A6106566C for ; Sat, 14 Mar 2009 11:40:01 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id EC2BC8FC17 for ; Sat, 14 Mar 2009 11:40:00 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n2EBe026082392 for ; Sat, 14 Mar 2009 11:40:00 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n2EBe07f082391; Sat, 14 Mar 2009 11:40:00 GMT (envelope-from gnats) Resent-Date: Sat, 14 Mar 2009 11:40:00 GMT Resent-Message-Id: <200903141140.n2EBe07f082391@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Patrick Lamaiziere Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8413D106564A for ; Sat, 14 Mar 2009 11:33:31 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 669338FC16 for ; Sat, 14 Mar 2009 11:33:31 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id n2EBXUQA028709 for ; Sat, 14 Mar 2009 11:33:30 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id n2EBXUOb028708; Sat, 14 Mar 2009 11:33:30 GMT (envelope-from nobody) Message-Id: <200903141133.n2EBXUOb028708@www.freebsd.org> Date: Sat, 14 Mar 2009 11:33:30 GMT From: Patrick Lamaiziere To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: kern/132622: [patch] glxsb(4) performs badly with ipsec X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Mar 2009 11:40:01 -0000 >Number: 132622 >Category: kern >Synopsis: [patch] glxsb(4) performs badly with ipsec >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Mar 14 11:40:00 UTC 2009 >Closed-Date: >Last-Modified: >Originator: Patrick Lamaiziere >Release: 8.0-CURRENT/i386 >Organization: >Environment: FreeBSD malpractice.lamaiziere.net 8.0-CURRENT FreeBSD 8.0-CURRENT #0: Mon Mar 9 21:25:42 CET 2009 patrick@malpractice.lamaiziere.net:/usr/obj/misc/src/sys/NET5501 i386 >Description: The glxsb(4) driver performs badly when using it with ipsec. With an ipsec tunnel using aes-128-cbc encryption and no hmac authentication the throughput is around 15 Mbits. The cryptosoft driver performs better! The problem is that glxsb(4) processes only one encryption request at a time. When it is busy, it blocks the Open Crypto Framework (OCF) and unblocks it when the previous request is completed. Then the OCF has to wake up and to resubmit the crypto request. This performs very badly with ipsec (tests show that there are a number of block/unblock occuring). The attached patch makes glxsb to not block the OCF, instead it queues the crypto requests into the driver. This performs a lot of better, the throughput is around 50 Mbits with ipsec. Another solution could be to perform the crypto request synchronously in the function glxsb_crypto_process() (ie without the use of a taskqueue). Let me know if you think that will be better, I will submit another patch. >How-To-Repeat: 3 machines: PC1 <---(ipsec)----> Soekris box (glxsb) <-------> PC2 PC1: 192.168.1.20 Soekris: 192.168.1.200 and 192.168.2.200 PC2: 192.168.2.97 Netperf TCP tests between PC1 and PC2 setkey file on PC1 flush; spdflush; add 192.168.1.20 192.168.1.200 esp 1011 -E rijndael-cbc "0123456789012345"; add 192.168.1.200 192.168.1.20 esp 1012 -E rijndael-cbc "0123456789012345"; spdadd 192.168.2.0/24 192.168.1.20 any -P in ipsec esp/tunnel/192.168.1.200-192.168.1.20/require; spdadd 192.168.1.20 192.168.2.0/24 any -P out ipsec esp/tunnel/192.168.1.20-192.168.1.200/require; setkey file on the Soekris flush; spdflush; add 192.168.1.20 192.168.1.200 esp 1011 -E rijndael-cbc "0123456789012345"; add 192.168.1.200 192.168.1.20 esp 1012 -E rijndael-cbc "0123456789012345"; spdadd 192.168.2.0/24 192.168.1.20 any -P out ipsec esp/tunnel/192.168.1.200-192.168.1.20 /require; spdadd 192.168.1.20 192.168.2.0/24 any -P in ipsec esp/tunnel/192.168.1.20-192.168.1.200/require; >Fix: See the attached patch Patch attached with submission follows: --- /sys/dev/glxsb/glxsb.c 2008-11-17 08:09:40.000000000 +0100 +++ glxsb.c 2009-03-14 11:06:48.000000000 +0100 @@ -40,10 +40,11 @@ __FBSDID("$FreeBSD: src/sys/dev/glxsb/gl #include #include #include #include #include +#include #include #include #include @@ -172,10 +173,11 @@ struct glxsb_dma_map { struct glxsb_taskop { struct glxsb_session *to_ses; /* crypto session */ struct cryptop *to_crp; /* cryptop to perfom */ struct cryptodesc *to_enccrd; /* enccrd to perform */ struct cryptodesc *to_maccrd; /* maccrd to perform */ + TAILQ_ENTRY(glxsb_taskop) to_next; }; struct glxsb_softc { device_t sc_dev; /* device backpointer */ struct resource *sc_sr; /* resource */ @@ -190,12 +192,13 @@ struct glxsb_softc { sc_sessions; /* crypto sessions */ struct rwlock sc_sessions_lock;/* sessions lock */ struct mtx sc_task_mtx; /* task mutex */ struct taskqueue *sc_tq; /* task queue */ struct task sc_cryptotask; /* task */ - struct glxsb_taskop sc_to; /* task's crypto operation */ - int sc_task_count; /* tasks count */ + TAILQ_HEAD(to_head, glxsb_taskop) + sc_taskops; /* tasks operations */ + uma_zone_t sc_taskops_zone;/* tasks operations' zone */ }; static int glxsb_probe(device_t); static int glxsb_attach(device_t); static int glxsb_detach(device_t); @@ -347,17 +350,25 @@ static int glxsb_detach(device_t dev) { struct glxsb_softc *sc = device_get_softc(dev); struct glxsb_session *ses; + /* do not detach if there are pending tasks */ + if (!mtx_trylock(&sc->sc_task_mtx)) + goto busy; + if (!TAILQ_EMPTY(&sc->sc_taskops)) { + mtx_unlock(&sc->sc_task_mtx); + goto busy; + } + mtx_unlock(&sc->sc_task_mtx); + + /* do not detach if there are active sessions */ rw_wlock(&sc->sc_sessions_lock); TAILQ_FOREACH(ses, &sc->sc_sessions, ses_next) { if (ses->ses_used) { rw_wunlock(&sc->sc_sessions_lock); - device_printf(dev, - "cannot detach, sessions still active.\n"); - return (EBUSY); + goto busy; } } while (!TAILQ_EMPTY(&sc->sc_sessions)) { ses = TAILQ_FIRST(&sc->sc_sessions); TAILQ_REMOVE(&sc->sc_sessions, ses, ses_next); @@ -371,11 +382,17 @@ glxsb_detach(device_t dev) glxsb_dma_free(sc, &sc->sc_dma); bus_release_resource(dev, SYS_RES_MEMORY, sc->sc_rid, sc->sc_sr); taskqueue_free(sc->sc_tq); rw_destroy(&sc->sc_sessions_lock); mtx_destroy(&sc->sc_task_mtx); + uma_zdestroy(sc->sc_taskops_zone); return (0); + +busy: + device_printf(dev, + "cannot detach, sessions still active.\n"); + return (EBUSY); } /* * callback for bus_dmamap_load() */ @@ -493,13 +510,21 @@ glxsb_crypto_setup(struct glxsb_softc *s return (ENOMEM); } TAILQ_INIT(&sc->sc_sessions); sc->sc_sid = 1; + TAILQ_INIT(&sc->sc_taskops); + sc->sc_taskops_zone = uma_zcreate("glxsb_taskop", + sizeof(struct glxsb_taskop), 0, 0, 0, 0, + UMA_ALIGN_PTR, UMA_ZONE_ZINIT); rw_init(&sc->sc_sessions_lock, "glxsb_sessions_lock"); mtx_init(&sc->sc_task_mtx, "glxsb_crypto_mtx", NULL, MTX_DEF); + if (sc->sc_taskops_zone == NULL) { + device_printf(sc->sc_dev, "cannot setup taskops zone\n"); + goto crypto_fail; + } if (crypto_register(sc->sc_cid, CRYPTO_AES_CBC, 0, 0) != 0) goto crypto_fail; if (crypto_register(sc->sc_cid, CRYPTO_NULL_HMAC, 0, 0) != 0) goto crypto_fail; if (crypto_register(sc->sc_cid, CRYPTO_MD5_HMAC, 0, 0) != 0) @@ -520,10 +545,11 @@ glxsb_crypto_setup(struct glxsb_softc *s crypto_fail: device_printf(sc->sc_dev, "cannot register crypto\n"); crypto_unregister_all(sc->sc_cid); rw_destroy(&sc->sc_sessions_lock); mtx_destroy(&sc->sc_task_mtx); + uma_zdestroy(sc->sc_taskops_zone); return (ENOMEM); } static int glxsb_crypto_newsession(device_t dev, uint32_t *sidp, struct cryptoini *cri) @@ -818,53 +844,65 @@ glxsb_crypto_encdec(struct cryptop *crp, static void glxsb_crypto_task(void *arg, int pending) { struct glxsb_softc *sc = arg; + struct glxsb_taskop *taskop; struct glxsb_session *ses; struct cryptop *crp; struct cryptodesc *enccrd, *maccrd; int error; - maccrd = sc->sc_to.to_maccrd; - enccrd = sc->sc_to.to_enccrd; - crp = sc->sc_to.to_crp; - ses = sc->sc_to.to_ses; - - /* Perform data authentication if requested before encryption */ - if (maccrd != NULL && maccrd->crd_next == enccrd) { - error = glxsb_hash_process(ses, maccrd, crp); - if (error != 0) - goto out; - } - - error = glxsb_crypto_encdec(crp, enccrd, ses, sc); - if (error != 0) - goto out; + for (; pending > 0; pending--) { + /* pop crypto request */ + mtx_lock(&sc->sc_task_mtx); + taskop = TAILQ_FIRST(&sc->sc_taskops); + if (taskop != NULL) { + TAILQ_REMOVE(&sc->sc_taskops, taskop, to_next); + mtx_unlock(&sc->sc_task_mtx); + } else { + /* should not happen */ + mtx_unlock(&sc->sc_task_mtx); + continue; + } + maccrd = taskop->to_maccrd; + enccrd = taskop->to_enccrd; + crp = taskop->to_crp; + ses = taskop->to_ses; + uma_zfree(sc->sc_taskops_zone, taskop); + + /* Perform data authentication if requested before encryption */ + if (maccrd != NULL && maccrd->crd_next == enccrd) { + error = glxsb_hash_process(ses, maccrd, crp); + if (error != 0) + goto crypto_out; + } - /* Perform data authentication if requested after encryption */ - if (maccrd != NULL && enccrd->crd_next == maccrd) { - error = glxsb_hash_process(ses, maccrd, crp); + error = glxsb_crypto_encdec(crp, enccrd, ses, sc); if (error != 0) - goto out; - } -out: - mtx_lock(&sc->sc_task_mtx); - sc->sc_task_count--; - mtx_unlock(&sc->sc_task_mtx); + goto crypto_out; - crp->crp_etype = error; - crypto_unblock(sc->sc_cid, CRYPTO_SYMQ); - crypto_done(crp); + /* Perform data authentication if requested after encryption */ + if (maccrd != NULL && enccrd->crd_next == maccrd) { + error = glxsb_hash_process(ses, maccrd, crp); + if (error != 0) + goto crypto_out; + } + +crypto_out: + crp->crp_etype = error; + crypto_done(crp); + } /* for */ } static int glxsb_crypto_process(device_t dev, struct cryptop *crp, int hint) { struct glxsb_softc *sc = device_get_softc(dev); struct glxsb_session *ses; struct cryptodesc *crd, *enccrd, *maccrd; + struct glxsb_taskop *taskop; uint32_t sid; int error = 0; enccrd = maccrd = NULL; @@ -920,23 +958,24 @@ glxsb_crypto_process(device_t dev, struc if (ses == NULL || !ses->ses_used) { error = EINVAL; goto fail; } + /* queue the crypto request */ mtx_lock(&sc->sc_task_mtx); - if (sc->sc_task_count != 0) { + taskop = uma_zalloc(sc->sc_taskops_zone, M_NOWAIT); + if (taskop == NULL) { mtx_unlock(&sc->sc_task_mtx); - return (ERESTART); + error = ENOMEM; + goto fail; } - sc->sc_task_count++; - - sc->sc_to.to_maccrd = maccrd; - sc->sc_to.to_enccrd = enccrd; - sc->sc_to.to_crp = crp; - sc->sc_to.to_ses = ses; + taskop->to_maccrd = maccrd; + taskop->to_enccrd = enccrd; + taskop->to_crp = crp; + taskop->to_ses = ses; + TAILQ_INSERT_TAIL(&sc->sc_taskops, taskop, to_next); mtx_unlock(&sc->sc_task_mtx); - taskqueue_enqueue(sc->sc_tq, &sc->sc_cryptotask); return(0); fail: crp->crp_etype = error; >Release-Note: >Audit-Trail: >Unformatted: