On Thu, Nov 11, 2021 at 9:02 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> From: Alexei Starovoitov <ast@xxxxxxxxxx>
>
> Make relo_core.c to be compiled with kernel and with libbpf.
>
> Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> ---
> include/linux/btf.h | 82 +++++++++++++++++++++++++++++++++++++++
> kernel/bpf/Makefile | 4 ++
> kernel/bpf/btf.c | 26 +++++++++++++
> tools/lib/bpf/relo_core.c | 71 ++++++++++++++++++++++++++++-----
> 4 files changed, 174 insertions(+), 9 deletions(-)
>
[...]
> static inline const struct btf_member *btf_type_member(const struct btf_type *t)
> {
> return (const struct btf_member *)(t + 1);
> }
>
> +
accidental empty line or intentional?
> +static inline struct btf_array *btf_array(const struct btf_type *t)
> +{
> + return (struct btf_array *)(t + 1);
> +}
> +
[...]
> +int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
> + const struct btf *targ_btf, __u32 targ_id)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static bool bpf_core_is_flavor_sep(const char *s)
> +{
> + /* check X___Y name pattern, where X and Y are not underscores */
> + return s[0] != '_' && /* X */
> + s[1] == '_' && s[2] == '_' && s[3] == '_' && /* ___ */
> + s[4] != '_'; /* Y */
> +}
> +
> +size_t bpf_core_essential_name_len(const char *name)
I might have missed something, but this seems to be used only
internally, so should be static. Otherwise there would be a
compilation warning due to the missing prototype, no?
> +{
> + size_t n = strlen(name);
> + int i;
> +
> + for (i = n - 5; i >= 0; i--) {
> + if (bpf_core_is_flavor_sep(name + i))
> + return i + 1;
> + }
> + return n;
> +}
[...]
> + t = btf_type_by_id(btf, type_id);
> + t = btf_resolve_size(btf, t, &size);
> + if (IS_ERR(t))
> + return PTR_ERR(t);
> + return size;
> +}
nit: empty line would be good here and after enum
> +enum libbpf_print_level {
> + LIBBPF_WARN,
> + LIBBPF_INFO,
> + LIBBPF_DEBUG,
> +};
> +#undef pr_warn
> +#undef pr_info
> +#undef pr_debug
> +#define pr_warn(fmt, log, ...) bpf_log((void *)log, fmt, "", ##__VA_ARGS__)
> +#define pr_info(fmt, log, ...) bpf_log((void *)log, fmt, "", ##__VA_ARGS__)
> +#define pr_debug(fmt, log, ...) bpf_log((void *)log, fmt, "", ##__VA_ARGS__)
> +#define libbpf_print(level, fmt, ...) bpf_log((void *)prog_name, fmt, ##__VA_ARGS__)
> +#else
> #include <stdio.h>
> #include <string.h>
> #include <errno.h>
> @@ -12,8 +64,9 @@
> #include "btf.h"
> #include "str_error.h"
> #include "libbpf_internal.h"
> +#endif
>
> -#define BPF_CORE_SPEC_MAX_LEN 64
> +#define BPF_CORE_SPEC_MAX_LEN 32
This is worth calling out in the commit description, should have
practical implications, but good to mention.
>
> /* represents BPF CO-RE field or array element accessor */
> struct bpf_core_accessor {
> @@ -272,8 +325,8 @@ static int bpf_core_parse_spec(const struct btf *btf,
> return sz;
> spec->bit_offset += access_idx * sz * 8;
> } else {
> - pr_warn("relo for [%u] %s (at idx %d) captures type [%d] of unexpected kind %s\n",
> - type_id, spec_str, i, id, btf_kind_str(t));
> +/* pr_warn("relo for [%u] %s (at idx %d) captures type [%d] of unexpected kind %s\n",
> + type_id, spec_str, i, id, btf_kind_str(t));*/
we can totally pass prog_name and add "prog '%s': " to uncomment this.
bpf_core_parse_spec() is called in the "context" of program, so it's
known
> return -EINVAL;
> }
> }
> @@ -346,8 +399,8 @@ static int bpf_core_fields_are_compat(const struct btf *local_btf,
> targ_id = btf_array(targ_type)->type;
> goto recur;
> default:
> - pr_warn("unexpected kind %d relocated, local [%d], target [%d]\n",
> - btf_kind(local_type), local_id, targ_id);
> +/* pr_warn("unexpected kind %d relocated, local [%d], target [%d]\n",
> + btf_kind(local_type), local_id, targ_id);*/
sigh... it's a bit too intrusive to pass prog_name here but it's also
highly unlikely that this happens (unless some compiler bug or
corruption) and even in that case it's semantically correct that
fields just don't match. So I'd just drop this pr_warn() instead of
commenting it out
> return 0;
> }
> }
[...]
