On 10/06, Song Liu wrote:
On Wed, Oct 6, 2021 at 5:41 PM Song Liu <song@xxxxxxxxxx> wrote:
>
> On Wed, Oct 6, 2021 at 9:04 AM YiFei Zhu <zhuyifei1999@xxxxxxxxx> wrote:
> >
> > From: YiFei Zhu <zhuyifei@xxxxxxxxxx>
> >
> > When passed in a positive errno, it sets the errno and returns 0.
> > When passed in 0, it gets the previously set errno. When passed in
> > an out of bound number, it returns -EINVAL. This is unambiguous:
> > negative return values are error in invoking the helper itself,
> > and positive return values are errnos being exported. Errnos once
> > set cannot be unset, but can be overridden.
> >
> > The errno value is stored inside bpf_cg_run_ctx for ease of access
> > different prog types with different context structs layouts. The
> > helper implementation can simply perform a container_of from
> > current->bpf_ctx to retrieve bpf_cg_run_ctx.
> >
> > For backward compatibility, if a program rejects without calling
> > the helper, and the errno has not been set by any prior progs, the
> > BPF_PROG_RUN_ARRAY_CG family macros automatically set the errno to
> > EPERM. If a prog sets an errno but returns 1 (allow), the outcome
> > is considered implementation-defined. This patch treat it the same
> > way as if 0 (reject) is returned.
> >
> > For BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY, the prior behavior is
> > that, if the return value is NET_XMIT_DROP, the packet is silently
> > dropped. We preserve this behavior for backward compatibility
> > reasons, so even if an errno is set, the errno does not return to
> > caller.
> >
> > For getsockopt hooks, they are different in that bpf progs runs
> > after kernel processes the getsockopt syscall instead of before.
> > There is also a retval in its context struct in which bpf progs
> > can unset the retval, and can force an -EPERM by returning 0.
> > We preseve the same semantics. Even though there is retval,
> > that value can only be unset, while progs can set (and not unset)
> > additional errno by using the helper, and that will override
> > whatever is in retval.
> >
> > Signed-off-by: YiFei Zhu <zhuyifei@xxxxxxxxxx>
> > Reviewed-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
>
> This is pretty complicated, but the logic looks all correct. Thus,
>
> Acked-by: Song Liu <songliubraving@xxxxxx>
>
> One question, if the program want to retrieve existing errno_val, and
> set a different one, it needs to call the helper twice, right? I guess
it
> is possible to do that in one call with a "swap" logic. Would this work?
Actually, how about we split this into two helpers:bpf_set_errno() and
bpf_get_errno(). This should avoid some confusion in long term.
We've agreed on the single helper during bpf office hours (about 2 weeks
ago), but we can do two, I don't think it matters that much.
