linux-kernel - Re: [RFC 1/1] net: introduce OpenVPN Data Channel Offload (ovpn-dco)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <04c967669e4ed8845323f1487fff86949f07a81d.camel@perches.com>
Date:   Wed, 03 Aug 2022 09:04:06 -0700
From:   Joe Perches <joe@...ches.com>
To:     Antonio Quartulli <antonio@...nvpn.net>, netdev@...r.kernel.org
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [RFC 1/1] net: introduce OpenVPN Data Channel Offload (ovpn-dco)

On Tue, 2022-07-19 at 03:47 +0200, Antonio Quartulli wrote:
> OpenVPN is a userspace software existing since around 2005 that allows
> users to create secure tunnels.
> 
> So far OpenVPN has implemented all operations in userspace, which
> implies several back and forth between kernel and user land in order to
> process packets (encapsulate/decapsulate, encrypt/decrypt, rerouting..).
> 
> With ovpn-dco, we intend to move the fast path (data channel) entirely
> in kernel space and thus improve user measured throughput over the
> tunnel.

Logging trivia:

> diff --git a/drivers/net/ovpn-dco/crypto.c b/drivers/net/ovpn-dco/crypto.c
> new file mode 100644
> index 000000000000..fcc3a351ba9d
> --- /dev/null
> +++ b/drivers/net/ovpn-dco/crypto.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*  OpenVPN data channel accelerator
> + *
> + *  Copyright (C) 2020-2022 OpenVPN, Inc.
> + *
> + *  Author:	James Yonan <james@...nvpn.net>
> + *		Antonio Quartulli <antonio@...nvpn.net>
> + */

Please add

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

before any #include when a logging message is output

[]
> +void ovpn_crypto_key_slot_delete(struct ovpn_crypto_state *cs,
> +				 enum ovpn_key_slot slot)
> +{
> +	struct ovpn_crypto_key_slot *ks = NULL;
> +
> +	mutex_lock(&cs->mutex);
> +	switch (slot) {
> +	case OVPN_KEY_SLOT_PRIMARY:
> +		ks = rcu_replace_pointer(cs->primary, NULL,
> +					 lockdep_is_held(&cs->mutex));
> +		break;
> +	case OVPN_KEY_SLOT_SECONDARY:
> +		ks = rcu_replace_pointer(cs->secondary, NULL,
> +					 lockdep_is_held(&cs->mutex));
> +		break;
> +	default:
> +		pr_warn("Invalid slot to release: %u\n", slot);

So messages like these are prefixed appropriately.

> +		break;
> +	}
> +	mutex_unlock(&cs->mutex);
> +
> +	if (!ks) {
> +		pr_debug("Key slot already released: %u\n", slot);
> +		return;
> +	}
> +	pr_debug("deleting key slot %u, key_id=%u\n", slot, ks->key_id);
> +
> +	ovpn_crypto_key_slot_put(ks);
> +}

> diff --git a/drivers/net/ovpn-dco/crypto_aead.c b/drivers/net/ovpn-dco/crypto_aead.c
[]
> +/* Initialize a struct crypto_aead object */
> +struct crypto_aead *ovpn_aead_init(const char *title, const char *alg_name,
> +				   const unsigned char *key, unsigned int keylen)
> +{
> +	struct crypto_aead *aead;
> +	int ret;
> +
> +	aead = crypto_alloc_aead(alg_name, 0, 0);
> +	if (IS_ERR(aead)) {
> +		ret = PTR_ERR(aead);
> +		pr_err("%s crypto_alloc_aead failed, err=%d\n", title, ret);
> +		aead = NULL;
> +		goto error;
> +	}
> +
> +	ret = crypto_aead_setkey(aead, key, keylen);
> +	if (ret) {
> +		pr_err("%s crypto_aead_setkey size=%u failed, err=%d\n", title,
> +		       keylen, ret);
> +		goto error;
> +	}
> +
> +	ret = crypto_aead_setauthsize(aead, AUTH_TAG_SIZE);
> +	if (ret) {
> +		pr_err("%s crypto_aead_setauthsize failed, err=%d\n", title,
> +		       ret);

Could use another #define pr_fmt(fmt) etc...