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.ambardar@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 ast@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);