On 4/25/19 12:32 PM, John Fastabend wrote:
> 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.
>
I'm going to throw that patch I sent earlier in this thread
on the series as well. Its the minimal set to get things working
again for me. Will follow up some selftests so we don't get
here again.
>>> + 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);
>
