IPsec parallelization

RFC PATCH In review Steffen Klassert 4 participants 37 messages Dec 1, 2008 – Dec 2, 2008

Subsystems (7)

Networking Network drivers Crypto API Arm STM32 +3 more subsystems

lore

Share & tools

lore

Steffen Klassert

author maintainer

2008-12-01 07:48 UTC #0 lore raw

cover IPsec parallelization

This is a first throw to try to parallelize the expensive part of xfrm by using a generic parallelization/serialization method. This method uses the remote softirq invocation infrastructure for parallelization and serialization. With this method data objects can be processed in parallel, starting at some given point. After doing some expensive operations in parallel, it is possible to serialize again. The parallelized data objects return after serialization in the order as they were before the parallelization. In the case of xfrm, this makes it possible to run the expensive part in parallel without getting packet reordering.

To use this parallelization method for xfrm, some changes in the crypto system were necessary. First of all, we need to force disabling async crypto transforms in the parallelization case, because we can't guarantee the packet order if the packets are put to a queue during the parallel processing.
A second thing was a very high contended lock in crypto_authenc_hash() if the crypto system runs in parallel. To get rid of this, the struct aead is moved to percpu data, what in turn means that we have percpu IV chains now. However, I'm not that familiar with the crypto system. So I'm not sure whether this is acceptable as I did it, this needs review.

I did forwarding tests with two quad core machines (Intel Core 2 Quad Q6600) used as IPsec routers (xfrm tunnel between the two quad core machines) and two notebooks T61 used as traffic generators.
With this testing environment I'm geting a throughput up to 910 Mbit/s (ipv4) and 880 Mbit/s (ipv6) with aes192-sha1 encryption (measured with iperf, _one_ tcp stream). Without the parallelization I'm getting with the same environment about 340 Mbit/s (ipv4) and 320 Mbit/s (ipv6).

If somebody wants to test it, the parallelization is switched off by default. To enable it, do 'echo 1 > /proc/sys/net/core/xfrm_padata'.

Steffen

Steffen Klassert

author maintainer

2008-12-01 07:48 UTC #1 lore raw

RFC PATCH 1/5 padata: generic interface for parallel processing

From: Steffen Klassert <[email protected]>

This patch introduces an interface to process data objects in parallel. On request it is possible to serialize again. The parallelized objects return in the same order as they were before the parallelization.

Signed-off-by Steffen Klassert <[email protected]>

---
 include/linux/interrupt.h |    1 +
 include/linux/padata.h    |  115 +++++++++++
 kernel/Makefile           |    2 +-
 kernel/padata.c           |  489 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 606 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/padata.h
 create mode 100644 kernel/padata.c

include/linux/interrupt.h +1 -0

index f58a0cf..4d2f4bb 100644
@@ -254,6 +254,7 @@ enum
 #ifdef CONFIG_HIGH_RES_TIMERS
 	HRTIMER_SOFTIRQ,
 #endif
+	PADATA_SOFTIRQ,
 	RCU_SOFTIRQ, 	/* Preferable RCU should always be the last softirq */
 
 	NR_SOFTIRQS

include/linux/padata.h new file +115 -0

115 lines not on this page · raw message

kernel/Makefile +1 -1

index 19fad00..730a401 100644
@@ -40,7 +40,7 @@ obj-$(CONFIG_RT_MUTEXES) += rtmutex.o
 obj-$(CONFIG_DEBUG_RT_MUTEXES) += rtmutex-debug.o
 obj-$(CONFIG_RT_MUTEX_TESTER) += rtmutex-tester.o
 obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o
-obj-$(CONFIG_USE_GENERIC_SMP_HELPERS) += smp.o
+obj-$(CONFIG_USE_GENERIC_SMP_HELPERS) += smp.o padata.o
 obj-$(CONFIG_SMP) += spinlock.o
 obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
 obj-$(CONFIG_PROVE_LOCKING) += spinlock.o

kernel/padata.c new file +489 -0

489 lines not on this page · raw message

Signature · 1 line

1.5.4.2

Steffen Klassert

author maintainer

2008-12-01 07:48 UTC #2 lore raw

RFC PATCH 2/5 xfrm: add possibility for parallel processing

In reply to Steffen Klassert

From: Steffen Klassert <[email protected]>

This patch uses the padata parallelization interface to run the expensive parts of xfrm in parallel.

Signed-off-by Steffen Klassert <[email protected]>

---
 include/linux/crypto.h     |    1 +
 include/linux/interrupt.h  |    2 +
 include/linux/padata.h     |    2 +
 include/linux/skbuff.h     |    5 +
 include/linux/sysctl.h     |    3 +-
 include/net/xfrm.h         |   38 ++++++
 kernel/sysctl_check.c      |    1 +
 net/core/skbuff.c          |    3 +
 net/core/sysctl_net_core.c |   11 ++
 net/xfrm/Kconfig           |    8 ++
 net/xfrm/Makefile          |    2 +-
 net/xfrm/xfrm_input.c      |    7 +-
 net/xfrm/xfrm_output.c     |    5 +
 net/xfrm/xfrm_padata.c     |  270 ++++++++++++++++++++++++++++++++++++++++++++
 net/xfrm/xfrm_policy.c     |    2 +
 15 files changed, 357 insertions(+), 3 deletions(-)
 create mode 100644 net/xfrm/xfrm_padata.c

include/linux/crypto.h +1 -0

index 3d2317e..d5dd094 100644
@@ -78,6 +78,7 @@
 #define CRYPTO_TFM_REQ_WEAK_KEY		0x00000100
 #define CRYPTO_TFM_REQ_MAY_SLEEP	0x00000200
 #define CRYPTO_TFM_REQ_MAY_BACKLOG	0x00000400
+#define CRYPTO_TFM_REQ_FORCE_SYNC	0x00000800
 #define CRYPTO_TFM_RES_WEAK_KEY		0x00100000
 #define CRYPTO_TFM_RES_BAD_KEY_LEN   	0x00200000
 #define CRYPTO_TFM_RES_BAD_KEY_SCHED 	0x00400000

include/linux/interrupt.h +2 -0

index 4d2f4bb..02b7fba 100644
@@ -248,6 +248,8 @@ enum
 	TIMER_SOFTIRQ,
 	NET_TX_SOFTIRQ,
 	NET_RX_SOFTIRQ,
+	XFRM_INPUT_SOFTIRQ,
+	XFRM_OUTPUT_SOFTIRQ,
 	BLOCK_SOFTIRQ,
 	TASKLET_SOFTIRQ,
 	SCHED_SOFTIRQ,

include/linux/padata.h +2 -0

index 6447c93..786ec44 100644
@@ -28,6 +28,8 @@
 enum
 {
 	NO_PADATA=0,
+	XFRM_INPUT_PADATA,
+	XFRM_OUTPUT_PADATA,
 	NR_PADATA
 };

include/linux/skbuff.h +5 -0

index 2725f4e..a5c9986 100644
@@ -18,6 +18,7 @@
 #include <linux/compiler.h>
 #include <linux/time.h>
 #include <linux/cache.h>
+#include <linux/padata.h>
 
 #include <asm/atomic.h>
 #include <asm/types.h>

@@ -261,6 +262,10 @@ struct sk_buff {
 	struct sk_buff		*next;
 	struct sk_buff		*prev;
 
+#ifdef CONFIG_XFRM_PADATA
+	struct padata_priv       padata;
+#endif
+
 	struct sock		*sk;
 	ktime_t			tstamp;
 	struct net_device	*dev;

include/linux/sysctl.h +2 -1

index 39d471d..fd86b44 100644
@@ -284,7 +284,8 @@ enum
 	NET_CORE_BUDGET=19,
 	NET_CORE_AEVENT_ETIME=20,
 	NET_CORE_AEVENT_RSEQTH=21,
-	NET_CORE_WARNINGS=22,
+	NET_CORE_PADATA=22,
+	NET_CORE_WARNINGS=23,
 };
 
 /* /proc/sys/net/ethernet */

include/net/xfrm.h +38 -0

index 11c890a..ee0ae79 100644
@@ -12,6 +12,7 @@
 #include <linux/in6.h>
 #include <linux/mutex.h>
 #include <linux/audit.h>
+#include <linux/crypto.h>
 
 #include <net/sock.h>
 #include <net/dst.h>

@@ -741,6 +742,43 @@ static inline void xfrm_pols_put(struct xfrm_policy **pols, int npols)
 }
 #endif
 
+#ifdef CONFIG_XFRM_PADATA
+extern u32 xfrm_padata_conf;
+extern int xfrm_do_parallel_input(struct sk_buff *skb);
+extern int xfrm_do_parallel_output(struct sk_buff *skb);
+extern void xfrm_init_padata(void);
+extern int xfrm_padata_strategy(ctl_table *ctl, void __user *oldval,
+		size_t __user *oldlenp, void __user *newval, size_t newlen);
+extern int xfrm_padata_sysctl(struct ctl_table *ctrl, int write,
+				struct file* filp, void __user *buffer,
+				size_t *lenp, loff_t *ppos);
+static inline u32 xfrm_aead_set_flags(struct sk_buff *skb, u32 flags)
+{
+	if (skb->padata.nr == XFRM_OUTPUT_PADATA ||
+			skb->padata.nr == XFRM_INPUT_PADATA)
+
+		flags |= CRYPTO_TFM_REQ_FORCE_SYNC;
+
+	return flags;
+}
+#else
+static inline int xfrm_do_parallel_input(struct sk_buff *skb)
+{
+	return 0;
+}
+static inline int xfrm_do_parallel_output(struct sk_buff *skb)
+{
+	return 0;
+}
+static inline void xfrm_init_padata(void)
+{
+}
+static inline u32 xfrm_aead_set_flags(struct sk_buff *skb, u32 flags)
+{
+	return 0;
+}
+#endif
+
 extern void __xfrm_state_destroy(struct xfrm_state *);
 
 static inline void __xfrm_state_put(struct xfrm_state *x)

kernel/sysctl_check.c +1 -0

index c35da23..011f74e 100644
@@ -161,6 +161,7 @@ static const struct trans_ctl_table trans_net_core_table[] = {
 	{ NET_CORE_BUDGET,		"netdev_budget" },
 	{ NET_CORE_AEVENT_ETIME,	"xfrm_aevent_etime" },
 	{ NET_CORE_AEVENT_RSEQTH,	"xfrm_aevent_rseqth" },
+	{ NET_CORE_PADATA,		"xfrm_padata" },
 	{ NET_CORE_WARNINGS,		"warnings" },
 	{},
 };

net/core/skbuff.c +3 -0

index d49ef83..6c8c86d 100644
@@ -495,6 +495,9 @@ EXPORT_SYMBOL(skb_recycle_check);
 
 static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 {
+#ifdef CONFIG_XFRM_PADATA
+	memset(&new->padata, 0, sizeof(struct padata_priv));
+#endif
 	new->tstamp		= old->tstamp;
 	new->dev		= old->dev;
 	new->transport_header	= old->transport_header;

net/core/sysctl_net_core.c +11 -0

index f686467..7688916 100644
@@ -122,6 +122,17 @@ static struct ctl_table net_core_table[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec
 	},
+#ifdef CONFIG_XFRM_PADATA
+	{
+		.ctl_name	= NET_CORE_PADATA,
+		.procname	= "xfrm_padata",
+		.data		= &xfrm_padata_conf,
+		.maxlen		= sizeof(u32),
+		.mode		= 0644,
+		.proc_handler	= &xfrm_padata_sysctl,
+		.strategy       = &xfrm_padata_strategy,
+	},
+#endif /* CONFIG_XFRM_PADATA */
 #endif /* CONFIG_XFRM */
 #endif /* CONFIG_NET */
 	{

net/xfrm/Kconfig +8 -0

index 6d08167..ba509e0 100644
@@ -46,6 +46,14 @@ config XFRM_STATISTICS
 
 	  If unsure, say N.
 
+config XFRM_PADATA
+	bool "Transformation parallel processing (EXPERIMENTAL)"
+	depends on INET && XFRM && USE_GENERIC_SMP_HELPERS && EXPERIMENTAL
+	---help---
+	  Support parallel processing of the expencive parts of IPsec.
+
+	  If unsure, say N.
+
 config XFRM_IPCOMP
 	tristate
 	select XFRM

net/xfrm/Makefile +1 -1

index 0f439a7..09f3f35 100644
@@ -7,4 +7,4 @@ obj-$(CONFIG_XFRM) := xfrm_policy.o xfrm_state.o xfrm_hash.o \
 obj-$(CONFIG_XFRM_STATISTICS) += xfrm_proc.o
 obj-$(CONFIG_XFRM_USER) += xfrm_user.o
 obj-$(CONFIG_XFRM_IPCOMP) += xfrm_ipcomp.o
-
+obj-$(CONFIG_XFRM_PADATA) += xfrm_padata.o

net/xfrm/xfrm_input.c +6 -1

index 7527940..28126cd 100644
@@ -115,7 +115,8 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 
 	/* A negative encap_type indicates async resumption. */
 	if (encap_type < 0) {
-		async = 1;
+		if (encap_type == -1)
+			async = 1;
 		x = xfrm_input_state(skb);
 		seq = XFRM_SKB_CB(skb)->seq.input;
 		goto resume;

@@ -185,6 +186,10 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 
 		XFRM_SKB_CB(skb)->seq.input = seq;
 
+
+		if (xfrm_do_parallel_input(skb))
+			return 0;
+
 		nexthdr = x->type->input(x, skb);
 
 		if (nexthdr == -EINPROGRESS)

net/xfrm/xfrm_output.c +5 -0

index dc50f1e..1fb134b 100644
@@ -83,6 +83,11 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 
 		spin_unlock_bh(&x->lock);
 
+		if (xfrm_do_parallel_output(skb)) {
+			err = -EINPROGRESS;
+			goto out_exit;
+		}
+
 		err = x->type->output(x, skb);
 		if (err == -EINPROGRESS)
 			goto out_exit;

net/xfrm/xfrm_padata.c new file +270 -0

270 lines not on this page · raw message

net/xfrm/xfrm_policy.c +2 -0

index 058f04f..41d3670 100644
@@ -2433,6 +2433,8 @@ static void __init xfrm_policy_init(void)
 
 void __init xfrm_init(void)
 {
+	xfrm_init_padata();
+
 #ifdef CONFIG_XFRM_STATISTICS
 	xfrm_statistics_init();
 #endif

Signature · 1 line

1.5.4.2

Steffen Klassert

author maintainer

2008-12-01 07:48 UTC #3 lore raw

RFC PATCH 3/5 crypto: add possibility to force sync transform

In reply to Steffen Klassert

From: Steffen Klassert <[email protected]>

If IPsec is parallelized by xfrm_padata, async transform is not allowed. So Add a possibility to force sync transform in this case.

Signed-off-by Steffen Klassert <[email protected]>

---
 crypto/chainiv.c |   14 ++++++++++++--
 net/ipv4/esp4.c  |   12 ++++++++++--
 net/ipv6/esp6.c  |   12 ++++++++++--
 3 files changed, 32 insertions(+), 6 deletions(-)

crypto/chainiv.c +12 -2

index 7c37a49..2079fdd 100644
@@ -168,8 +168,12 @@ static int async_chainiv_givencrypt_tail(struct skcipher_givcrypt_request *req)
 	memcpy(subreq->info, ctx->iv, ivsize);
 
 	ctx->err = crypto_ablkcipher_encrypt(subreq);
-	if (ctx->err)
-		goto out;
+	if (ctx->err) {
+		if (ablkcipher_request_flags(&req->creq) & CRYPTO_TFM_REQ_FORCE_SYNC)
+			return ctx->err;
+		else
+			goto out;
+	}
 
 	memcpy(ctx->iv, subreq->info, ivsize);

@@ -190,6 +194,11 @@ static int async_chainiv_givencrypt(struct skcipher_givcrypt_request *req)
 	ablkcipher_request_set_crypt(subreq, req->creq.src, req->creq.dst,
 				     req->creq.nbytes, req->creq.info);
 
+	 if (ablkcipher_request_flags(&req->creq) & CRYPTO_TFM_REQ_FORCE_SYNC) {
+		 clear_bit(CHAINIV_STATE_INUSE, &ctx->state);
+		 goto sync_out;
+	 }
+
 	if (test_and_set_bit(CHAINIV_STATE_INUSE, &ctx->state))
 		goto postpone;

@@ -198,6 +207,7 @@ static int async_chainiv_givencrypt(struct skcipher_givcrypt_request *req)
 		goto postpone;
 	}
 
+sync_out:
 	return async_chainiv_givencrypt_tail(req);
 
 postpone:

net/ipv4/esp4.c +10 -2

index 21515d4..3ea3fb2 100644
@@ -114,6 +114,7 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 	void *tmp;
 	u8 *iv;
 	u8 *tail;
+	u32 flags;
 	int blksize;
 	int clen;
 	int alen;

@@ -123,6 +124,8 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 
 	err = -ENOMEM;
 
+	flags = 0;
+
 	/* Round to block size */
 	clen = skb->len;

@@ -207,7 +210,9 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 		     clen + alen);
 	sg_init_one(asg, esph, sizeof(*esph));
 
-	aead_givcrypt_set_callback(req, 0, esp_output_done, skb);
+	xfrm_aead_set_flags(skb, flags);
+
+	aead_givcrypt_set_callback(req, flags, esp_output_done, skb);
 	aead_givcrypt_set_crypt(req, sg, sg, clen, iv);
 	aead_givcrypt_set_assoc(req, asg, sizeof(*esph));
 	aead_givcrypt_set_giv(req, esph->enc_data,

@@ -332,6 +337,7 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
 	int nfrags;
 	void *tmp;
 	u8 *iv;
+	u32 flags = 0;
 	struct scatterlist *sg;
 	struct scatterlist *asg;
 	int err = -EINVAL;

@@ -368,7 +374,9 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
 	skb_to_sgvec(skb, sg, sizeof(*esph) + crypto_aead_ivsize(aead), elen);
 	sg_init_one(asg, esph, sizeof(*esph));
 
-	aead_request_set_callback(req, 0, esp_input_done, skb);
+	xfrm_aead_set_flags(skb, flags);
+
+	aead_request_set_callback(req, flags, esp_input_done, skb);
 	aead_request_set_crypt(req, sg, sg, elen, iv);
 	aead_request_set_assoc(req, asg, sizeof(*esph));

net/ipv6/esp6.c +10 -2

index b181b08..9874adf 100644
@@ -143,11 +143,14 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 	int nfrags;
 	u8 *iv;
 	u8 *tail;
+	u32 flags;
 	struct esp_data *esp = x->data;
 
 	/* skb is pure payload to encrypt */
 	err = -ENOMEM;
 
+	flags = 0;
+
 	/* Round to block size */
 	clen = skb->len;

@@ -196,7 +199,9 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 		     clen + alen);
 	sg_init_one(asg, esph, sizeof(*esph));
 
-	aead_givcrypt_set_callback(req, 0, esp_output_done, skb);
+	xfrm_aead_set_flags(skb, flags);
+
+	aead_givcrypt_set_callback(req, flags, esp_output_done, skb);
 	aead_givcrypt_set_crypt(req, sg, sg, clen, iv);
 	aead_givcrypt_set_assoc(req, asg, sizeof(*esph));
 	aead_givcrypt_set_giv(req, esph->enc_data,

@@ -279,6 +284,7 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
 	int ret = 0;
 	void *tmp;
 	u8 *iv;
+	u32 flags = 0;
 	struct scatterlist *sg;
 	struct scatterlist *asg;

@@ -319,7 +325,9 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
 	skb_to_sgvec(skb, sg, sizeof(*esph) + crypto_aead_ivsize(aead), elen);
 	sg_init_one(asg, esph, sizeof(*esph));
 
-	aead_request_set_callback(req, 0, esp_input_done, skb);
+	xfrm_aead_set_flags(skb, flags);
+
+	aead_request_set_callback(req, flags, esp_input_done, skb);
 	aead_request_set_crypt(req, sg, sg, elen, iv);
 	aead_request_set_assoc(req, asg, sizeof(*esph));

Signature · 1 line

1.5.4.2

Herbert Xu

maintainer

2008-12-01 11:22 UTC #4 lore raw

Re: [RFC PATCH 3/5] crypto: add possibility to force sync transform

On Mon, Dec 01, 2008 at 08:19:03AM +0100, Steffen Klassert wrote:
.

crypto/chainiv.c static int async_chainiv_givencrypt_tail(struct skcipher_givcrypt_request *req) · line 172

Quoted 12 lines

crypto/chainiv.c +2 -2

index 7c37a49..2079fdd 100644
@@ -168,8 +168,12 @@ static int async_chainiv_givencrypt_tail(struct skcipher_givcrypt_request *req)
 	memcpy(subreq->info, ctx->iv, ivsize);
 
 	ctx->err = crypto_ablkcipher_encrypt(subreq);
-	if (ctx->err)
-		goto out;
+	if (ctx->err) {
+		if (ablkcipher_request_flags(&req->creq) & CRYPTO_TFM_REQ_FORCE_SYNC)

If you want sync algorithms, you should request for that at alloc time:

	crypto_alloc_ablkcipher("foo", 0, CRYPTO_ALG_ASYNC);

Doing so at run-time is going to end in tears.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-01 13:18 UTC #5 lore raw

Re: [RFC PATCH 3/5] crypto: add possibility to force sync transform

Quoted 5 lines · Herbert Xu

If you want sync algorithms, you should request for that at alloc time:
	crypto_alloc_ablkcipher("foo", 0, CRYPTO_ALG_ASYNC);

Ok, will do that.

Quoted 3 lines

Doing so at run-time is going to end in tears.

As I wrote, I'm not that familiar with the crypto system :-)

Steffen Klassert

author maintainer

2008-12-01 07:48 UTC #6 lore raw

RFC PATCH 4/5 crypto: allow allocation of percpu crypto transforms

In reply to Steffen Klassert

From: Steffen Klassert <[email protected]>

This patch adds functions to alloc/free crypto transforms as percpu data.

Signed-off-by Steffen Klassert <[email protected]>

---
 crypto/api.c           |   86 ++++++++++++++++++++++++++++++++++++++++++-----
 crypto/internal.h      |    3 ++
 include/linux/crypto.h |    1 +
 3 files changed, 80 insertions(+), 10 deletions(-)

crypto/api.c +76 ~~-10~~

index 0444d24..6c8fcfb 100644
@@ -361,6 +361,55 @@ void crypto_shoot_alg(struct crypto_alg *alg)
 }
 EXPORT_SYMBOL_GPL(crypto_shoot_alg);
 
+struct crypto_tfm *__crypto_alloc_tfm_percpu(struct crypto_alg *alg, u32 type,
+				      u32 mask)
+{
+	struct crypto_tfm *tfm = NULL;
+	struct crypto_tfm *tfm_percpu = NULL;
+	unsigned int tfm_size, cpu, cpu_err;
+	int err = -ENOMEM;
+
+	tfm_size = sizeof(*tfm) + crypto_ctxsize(alg, type, mask);
+
+	tfm_percpu =  __alloc_percpu(tfm_size);
+	if (tfm_percpu == NULL)
+		goto out_err;
+
+	for_each_possible_cpu(cpu) {
+
+		tfm = per_cpu_ptr(tfm_percpu, cpu);
+
+		tfm->__crt_alg = alg;
+
+		err = crypto_init_ops(tfm, type, mask);
+		if (err)
+			goto out_free_tfm;
+
+		if (alg->cra_init && (err = alg->cra_init(tfm))) {
+			if (err == -EAGAIN)
+				crypto_shoot_alg(alg);
+			goto cra_init_failed;
+		}
+	}
+
+	goto out;
+
+cra_init_failed:
+	for_each_possible_cpu(cpu_err) {
+		tfm = per_cpu_ptr(tfm_percpu, cpu);
+		crypto_exit_ops(tfm);
+		if (cpu_err == cpu)
+			break;
+	}
+out_free_tfm:
+	free_percpu(tfm_percpu);
+out_err:
+	tfm_percpu = ERR_PTR(err);
+out:
+	return tfm_percpu;
+}
+EXPORT_SYMBOL_GPL(__crypto_alloc_tfm_percpu);
+
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
 				      u32 mask)
 {

@@ -450,15 +499,8 @@ err:
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(crypto_alloc_base);
- 
-/*
- *	crypto_free_tfm - Free crypto transform
- *	@tfm: Transform to free
- *
- *	crypto_free_tfm() frees up the transform and any associated resources,
- *	then drops the refcount on the associated algorithm.
- */
-void crypto_free_tfm(struct crypto_tfm *tfm)
+
+void __crypto_free_tfm(struct crypto_tfm *tfm)
 {
 	struct crypto_alg *alg;
 	int size;

@@ -474,11 +516,35 @@ void crypto_free_tfm(struct crypto_tfm *tfm)
 	crypto_exit_ops(tfm);
 	crypto_mod_put(alg);
 	memset(tfm, 0, size);
-	kfree(tfm);
 }
 
+/*
+ *	crypto_free_tfm - Free crypto transform
+ *	@tfm: Transform to free
+ *
+ *	crypto_free_tfm() frees up the transform and any associated resources,
+ *	then drops the refcount on the associated algorithm.
+ */
+void crypto_free_tfm(struct crypto_tfm *tfm)
+{
+	__crypto_free_tfm(tfm);
+	kfree(tfm);
+}
 EXPORT_SYMBOL_GPL(crypto_free_tfm);
 
+void crypto_free_tfm_percpu(struct crypto_tfm *tfm_percpu)
+{
+	unsigned int cpu;
+	struct crypto_tfm *tfm;
+
+	for_each_possible_cpu(cpu) {
+		tfm = per_cpu_ptr(tfm_percpu, cpu);
+		__crypto_free_tfm(tfm);
+	}
+	free_percpu(tfm_percpu);
+}
+EXPORT_SYMBOL_GPL(crypto_free_tfm_percpu);
+
 int crypto_has_alg(const char *name, u32 type, u32 mask)
 {
 	int ret = 0;

crypto/internal.h +3 -0

index 8ef72d7..8f0fa9e 100644
@@ -109,6 +109,9 @@ void crypto_alg_tested(const char *name, int err);
 void crypto_shoot_alg(struct crypto_alg *alg);
 struct crypto_tfm *__crypto_alloc_tfm(struct crypto_alg *alg, u32 type,
 				      u32 mask);
+struct crypto_tfm *__crypto_alloc_tfm_percpu(struct crypto_alg *alg, u32 type,
+				      u32 mask);
+void __crypto_free_tfm(struct crypto_tfm *tfm);
 
 int crypto_register_instance(struct crypto_template *tmpl,
 			     struct crypto_instance *inst);

include/linux/crypto.h +1 -0

index d5dd094..da07852 100644
@@ -548,6 +548,7 @@ struct crypto_attr_u32 {
 struct crypto_tfm *crypto_alloc_tfm(const char *alg_name, u32 tfm_flags);
 struct crypto_tfm *crypto_alloc_base(const char *alg_name, u32 type, u32 mask);
 void crypto_free_tfm(struct crypto_tfm *tfm);
+void crypto_free_tfm_percpu(struct crypto_tfm *tfm);
 
 int alg_test(const char *driver, const char *alg, u32 type, u32 mask);

Signature · 1 line

1.5.4.2

Herbert Xu

maintainer

2008-12-01 11:38 UTC #7 lore raw

Re: [RFC PATCH 4/5] crypto: allow allocation of percpu crypto transforms

Quoted 6 lines · Steffen Klassert

From: Steffen Klassert <[email protected]>
This patch adds functions to alloc/free crypto transforms as percpu data.
Signed-off-by Steffen Klassert <[email protected]>

I'd prefer to just have the user call crypto_alloc_tfm for each CPU. This is the slow path after all so we don't need to optimise it that much.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2008-12-01 12:25 UTC #8 lore raw

Re: [RFC PATCH 4/5] crypto: allow allocation of percpu crypto transforms

From: Herbert Xu <[email protected]>
Date: Mon, 1 Dec 2008 19:38:28 +0800

Quoted 11 lines · Steffen Klassert

Quoted 6 lines
From: Steffen Klassert <[email protected]>
This patch adds functions to alloc/free crypto transforms as percpu data.
Signed-off-by Steffen Klassert <[email protected]>
I'd prefer to just have the user call crypto_alloc_tfm for each CPU. This is the slow path after all so we don't need to optimise it that much.

I somewhat disagree.

If you make this num_cpus expensive, I can nearly guarentee you this will show up in the profiles for the workloads I was using to test the control path optimizations with all the dynamic hash tables and streamlined lookups.

Some cell phones networks, I am to understand, can allocate 6 SAs when they come onto the network, perhaps never use them, then go away. Multiply that by the number of cell phones using a network in a busy downtown area of a city and it could be very interesting.

If, in those changes, I was removing atomics to make rule insert and delete faster, making more TFM allocs happen might be a big no-no :)

Herbert Xu

maintainer

2008-12-01 12:39 UTC #9 lore raw

Re: [RFC PATCH 4/5] crypto: allow allocation of percpu crypto transforms

Quoted 3 lines · David Miller

If, in those changes, I was removing atomics to make rule insert and delete faster, making more TFM allocs happen might be a big no-no :)

Fair enough. But it looks like this stuff only exists because of the spinlock on the hash which shash will get rid of anyway.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-01 13:21 UTC #10 lore raw

Re: [RFC PATCH 4/5] crypto: allow allocation of percpu crypto transforms

Quoted 8 lines · Herbert Xu

Quoted 3 lines · David Miller
If, in those changes, I was removing atomics to make rule insert and delete faster, making more TFM allocs happen might be a big no-no :)
Fair enough. But it looks like this stuff only exists because of the spinlock on the hash which shash will get rid of anyway.

Yes exactly, it was just to get rid of the lock.

Steffen Klassert

author maintainer

2008-12-01 07:48 UTC #11 lore raw

RFC PATCH 5/5 crypto: make struct aead percpu data

In reply to Steffen Klassert

From: Steffen Klassert <[email protected]>

The struct aead is now allocated as percpu data to get rid of a very high contended lock in crypto_authenc_hash() if IPsec runs in parallel.

Signed-off-by Steffen Klassert <[email protected]>

---
 crypto/aead.c          |    2 +-
 crypto/chainiv.c       |    3 +-
 crypto/testmgr.c       |   14 +++++---
 include/linux/crypto.h |    2 +-
 net/ipv4/esp4.c        |   79 +++++++++++++++++++++++++++-----------------
 net/ipv6/esp6.c        |   84 +++++++++++++++++++++++++++++------------------
 6 files changed, 113 insertions(+), 71 deletions(-)

crypto/aead.c +1 -1

index 3a6f3f5..8f9a2a0 100644
@@ -465,7 +465,7 @@ struct crypto_aead *crypto_alloc_aead(const char *alg_name, u32 type, u32 mask)
 			goto err;
 		}
 
-		tfm = __crypto_alloc_tfm(alg, type, mask);
+		tfm = __crypto_alloc_tfm_percpu(alg, type, mask);
 		if (!IS_ERR(tfm))
 			return __crypto_aead_cast(tfm);

crypto/chainiv.c +2 -1

index 2079fdd..b3e2b28 100644
@@ -133,7 +133,8 @@ static int async_chainiv_schedule_work(struct async_chainiv_ctx *ctx)
 			goto out;
 	}
 
-	queued = schedule_work(&ctx->postponed);
+	queued = schedule_work_on(get_cpu(), &ctx->postponed);
+	put_cpu();
 	BUG_ON(!queued);
 
 out:

crypto/testmgr.c +9 -5

index b828c6c..96418cd 100644
@@ -885,16 +885,20 @@ out:
 static int alg_test_aead(const struct alg_test_desc *desc, const char *driver,
 			 u32 type, u32 mask)
 {
-	struct crypto_aead *tfm;
+	struct crypto_aead *tfm, *tfm_percpu;
 	int err = 0;
 
-	tfm = crypto_alloc_aead(driver, type, mask);
-	if (IS_ERR(tfm)) {
+	tfm_percpu = crypto_alloc_aead(driver, type, mask);
+
+	if (IS_ERR(tfm_percpu)) {
 		printk(KERN_ERR "alg: aead: Failed to load transform for %s: "
-		       "%ld\n", driver, PTR_ERR(tfm));
-		return PTR_ERR(tfm);
+		       "%ld\n", driver, PTR_ERR(tfm_percpu));
+		return PTR_ERR(tfm_percpu);
 	}
 
+	tfm = per_cpu_ptr(tfm_percpu, get_cpu());
+	put_cpu();
+
 	if (desc->suite.aead.enc.vecs) {
 		err = test_aead(tfm, ENCRYPT, desc->suite.aead.enc.vecs,
 				desc->suite.aead.enc.count);

include/linux/crypto.h +1 -1

index da07852..f9c4bf2 100644
@@ -800,7 +800,7 @@ static inline struct crypto_tfm *crypto_aead_tfm(struct crypto_aead *tfm)
 
 static inline void crypto_free_aead(struct crypto_aead *tfm)
 {
-	crypto_free_tfm(crypto_aead_tfm(tfm));
+	crypto_free_tfm_percpu(crypto_aead_tfm(tfm));
 }
 
 static inline struct aead_tfm *crypto_aead_crt(struct crypto_aead *tfm)

net/ipv4/esp4.c +48 ~~-31~~

166 lines not on this page · raw message

net/ipv6/esp6.c +52 ~~-32~~

188 lines not on this page · raw message

Signature · 1 line

1.5.4.2

Herbert Xu

maintainer

2008-12-01 11:40 UTC #12 lore raw

Re: [RFC PATCH 5/5] crypto: make struct aead percpu data

Quoted 7 lines · Steffen Klassert

From: Steffen Klassert <[email protected]>
The struct aead is now allocated as percpu data to get rid of a very high contended lock in crypto_authenc_hash() if IPsec runs in parallel.
Signed-off-by Steffen Klassert <[email protected]>

Well you're in luck :) One of the objectives of the shash work is to get rid of this lock. So instead of doing this, please push the shash work along and we can eliminate this without allocating loads of duplicate tfm's.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-01 13:36 UTC #13 lore raw

Re: [RFC PATCH 5/5] crypto: make struct aead percpu data

Quoted 7 lines · Herbert Xu

Quoted 6 lines · Steffen Klassert
From: Steffen Klassert <[email protected]>
The struct aead is now allocated as percpu data to get rid of a very high contended lock in crypto_authenc_hash() if IPsec runs in parallel.

Quoted 7 lines

Quoted 1 line
Signed-off-by Steffen Klassert <[email protected]>
Well you're in luck :) One of the objectives of the shash work is to get rid of this lock. So instead of doing this, please push the shash work along and we can eliminate this without allocating loads of duplicate tfm's.

I searched for your shash work. Is there already some work in progress aside from crc32?

Herbert Xu

maintainer

2008-12-01 13:44 UTC #14 lore raw

Re: [RFC PATCH 5/5] crypto: make struct aead percpu data

Quoted 3 lines · Steffen Klassert

I searched for your shash work. Is there already some work in progress aside from crc32?

I started on the algorithm conversion but have only made it as far as null and rmd*. But it should serve as a good template for doing the rest.

If you could finish them for me I'd be most grateful :)

I'll push what I've got out now.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

maintainer

2008-12-01 13:47 UTC #15 lore raw

PATCH 1/6 crypto: hash - Make setkey optional

crypto: hash - Make setkey optional

Since most cryptographic hash algorithms have no keys, this patch makes the setkey function optional for ahash and shash.

Signed-off-by Herbert Xu <[email protected]>

---

 crypto/ahash.c |    8 +++++++-
 crypto/shash.c |    3 +++
 2 files changed, 10 insertions(+), 1 deletion(-)

crypto/ahash.c +7 -1

index 9f98956..ba5292d 100644
@@ -162,6 +162,12 @@ static int ahash_setkey(struct crypto_ahash *tfm, const u8 *key,
 	return ahash->setkey(tfm, key, keylen);
 }
 
+static int ahash_nosetkey(struct crypto_ahash *tfm, const u8 *key,
+			  unsigned int keylen)
+{
+	return -ENOSYS;
+}
+
 int crypto_ahash_import(struct ahash_request *req, const u8 *in)
 {
 	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);

@@ -194,7 +200,7 @@ static int crypto_init_ahash_ops(struct crypto_tfm *tfm, u32 type, u32 mask)
 	crt->update = alg->update;
 	crt->final  = alg->final;
 	crt->digest = alg->digest;
-	crt->setkey = ahash_setkey;
+	crt->setkey = alg->setkey ? ahash_setkey : ahash_nosetkey;
 	crt->digestsize = alg->digestsize;
 
 	return 0;

crypto/shash.c +3 -0

index 50d69a4..c9df367 100644
@@ -55,6 +55,9 @@ int crypto_shash_setkey(struct crypto_shash *tfm, const u8 *key,
 	struct shash_alg *shash = crypto_shash_alg(tfm);
 	unsigned long alignmask = crypto_shash_alignmask(tfm);
 
+	if (!shash->setkey)
+		return -ENOSYS;
+
 	if ((unsigned long)key & alignmask)
 		return shash_setkey_unaligned(tfm, key, keylen);

Herbert Xu

maintainer

2008-12-01 13:47 UTC #16 lore raw

PATCH 2/6 crypto: null - Switch to shash

In reply to Herbert Xu

crypto: null - Switch to shash

This patch changes digest_null to the new shash interface.

Signed-off-by Herbert Xu <[email protected]>

---

 crypto/crypto_null.c |   64 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 23 deletions(-)

crypto/crypto_null.c +41 ~~-23~~

index 1f7d530..cb71c91 100644
@@ -17,6 +17,7 @@
  *
  */
 
+#include <crypto/internal/hash.h>
 #include <crypto/internal/skcipher.h>
 #include <linux/init.h>
 #include <linux/module.h>

@@ -38,15 +39,31 @@ static int null_compress(struct crypto_tfm *tfm, const u8 *src,
 	return 0;
 }
 
-static void null_init(struct crypto_tfm *tfm)
-{ }
+static int null_init(struct shash_desc *desc)
+{
+	return 0;
+}
 
-static void null_update(struct crypto_tfm *tfm, const u8 *data,
-			unsigned int len)
-{ }
+static int null_update(struct shash_desc *desc, const u8 *data,
+		       unsigned int len)
+{
+	return 0;
+}
 
-static void null_final(struct crypto_tfm *tfm, u8 *out)
-{ }
+static int null_final(struct shash_desc *desc, u8 *out)
+{
+	return 0;
+}
+
+static int null_digest(struct shash_desc *desc, const u8 *data,
+		       unsigned int len, u8 *out)
+{
+	return 0;
+}
+
+static int null_hash_setkey(struct crypto_shash *tfm, const u8 *key,
+			    unsigned int keylen)
+{ return 0; }
 
 static int null_setkey(struct crypto_tfm *tfm, const u8 *key,
 		       unsigned int keylen)

@@ -89,19 +106,20 @@ static struct crypto_alg compress_null = {
 	.coa_decompress		=	null_compress } }
 };
 
-static struct crypto_alg digest_null = {
-	.cra_name		=	"digest_null",
-	.cra_flags		=	CRYPTO_ALG_TYPE_DIGEST,
-	.cra_blocksize		=	NULL_BLOCK_SIZE,
-	.cra_ctxsize		=	0,
-	.cra_module		=	THIS_MODULE,
-	.cra_list		=       LIST_HEAD_INIT(digest_null.cra_list),	
-	.cra_u			=	{ .digest = {
-	.dia_digestsize		=	NULL_DIGEST_SIZE,
-	.dia_setkey   		=	null_setkey,
-	.dia_init   		=	null_init,
-	.dia_update 		=	null_update,
-	.dia_final  		=	null_final } }
+static struct shash_alg digest_null = {
+	.digestsize		=	NULL_DIGEST_SIZE,
+	.setkey   		=	null_hash_setkey,
+	.init   		=	null_init,
+	.update 		=	null_update,
+	.finup 			=	null_digest,
+	.digest 		=	null_digest,
+	.final  		=	null_final,
+	.base			=	{
+		.cra_name		=	"digest_null",
+		.cra_flags		=	CRYPTO_ALG_TYPE_SHASH,
+		.cra_blocksize		=	NULL_BLOCK_SIZE,
+		.cra_module		=	THIS_MODULE,
+	}
 };
 
 static struct crypto_alg cipher_null = {

@@ -154,7 +172,7 @@ static int __init crypto_null_mod_init(void)
 	if (ret < 0)
 		goto out_unregister_cipher;
 
-	ret = crypto_register_alg(&digest_null);
+	ret = crypto_register_shash(&digest_null);
 	if (ret < 0)
 		goto out_unregister_skcipher;

@@ -166,7 +184,7 @@ out:
 	return ret;
 
 out_unregister_digest:
-	crypto_unregister_alg(&digest_null);
+	crypto_unregister_shash(&digest_null);
 out_unregister_skcipher:
 	crypto_unregister_alg(&skcipher_null);
 out_unregister_cipher:

@@ -177,7 +195,7 @@ out_unregister_cipher:
 static void __exit crypto_null_mod_fini(void)
 {
 	crypto_unregister_alg(&compress_null);
-	crypto_unregister_alg(&digest_null);
+	crypto_unregister_shash(&digest_null);
 	crypto_unregister_alg(&skcipher_null);
 	crypto_unregister_alg(&cipher_null);
 }

Herbert Xu

maintainer

2008-12-01 13:47 UTC #17 lore raw

PATCH 3/6 crypto: rmd128 - Switch to shash

In reply to Herbert Xu

crypto: rmd128 - Switch to shash

This patch changes rmd128 to the new shash interface.

Signed-off-by Herbert Xu <[email protected]>

---

 crypto/rmd128.c |   61 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

crypto/rmd128.c +32 ~~-29~~

index 5de6fa2..1ceb673 100644
@@ -13,11 +13,10 @@
  * any later version.
  *
  */
+#include <crypto/internal/hash.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/crypto.h>
-#include <linux/cryptohash.h>
 #include <linux/types.h>
 #include <asm/byteorder.h>

@@ -218,9 +217,9 @@ static void rmd128_transform(u32 *state, const __le32 *in)
 	return;
 }
 
-static void rmd128_init(struct crypto_tfm *tfm)
+static int rmd128_init(struct shash_desc *desc)
 {
-	struct rmd128_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd128_ctx *rctx = shash_desc_ctx(desc);
 
 	rctx->byte_count = 0;

@@ -230,12 +229,14 @@ static void rmd128_init(struct crypto_tfm *tfm)
 	rctx->state[3] = RMD_H3;
 
 	memset(rctx->buffer, 0, sizeof(rctx->buffer));
+
+	return 0;
 }
 
-static void rmd128_update(struct crypto_tfm *tfm, const u8 *data,
-			  unsigned int len)
+static int rmd128_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int len)
 {
-	struct rmd128_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd128_ctx *rctx = shash_desc_ctx(desc);
 	const u32 avail = sizeof(rctx->buffer) - (rctx->byte_count & 0x3f);
 
 	rctx->byte_count += len;

@@ -244,7 +245,7 @@ static void rmd128_update(struct crypto_tfm *tfm, const u8 *data,
 	if (avail > len) {
 		memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),
 		       data, len);
-		return;
+		goto out;
 	}
 
 	memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),

@@ -262,12 +263,15 @@ static void rmd128_update(struct crypto_tfm *tfm, const u8 *data,
 	}
 
 	memcpy(rctx->buffer, data, len);
+
+out:
+	return 0;
 }
 
 /* Add padding and return the message digest. */
-static void rmd128_final(struct crypto_tfm *tfm, u8 *out)
+static int rmd128_final(struct shash_desc *desc, u8 *out)
 {
-	struct rmd128_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd128_ctx *rctx = shash_desc_ctx(desc);
 	u32 i, index, padlen;
 	__le64 bits;
 	__le32 *dst = (__le32 *)out;

@@ -278,10 +282,10 @@ static void rmd128_final(struct crypto_tfm *tfm, u8 *out)
 	/* Pad out to 56 mod 64 */
 	index = rctx->byte_count & 0x3f;
 	padlen = (index < 56) ? (56 - index) : ((64+56) - index);
-	rmd128_update(tfm, padding, padlen);
+	rmd128_update(desc, padding, padlen);
 
 	/* Append length */
-	rmd128_update(tfm, (const u8 *)&bits, sizeof(bits));
+	rmd128_update(desc, (const u8 *)&bits, sizeof(bits));
 
 	/* Store state in digest */
 	for (i = 0; i < 4; i++)

@@ -289,31 +293,32 @@ static void rmd128_final(struct crypto_tfm *tfm, u8 *out)
 
 	/* Wipe context */
 	memset(rctx, 0, sizeof(*rctx));
+
+	return 0;
 }
 
-static struct crypto_alg alg = {
-	.cra_name	 =	"rmd128",
-	.cra_driver_name =	"rmd128",
-	.cra_flags	 =	CRYPTO_ALG_TYPE_DIGEST,
-	.cra_blocksize	 =	RMD128_BLOCK_SIZE,
-	.cra_ctxsize	 =	sizeof(struct rmd128_ctx),
-	.cra_module	 =	THIS_MODULE,
-	.cra_list	 =	LIST_HEAD_INIT(alg.cra_list),
-	.cra_u		 =	{ .digest = {
-	.dia_digestsize	 =	RMD128_DIGEST_SIZE,
-	.dia_init	 =	rmd128_init,
-	.dia_update	 =	rmd128_update,
-	.dia_final	 =	rmd128_final } }
+static struct shash_alg alg = {
+	.digestsize	=	RMD128_DIGEST_SIZE,
+	.init		=	rmd128_init,
+	.update		=	rmd128_update,
+	.final		=	rmd128_final,
+	.descsize	=	sizeof(struct rmd128_ctx),
+	.base		=	{
+		.cra_name	 =	"rmd128",
+		.cra_flags	 =	CRYPTO_ALG_TYPE_SHASH,
+		.cra_blocksize	 =	RMD128_BLOCK_SIZE,
+		.cra_module	 =	THIS_MODULE,
+	}
 };
 
 static int __init rmd128_mod_init(void)
 {
-	return crypto_register_alg(&alg);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit rmd128_mod_fini(void)
 {
-	crypto_unregister_alg(&alg);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(rmd128_mod_init);

@@ -321,5 +326,3 @@ module_exit(rmd128_mod_fini);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("RIPEMD-128 Message Digest");
-
-MODULE_ALIAS("rmd128");

Herbert Xu

maintainer

2008-12-01 13:47 UTC #18 lore raw

PATCH 4/6 crypto: rmd160 - Switch to shash

In reply to Herbert Xu

crypto: rmd160 - Switch to shash

This patch changes rmd160 to the new shash interface.

Signed-off-by Herbert Xu <[email protected]>

---

 crypto/rmd160.c |   61 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

crypto/rmd160.c +32 ~~-27~~

index f001ec7..472261f 100644
@@ -13,11 +13,10 @@
  * any later version.
  *
  */
+#include <crypto/internal/hash.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/crypto.h>
-#include <linux/cryptohash.h>
 #include <linux/types.h>
 #include <asm/byteorder.h>

@@ -261,9 +260,9 @@ static void rmd160_transform(u32 *state, const __le32 *in)
 	return;
 }
 
-static void rmd160_init(struct crypto_tfm *tfm)
+static int rmd160_init(struct shash_desc *desc)
 {
-	struct rmd160_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd160_ctx *rctx = shash_desc_ctx(desc);
 
 	rctx->byte_count = 0;

@@ -274,12 +273,14 @@ static void rmd160_init(struct crypto_tfm *tfm)
 	rctx->state[4] = RMD_H4;
 
 	memset(rctx->buffer, 0, sizeof(rctx->buffer));
+
+	return 0;
 }
 
-static void rmd160_update(struct crypto_tfm *tfm, const u8 *data,
-			  unsigned int len)
+static int rmd160_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int len)
 {
-	struct rmd160_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd160_ctx *rctx = shash_desc_ctx(desc);
 	const u32 avail = sizeof(rctx->buffer) - (rctx->byte_count & 0x3f);
 
 	rctx->byte_count += len;

@@ -288,7 +289,7 @@ static void rmd160_update(struct crypto_tfm *tfm, const u8 *data,
 	if (avail > len) {
 		memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),
 		       data, len);
-		return;
+		goto out;
 	}
 
 	memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),

@@ -306,12 +307,15 @@ static void rmd160_update(struct crypto_tfm *tfm, const u8 *data,
 	}
 
 	memcpy(rctx->buffer, data, len);
+
+out:
+	return 0;
 }
 
 /* Add padding and return the message digest. */
-static void rmd160_final(struct crypto_tfm *tfm, u8 *out)
+static int rmd160_final(struct shash_desc *desc, u8 *out)
 {
-	struct rmd160_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd160_ctx *rctx = shash_desc_ctx(desc);
 	u32 i, index, padlen;
 	__le64 bits;
 	__le32 *dst = (__le32 *)out;

@@ -322,10 +326,10 @@ static void rmd160_final(struct crypto_tfm *tfm, u8 *out)
 	/* Pad out to 56 mod 64 */
 	index = rctx->byte_count & 0x3f;
 	padlen = (index < 56) ? (56 - index) : ((64+56) - index);
-	rmd160_update(tfm, padding, padlen);
+	rmd160_update(desc, padding, padlen);
 
 	/* Append length */
-	rmd160_update(tfm, (const u8 *)&bits, sizeof(bits));
+	rmd160_update(desc, (const u8 *)&bits, sizeof(bits));
 
 	/* Store state in digest */
 	for (i = 0; i < 5; i++)

@@ -333,31 +337,32 @@ static void rmd160_final(struct crypto_tfm *tfm, u8 *out)
 
 	/* Wipe context */
 	memset(rctx, 0, sizeof(*rctx));
+
+	return 0;
 }
 
-static struct crypto_alg alg = {
-	.cra_name	 =	"rmd160",
-	.cra_driver_name =	"rmd160",
-	.cra_flags	 =	CRYPTO_ALG_TYPE_DIGEST,
-	.cra_blocksize	 =	RMD160_BLOCK_SIZE,
-	.cra_ctxsize	 =	sizeof(struct rmd160_ctx),
-	.cra_module	 =	THIS_MODULE,
-	.cra_list	 =	LIST_HEAD_INIT(alg.cra_list),
-	.cra_u		 =	{ .digest = {
-	.dia_digestsize	 =	RMD160_DIGEST_SIZE,
-	.dia_init	 =	rmd160_init,
-	.dia_update	 =	rmd160_update,
-	.dia_final	 =	rmd160_final } }
+static struct shash_alg alg = {
+	.digestsize	=	RMD160_DIGEST_SIZE,
+	.init		=	rmd160_init,
+	.update		=	rmd160_update,
+	.final		=	rmd160_final,
+	.descsize	=	sizeof(struct rmd160_ctx),
+	.base		=	{
+		.cra_name	 =	"rmd160",
+		.cra_flags	 =	CRYPTO_ALG_TYPE_SHASH,
+		.cra_blocksize	 =	RMD160_BLOCK_SIZE,
+		.cra_module	 =	THIS_MODULE,
+	}
 };
 
 static int __init rmd160_mod_init(void)
 {
-	return crypto_register_alg(&alg);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit rmd160_mod_fini(void)
 {
-	crypto_unregister_alg(&alg);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(rmd160_mod_init);

@@ -365,5 +370,3 @@ module_exit(rmd160_mod_fini);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("RIPEMD-160 Message Digest");

Herbert Xu

maintainer

2008-12-01 13:47 UTC #19 lore raw

PATCH 5/6 crypto: rmd256 - Switch to shash

In reply to Herbert Xu

crypto: rmd256 - Switch to shash

This patch changes rmd256 to the new shash interface.

Signed-off-by Herbert Xu <[email protected]>

---

 crypto/rmd256.c |   61 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

crypto/rmd256.c +32 ~~-27~~

index e3de5b4..72eafa8 100644
@@ -13,11 +13,10 @@
  * any later version.
  *
  */
+#include <crypto/internal/hash.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/crypto.h>
-#include <linux/cryptohash.h>
 #include <linux/types.h>
 #include <asm/byteorder.h>

@@ -233,9 +232,9 @@ static void rmd256_transform(u32 *state, const __le32 *in)
 	return;
 }
 
-static void rmd256_init(struct crypto_tfm *tfm)
+static int rmd256_init(struct shash_desc *desc)
 {
-	struct rmd256_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd256_ctx *rctx = shash_desc_ctx(desc);
 
 	rctx->byte_count = 0;

@@ -249,12 +248,14 @@ static void rmd256_init(struct crypto_tfm *tfm)
 	rctx->state[7] = RMD_H8;
 
 	memset(rctx->buffer, 0, sizeof(rctx->buffer));
+
+	return 0;
 }
 
-static void rmd256_update(struct crypto_tfm *tfm, const u8 *data,
-			  unsigned int len)
+static int rmd256_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int len)
 {
-	struct rmd256_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd256_ctx *rctx = shash_desc_ctx(desc);
 	const u32 avail = sizeof(rctx->buffer) - (rctx->byte_count & 0x3f);
 
 	rctx->byte_count += len;

@@ -263,7 +264,7 @@ static void rmd256_update(struct crypto_tfm *tfm, const u8 *data,
 	if (avail > len) {
 		memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),
 		       data, len);
-		return;
+		goto out;
 	}
 
 	memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),

@@ -281,12 +282,15 @@ static void rmd256_update(struct crypto_tfm *tfm, const u8 *data,
 	}
 
 	memcpy(rctx->buffer, data, len);
+
+out:
+	return 0;
 }
 
 /* Add padding and return the message digest. */
-static void rmd256_final(struct crypto_tfm *tfm, u8 *out)
+static int rmd256_final(struct shash_desc *desc, u8 *out)
 {
-	struct rmd256_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd256_ctx *rctx = shash_desc_ctx(desc);
 	u32 i, index, padlen;
 	__le64 bits;
 	__le32 *dst = (__le32 *)out;

@@ -297,10 +301,10 @@ static void rmd256_final(struct crypto_tfm *tfm, u8 *out)
 	/* Pad out to 56 mod 64 */
 	index = rctx->byte_count & 0x3f;
 	padlen = (index < 56) ? (56 - index) : ((64+56) - index);
-	rmd256_update(tfm, padding, padlen);
+	rmd256_update(desc, padding, padlen);
 
 	/* Append length */
-	rmd256_update(tfm, (const u8 *)&bits, sizeof(bits));
+	rmd256_update(desc, (const u8 *)&bits, sizeof(bits));
 
 	/* Store state in digest */
 	for (i = 0; i < 8; i++)

@@ -308,31 +312,32 @@ static void rmd256_final(struct crypto_tfm *tfm, u8 *out)
 
 	/* Wipe context */
 	memset(rctx, 0, sizeof(*rctx));
+
+	return 0;
 }
 
-static struct crypto_alg alg = {
-	.cra_name	 =	"rmd256",
-	.cra_driver_name =	"rmd256",
-	.cra_flags	 =	CRYPTO_ALG_TYPE_DIGEST,
-	.cra_blocksize	 =	RMD256_BLOCK_SIZE,
-	.cra_ctxsize	 =	sizeof(struct rmd256_ctx),
-	.cra_module	 =	THIS_MODULE,
-	.cra_list	 =	LIST_HEAD_INIT(alg.cra_list),
-	.cra_u		 =	{ .digest = {
-	.dia_digestsize	 =	RMD256_DIGEST_SIZE,
-	.dia_init	 =	rmd256_init,
-	.dia_update	 =	rmd256_update,
-	.dia_final	 =	rmd256_final } }
+static struct shash_alg alg = {
+	.digestsize	=	RMD256_DIGEST_SIZE,
+	.init		=	rmd256_init,
+	.update		=	rmd256_update,
+	.final		=	rmd256_final,
+	.descsize	=	sizeof(struct rmd256_ctx),
+	.base		=	{
+		.cra_name	 =	"rmd256",
+		.cra_flags	 =	CRYPTO_ALG_TYPE_SHASH,
+		.cra_blocksize	 =	RMD256_BLOCK_SIZE,
+		.cra_module	 =	THIS_MODULE,
+	}
 };
 
 static int __init rmd256_mod_init(void)
 {
-	return crypto_register_alg(&alg);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit rmd256_mod_fini(void)
 {
-	crypto_unregister_alg(&alg);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(rmd256_mod_init);

@@ -340,5 +345,3 @@ module_exit(rmd256_mod_fini);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("RIPEMD-256 Message Digest");

Herbert Xu

maintainer

2008-12-01 13:47 UTC #20 lore raw

PATCH 6/6 crypto: rmd320 - Switch to shash

In reply to Herbert Xu

crypto: rmd320 - Switch to shash

This patch changes rmd320 to the new shash interface.

Signed-off-by Herbert Xu <[email protected]>

---

 crypto/rmd320.c |   61 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

crypto/rmd320.c +32 ~~-29~~

index b143d66..86becab 100644
@@ -13,11 +13,10 @@
  * any later version.
  *
  */
+#include <crypto/internal/hash.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/crypto.h>
-#include <linux/cryptohash.h>
 #include <linux/types.h>
 #include <asm/byteorder.h>

@@ -280,9 +279,9 @@ static void rmd320_transform(u32 *state, const __le32 *in)
 	return;
 }
 
-static void rmd320_init(struct crypto_tfm *tfm)
+static int rmd320_init(struct shash_desc *desc)
 {
-	struct rmd320_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd320_ctx *rctx = shash_desc_ctx(desc);
 
 	rctx->byte_count = 0;

@@ -298,12 +297,14 @@ static void rmd320_init(struct crypto_tfm *tfm)
 	rctx->state[9] = RMD_H9;
 
 	memset(rctx->buffer, 0, sizeof(rctx->buffer));
+
+	return 0;
 }
 
-static void rmd320_update(struct crypto_tfm *tfm, const u8 *data,
-			  unsigned int len)
+static int rmd320_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int len)
 {
-	struct rmd320_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd320_ctx *rctx = shash_desc_ctx(desc);
 	const u32 avail = sizeof(rctx->buffer) - (rctx->byte_count & 0x3f);
 
 	rctx->byte_count += len;

@@ -312,7 +313,7 @@ static void rmd320_update(struct crypto_tfm *tfm, const u8 *data,
 	if (avail > len) {
 		memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),
 		       data, len);
-		return;
+		goto out;
 	}
 
 	memcpy((char *)rctx->buffer + (sizeof(rctx->buffer) - avail),

@@ -330,12 +331,15 @@ static void rmd320_update(struct crypto_tfm *tfm, const u8 *data,
 	}
 
 	memcpy(rctx->buffer, data, len);
+
+out:
+	return 0;
 }
 
 /* Add padding and return the message digest. */
-static void rmd320_final(struct crypto_tfm *tfm, u8 *out)
+static int rmd320_final(struct shash_desc *desc, u8 *out)
 {
-	struct rmd320_ctx *rctx = crypto_tfm_ctx(tfm);
+	struct rmd320_ctx *rctx = shash_desc_ctx(desc);
 	u32 i, index, padlen;
 	__le64 bits;
 	__le32 *dst = (__le32 *)out;

@@ -346,10 +350,10 @@ static void rmd320_final(struct crypto_tfm *tfm, u8 *out)
 	/* Pad out to 56 mod 64 */
 	index = rctx->byte_count & 0x3f;
 	padlen = (index < 56) ? (56 - index) : ((64+56) - index);
-	rmd320_update(tfm, padding, padlen);
+	rmd320_update(desc, padding, padlen);
 
 	/* Append length */
-	rmd320_update(tfm, (const u8 *)&bits, sizeof(bits));
+	rmd320_update(desc, (const u8 *)&bits, sizeof(bits));
 
 	/* Store state in digest */
 	for (i = 0; i < 10; i++)

@@ -357,31 +361,32 @@ static void rmd320_final(struct crypto_tfm *tfm, u8 *out)
 
 	/* Wipe context */
 	memset(rctx, 0, sizeof(*rctx));
+
+	return 0;
 }
 
-static struct crypto_alg alg = {
-	.cra_name	 =	"rmd320",
-	.cra_driver_name =	"rmd320",
-	.cra_flags	 =	CRYPTO_ALG_TYPE_DIGEST,
-	.cra_blocksize	 =	RMD320_BLOCK_SIZE,
-	.cra_ctxsize	 =	sizeof(struct rmd320_ctx),
-	.cra_module	 =	THIS_MODULE,
-	.cra_list	 =	LIST_HEAD_INIT(alg.cra_list),
-	.cra_u		 =	{ .digest = {
-	.dia_digestsize	 =	RMD320_DIGEST_SIZE,
-	.dia_init	 =	rmd320_init,
-	.dia_update	 =	rmd320_update,
-	.dia_final	 =	rmd320_final } }
+static struct shash_alg alg = {
+	.digestsize	=	RMD320_DIGEST_SIZE,
+	.init		=	rmd320_init,
+	.update		=	rmd320_update,
+	.final		=	rmd320_final,
+	.descsize	=	sizeof(struct rmd320_ctx),
+	.base		=	{
+		.cra_name	 =	"rmd320",
+		.cra_flags	 =	CRYPTO_ALG_TYPE_SHASH,
+		.cra_blocksize	 =	RMD320_BLOCK_SIZE,
+		.cra_module	 =	THIS_MODULE,
+	}
 };
 
 static int __init rmd320_mod_init(void)
 {
-	return crypto_register_alg(&alg);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit rmd320_mod_fini(void)
 {
-	crypto_unregister_alg(&alg);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(rmd320_mod_init);

@@ -389,5 +394,3 @@ module_exit(rmd320_mod_fini);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("RIPEMD-320 Message Digest");
-
-MODULE_ALIAS("rmd320");

Herbert Xu

maintainer

2008-12-01 13:51 UTC #21 lore raw

Re: [RFC PATCH 5/5] crypto: make struct aead percpu data

In reply to Herbert Xu

Quoted 2 lines · Herbert Xu

If you could finish them for me I'd be most grateful :)

In case it isn't clear from the code, once we've converted all the algorithms then we can start converting the existing users across to the new interface. In particular, authenc can be made to use shash (or ahash) which would render the spinlock useless.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-01 13:54 UTC #22 lore raw

Re: [RFC PATCH 5/5] crypto: make struct aead percpu data

In reply to Herbert Xu

Quoted 5 lines · Herbert Xu

Quoted 3 lines · Steffen Klassert
I searched for your shash work. Is there already some work in progress aside from crc32?

Quoted 7 lines

I started on the algorithm conversion but have only made it as far as null and rmd*. But it should serve as a good template for doing the rest.
If you could finish them for me I'd be most grateful :)
I'll push what I've got out now.

Thanks, I'll see what I can do :-)

Herbert Xu

maintainer

2008-12-01 08:49 UTC #23 lore raw

In reply to Steffen Klassert

Quoted 9 lines · Steffen Klassert

This is a first throw to try to parallelize the expensive part of xfrm by using a generic parallelization/serialization method. This method uses the remote softirq invocation infrastructure for parallelization and serialization. With this method data objects can be processed in parallel, starting at some given point. After doing some expensive operations in parallel, it is possible to serialize again. The parallelized data objects return after serialization in the order as they were before the parallelization. In the case of xfrm, this makes it possible to run the expensive part in parallel without getting packet reordering.

I still think that you're much better off doing this in the crypto layer. As it stands the only reason why this is attractive is because crypto is slow.

Pretty soon processors will start providing crypto support natively so this will no longer be the case. I'd rather see this stuff contained in a small area instead of having it spread all over the place as this may become obsolete any day now.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2008-12-01 10:29 UTC #24 lore raw

From: Herbert Xu <[email protected]>
Date: Mon, 1 Dec 2008 16:49:02 +0800

Quoted 11 lines · Steffen Klassert

Quoted 9 lines
This is a first throw to try to parallelize the expensive part of xfrm by using a generic parallelization/serialization method. This method uses the remote softirq invocation infrastructure for parallelization and serialization. With this method data objects can be processed in parallel, starting at some given point. After doing some expensive operations in parallel, it is possible to serialize again. The parallelized data objects return after serialization in the order as they were before the parallelization. In the case of xfrm, this makes it possible to run the expensive part in parallel without getting packet reordering.

Quoted 8 lines

I still think that you're much better off doing this in the crypto layer. As it stands the only reason why this is attractive is because crypto is slow.
Pretty soon processors will start providing crypto support natively so this will no longer be the case. I'd rather see this stuff contained in a small area instead of having it spread all over the place as this may become obsolete any day now.

Herbert, I'm not completely convinced of this line of thinking :-)

Will crypto be faster than a routing cache lookup? Because flow seperation helps routing performance, significantly.

The problem is that we can't seperate within a flow, that's why we need something like Steffen's work.

And the reconstitution of the seperated crypto operations into the original properly ordered flow can be done most cleanly (IMHO) in the networking layer as far as I have seen so far.

Herbert Xu

maintainer

2008-12-01 11:15 UTC #25 lore raw

Quoted 2 lines · David Miller

Herbert, I'm not completely convinced of this line of thinking :-)

Well you've put some doubts into mind for a second there, but after thinking some more I'm sticking to my position :)

Quoted 2 lines

Will crypto be faster than a routing cache lookup? Because flow seperation helps routing performance, significantly.

No to the first part, but that's not the point. I'm saying that you can do flow separation within the crypto layer, well, for output anyway. All you need to do is to provide some key to the crypto layer that tells it that ordering is only required within that key. Alternatively, just allocate plenty of crypto tfms as these patches do, and feed the flows down to different tfms.

For input there's nothing we can do apart from processing completed requests in sequence so no flow information is going to help (and in fact if it did one could argue that the purpose of IPsec has been defeated :)

Quoted 3 lines

And the reconstitution of the seperated crypto operations into the original properly ordered flow can be done most cleanly (IMHO) in the networking layer as far as I have seen so far.

Well once I've finished the GRO stuff I can give this a go. But I'm pretty sure we can achieve exactly the same thing with only minimal changes to xfrm. Really all it needs to do is allocate more tfm objects for the output path.

All the serialisation logic can stay in the crypto layer because AFAICS no network-specific knowledge is being used in this patch set.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-02 07:57 UTC #26 lore raw

Quoted 27 lines · Herbert Xu

Quoted 2 lines · David Miller
Herbert, I'm not completely convinced of this line of thinking :-)
Well you've put some doubts into mind for a second there, but after thinking some more I'm sticking to my position :)
Quoted 2 lines
Will crypto be faster than a routing cache lookup? Because flow seperation helps routing performance, significantly.
No to the first part, but that's not the point. I'm saying that you can do flow separation within the crypto layer, well, for output anyway. All you need to do is to provide some key to the crypto layer that tells it that ordering is only required within that key. Alternatively, just allocate plenty of crypto tfms as these patches do, and feed the flows down to different tfms.
For input there's nothing we can do apart from processing completed requests in sequence so no flow information is going to help (and in fact if it did one could argue that the purpose of IPsec has been defeated :)
Quoted 3 lines
And the reconstitution of the seperated crypto operations into the original properly ordered flow can be done most cleanly (IMHO) in the networking layer as far as I have seen so far.

Quoted 9 lines

Well once I've finished the GRO stuff I can give this a go. But I'm pretty sure we can achieve exactly the same thing with only minimal changes to xfrm. Really all it needs to do is allocate more tfm objects for the output path.
All the serialisation logic can stay in the crypto layer because AFAICS no network-specific knowledge is being used in this patch set.

The parallelization/serialization interface is generic, so the crypto layer can use it in the same way as xfrm.

The only network specific information that is used is the knowledge of the xfrm_state that a packet uses. On serialization, this helps to stay as parallel as possible. Packets that uses the same state must go to the same cpu (because of the replay window) packets that use different states can go to different cpus.

Herbert Xu

maintainer

2008-12-02 08:19 UTC #27 lore raw

Quoted 3 lines · Steffen Klassert

The parallelization/serialization interface is generic, so the crypto layer can use it in the same way as xfrm.

I don't really have an opinion on the padata stuff. What I'm objecting to is the way you're putting the parallelism into xfrm.

Quoted 5 lines

The only network specific information that is used is the knowledge of the xfrm_state that a packet uses. On serialization, this helps to stay as parallel as possible. Packets that uses the same state must go to the same cpu (because of the replay window) packets that use different states can go to different cpus.

Maybe I'm missing somthing, but all you're doing is paralleising based on xfrm_state objects. You can already do that trivially in the crypto layer with no network-specific knowledge at all because each xfrm_state allocates its own tfm objects.

What I'm proposing is that you create a single parallel crypto algorithm template similar to cryptd (FWIW you could even just extend cryptd to do this) that does the parallel processing based on tfms.

This achieves exactly the same thing as your current patch-set plus:

1) The uesr no longer has to make a system-wide choice of whether to enable this, instead the control is per-SA through the usual algorithm selection mechanism which means that this no longer conflicts with async crypto;

2) There is no change to the xfrm code;

3) The same mechanism can benefit other crypto users such as disk encryption.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-02 08:43 UTC #28 lore raw

Quoted 15 lines · Herbert Xu

Quoted 3 lines · Steffen Klassert
The parallelization/serialization interface is generic, so the crypto layer can use it in the same way as xfrm.
I don't really have an opinion on the padata stuff. What I'm objecting to is the way you're putting the parallelism into xfrm.
Quoted 5 lines
The only network specific information that is used is the knowledge of the xfrm_state that a packet uses. On serialization, this helps to stay as parallel as possible. Packets that uses the same state must go to the same cpu (because of the replay window) packets that use different states can go to different cpus.

Quoted 4 lines

Maybe I'm missing somthing, but all you're doing is paralleising based on xfrm_state objects. You can already do that trivially in the crypto layer with no network-specific knowledge at all because each xfrm_state allocates its own tfm objects.

No, it's not just parallelizing based on xfrm_states. We are running in parallel even within the same state. That's why I'm getting a bandwith up to 900Mbit/s when sending one tcp stream. Parallelizing based on state would not help that much if you are sending just one stream.

What happens is that we are sending the packets round robin to the different cpus on parallelization. On serialization, the packets will be brought back to the order as they were before the parallelization. And to not just send all the packets back to one cpu, I'm doing parallelization based on xfrm_state after that.

Quoted 14 lines

What I'm proposing is that you create a single parallel crypto algorithm template similar to cryptd (FWIW you could even just extend cryptd to do this) that does the parallel processing based on tfms.
This achieves exactly the same thing as your current patch-set plus:
1) The uesr no longer has to make a system-wide choice of whether to enable this, instead the control is per-SA through the usual algorithm selection mechanism which means that this no longer conflicts with async crypto;

Quoted 4 lines

2) There is no change to the xfrm code;
3) The same mechanism can benefit other crypto users such as disk encryption.

The padata stuff is generic, so it can be used even for disk encryption or for anything else that should run in parallel but needs a certain order at a given point.

Herbert Xu

maintainer

2008-12-02 08:50 UTC #29 lore raw

Quoted 11 lines · Steffen Klassert

Quoted 5 lines
Maybe I'm missing somthing, but all you're doing is paralleising based on xfrm_state objects. You can already do that trivially in the crypto layer with no network-specific knowledge at all because each xfrm_state allocates its own tfm objects.
No, it's not just parallelizing based on xfrm_states. We are running in parallel even within the same state. That's why I'm getting a bandwith up to 900Mbit/s when sending one tcp stream. Parallelizing based on state would not help that much if you are sending just one stream.

I understand. What I meant was maintaining ordering within an xfrm_state. Since each xfrm_state has its own tfms, all you have to do is round-robin within each crypto tfm while maintaining ordering to achieve the same result.

Quoted 8 lines

Quoted 8 lines
This achieves exactly the same thing as your current patch-set plus:
1) The uesr no longer has to make a system-wide choice of whether to enable this, instead the control is per-SA through the usual algorithm selection mechanism which means that this no longer conflicts with async crypto;

Quoted 8 lines

Quoted 4 lines
2) There is no change to the xfrm code;
3) The same mechanism can benefit other crypto users such as disk encryption.
The padata stuff is generic, so it can be used even for disk encryption or for anything else that should run in parallel but needs a certain order at a given point.

What about the first issue?

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-02 09:21 UTC #30 lore raw

Quoted 11 lines · Herbert Xu

Quoted 11 lines
Quoted 11 lines
This achieves exactly the same thing as your current patch-set plus:
1) The uesr no longer has to make a system-wide choice of whether to enable this, instead the control is per-SA through the usual algorithm selection mechanism which means that this no longer conflicts with async crypto;
2) There is no change to the xfrm code;

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
3) The same mechanism can benefit other crypto users such as disk encryption.
The padata stuff is generic, so it can be used even for disk encryption or for anything else that should run in parallel but needs a certain order at a given point.
What about the first issue?

Yes, that's a point. If it is in the network layer we probaply need this system-wide choice because it would bring a useless overhead to the sk_buff if this thing is not used. So this could be a plus for putting it to the crypto layer if we don't need this system-wide choice there.

Herbert Xu

maintainer

2008-12-02 08:53 UTC #31 lore raw

In reply to Steffen Klassert

Quoted 7 lines · Steffen Klassert

Quoted 3 lines
3) The same mechanism can benefit other crypto users such as disk encryption.
The padata stuff is generic, so it can be used even for disk encryption or for anything else that should run in parallel but needs a certain order at a given point.

The padata stuff might be great, but does that mean that we really want to reimplement exactly the same logic in two different places when we can do it just once in the crypto layer (which could also use padata as you suggested)?

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-02 09:38 UTC #32 lore raw

Quoted 5 lines · Herbert Xu

Quoted 4 lines · Steffen Klassert
Quoted 3 lines
3) The same mechanism can benefit other crypto users such as disk encryption.

Quoted 9 lines

Quoted 3 lines
The padata stuff is generic, so it can be used even for disk encryption or for anything else that should run in parallel but needs a certain order at a given point.
The padata stuff might be great, but does that mean that we really want to reimplement exactly the same logic in two different places when we can do it just once in the crypto layer (which could also use padata as you suggested)?

I would not mind to move the padata hooks to the crypto layer.

But what's with the interface? Should it be moved to the crypto layer too? I'm not sure, but perhaps it could find users beyond the crypto layer.

Herbert Xu

maintainer

2008-12-02 10:37 UTC #33 lore raw

Quoted 6 lines · Steffen Klassert

I would not mind to move the padata hooks to the crypto layer.
But what's with the interface? Should it be moved to the crypto layer too? I'm not sure, but perhaps it could find users beyond the crypto layer.

As I said I have no objections to the padata stuff itself so I don't really mind either way.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Andi Kleen

2008-12-01 11:20 UTC #34 lore raw

In reply to Herbert Xu

Quoted 7 lines · Herbert Xu

I still think that you're much better off doing this in the crypto layer. As it stands the only reason why this is attractive is because crypto is slow.
Pretty soon processors will start providing crypto support natively so this will no longer be the case.

I'm not sure that's a useful argument. When cryptography is not CPU bound anymore it will be memory bandwidth bound. And in this case you can still get a win out of parallelization if you parallelize over multiple sockets with own memory controller or own FSB because that will give you more bandwidth.

-Andi

Signature · 1 line

[email protected]

Herbert Xu

maintainer

2008-12-01 13:40 UTC #35 lore raw

Quoted 6 lines · Andi Kleen

I'm not sure that's a useful argument. When cryptography is not CPU bound anymore it will be memory bandwidth bound. And in this case you can still get a win out of parallelization if you parallelize over multiple sockets with own memory controller or own FSB because that will give you more bandwidth.

That's a good point.

Of course you can achieve the same thing by doing the parallel processing within the crypto layer, which would also benefit other crypto users, such as disk encryption.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Steffen Klassert

author maintainer

2008-12-02 07:59 UTC #36 lore raw

In reply to Herbert Xu

Quoted 11 lines · Herbert Xu

Quoted 9 lines · Steffen Klassert
This is a first throw to try to parallelize the expensive part of xfrm by using a generic parallelization/serialization method. This method uses the remote softirq invocation infrastructure for parallelization and serialization. With this method data objects can be processed in parallel, starting at some given point. After doing some expensive operations in parallel, it is possible to serialize again. The parallelized data objects return after serialization in the order as they were before the parallelization. In the case of xfrm, this makes it possible to run the expensive part in parallel without getting packet reordering.

Quoted 9 lines

I still think that you're much better off doing this in the crypto layer. As it stands the only reason why this is attractive is because crypto is slow.
Pretty soon processors will start providing crypto support natively so this will no longer be the case. I'd rather see this stuff contained in a small area instead of having it spread all over the place as this may become obsolete any day now.

Now, that most of the crypto changes of this patchset are obsolete by shash, 'spreading all over the place' is probaply not a reason any more not to keep it in the network layer.