On Fri, Sep 25, 2020 at 11:25:03PM +0200, Toke Høiland-Jørgensen wrote:
>
> int bpf_check_attach_target(struct bpf_verifier_log *log,
> const struct bpf_prog *prog,
> - const struct bpf_prog *tgt_prog,
> + const struct bpf_prog *dst_prog,
so you really did blind search and replace?
That's not at all what I was asking.
The function is called check_attach_target and the argument name
'tgt_prog' fits perfectly.
> u32 btf_id,
> struct bpf_attach_target_info *tgt_info);
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 868c03a24d0a..faf57c6f8804 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3706,10 +3706,10 @@ struct btf *btf_parse_vmlinux(void)
>
> struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog)
> {
> - struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> + struct bpf_prog *dst_prog = prog->aux->dst_prog;
same here. tgt_prog fits just fine as a name.
>
> - if (tgt_prog) {
> - return tgt_prog->aux->btf;
> + if (dst_prog) {
> + return dst_prog->aux->btf;
> } else {
> return btf_vmlinux;
> }
> @@ -3733,7 +3733,7 @@ bool btf_ctx_access(int off, int size, enum
> bpf_access_type type,
> struct bpf_insn_access_aux *info)
> {
> const struct btf_type *t = prog->aux->attach_func_proto;
> - struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> + struct bpf_prog *dst_prog = prog->aux->dst_prog;
here as well.
it's a tgt_prog being checked.
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2740df19f55e..099a651efe8b 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2154,14 +2154,14 @@ static int bpf_prog_load(union bpf_attr *attr, union
> bpf_attr __user *uattr)
> prog->expected_attach_type = attr->expected_attach_type;
> prog->aux->attach_btf_id = attr->attach_btf_id;
> if (attr->attach_prog_fd) {
> - struct bpf_prog *tgt_prog;
> + struct bpf_prog *dst_prog;
>
> - tgt_prog = bpf_prog_get(attr->attach_prog_fd);
> - if (IS_ERR(tgt_prog)) {
> - err = PTR_ERR(tgt_prog);
> + dst_prog = bpf_prog_get(attr->attach_prog_fd);
> + if (IS_ERR(dst_prog)) {
> + err = PTR_ERR(dst_prog);
> goto free_prog_nouncharge;
> }
> - prog->aux->linked_prog = tgt_prog;
> + prog->aux->dst_prog = dst_prog;
Here 'dst_prog' makes logical sense, but I wouldn't bother renaming.
You can keep this hunk, if you like.
> int bpf_check_attach_target(struct bpf_verifier_log *log,
> const struct bpf_prog *prog,
> - const struct bpf_prog *tgt_prog,
> + const struct bpf_prog *dst_prog,
pls keep it as 'tgt_prog' here and through the function.
> static int check_attach_btf_id(struct bpf_verifier_env *env)
> {
> struct bpf_prog *prog = env->prog;
> - struct bpf_prog *tgt_prog = prog->aux->linked_prog;
> + struct bpf_prog *dst_prog = prog->aux->dst_prog;
no need to rename either. It's a target program being checked.
