Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <20231018061746.111364-1-zhouchuyi@bytedance.com> <20231018061746.111364-9-zhouchuyi@bytedance.com>
In-Reply-To: <20231018061746.111364-9-zhouchuyi@bytedance.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Thu, 19 Oct 2023 17:03:53 -0700
Message-ID: <CAADnVQKafk_junRyE=-FVAik4hjTRDtThymYGEL8hGTuYoOGpA@mail.gmail.com>
Subject: Re: [RESEND PATCH bpf-next v6 8/8] selftests/bpf: Add tests for
 open-coded task and css iter
To:     Chuyi Zhou <zhouchuyi@bytedance.com>
Cc:     bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@kernel.org>,
        Tejun Heo <tj@kernel.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Oct 17, 2023 at 11:18=E2=80=AFPM Chuyi Zhou <zhouchuyi@bytedance.co=
m> wrote:
>
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +__failure __msg("css_task_iter is only allowed in bpf_lsm and bpf iter-s=
")
> +int BPF_PROG(iter_css_task_for_each)
> +{
> +       u64 cg_id =3D bpf_get_current_cgroup_id();
> +       struct cgroup *cgrp =3D bpf_cgroup_from_id(cg_id);
> +       struct cgroup_subsys_state *css;
> +       struct task_struct *task;
> +
> +       if (cgrp =3D=3D NULL)
> +               return 0;
> +       css =3D &cgrp->self;
> +
> +       bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {
> +
> +       }
> +       bpf_cgroup_release(cgrp);
> +       return 0;
> +}

I think we should relax allowlist in patch 2 further.
Any sleepable is safe.
Allowlist is needed to avoid dead locking on css_set_lock.
Any lsm and any iter (even non-sleepable) and any sleepable
seems to be safe.

Then the above test would need s/fentry.s/fentry/ to stay relevant.

I would also add:

SEC("iter/cgroup")
int cgroup_id_printer(struct bpf_iter__cgroup *ctx)
{
        struct seq_file *seq =3D ctx->meta->seq;
        struct cgroup *cgrp =3D ctx->cgroup;

        /* epilogue */
        if (cgrp =3D=3D NULL) ..

        bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {
           BPF_SEQ_PRINTF(); // something about task
        }

To demonstrate how new kfunc iter can be combined with cgroup iter and
it won't deadlock, though cgroup iter is not sleepable.

I've applied the current set. Pls send a follow up. Thanks