Jakub Kicinski wrote:
> On Wed, 10 Jul 2019 12:34:17 -0700, Jakub Kicinski wrote:
> > > > > + if (sk->sk_prot->unhash)
> > > > > + sk->sk_prot->unhash(sk);
> > > > > + }
> > > > > +
> > > > > + ctx = tls_get_ctx(sk);
> > > > > + if (ctx->tx_conf == TLS_SW || ctx->rx_conf == TLS_SW)
> > > > > + tls_sk_proto_cleanup(sk, ctx, timeo);
>
> Do we still need to hook into unhash? With patch 6 in place perhaps we
> can just do disconnect 🥺
?? "can just do a disconnect", not sure I folow. We still need unhash
in cases where we have a TLS socket transition from ESTABLISHED
to LISTEN state without calling close(). This is independent of if
sockmap is running or not.
Originally, I thought this would be extremely rare but I did see it
in real applications on the sockmap side so presumably it is possible
here as well.
>
> cleanup is going to kick off TX but also:
>
> if (unlikely(sk->sk_write_pending) &&
> !wait_on_pending_writer(sk, &timeo))
> tls_handle_open_record(sk, 0);
>
> Are we guaranteed that sk_write_pending is 0? Otherwise
> wait_on_pending_writer is hiding yet another release_sock() :(
Not seeing the path to release_sock() at the moment?
tls_handle_open_record
push_pending_record
tls_sw_push_pending_record
bpf_exec_tx_verdict
If bpf_exec_tx_verdict does a redirect we could hit a relase but that
is another fix I have to get queued up shortly. I think we can fix
that in another series.
