On 4/25/19 12:29 PM, Jakub Kicinski wrote: > On Thu, 25 Apr 2019 09:03:08 -0700, John Fastabend wrote: >> +static void tls_sk_proto_unhash(struct sock *sk) >> +{ >> + struct tls_context *ctx = tls_get_ctx(sk); >> + void (*sk_proto_unhash)(struct sock *sk); >> + bool free_ctx; >> + >> + if (!ctx) >> + return sk->sk_prot->unhash(sk); >> + sk_proto_unhash = ctx->sk_proto_unhash; >> + free_ctx = tls_sk_proto_destroy(sk, ctx, false); >> + tls_put_ctx(sk); > > Oh, I think you can't put_ctx() unconditionally, > when free_ctx is false, tls_device_sk_destruct() > needs it the ctx pointer. > > I think this explains the offload crashing. > ugh yeah. So we need to _not_ free it from tls_sk_proto_destroy do the put_ctx and then finally free it. Otherwise we can't restore the sk_proto fields. v3 on its way. Thanks. >> + if (sk_proto_unhash) >> + sk_proto_unhash(sk); >> + if (free_ctx) >> + tls_ctx_free(ctx); >> +} >> >> -skip_tx_cleanup: >> +static void tls_sk_proto_close(struct sock *sk, long timeout) >> +{ >> + void (*sk_proto_close)(struct sock *sk, long timeout); >> + struct tls_context *ctx = tls_get_ctx(sk); >> + bool free_ctx; >> + >> + if (!ctx) >> + return sk->sk_prot->destroy(sk); >> + >> + lock_sock(sk); >> + sk_proto_close = ctx->sk_proto_close; >> + free_ctx = tls_sk_proto_destroy(sk, ctx, true); >> + tls_put_ctx(sk);