Re: [PATCH bpf-next v5 7/8] libbpf: Support creating light skeleton of either endianness

[Tony Ambardar]

On Tue, Sep 03, 2024 at 12:57:51PM -0700, Alexei Starovoitov wrote:
> On Tue, Sep 3, 2024 at 12:34 AM Tony Ambardar <tony.ambar...@gmail.com> wrote:
> >
> > @@ -1040,10 +1121,11 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, 
> > int map_idx, void *pvalue,
> >         int zero = 0;
> >
> >         memset(&attr, 0, attr_size);
> > -       pr_debug("gen: map_update_elem: idx %d\n", map_idx);
> >
> >         value = add_data(gen, pvalue, value_size);
> >         key = add_data(gen, &zero, sizeof(zero));
> > +       pr_debug("gen: map_update_elem: idx %d, value: off %d size %d\n",
> > +                map_idx, value, value_size);
> >
> >         /* if (map_desc[map_idx].initial_value) {
> >          *    if (ctx->flags & BPF_SKEL_KERNEL)
> > @@ -1068,6 +1150,8 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, 
> > int map_idx, void *pvalue,
> >         emit(gen, BPF_EMIT_CALL(BPF_FUNC_probe_read_kernel));
> >
> >         map_update_attr = add_data(gen, &attr, attr_size);
> > +       pr_debug("gen: map_update_elem: attr: off %d size %d\n",
> > +                map_update_attr, attr_size);
> >         move_blob2blob(gen, attr_field(map_update_attr, map_fd), 4,
> >                        blob_fd_array_off(gen, map_idx));
> >         emit_rel_store(gen, attr_field(map_update_attr, key), key);
> 
> I don't see the point of two pr_debug("gen: map_update_elem...
> just a few lines from each other.
> 
> Other than that:
> Acked-by: Alexei Starovoitov <a...@kernel.org>

Thanks for reviewing, Alexei. I agree those could be consolidated, and I
tested the following patch to do so. I'll include it if another respin is
needed, or someone else could modify during merge otherwise.

--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -1124,8 +1124,6 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, int 
map_idx, void *pvalue,

        value = add_data(gen, pvalue, value_size);
        key = add_data(gen, &zero, sizeof(zero));
-       pr_debug("gen: map_update_elem: idx %d, value: off %d size %d\n",
-                map_idx, value, value_size);

        /* if (map_desc[map_idx].initial_value) {
         *    if (ctx->flags & BPF_SKEL_KERNEL)
@@ -1150,8 +1148,8 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, int 
map_idx, void *pvalue,
        emit(gen, BPF_EMIT_CALL(BPF_FUNC_probe_read_kernel));

        map_update_attr = add_data(gen, &attr, attr_size);
-       pr_debug("gen: map_update_elem: attr: off %d size %d\n",
-                map_update_attr, attr_size);
+       pr_debug("gen: map_update_elem: idx %d, value: off %d size %d, attr: 
off %d size %d\n",
+                map_idx, value, value_size, map_update_attr, attr_size);
        move_blob2blob(gen, attr_field(map_update_attr, map_fd), 4,
                       blob_fd_array_off(gen, map_idx));
        emit_rel_store(gen, attr_field(map_update_attr, key), key);