1. 20 Oct, 2023 16 commits
  2. 19 Oct, 2023 2 commits
  3. 18 Oct, 2023 2 commits
  4. 17 Oct, 2023 8 commits
    • Daniel Borkmann's avatar
      selftests/bpf: Add additional mprog query test coverage · 24516309
      Daniel Borkmann authored
      Add several new test cases which assert corner cases on the mprog query
      mechanism, for example, around passing in a too small or a larger array
      than the current count.
      
        ./test_progs -t tc_opts
        #252     tc_opts_after:OK
        #253     tc_opts_append:OK
        #254     tc_opts_basic:OK
        #255     tc_opts_before:OK
        #256     tc_opts_chain_classic:OK
        #257     tc_opts_chain_mixed:OK
        #258     tc_opts_delete_empty:OK
        #259     tc_opts_demixed:OK
        #260     tc_opts_detach:OK
        #261     tc_opts_detach_after:OK
        #262     tc_opts_detach_before:OK
        #263     tc_opts_dev_cleanup:OK
        #264     tc_opts_invalid:OK
        #265     tc_opts_max:OK
        #266     tc_opts_mixed:OK
        #267     tc_opts_prepend:OK
        #268     tc_opts_query:OK
        #269     tc_opts_query_attach:OK
        #270     tc_opts_replace:OK
        #271     tc_opts_revision:OK
        Summary: 20/0 PASSED, 0 SKIPPED, 0 FAILED
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Reviewed-by: default avatarAlan Maguire <alan.maguire@oracle.com>
      Link: https://lore.kernel.org/bpf/20231017081728.24769-1-daniel@iogearbox.net
      24516309
    • Yafang Shao's avatar
      selftests/bpf: Add selftest for bpf_task_under_cgroup() in sleepable prog · 44cb03f1
      Yafang Shao authored
      The result is as follows:
      
        $ tools/testing/selftests/bpf/test_progs --name=task_under_cgroup
        #237     task_under_cgroup:OK
        Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
      
      Without the previous patch, there will be RCU warnings in dmesg when
      CONFIG_PROVE_RCU is enabled. While with the previous patch, there will
      be no warnings.
      Signed-off-by: default avatarYafang Shao <laoar.shao@gmail.com>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarStanislav Fomichev <sdf@google.com>
      Link: https://lore.kernel.org/bpf/20231007135945.4306-2-laoar.shao@gmail.com
      44cb03f1
    • Yafang Shao's avatar
      bpf: Fix missed rcu read lock in bpf_task_under_cgroup() · 29a7e00f
      Yafang Shao authored
      When employed within a sleepable program not under RCU protection, the
      use of 'bpf_task_under_cgroup()' may trigger a warning in the kernel log,
      particularly when CONFIG_PROVE_RCU is enabled:
      
        [ 1259.662357] WARNING: suspicious RCU usage
        [ 1259.662358] 6.5.0+ #33 Not tainted
        [ 1259.662360] -----------------------------
        [ 1259.662361] include/linux/cgroup.h:423 suspicious rcu_dereference_check() usage!
      
      Other info that might help to debug this:
      
        [ 1259.662366] rcu_scheduler_active = 2, debug_locks = 1
        [ 1259.662368] 1 lock held by trace/72954:
        [ 1259.662369]  #0: ffffffffb5e3eda0 (rcu_read_lock_trace){....}-{0:0}, at: __bpf_prog_enter_sleepable+0x0/0xb0
      
      Stack backtrace:
      
        [ 1259.662385] CPU: 50 PID: 72954 Comm: trace Kdump: loaded Not tainted 6.5.0+ #33
        [ 1259.662391] Call Trace:
        [ 1259.662393]  <TASK>
        [ 1259.662395]  dump_stack_lvl+0x6e/0x90
        [ 1259.662401]  dump_stack+0x10/0x20
        [ 1259.662404]  lockdep_rcu_suspicious+0x163/0x1b0
        [ 1259.662412]  task_css_set.part.0+0x23/0x30
        [ 1259.662417]  bpf_task_under_cgroup+0xe7/0xf0
        [ 1259.662422]  bpf_prog_7fffba481a3bcf88_lsm_run+0x5c/0x93
        [ 1259.662431]  bpf_trampoline_6442505574+0x60/0x1000
        [ 1259.662439]  bpf_lsm_bpf+0x5/0x20
        [ 1259.662443]  ? security_bpf+0x32/0x50
        [ 1259.662452]  __sys_bpf+0xe6/0xdd0
        [ 1259.662463]  __x64_sys_bpf+0x1a/0x30
        [ 1259.662467]  do_syscall_64+0x38/0x90
        [ 1259.662472]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
        [ 1259.662479] RIP: 0033:0x7f487baf8e29
        [...]
        [ 1259.662504]  </TASK>
      
      This issue can be reproduced by executing a straightforward program, as
      demonstrated below:
      
      SEC("lsm.s/bpf")
      int BPF_PROG(lsm_run, int cmd, union bpf_attr *attr, unsigned int size)
      {
              struct cgroup *cgrp = NULL;
              struct task_struct *task;
              int ret = 0;
      
              if (cmd != BPF_LINK_CREATE)
                      return 0;
      
              // The cgroup2 should be mounted first
              cgrp = bpf_cgroup_from_id(1);
              if (!cgrp)
                      goto out;
              task = bpf_get_current_task_btf();
              if (bpf_task_under_cgroup(task, cgrp))
                      ret = -1;
              bpf_cgroup_release(cgrp);
      
      out:
              return ret;
      }
      
      After running the program, if you subsequently execute another BPF program,
      you will encounter the warning.
      
      It's worth noting that task_under_cgroup_hierarchy() is also utilized by
      bpf_current_task_under_cgroup(). However, bpf_current_task_under_cgroup()
      doesn't exhibit this issue because it cannot be used in sleepable BPF
      programs.
      
      Fixes: b5ad4cdc ("bpf: Add bpf_task_under_cgroup() kfunc")
      Signed-off-by: default avatarYafang Shao <laoar.shao@gmail.com>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarStanislav Fomichev <sdf@google.com>
      Cc: Feng Zhou <zhoufeng.zf@bytedance.com>
      Cc: KP Singh <kpsingh@kernel.org>
      Link: https://lore.kernel.org/bpf/20231007135945.4306-1-laoar.shao@gmail.com
      29a7e00f
    • Sebastian Andrzej Siewior's avatar
      net, bpf: Add a warning if NAPI cb missed xdp_do_flush(). · 9a675ba5
      Sebastian Andrzej Siewior authored
      A few drivers were missing a xdp_do_flush() invocation after
      XDP_REDIRECT.
      
      Add three helper functions each for one of the per-CPU lists. Return
      true if the per-CPU list is non-empty and flush the list.
      
      Add xdp_do_check_flushed() which invokes each helper functions and
      creates a warning if one of the functions had a non-empty list.
      
      Hide everything behind CONFIG_DEBUG_NET.
      Suggested-by: default avatarJesper Dangaard Brouer <hawk@kernel.org>
      Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Reviewed-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
      Acked-by: default avatarJakub Kicinski <kuba@kernel.org>
      Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
      Link: https://lore.kernel.org/bpf/20231016125738.Yt79p1uF@linutronix.de
      9a675ba5
    • Andrii Nakryiko's avatar
      libbpf: Don't assume SHT_GNU_verdef presence for SHT_GNU_versym section · 137df118
      Andrii Nakryiko authored
      Fix too eager assumption that SHT_GNU_verdef ELF section is going to be
      present whenever binary has SHT_GNU_versym section. It seems like either
      SHT_GNU_verdef or SHT_GNU_verneed can be used, so failing on missing
      SHT_GNU_verdef actually breaks use cases in production.
      
      One specific reported issue, which was used to manually test this fix,
      was trying to attach to `readline` function in BASH binary.
      
      Fixes: bb7fa093 ("libbpf: Support symbol versioning for uprobe")
      Reported-by: default avatarLiam Wisehart <liamwisehart@meta.com>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Tested-by: default avatarManu Bretelle <chantr4@gmail.com>
      Reviewed-by: default avatarFangrui Song <maskray@google.com>
      Acked-by: default avatarHengqi Chen <hengqi.chen@gmail.com>
      Link: https://lore.kernel.org/bpf/20231016182840.4033346-1-andrii@kernel.org
      137df118
    • Jakub Kicinski's avatar
      Merge tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next · a3c2dd96
      Jakub Kicinski authored
      Daniel Borkmann says:
      
      ====================
      pull-request: bpf-next 2023-10-16
      
      We've added 90 non-merge commits during the last 25 day(s) which contain
      a total of 120 files changed, 3519 insertions(+), 895 deletions(-).
      
      The main changes are:
      
      1) Add missed stats for kprobes to retrieve the number of missed kprobe
         executions and subsequent executions of BPF programs, from Jiri Olsa.
      
      2) Add cgroup BPF sockaddr hooks for unix sockets. The use case is
         for systemd to reimplement the LogNamespace feature which allows
         running multiple instances of systemd-journald to process the logs
         of different services, from Daan De Meyer.
      
      3) Implement BPF CPUv4 support for s390x BPF JIT, from Ilya Leoshkevich.
      
      4) Improve BPF verifier log output for scalar registers to better
         disambiguate their internal state wrt defaults vs min/max values
         matching, from Andrii Nakryiko.
      
      5) Extend the BPF fib lookup helpers for IPv4/IPv6 to support retrieving
         the source IP address with a new BPF_FIB_LOOKUP_SRC flag,
         from Martynas Pumputis.
      
      6) Add support for open-coded task_vma iterator to help with symbolization
         for BPF-collected user stacks, from Dave Marchevsky.
      
      7) Add libbpf getters for accessing individual BPF ring buffers which
         is useful for polling them individually, for example, from Martin Kelly.
      
      8) Extend AF_XDP selftests to validate the SHARED_UMEM feature,
         from Tushar Vyavahare.
      
      9) Improve BPF selftests cross-building support for riscv arch,
         from Björn Töpel.
      
      10) Add the ability to pin a BPF timer to the same calling CPU,
         from David Vernet.
      
      11) Fix libbpf's bpf_tracing.h macros for riscv to use the generic
         implementation of PT_REGS_SYSCALL_REGS() to access syscall arguments,
         from Alexandre Ghiti.
      
      12) Extend libbpf to support symbol versioning for uprobes, from Hengqi Chen.
      
      13) Fix bpftool's skeleton code generation to guarantee that ELF data
          is 8 byte aligned, from Ian Rogers.
      
      14) Inherit system-wide cpu_mitigations_off() setting for Spectre v1/v4
          security mitigations in BPF verifier, from Yafang Shao.
      
      15) Annotate struct bpf_stack_map with __counted_by attribute to prepare
          BPF side for upcoming __counted_by compiler support, from Kees Cook.
      
      * tag 'for-netdev' of https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next: (90 commits)
        bpf: Ensure proper register state printing for cond jumps
        bpf: Disambiguate SCALAR register state output in verifier logs
        selftests/bpf: Make align selftests more robust
        selftests/bpf: Improve missed_kprobe_recursion test robustness
        selftests/bpf: Improve percpu_alloc test robustness
        selftests/bpf: Add tests for open-coded task_vma iter
        bpf: Introduce task_vma open-coded iterator kfuncs
        selftests/bpf: Rename bpf_iter_task_vma.c to bpf_iter_task_vmas.c
        bpf: Don't explicitly emit BTF for struct btf_iter_num
        bpf: Change syscall_nr type to int in struct syscall_tp_t
        net/bpf: Avoid unused "sin_addr_len" warning when CONFIG_CGROUP_BPF is not set
        bpf: Avoid unnecessary audit log for CPU security mitigations
        selftests/bpf: Add tests for cgroup unix socket address hooks
        selftests/bpf: Make sure mount directory exists
        documentation/bpf: Document cgroup unix socket address hooks
        bpftool: Add support for cgroup unix socket address hooks
        libbpf: Add support for cgroup unix socket address hooks
        bpf: Implement cgroup sockaddr hooks for unix sockets
        bpf: Add bpf_sock_addr_set_sun_path() to allow writing unix sockaddr from bpf
        bpf: Propagate modified uaddrlen from cgroup sockaddr programs
        ...
      ====================
      
      Link: https://lore.kernel.org/r/20231016204803.30153-1-daniel@iogearbox.netSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      a3c2dd96
    • Yunsheng Lin's avatar
      page_pool: fragment API support for 32-bit arch with 64-bit DMA · 90de47f0
      Yunsheng Lin authored
      Currently page_pool_alloc_frag() is not supported in 32-bit
      arch with 64-bit DMA because of the overlap issue between
      pp_frag_count and dma_addr_upper in 'struct page' for those
      arches, which seems to be quite common, see [1], which means
      driver may need to handle it when using fragment API.
      
      It is assumed that the combination of the above arch with an
      address space >16TB does not exist, as all those arches have
      64b equivalent, it seems logical to use the 64b version for a
      system with a large address space. It is also assumed that dma
      address is page aligned when we are dma mapping a page aligned
      buffer, see [2].
      
      That means we're storing 12 bits of 0 at the lower end for a
      dma address, we can reuse those bits for the above arches to
      support 32b+12b, which is 16TB of memory.
      
      If we make a wrong assumption, a warning is emitted so that
      user can report to us.
      
      1. https://lore.kernel.org/all/20211117075652.58299-1-linyunsheng@huawei.com/
      2. https://lore.kernel.org/all/20230818145145.4b357c89@kernel.org/Tested-by: default avatarAlexander Lobakin <aleksander.lobakin@intel.com>
      Signed-off-by: default avatarYunsheng Lin <linyunsheng@huawei.com>
      CC: Lorenzo Bianconi <lorenzo@kernel.org>
      CC: Alexander Duyck <alexander.duyck@gmail.com>
      CC: Liang Chen <liangchen.linux@gmail.com>
      CC: Guillaume Tucker <guillaume.tucker@collabora.com>
      CC: Matthew Wilcox <willy@infradead.org>
      CC: Linux-MM <linux-mm@kvack.org>
      Link: https://lore.kernel.org/r/20231013064827.61135-2-linyunsheng@huawei.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      90de47f0
    • Jacob Keller's avatar
      net: stub tcp_gro_complete if CONFIG_INET=n · e411a8e3
      Jacob Keller authored
      A few networking drivers including bnx2x, bnxt, qede, and idpf call
      tcp_gro_complete as part of offloading TCP GRO. The function is only
      defined if CONFIG_INET is true, since its TCP specific and is meaningless
      if the kernel lacks IP networking support.
      
      The combination of trying to use the complex network drivers with
      CONFIG_NET but not CONFIG_INET is rather unlikely in practice: most use
      cases are going to need IP networking.
      
      The tcp_gro_complete function just sets some data in the socket buffer for
      use in processing the TCP packet in the event that the GRO was offloaded to
      the device. If the kernel lacks TCP support, such setup will simply go
      unused.
      
      The bnx2x, bnxt, and qede drivers wrap their TCP offload support in
      CONFIG_INET checks and skip handling on such kernels.
      
      The idpf driver did not check CONFIG_INET and thus fails to link if the
      kernel is configured  with CONFIG_NET=y, CONFIG_IDPF=(m|y), and
      CONFIG_INET=n.
      
      While checking CONFIG_INET does allow the driver to bypass significantly
      more instructions in the event that we know TCP networking isn't supported,
      the configuration is unlikely to be used widely.
      
      Rather than require driver authors to care about this, stub the
      tcp_gro_complete function when CONFIG_INET=n. This allows drivers to be
      left as-is. It does mean the idpf driver will perform slightly more work
      than strictly necessary when CONFIG_INET=n, since it will still execute
      some of the skb setup in idpf_rx_rsc. However, that work would be performed
      in the case where CONFIG_INET=y anyways.
      
      I did not change the existing drivers, since they appear to wrap a
      significant portion of code when CONFIG_INET=n. There is little benefit in
      trashing these drivers just to unwrap and remove the CONFIG_INET check.
      
      Using a stub for tcp_gro_complete is still beneficial, as it means future
      drivers no longer need to worry about this case of CONFIG_NET=y and
      CONFIG_INET=n, which should reduce noise from buildbots that check such a
      configuration.
      Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
      Acked-by: default avatarRandy Dunlap <rdunlap@infradead.org>
      Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
      Link: https://lore.kernel.org/r/20231013185502.1473541-1-jacob.e.keller@intel.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      e411a8e3
  5. 16 Oct, 2023 12 commits
    • Muhammad Muzammil's avatar
    • Liansen Zhai's avatar
      cgroup, netclassid: on modifying netclassid in cgroup, only consider the main process. · c60991f8
      Liansen Zhai authored
      When modifying netclassid, the command("echo 0x100001 > net_cls.classid")
      will take more time on many threads of one process, because the process
      create many fds.
      for example, one process exists 28000 fds and 60000 threads, echo command
      will task 45 seconds.
      Now, we only consider the main process when exec "iterate_fd", and the
      time is about 52 milliseconds.
      Signed-off-by: default avatarLiansen Zhai <zhailiansen@kuaishou.com>
      Reviewed-by: default avatarSimon Horman <horms@kernel.org>
      Link: https://lore.kernel.org/r/20231012090330.29636-1-zhailiansen@kuaishou.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      c60991f8
    • Justin Stitt's avatar
      net: usb: replace deprecated strncpy with strscpy · 1cfce826
      Justin Stitt authored
      strncpy() is deprecated for use on NUL-terminated destination strings
      [1] and as such we should prefer more robust and less ambiguous string
      interfaces.
      
      Other implementations of .*get_drvinfo use strscpy so this patch brings
      sr_get_drvinfo() in line as well:
      
      igb/igb_ethtool.c +851
      static void igb_get_drvinfo(struct net_device *netdev,
      
      igbvf/ethtool.c
      167:static void igbvf_get_drvinfo(struct net_device *netdev,
      
      i40e/i40e_ethtool.c
      1999:static void i40e_get_drvinfo(struct net_device *netdev,
      
      e1000/e1000_ethtool.c
      529:static void e1000_get_drvinfo(struct net_device *netdev,
      
      ixgbevf/ethtool.c
      211:static void ixgbevf_get_drvinfo(struct net_device *netdev,
      
      ...
      
      Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
      Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
      Link: https://github.com/KSPP/linux/issues/90Signed-off-by: default avatarJustin Stitt <justinstitt@google.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Link: https://lore.kernel.org/r/20231012-strncpy-drivers-net-usb-sr9800-c-v1-1-5540832c8ec2@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      1cfce826
    • Justin Stitt's avatar
      lan78xx: replace deprecated strncpy with strscpy · 2242f22a
      Justin Stitt authored
      strncpy() is deprecated for use on NUL-terminated destination strings
      [1] and as such we should prefer more robust and less ambiguous string
      interfaces.
      
      Other implementations of .*get_drvinfo use strscpy so this patch brings
      lan78xx_get_drvinfo() in line as well:
      
      igb/igb_ethtool.c +851
      static void igb_get_drvinfo(struct net_device *netdev,
      
      igbvf/ethtool.c
      167:static void igbvf_get_drvinfo(struct net_device *netdev,
      
      i40e/i40e_ethtool.c
      1999:static void i40e_get_drvinfo(struct net_device *netdev,
      
      e1000/e1000_ethtool.c
      529:static void e1000_get_drvinfo(struct net_device *netdev,
      
      ixgbevf/ethtool.c
      211:static void ixgbevf_get_drvinfo(struct net_device *netdev,
      
      Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
      Link: https://github.com/KSPP/linux/issues/90Signed-off-by: default avatarJustin Stitt <justinstitt@google.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Link: https://lore.kernel.org/r/20231012-strncpy-drivers-net-usb-lan78xx-c-v1-1-99d513061dfc@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      2242f22a
    • Justin Stitt's avatar
      net: phy: smsc: replace deprecated strncpy with ethtool_sprintf · 4ddc1f1f
      Justin Stitt authored
      strncpy() is deprecated for use on NUL-terminated destination strings
      [1] and as such we should prefer more robust and less ambiguous string
      interfaces.
      
      ethtool_sprintf() is designed specifically for get_strings() usage.
      Let's replace strncpy in favor of this dedicated helper function.
      
      Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
      Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
      Link: https://github.com/KSPP/linux/issues/90Signed-off-by: default avatarJustin Stitt <justinstitt@google.com>
      Reviewed-by: default avatarSimon Horman <horms@kernel.org>
      Link: https://lore.kernel.org/r/20231012-strncpy-drivers-net-phy-smsc-c-v1-1-00528f7524b3@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      4ddc1f1f
    • Justin Stitt's avatar
      net: netcp: replace deprecated strncpy with strscpy · eb7fa2eb
      Justin Stitt authored
      strncpy() is deprecated for use on NUL-terminated destination strings
      [1] and as such we should prefer more robust and less ambiguous string
      interfaces.
      
      Considering the above, a suitable replacement is `strscpy` [2] due to
      the fact that it guarantees NUL-termination on the destination buffer
      without unnecessarily NUL-padding.
      
      Other implementations of .*get_drvinfo also use strscpy so this patch
      brings keystone_get_drvinfo() in line as well:
      
      igb/igb_ethtool.c +851
      static void igb_get_drvinfo(struct net_device *netdev,
      
      igbvf/ethtool.c
      167:static void igbvf_get_drvinfo(struct net_device *netdev,
      
      i40e/i40e_ethtool.c
      1999:static void i40e_get_drvinfo(struct net_device *netdev,
      
      e1000/e1000_ethtool.c
      529:static void e1000_get_drvinfo(struct net_device *netdev,
      
      ixgbevf/ethtool.c
      211:static void ixgbevf_get_drvinfo(struct net_device *netdev,
      
      Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
      Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
      Link: https://github.com/KSPP/linux/issues/90Signed-off-by: default avatarJustin Stitt <justinstitt@google.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Link: https://lore.kernel.org/r/20231012-strncpy-drivers-net-ethernet-ti-netcp_ethss-c-v1-1-93142e620864@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      eb7fa2eb
    • Haiyang Zhang's avatar
      tcp: Set pingpong threshold via sysctl · 562b1fdf
      Haiyang Zhang authored
      TCP pingpong threshold is 1 by default. But some applications, like SQL DB
      may prefer a higher pingpong threshold to activate delayed acks in quick
      ack mode for better performance.
      
      The pingpong threshold and related code were changed to 3 in the year
      2019 in:
        commit 4a41f453 ("tcp: change pingpong threshold to 3")
      And reverted to 1 in the year 2022 in:
        commit 4d8f24ee ("Revert "tcp: change pingpong threshold to 3"")
      
      There is no single value that fits all applications.
      Add net.ipv4.tcp_pingpong_thresh sysctl tunable, so it can be tuned for
      optimal performance based on the application needs.
      Signed-off-by: default avatarHaiyang Zhang <haiyangz@microsoft.com>
      Reviewed-by: default avatarSimon Horman <horms@kernel.org>
      Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
      Acked-by: default avatarNeal Cardwell <ncardwell@google.com>
      Reviewed-by: default avatarKuniyuki Iwashima <kuniyu@amazon.com>
      Link: https://lore.kernel.org/r/1697056244-21888-1-git-send-email-haiyangz@microsoft.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      562b1fdf
    • Daniel Borkmann's avatar
      net, sched: Add tcf_set_drop_reason for {__,}tcf_classify · 39d08b91
      Daniel Borkmann authored
      Add an initial user for the newly added tcf_set_drop_reason() helper to set the
      drop reason for internal errors leading to TC_ACT_SHOT inside {__,}tcf_classify().
      
      Right now this only adds a very basic SKB_DROP_REASON_TC_ERROR as a generic
      fallback indicator to mark drop locations. Where needed, such locations can be
      converted to more specific codes, for example, when hitting the reclassification
      limit, etc.
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Cc: Jamal Hadi Salim <jhs@mojatatu.com>
      Cc: Victor Nogueira <victor@mojatatu.com>
      Link: https://lore.kernel.org/r/20231009092655.22025-2-daniel@iogearbox.netSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      39d08b91
    • Daniel Borkmann's avatar
      net, sched: Make tc-related drop reason more flexible · 54a59aed
      Daniel Borkmann authored
      Currently, the kfree_skb_reason() in sch_handle_{ingress,egress}() can only
      express a basic SKB_DROP_REASON_TC_INGRESS or SKB_DROP_REASON_TC_EGRESS reason.
      
      Victor kicked-off an initial proposal to make this more flexible by disambiguating
      verdict from return code by moving the verdict into struct tcf_result and
      letting tcf_classify() return a negative error. If hit, then two new drop
      reasons were added in the proposal, that is SKB_DROP_REASON_TC_INGRESS_ERROR
      as well as SKB_DROP_REASON_TC_EGRESS_ERROR. Further analysis of the actual
      error codes would have required to attach to tcf_classify via kprobe/kretprobe
      to more deeply debug skb and the returned error.
      
      In order to make the kfree_skb_reason() in sch_handle_{ingress,egress}() more
      extensible, it can be addressed in a more straight forward way, that is: Instead
      of placing the verdict into struct tcf_result, we can just put the drop reason
      in there, which does not require changes throughout various classful schedulers
      given the existing verdict logic can stay as is.
      
      Then, SKB_DROP_REASON_TC_ERROR{,_*} can be added to the enum skb_drop_reason
      to disambiguate between an error or an intentional drop. New drop reason error
      codes can be added successively to the tc code base.
      
      For internal error locations which have not yet been annotated with a
      SKB_DROP_REASON_TC_ERROR{,_*}, the fallback is SKB_DROP_REASON_TC_INGRESS and
      SKB_DROP_REASON_TC_EGRESS, respectively. Generic errors could be marked with a
      SKB_DROP_REASON_TC_ERROR code until they are converted to more specific ones
      if it is found that they would be useful for troubleshooting.
      
      While drop reasons have infrastructure for subsystem specific error codes which
      are currently used by mac80211 and ovs, Jakub mentioned that it is preferred
      for tc to use the enum skb_drop_reason core codes given it is a better fit and
      currently the tooling support is better, too.
      
      With regards to the latter:
      
        [...] I think Alastair (bpftrace) is working on auto-prettifying enums when
        bpftrace outputs maps. So we can do something like:
      
        $ bpftrace -e 'tracepoint:skb:kfree_skb { @[args->reason] = count(); }'
        Attaching 1 probe...
        ^C
      
        @[SKB_DROP_REASON_TC_INGRESS]: 2
        @[SKB_CONSUMED]: 34
      
        ^^^^^^^^^^^^ names!!
      
        Auto-magically. [...]
      
      Add a small helper tcf_set_drop_reason() which can be used to set the drop reason
      into the tcf_result.
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Cc: Jamal Hadi Salim <jhs@mojatatu.com>
      Cc: Victor Nogueira <victor@mojatatu.com>
      Link: https://lore.kernel.org/netdev/20231006063233.74345d36@kernel.orgReviewed-by: default avatarJakub Kicinski <kuba@kernel.org>
      Link: https://lore.kernel.org/r/20231009092655.22025-1-daniel@iogearbox.netSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      54a59aed
    • Daniel Borkmann's avatar
      Merge branch 'bpf-log-improvements' · 99c9991f
      Daniel Borkmann authored
      Andrii Nakryiko says:
      
      ====================
      This patch set fixes ambiguity in BPF verifier log output of SCALAR register
      in the parts that emit umin/umax, smin/smax, etc ranges. See patch #4 for
      details.
      
      Also, patch #5 fixes an issue with verifier log missing instruction context
      (state) output for conditionals that trigger precision marking. See details in
      the patch.
      
      First two patches are just improvements to two selftests that are very flaky
      locally when run in parallel mode.
      
      Patch #3 changes 'align' selftest to be less strict about exact verifier log
      output (which patch #4 changes, breaking lots of align tests as written). Now
      test does more of a register substate checks, mostly around expected var_off()
      values. This 'align' selftests is one of the more brittle ones and requires
      constant adjustment when verifier log output changes, without really catching
      any new issues. So hopefully these changes can minimize future support efforts
      for this specific set of tests.
      ====================
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      99c9991f
    • Andrii Nakryiko's avatar
      bpf: Ensure proper register state printing for cond jumps · 1a8a315f
      Andrii Nakryiko authored
      Verifier emits relevant register state involved in any given instruction
      next to it after `;` to the right, if possible. Or, worst case, on the
      separate line repeating instruction index.
      
      E.g., a nice and simple case would be:
      
        2: (d5) if r0 s<= 0x0 goto pc+1       ; R0_w=0
      
      But if there is some intervening extra output (e.g., precision
      backtracking log) involved, we are supposed to see the state after the
      precision backtrack log:
      
        4: (75) if r0 s>= 0x0 goto pc+1
        mark_precise: frame0: last_idx 4 first_idx 0 subseq_idx -1
        mark_precise: frame0: regs=r0 stack= before 2: (d5) if r0 s<= 0x0 goto pc+1
        mark_precise: frame0: regs=r0 stack= before 1: (b7) r0 = 0
        6: R0_w=0
      
      First off, note that in `6: R0_w=0` instruction index corresponds to the
      next instruction, not to the conditional jump instruction itself, which
      is wrong and we'll get to that.
      
      But besides that, the above is a happy case that does work today. Yet,
      if it so happens that precision backtracking had to traverse some of the
      parent states, this `6: R0_w=0` state output would be missing.
      
      This is due to a quirk of print_verifier_state() routine, which performs
      mark_verifier_state_clean(env) at the end. This marks all registers as
      "non-scratched", which means that subsequent logic to print *relevant*
      registers (that is, "scratched ones") fails and doesn't see anything
      relevant to print and skips the output altogether.
      
      print_verifier_state() is used both to print instruction context, but
      also to print an **entire** verifier state indiscriminately, e.g.,
      during precision backtracking (and in a few other situations, like
      during entering or exiting subprogram).  Which means if we have to print
      entire parent state before getting to printing instruction context
      state, instruction context is marked as clean and is omitted.
      
      Long story short, this is definitely not intentional. So we fix this
      behavior in this patch by teaching print_verifier_state() to clear
      scratch state only if it was used to print instruction state, not the
      parent/callback state. This is determined by print_all option, so if
      it's not set, we don't clear scratch state. This fixes missing
      instruction state for these cases.
      
      As for the mismatched instruction index, we fix that by making sure we
      call print_insn_state() early inside check_cond_jmp_op() before we
      adjusted insn_idx based on jump branch taken logic. And with that we get
      desired correct information:
      
        9: (16) if w4 == 0x1 goto pc+9
        mark_precise: frame0: last_idx 9 first_idx 9 subseq_idx -1
        mark_precise: frame0: parent state regs=r4 stack=: R2_w=1944 R4_rw=P1 R10=fp0
        mark_precise: frame0: last_idx 8 first_idx 0 subseq_idx 9
        mark_precise: frame0: regs=r4 stack= before 8: (66) if w4 s> 0x3 goto pc+5
        mark_precise: frame0: regs=r4 stack= before 7: (b7) r4 = 1
        9: R4=1
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20231011223728.3188086-6-andrii@kernel.org
      1a8a315f
    • Andrii Nakryiko's avatar
      bpf: Disambiguate SCALAR register state output in verifier logs · 72f8a1de
      Andrii Nakryiko authored
      Currently the way that verifier prints SCALAR_VALUE register state (and
      PTR_TO_PACKET, which can have var_off and ranges info as well) is very
      ambiguous.
      
      In the name of brevity we are trying to eliminate "unnecessary" output
      of umin/umax, smin/smax, u32_min/u32_max, and s32_min/s32_max values, if
      possible. Current rules are that if any of those have their default
      value (which for mins is the minimal value of its respective types: 0,
      S32_MIN, or S64_MIN, while for maxs it's U32_MAX, S32_MAX, S64_MAX, or
      U64_MAX) *OR* if there is another min/max value that as matching value.
      E.g., if smin=100 and umin=100, we'll emit only umin=10, omitting smin
      altogether. This approach has a few problems, being both ambiguous and
      sort-of incorrect in some cases.
      
      Ambiguity is due to missing value could be either default value or value
      of umin/umax or smin/smax. This is especially confusing when we mix
      signed and unsigned ranges. Quite often, umin=0 and smin=0, and so we'll
      have only `umin=0` leaving anyone reading verifier log to guess whether
      smin is actually 0 or it's actually -9223372036854775808 (S64_MIN). And
      often times it's important to know, especially when debugging tricky
      issues.
      
      "Sort-of incorrectness" comes from mixing negative and positive values.
      E.g., if umin is some large positive number, it can be equal to smin
      which is, interpreted as signed value, is actually some negative value.
      Currently, that smin will be omitted and only umin will be emitted with
      a large positive value, giving an impression that smin is also positive.
      
      Anyway, ambiguity is the biggest issue making it impossible to have an
      exact understanding of register state, preventing any sort of automated
      testing of verifier state based on verifier log. This patch is
      attempting to rectify the situation by removing ambiguity, while
      minimizing the verboseness of register state output.
      
      The rules are straightforward:
        - if some of the values are missing, then it definitely has a default
        value. I.e., `umin=0` means that umin is zero, but smin is actually
        S64_MIN;
        - all the various boundaries that happen to have the same value are
        emitted in one equality separated sequence. E.g., if umin and smin are
        both 100, we'll emit `smin=umin=100`, making this explicit;
        - we do not mix negative and positive values together, and even if
        they happen to have the same bit-level value, they will be emitted
        separately with proper sign. I.e., if both umax and smax happen to be
        0xffffffffffffffff, we'll emit them both separately as
        `smax=-1,umax=18446744073709551615`;
        - in the name of a bit more uniformity and consistency,
        {u32,s32}_{min,max} are renamed to {s,u}{min,max}32, which seems to
        improve readability.
      
      The above means that in case of all 4 ranges being, say, [50, 100] range,
      we'd previously see hugely ambiguous:
      
          R1=scalar(umin=50,umax=100)
      
      Now, we'll be more explicit:
      
          R1=scalar(smin=umin=smin32=umin32=50,smax=umax=smax32=umax32=100)
      
      This is slightly more verbose, but distinct from the case when we don't
      know anything about signed boundaries and 32-bit boundaries, which under
      new rules will match the old case:
      
          R1=scalar(umin=50,umax=100)
      
      Also, in the name of simplicity of implementation and consistency, order
      for {s,u}32_{min,max} are emitted *before* var_off. Previously they were
      emitted afterwards, for unclear reasons.
      
      This patch also includes a few fixes to selftests that expect exact
      register state to accommodate slight changes to verifier format. You can
      see that the changes are pretty minimal in common cases.
      
      Note, the special case when SCALAR_VALUE register is a known constant
      isn't changed, we'll emit constant value once, interpreted as signed
      value.
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
      Acked-by: default avatarEduard Zingerman <eddyz87@gmail.com>
      Link: https://lore.kernel.org/bpf/20231011223728.3188086-5-andrii@kernel.org
      72f8a1de