diff mbox

[Crypto,v6,03/12] tls: support for inline tls

Message ID 1519022990-23126-2-git-send-email-atul.gupta@chelsio.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show

Commit Message

Atul Gupta Feb. 19, 2018, 6:49 a.m. UTC 
Facility to register Inline TLS drivers to net/tls. Setup
TLS_FULL_HW prot to listen on offload device.

Cases handled
1. Inline TLS device exists, setup prot for TLS_FULL_HW
2. Atleast one Inline TLS exists, sets TLS_FULL_HW. If
non-inline capable device establish connection, move to TLS_SW_TX
3. default mode TLS_SW_TX continues

Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
 net/tls/tls_main.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 116 insertions(+), 7 deletions(-)

Comments

David Miller Feb. 20, 2018, 4:15 p.m. UTC | #1 
From: Atul Gupta <atul.gupta@chelsio.com>
Date: Mon, 19 Feb 2018 12:19:41 +0530

> +	struct net_device *netdev = NULL;
> +
> +	netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);

No need for an assignment in the variable declaration here.
You immediately set it to something else unconditionally.

> +static int get_tls_offload_dev(struct sock *sk)
> +{
> +	struct net_device *netdev;
> +	struct tls_device *dev;
> +	int rc = 0;
> +
> +	netdev = get_netdev(sk);
> +	if (!netdev)
> +		return -EINVAL;
> +
> +	mutex_lock(&device_mutex);
> +	list_for_each_entry(dev, &device_list, dev_list) {
> +		if (dev->netdev && dev->netdev(dev, netdev)) {
> +			rc = -EEXIST;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&device_mutex);
> +	dev_put(netdev);
> +	return rc;
> +}

This is really a confusing function.

It's name suggests that it "gets" the offload device.  In that case,
if it is found it should return success.  Instead we get an -EEXIST
error in that case.  And it returns 0 if not found.

Better to make this do what it says it does, which would be to return
'0' when the device is found and return -ENODEV when it is not found.

> +	tcp_prot.unhash(sk);

Do not force this to the ipv4 TCP instance, use the pointer through
the socket to call the proper unhash method.

> +	err = tcp_prot.hash(sk);

Likewise.
Atul Gupta Feb. 20, 2018, 4:42 p.m. UTC | #2 
-----Original Message-----
From: linux-crypto-owner@vger.kernel.org [mailto:linux-crypto-owner@vger.kernel.org] On Behalf Of David Miller
Sent: Tuesday, February 20, 2018 9:46 PM
To: Atul Gupta <atul.gupta@chelsio.com>
Cc: davejwatson@fb.com; herbert@gondor.apana.org.au; sd@queasysnail.net; linux-crypto@vger.kernel.org; netdev@vger.kernel.org; Ganesh GR <ganeshgr@chelsio.com>
Subject: Re: [Crypto v6 03/12] tls: support for inline tls

From: Atul Gupta <atul.gupta@chelsio.com>
Date: Mon, 19 Feb 2018 12:19:41 +0530

> +	struct net_device *netdev = NULL;
> +
> +	netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);

No need for an assignment in the variable declaration here.
You immediately set it to something else unconditionally.
Sure.

> +static int get_tls_offload_dev(struct sock *sk) {
> +	struct net_device *netdev;
> +	struct tls_device *dev;
> +	int rc = 0;
> +
> +	netdev = get_netdev(sk);
> +	if (!netdev)
> +		return -EINVAL;
> +
> +	mutex_lock(&device_mutex);
> +	list_for_each_entry(dev, &device_list, dev_list) {
> +		if (dev->netdev && dev->netdev(dev, netdev)) {
> +			rc = -EEXIST;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&device_mutex);
> +	dev_put(netdev);
> +	return rc;
> +}

This is really a confusing function.

It's name suggests that it "gets" the offload device.  In that case, if it is found it should return success.  Instead we get an -EEXIST error in that case.  And it returns 0 if not found.
Will rename to no_tls_offload_dev, return 0 is used as default condition in calling function.

Better to make this do what it says it does, which would be to return '0' when the device is found and return -ENODEV when it is not found.

> +	tcp_prot.unhash(sk);
Done

Do not force this to the ipv4 TCP instance, use the pointer through the socket to call the proper unhash method.

> +	err = tcp_prot.hash(sk);
Done

Likewise.
Thanks
diff mbox

Patch

diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index b0d5fce..88eafec 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -38,6 +38,7 @@ 
 #include <linux/highmem.h>
 #include <linux/netdevice.h>
 #include <linux/sched/signal.h>
+#include <linux/inetdevice.h>
 
 #include <net/tls.h>
 
@@ -45,13 +46,9 @@ 
 MODULE_DESCRIPTION("Transport Layer Security Support");
 MODULE_LICENSE("Dual BSD/GPL");
 
-enum {
-	TLS_BASE_TX,
-	TLS_SW_TX,
-	TLS_NUM_CONFIG,
-};
-
-static struct proto tls_prots[TLS_NUM_CONFIG];
+static LIST_HEAD(device_list);
+static DEFINE_MUTEX(device_mutex);
+struct proto tls_prots[TLS_NUM_CONFIG];
 
 static inline void update_sk_prot(struct sock *sk, struct tls_context *ctx)
 {
@@ -260,6 +257,37 @@  static void tls_sk_proto_close(struct sock *sk, long timeout)
 	sk_proto_close(sk, timeout);
 }
 
+static struct net_device *get_netdev(struct sock *sk)
+{
+	struct inet_sock *inet = inet_sk(sk);
+	struct net_device *netdev = NULL;
+
+	netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);
+	return netdev;
+}
+
+static int get_tls_offload_dev(struct sock *sk)
+{
+	struct net_device *netdev;
+	struct tls_device *dev;
+	int rc = 0;
+
+	netdev = get_netdev(sk);
+	if (!netdev)
+		return -EINVAL;
+
+	mutex_lock(&device_mutex);
+	list_for_each_entry(dev, &device_list, dev_list) {
+		if (dev->netdev && dev->netdev(dev, netdev)) {
+			rc = -EEXIST;
+			break;
+		}
+	}
+	mutex_unlock(&device_mutex);
+	dev_put(netdev);
+	return rc;
+}
+
 static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
 				int __user *optlen)
 {
@@ -403,6 +431,15 @@  static int do_tls_setsockopt_tx(struct sock *sk, char __user *optval,
 		goto err_crypto_info;
 	}
 
+	rc = get_tls_offload_dev(sk);
+	if (rc == -EINVAL) {
+		goto out;
+	} else if (rc == -EEXIST) {
+		/* Retain HW unhash for cleanup and move to SW Tx */
+		sk->sk_prot[TLS_BASE_TX].unhash =
+			sk->sk_prot[TLS_FULL_HW].unhash;
+	}
+
 	/* currently SW is default, we will have ethtool in future */
 	rc = tls_set_sw_offload(sk, ctx);
 	tx_conf = TLS_SW_TX;
@@ -450,6 +487,54 @@  static int tls_setsockopt(struct sock *sk, int level, int optname,
 	return do_tls_setsockopt(sk, optname, optval, optlen);
 }
 
+static int tls_hw_prot(struct sock *sk)
+{
+	struct tls_context *ctx = tls_get_ctx(sk);
+	struct tls_device *dev;
+
+	mutex_lock(&device_mutex);
+	list_for_each_entry(dev, &device_list, dev_list) {
+		if (dev->feature && dev->feature(dev)) {
+			ctx->tx_conf = TLS_FULL_HW;
+			update_sk_prot(sk, ctx);
+			break;
+		}
+	}
+	mutex_unlock(&device_mutex);
+	return ctx->tx_conf;
+}
+
+static void tls_hw_unhash(struct sock *sk)
+{
+	struct tls_device *dev;
+
+	mutex_lock(&device_mutex);
+	list_for_each_entry(dev, &device_list, dev_list) {
+		if (dev->unhash)
+			dev->unhash(dev, sk);
+	}
+	mutex_unlock(&device_mutex);
+	tcp_prot.unhash(sk);
+}
+
+static int tls_hw_hash(struct sock *sk)
+{
+	struct tls_device *dev;
+	int err;
+
+	err = tcp_prot.hash(sk);
+	mutex_lock(&device_mutex);
+	list_for_each_entry(dev, &device_list, dev_list) {
+		if (dev->hash)
+			err |= dev->hash(dev, sk);
+	}
+	mutex_unlock(&device_mutex);
+
+	if (err)
+		tls_hw_unhash(sk);
+	return err;
+}
+
 static int tls_init(struct sock *sk)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
@@ -477,6 +562,9 @@  static int tls_init(struct sock *sk)
 	ctx->sk_proto_close = sk->sk_prot->close;
 
 	ctx->tx_conf = TLS_BASE_TX;
+	if (tls_hw_prot(sk) == TLS_FULL_HW)
+		goto out;
+
 	update_sk_prot(sk, ctx);
 out:
 	return rc;
@@ -500,7 +588,27 @@  static void build_protos(struct proto *prot, struct proto *base)
 	prot[TLS_SW_TX] = prot[TLS_BASE_TX];
 	prot[TLS_SW_TX].sendmsg		= tls_sw_sendmsg;
 	prot[TLS_SW_TX].sendpage	= tls_sw_sendpage;
+
+	prot[TLS_FULL_HW] = prot[TLS_BASE_TX];
+	prot[TLS_FULL_HW].hash		= tls_hw_hash;
+	prot[TLS_FULL_HW].unhash	= tls_hw_unhash;
+}
+
+void tls_register_device(struct tls_device *device)
+{
+	mutex_lock(&device_mutex);
+	list_add_tail(&device->dev_list, &device_list);
+	mutex_unlock(&device_mutex);
+}
+EXPORT_SYMBOL(tls_register_device);
+
+void tls_unregister_device(struct tls_device *device)
+{
+	mutex_lock(&device_mutex);
+	list_del(&device->dev_list);
+	mutex_unlock(&device_mutex);
 }
+EXPORT_SYMBOL(tls_unregister_device);
 
 static int __init tls_register(void)
 {
@@ -510,6 +618,7 @@  static int __init tls_register(void)
 
 	return 0;
 }
+EXPORT_SYMBOL(tls_prots);
 
 static void __exit tls_unregister(void)
 {