Re: [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/5/24 11:42 AM, Vadim Fedorenko wrote:
On 27/05/2024 19:59, Vadim Fedorenko wrote:
Add special flag to validate that TC BPF program properly updates
checksum information in skb.

Signed-off-by: Vadim Fedorenko <vadfed@xxxxxxxx>
---
  include/uapi/linux/bpf.h       |  2 ++
  net/bpf/test_run.c             | 18 +++++++++++++++++-
  tools/include/uapi/linux/bpf.h |  2 ++
  3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 90706a47f6ff..f7d458d88111 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1425,6 +1425,8 @@ enum {
  #define BPF_F_TEST_RUN_ON_CPU    (1U << 0)
  /* If set, XDP frames will be transmitted after processing */
  #define BPF_F_TEST_XDP_LIVE_FRAMES    (1U << 1)
+/* If set, apply CHECKSUM_COMPLETE to skb and validate the checksum */
+#define BPF_F_TEST_SKB_CHECKSUM_COMPLETE    (1U << 2)
  /* type for BPF_ENABLE_STATS */
  enum bpf_stats_type {
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index f6aad4ed2ab2..4c21562ad526 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -977,7 +977,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
      void *data;
      int ret;
-    if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
+    if ((kattr->test.flags & ~BPF_F_TEST_SKB_CHECKSUM_COMPLETE) ||
+        kattr->test.cpu || kattr->test.batch_size)
          return -EINVAL;
      data = bpf_test_init(kattr, kattr->test.data_size_in,
@@ -1025,6 +1026,12 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
      skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
      __skb_put(skb, size);
+
+    if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
+        skb->csum = skb_checksum(skb, 0, skb->len, 0);
+        skb->ip_summed = CHECKSUM_COMPLETE;
+    }
+
      if (ctx && ctx->ifindex > 1) {
          dev = dev_get_by_index(net, ctx->ifindex);
          if (!dev) {
@@ -1079,6 +1086,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
      }
      convert_skb_to___skb(skb, ctx);
+    if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
+        __wsum csum = skb_checksum(skb, 0, skb->len, 0);
+
+        if (skb->csum != csum) {
+            ret = -EBADMSG;
+            goto out;
+        }
+    }
+
      size = skb->len;
      /* bpf program can never convert linear skb to non-linear */
      if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 90706a47f6ff..f7d458d88111 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1425,6 +1425,8 @@ enum {
  #define BPF_F_TEST_RUN_ON_CPU    (1U << 0)
  /* If set, XDP frames will be transmitted after processing */
  #define BPF_F_TEST_XDP_LIVE_FRAMES    (1U << 1)
+/* If set, apply CHECKSUM_COMPLETE to skb and validate the checksum */
+#define BPF_F_TEST_SKB_CHECKSUM_COMPLETE    (1U << 2)
  /* type for BPF_ENABLE_STATS */
  enum bpf_stats_type {

Hi Daniel!

Have you had a chance to look at v3 of this patch?
I think I addressed all your comments form v2.

Looks good, but I think there is something off given the test on arm64 and s390x
fails in skb_pkt_end with EBADMSG. I wonder if we're missing a :

  skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);

right after the eth_type_trans()?

Thanks,
Daniel




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux