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. Thanks, Song
