lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 19 Jul 2022 12:00:01 +0800
From:   Pu Lehui <pulehui@...wei.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     bpf <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Quentin Monnet <quentin@...valent.com>,
        Martin KaFai Lau <kafai@...com>,
        "Song Liu" <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "Jean-Philippe Brucker" <jean-philippe@...aro.org>
Subject: Re: [PATCH bpf-next v2 0/5] cleanup for data casting



On 2022/7/19 11:12, Alexei Starovoitov wrote:
> On Mon, Jul 18, 2022 at 5:59 AM Pu Lehui <pulehui@...wei.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.

Powered by blists - more mailing lists