From: Martin KaFai Lau <kafai@xxxxxx>
Date: Wed, 19 May 2021 23:26:48 -0700
> On Mon, May 17, 2021 at 09:22:50AM +0900, Kuniyuki Iwashima wrote:
>
> > +static int reuseport_resurrect(struct sock *sk, struct sock_reuseport *old_reuse,
> > + struct sock_reuseport *reuse, bool bind_inany)
> > +{
> > + if (old_reuse == reuse) {
> > + /* If sk was in the same reuseport group, just pop sk out of
> > + * the closed section and push sk into the listening section.
> > + */
> > + __reuseport_detach_closed_sock(sk, old_reuse);
> > + __reuseport_add_sock(sk, old_reuse);
> > + return 0;
> > + }
> > +
> > + if (!reuse) {
> > + /* In bind()/listen() path, we cannot carry over the eBPF prog
> > + * for the shutdown()ed socket. In setsockopt() path, we should
> > + * not change the eBPF prog of listening sockets by attaching a
> > + * prog to the shutdown()ed socket. Thus, we will allocate a new
> > + * reuseport group and detach sk from the old group.
> > + */
> For the reuseport_attach_prog() path, I think it needs to consider
> the reuse->num_closed_socks != 0 case also and that should belong
> to the resurrect case. For example, when
> sk_unhashed(sk) but sk->sk_reuseport == 0.
In the path, reuseport_resurrect() is called from reuseport_alloc() only
if reuse->num_closed_socks != 0.
> @@ -92,6 +117,14 @@ int reuseport_alloc(struct sock *sk, bool bind_inany)
> reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
> lockdep_is_held(&reuseport_lock));
> if (reuse) {
> + if (reuse->num_closed_socks) {
But, should this be
if (sk->sk_state == TCP_CLOSE && reuse->num_closed_socks)
because we need not allocate a new group when we attach a bpf prog to
listeners?
> + /* sk was shutdown()ed before */
> + int err = reuseport_resurrect(sk, reuse, NULL, bind_inany);
> +
> + spin_unlock_bh(&reuseport_lock);
> + return err;
> + }
> +
> /* Only set reuse->bind_inany if the bind_inany is true.
> * Otherwise, it will overwrite the reuse->bind_inany
> * which was set by the bind/hash path.
