Re: [bpf PATCH 1/2] bpf, sockmap: fix sk->prot unhash op reset

On Wed, Mar 24, 2021 at 7:28 PM John Fastabend [off-list ref] wrote:

Cong Wang wrote:

quoted

On Wed, Mar 24, 2021 at 1:59 PM John Fastabend [off-list ref] wrote:

quoted

diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 47b7c5334c34..ecb5634b4c4a 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c

@@ -754,6 +754,12 @@ static void tls_update(struct sock *sk, struct proto *p,

        ctx = tls_get_ctx(sk);
        if (likely(ctx)) {
+               /* TLS does not have an unhash proto in SW cases, but we need
+                * to ensure we stop using the sock_map unhash routine because
+                * the associated psock is being removed. So use the original
+                * unhash handler.
+                */
+               WRITE_ONCE(sk->sk_prot->unhash, p->unhash);
                ctx->sk_write_space = write_space;
                ctx->sk_proto = p;

It looks awkward to update sk->sk_proto inside tls_update(),
at least when ctx!=NULL.

hmm. It doesn't strike me as paticularly awkward but OK.

I read tls_update() as "updating ctx when it is initialized", with your
patch, we are updating sk->sk_prot->unhash too when updating ctx,
pretty much like a piggyback, hence it reads odd to me.

Thanks.