On Wed, 26 Feb 2020 at 18:37, Martin KaFai Lau <kafai@xxxxxx> wrote:
>
> > +int sk_psock_hooks_install(struct sk_psock_hooks *hooks, struct sock *sk)
> > +{
> > + struct sk_psock *psock = sk_psock(sk);
> > + struct proto *prot_base;
> > +
> > + WARN_ON_ONCE(!rcu_read_lock_held());
> Is this only for the earlier sk_psock(sk)?
The function is an amalgamation of tcp_bpf_reinit and tcp_bpf_init,
which both take the
read lock. I figured it would make sense to assert this behaviour in
sk_psock_hooks_install.
>
> > +
> > + if (unlikely(!psock))
> When will this happen?
I don't know to be honest, this is adapted from tcp_bpf_init:
psock = sk_psock(sk);
if (unlikely(!psock || psock->sk_proto ||
tcp_bpf_assert_proto_ops(ops))) {
rcu_read_unlock();
return -EINVAL;
}
>
> > + return -EINVAL;
> > +
> > + /* Initialize saved callbacks and original proto only once.
> > + * Since we've not installed the hooks, psock is not yet in use and
> > + * we can initialize it without synchronization.
> > + */
> > + if (!psock->sk_proto) {
> If I read it correctly, this is to replace the tcp_bpf_reinit_sk_prot()?
>
> I think some of the current reinit comment is useful to keep also:
>
> /* Reinit occurs when program types change e.g. TCP_BPF_TX is removed ... */
Ack, I will elaborate.
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
