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.
> +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().
It looks like in patch 11 you directly call tcp_prot.hash/unhash, so
it doesn't have this issue.
