| Message ID | 20220718132938.1031864-1-pulehui@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | cleanup for data casting | expand |
On Mon, Jul 18, 2022 at 5:59 AM Pu Lehui <pulehui@huawei.com> wrote: > > Previously, we found that memory address casting in libbpf > was not appropriate [0]. Memory addresses are conceptually > unsigned, (unsigned long) casting makes more sense. With the > suggestion of Daniel, we applied this cleanup to the entire > bpf, and there is no functional change. > > [0] https://lore.kernel.org/bpf/a31efed5-a436-49c9-4126-902303df9766@iogearbox.net/ > > v2: > - update subject and commit msg of patch 5. > - attach ack of Yonghong. > > v1: https://lore.kernel.org/bpf/20220716125108.1011206-1-pulehui@huawei.com/ > > Pu Lehui (5): > bpf: Unify memory address casting operation style > libbpf: Unify memory address casting operation style > selftests: bpf: Unify memory address casting operation style > samples: bpf: Unify memory address casting operation style > selftests/bpf: Change the casting about jited_ksyms and jited_linfo > > kernel/bpf/core.c | 2 +- > kernel/bpf/helpers.c | 6 +-- > kernel/bpf/syscall.c | 2 +- > kernel/bpf/verifier.c | 6 +-- > samples/bpf/parse_simple.c | 4 +- > samples/bpf/parse_varlen.c | 4 +- > samples/bpf/tc_l2_redirect_kern.c | 16 +++---- > samples/bpf/test_cgrp2_tc_kern.c | 4 +- > samples/bpf/test_lwt_bpf.c | 4 +- > samples/bpf/xdp_adjust_tail_kern.c | 12 ++--- > samples/bpf/xdp_fwd_kern.c | 4 +- > samples/bpf/xdp_redirect.bpf.c | 4 +- > samples/bpf/xdp_redirect_cpu.bpf.c | 48 +++++++++---------- > samples/bpf/xdp_redirect_map.bpf.c | 8 ++-- > samples/bpf/xdp_redirect_map_multi.bpf.c | 4 +- > samples/bpf/xdp_router_ipv4.bpf.c | 4 +- > samples/bpf/xdp_rxq_info_kern.c | 4 +- > samples/bpf/xdp_sample_pkts_kern.c | 4 +- > samples/bpf/xdp_tx_iptunnel_kern.c | 20 ++++---- > tools/lib/bpf/bpf_prog_linfo.c | 8 ++-- > tools/lib/bpf/btf.c | 7 +-- > tools/lib/bpf/skel_internal.h | 4 +- > tools/lib/bpf/usdt.c | 4 +- > tools/testing/selftests/bpf/bench.c | 4 +- > .../selftests/bpf/prog_tests/bpf_obj_id.c | 10 ++-- > .../selftests/bpf/prog_tests/bpf_tcp_ca.c | 2 +- > tools/testing/selftests/bpf/prog_tests/btf.c | 20 ++++---- > .../bpf/prog_tests/core_read_macros.c | 8 ++-- > .../selftests/bpf/prog_tests/hashmap.c | 8 ++-- > .../selftests/bpf/prog_tests/ringbuf.c | 4 +- > .../selftests/bpf/prog_tests/ringbuf_multi.c | 4 +- > .../bpf/prog_tests/sockopt_inherit.c | 2 +- > tools/testing/selftests/bpf/progs/bpf_flow.c | 10 ++-- > tools/testing/selftests/bpf/progs/core_kern.c | 4 +- > .../selftests/bpf/progs/fexit_bpf2bpf.c | 8 ++-- > tools/testing/selftests/bpf/progs/pyperf.h | 4 +- > .../testing/selftests/bpf/progs/skb_pkt_end.c | 4 +- > .../selftests/bpf/progs/sockmap_parse_prog.c | 8 ++-- > .../bpf/progs/sockmap_verdict_prog.c | 4 +- > .../bpf/progs/test_btf_skc_cls_ingress.c | 8 ++-- > .../selftests/bpf/progs/test_check_mtu.c | 16 +++---- > .../selftests/bpf/progs/test_cls_redirect.c | 8 ++-- > tools/testing/selftests/bpf/progs/test_l4lb.c | 6 +-- > .../selftests/bpf/progs/test_l4lb_noinline.c | 6 +-- > .../selftests/bpf/progs/test_lwt_seg6local.c | 10 ++-- > .../bpf/progs/test_migrate_reuseport.c | 4 +- > .../selftests/bpf/progs/test_pkt_access.c | 8 ++-- > .../bpf/progs/test_queue_stack_map.h | 4 +- > .../selftests/bpf/progs/test_seg6_loop.c | 8 ++-- > .../selftests/bpf/progs/test_sk_assign.c | 8 ++-- > .../selftests/bpf/progs/test_sk_lookup_kern.c | 4 +- > .../selftests/bpf/progs/test_sockmap_kern.h | 12 ++--- > .../selftests/bpf/progs/test_tc_dtime.c | 2 +- > .../testing/selftests/bpf/progs/test_tc_edt.c | 6 +-- > .../selftests/bpf/progs/test_tc_neigh.c | 2 +- > .../selftests/bpf/progs/test_tc_neigh_fib.c | 2 +- > .../bpf/progs/test_tcp_check_syncookie_kern.c | 8 ++-- > .../selftests/bpf/progs/test_tunnel_kern.c | 12 ++--- > .../selftests/bpf/progs/test_verif_scale1.c | 4 +- > .../selftests/bpf/progs/test_verif_scale2.c | 4 +- > .../selftests/bpf/progs/test_verif_scale3.c | 4 +- > tools/testing/selftests/bpf/progs/test_xdp.c | 20 ++++---- > .../bpf/progs/test_xdp_adjust_tail_grow.c | 4 +- > .../bpf/progs/test_xdp_adjust_tail_shrink.c | 4 +- > .../selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 +- > .../bpf/progs/test_xdp_context_test_run.c | 4 +- > .../bpf/progs/test_xdp_devmap_helpers.c | 4 +- > .../bpf/progs/test_xdp_do_redirect.c | 14 +++--- > .../selftests/bpf/progs/test_xdp_loop.c | 20 ++++---- > .../selftests/bpf/progs/test_xdp_noinline.c | 28 +++++------ > .../bpf/progs/test_xdp_update_frags.c | 4 +- > .../selftests/bpf/progs/test_xdp_vlan.c | 16 +++---- > .../bpf/progs/test_xdp_with_devmap_helpers.c | 4 +- > .../bpf/progs/xdp_redirect_multi_kern.c | 8 ++-- > .../selftests/bpf/progs/xdp_synproxy_kern.c | 16 +++---- > .../testing/selftests/bpf/progs/xdping_kern.c | 12 ++--- > tools/testing/selftests/bpf/progs/xdpwall.c | 4 +- > 77 files changed, 301 insertions(+), 298 deletions(-) I really don't like it. Looks like a huge amount of churn for no good reason. (void *)(long) is shorter and easier to read.
On 2022/7/19 11:12, Alexei Starovoitov wrote: > On Mon, Jul 18, 2022 at 5:59 AM Pu Lehui <pulehui@huawei.com> wrote: >> >> Previously, we found that memory address casting in libbpf >> was not appropriate [0]. Memory addresses are conceptually >> unsigned, (unsigned long) casting makes more sense. With the >> suggestion of Daniel, we applied this cleanup to the entire >> bpf, and there is no functional change. >> >> [0] https://lore.kernel.org/bpf/a31efed5-a436-49c9-4126-902303df9766@iogearbox.net/ >> >> v2: >> - update subject and commit msg of patch 5. >> - attach ack of Yonghong. >> >> v1: https://lore.kernel.org/bpf/20220716125108.1011206-1-pulehui@huawei.com/ >> >> Pu Lehui (5): >> bpf: Unify memory address casting operation style >> libbpf: Unify memory address casting operation style >> selftests: bpf: Unify memory address casting operation style >> samples: bpf: Unify memory address casting operation style >> selftests/bpf: Change the casting about jited_ksyms and jited_linfo >> >> kernel/bpf/core.c | 2 +- >> kernel/bpf/helpers.c | 6 +-- >> kernel/bpf/syscall.c | 2 +- >> kernel/bpf/verifier.c | 6 +-- >> samples/bpf/parse_simple.c | 4 +- >> samples/bpf/parse_varlen.c | 4 +- >> samples/bpf/tc_l2_redirect_kern.c | 16 +++---- >> samples/bpf/test_cgrp2_tc_kern.c | 4 +- >> samples/bpf/test_lwt_bpf.c | 4 +- >> samples/bpf/xdp_adjust_tail_kern.c | 12 ++--- >> samples/bpf/xdp_fwd_kern.c | 4 +- >> samples/bpf/xdp_redirect.bpf.c | 4 +- >> samples/bpf/xdp_redirect_cpu.bpf.c | 48 +++++++++---------- >> samples/bpf/xdp_redirect_map.bpf.c | 8 ++-- >> samples/bpf/xdp_redirect_map_multi.bpf.c | 4 +- >> samples/bpf/xdp_router_ipv4.bpf.c | 4 +- >> samples/bpf/xdp_rxq_info_kern.c | 4 +- >> samples/bpf/xdp_sample_pkts_kern.c | 4 +- >> samples/bpf/xdp_tx_iptunnel_kern.c | 20 ++++---- >> tools/lib/bpf/bpf_prog_linfo.c | 8 ++-- >> tools/lib/bpf/btf.c | 7 +-- >> tools/lib/bpf/skel_internal.h | 4 +- >> tools/lib/bpf/usdt.c | 4 +- >> tools/testing/selftests/bpf/bench.c | 4 +- >> .../selftests/bpf/prog_tests/bpf_obj_id.c | 10 ++-- >> .../selftests/bpf/prog_tests/bpf_tcp_ca.c | 2 +- >> tools/testing/selftests/bpf/prog_tests/btf.c | 20 ++++---- >> .../bpf/prog_tests/core_read_macros.c | 8 ++-- >> .../selftests/bpf/prog_tests/hashmap.c | 8 ++-- >> .../selftests/bpf/prog_tests/ringbuf.c | 4 +- >> .../selftests/bpf/prog_tests/ringbuf_multi.c | 4 +- >> .../bpf/prog_tests/sockopt_inherit.c | 2 +- >> tools/testing/selftests/bpf/progs/bpf_flow.c | 10 ++-- >> tools/testing/selftests/bpf/progs/core_kern.c | 4 +- >> .../selftests/bpf/progs/fexit_bpf2bpf.c | 8 ++-- >> tools/testing/selftests/bpf/progs/pyperf.h | 4 +- >> .../testing/selftests/bpf/progs/skb_pkt_end.c | 4 +- >> .../selftests/bpf/progs/sockmap_parse_prog.c | 8 ++-- >> .../bpf/progs/sockmap_verdict_prog.c | 4 +- >> .../bpf/progs/test_btf_skc_cls_ingress.c | 8 ++-- >> .../selftests/bpf/progs/test_check_mtu.c | 16 +++---- >> .../selftests/bpf/progs/test_cls_redirect.c | 8 ++-- >> tools/testing/selftests/bpf/progs/test_l4lb.c | 6 +-- >> .../selftests/bpf/progs/test_l4lb_noinline.c | 6 +-- >> .../selftests/bpf/progs/test_lwt_seg6local.c | 10 ++-- >> .../bpf/progs/test_migrate_reuseport.c | 4 +- >> .../selftests/bpf/progs/test_pkt_access.c | 8 ++-- >> .../bpf/progs/test_queue_stack_map.h | 4 +- >> .../selftests/bpf/progs/test_seg6_loop.c | 8 ++-- >> .../selftests/bpf/progs/test_sk_assign.c | 8 ++-- >> .../selftests/bpf/progs/test_sk_lookup_kern.c | 4 +- >> .../selftests/bpf/progs/test_sockmap_kern.h | 12 ++--- >> .../selftests/bpf/progs/test_tc_dtime.c | 2 +- >> .../testing/selftests/bpf/progs/test_tc_edt.c | 6 +-- >> .../selftests/bpf/progs/test_tc_neigh.c | 2 +- >> .../selftests/bpf/progs/test_tc_neigh_fib.c | 2 +- >> .../bpf/progs/test_tcp_check_syncookie_kern.c | 8 ++-- >> .../selftests/bpf/progs/test_tunnel_kern.c | 12 ++--- >> .../selftests/bpf/progs/test_verif_scale1.c | 4 +- >> .../selftests/bpf/progs/test_verif_scale2.c | 4 +- >> .../selftests/bpf/progs/test_verif_scale3.c | 4 +- >> tools/testing/selftests/bpf/progs/test_xdp.c | 20 ++++---- >> .../bpf/progs/test_xdp_adjust_tail_grow.c | 4 +- >> .../bpf/progs/test_xdp_adjust_tail_shrink.c | 4 +- >> .../selftests/bpf/progs/test_xdp_bpf2bpf.c | 4 +- >> .../bpf/progs/test_xdp_context_test_run.c | 4 +- >> .../bpf/progs/test_xdp_devmap_helpers.c | 4 +- >> .../bpf/progs/test_xdp_do_redirect.c | 14 +++--- >> .../selftests/bpf/progs/test_xdp_loop.c | 20 ++++---- >> .../selftests/bpf/progs/test_xdp_noinline.c | 28 +++++------ >> .../bpf/progs/test_xdp_update_frags.c | 4 +- >> .../selftests/bpf/progs/test_xdp_vlan.c | 16 +++---- >> .../bpf/progs/test_xdp_with_devmap_helpers.c | 4 +- >> .../bpf/progs/xdp_redirect_multi_kern.c | 8 ++-- >> .../selftests/bpf/progs/xdp_synproxy_kern.c | 16 +++---- >> .../testing/selftests/bpf/progs/xdping_kern.c | 12 ++--- >> tools/testing/selftests/bpf/progs/xdpwall.c | 4 +- >> 77 files changed, 301 insertions(+), 298 deletions(-) > > I really don't like it. > Looks like a huge amount of churn for no good reason. > (void *)(long) is shorter and easier to read. > . > Hi Alexei, In fact, this cleanup can be done or not, just to fulfill the promise [0], accept or reject it is fine for me. Actually, I'm more interested in feature improvements or issue fixes. [0] https://lore.kernel.org/bpf/a31efed5-a436-49c9-4126-902303df9766@iogearbox.net/ Lehui, Thanks.
>> On Mon, Jul 18, 2022 at 5:59 AM Pu Lehui <pulehui@huawei.com> wrote: >>> Previously, we found that memory address casting in libbpf >>> was not appropriate [0]. Memory addresses are conceptually >>> unsigned, (unsigned long) casting makes more sense. With the >>> suggestion of Daniel, we applied this cleanup to the entire >>> bpf, and there is no functional change. Fwiw, pointers in C aren't necessarily unsigned; some versions of gcc have treated them as signed and — if no object can straddle the sign boundary — it's even allowed by the standard. [1] (And at a hardware level, a memory address is just a pattern of bits on an address bus, which isn't arithmetic at all.) -ed [1]: https://yarchive.net/comp/linux/signed_pointers.html