Date: Sat, 14 Mar 2009 11:33:30 GMT From: Patrick Lamaiziere <patfbsd@davenulle.org> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/132622: [patch] glxsb(4) performs badly with ipsec Message-ID: <200903141133.n2EBXUOb028708@www.freebsd.org> Resent-Message-ID: <200903141140.n2EBe07f082391@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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 <sys/random.h>
#include <sys/rman.h>
#include <sys/rwlock.h>
#include <sys/sysctl.h>
#include <sys/taskqueue.h>
+#include <vm/uma.h>
#include <machine/bus.h>
#include <machine/cpufunc.h>
#include <machine/resource.h>
@@ -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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903141133.n2EBXUOb028708>