On 02/23/18 04:58 PM, Atul Gupta wrote: > > 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. I still don't follow. If it doesn't impact SW, then what is it doing? According to the comment, we're moving to SW tx, where sk_prot will be &tls_prot[TLS_SW_TX], and the unhash function you set here in TLS_BASE_TX won't be called.