-----Original Message-----
From: linux-crypto-ow...@vger.kernel.org
[mailto:linux-crypto-ow...@vger.kernel.org] On Behalf Of David Miller
Sent: Tuesday, February 20, 2018 9:46 PM
To: Atul Gupta <atul.gu...@chelsio.com>
Cc: davejwat...@fb.com; herb...@gondor.apana.org.au; s...@queasysnail.net;
linux-cry...@vger.kernel.org; netdev@vger.kernel.org; Ganesh GR
<ganes...@chelsio.com>
Subject: Re: [Crypto v6 03/12] tls: support for inline tls
From: Atul Gupta <atul.gu...@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
