-----Original Message-----
From: Dave Watson [mailto:davejwat...@fb.com] 
Sent: Friday, February 23, 2018 9:53 PM
To: Atul Gupta <atul.gu...@chelsio.com>
Cc: da...@davemloft.net; herb...@gondor.apana.org.au; s...@queasysnail.net; 
linux-crypto@vger.kernel.org; net...@vger.kernel.org; Ganesh GR 
Subject: Re: [Crypto v7 03/12] tls: support for inline tls

On 02/22/18 11:21 PM, Atul Gupta wrote:
> @@ -403,6 +431,15 @@ static int do_tls_setsockopt_tx(struct sock *sk, char 
> __user *optval,
>               goto err_crypto_info;
>       }
> +     rc = tls_offload_dev_absent(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;

I'm still confused by this, it lookes like it is modifying the global tls_prots 
without taking a lock?  And modifying it for all sockets, not just this one?  
One way to fix might be to always set an unhash in TLS_BASE_TX, and then have a 
function pointer unhash in ctx.

code enters do_tls_setsockopt_tx only for those offload capable dev which does 
not define FULL_HW setsockopt as done by chtls, unhash prot update is required 
for cleanup/revert of setup done in tls_hw_hash. This update does not impact SW 
or other Inline HW path. 

> +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);
> +     sk->sk_prot->unhash(sk);

I would have thought unhash() here was tls_hw_unhash, doesn't the original 
callback need to be saved like the other ones (set/getsockopt, etc) in 
tls_init?  Similar for hash().
Yes, good to store it or have it the way I had in v6 [tcp_prot.hash], can this 
correction go in patch than submit the whole series?

It looks like in patch 11 you directly call tcp_prot.hash/unhash, so it doesn't 
have this issue.

Reply via email to