[PATCH bpf-next v5 0/5] Support non-linear skbs for BPF_PROG_TEST_RUN

[PATCH bpf-next v5 0/5] Support non-linear skbs for BPF_PROG_TEST_RUN

[Paul Chaignon]

This patchset adds support for non-linear skbs when running tc programs
with BPF_PROG_TEST_RUN.

We've had multiple bugs in the past few years in Cilium caused by
missing calls to bpf_skb_pull_data(). Daniel suggested this new
BPF_PROG_TEST_RUN flag as a way to uncover these bugs in our BPF tests.

Changes in v5:
  - Fix double free on data in first patch.
Changes in v4:
  - Per Martin's suggestion, follow the XDP code pattern and use
    bpf_test_init only to initialize the linear area. That way data is
    directly copied to the right areas and we avoid the call to
    __pskb_pull_tail.
  - Fixed outdated commit descriptions.
  - Rebased.
Changes in v3:
  - Dropped BPF_F_TEST_SKB_NON_LINEAR and used the ctx->data_end to
    determine if the user wants non-linear skb, as suggested by Amery.
  - Introduced a second commit with a bit of refactoring to allow for
    the above requested change.
  - Fix bug found by syzkaller on third commit.
  - Rebased.
Changes in v2:
  - Made the linear size configurable via ctx->data_end, as suggested
    by Amery.
  - Reworked the selftests to allow testing the configurable linear
    size.
  - Fix warnings reported by kernel test robot on first commit.
  - Rebased.

Paul Chaignon (5):
  bpf: Refactor cleanup of bpf_prog_test_run_skb
  bpf: Reorder bpf_prog_test_run_skb initialization
  bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN
  selftests/bpf: Support non-linear flag in test loader
  selftests/bpf: Test direct packet access on non-linear skbs

 net/bpf/test_run.c                            | 107 ++++++++++++++----
 tools/testing/selftests/bpf/progs/bpf_misc.h  |   4 +
 .../bpf/progs/verifier_direct_packet_access.c |  54 +++++++++
 tools/testing/selftests/bpf/test_loader.c     |  19 +++-
 4 files changed, 157 insertions(+), 27 deletions(-)

-- 
2.43.0

[PATCH bpf-next v5 1/5] bpf: Refactor cleanup of bpf_prog_test_run_skb

[Paul Chaignon]

This bit of refactoring aims to simplify how we free memory in
bpf_prog_test_run_skb to avoid code duplication.

Tested-by: syzbot@syzkaller.appspotmail.com
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 net/bpf/test_run.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index dfb03ee0bb62..4b52db9e6433 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -990,10 +990,10 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	struct __sk_buff *ctx = NULL;
+	struct sk_buff *skb = NULL;
+	struct sock *sk = NULL;
 	u32 retval, duration;
 	int hh_len = ETH_HLEN;
-	struct sk_buff *skb;
-	struct sock *sk;
 	void *data;
 	int ret;
 
@@ -1012,8 +1012,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 
 	ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
 	if (IS_ERR(ctx)) {
-		kfree(data);
-		return PTR_ERR(ctx);
+		ret = PTR_ERR(ctx);
+		ctx = NULL;
+		goto out;
 	}
 
 	switch (prog->type) {
@@ -1033,24 +1034,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 
 	sk = sk_alloc(net, AF_UNSPEC, GFP_USER, &bpf_dummy_proto, 1);
 	if (!sk) {
-		kfree(data);
-		kfree(ctx);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 	sock_init_data(NULL, sk);
 
 	skb = slab_build_skb(data);
 	if (!skb) {
-		kfree(data);
-		kfree(ctx);
-		sk_free(sk);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 	skb->sk = sk;
 
 	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
 	__skb_put(skb, size);
 
+	data = NULL; /* data released via kfree_skb */
+
 	if (ctx && ctx->ifindex > 1) {
 		dev = dev_get_by_index(net, ctx->ifindex);
 		if (!dev) {
@@ -1142,7 +1142,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	if (dev && dev != net->loopback_dev)
 		dev_put(dev);
 	kfree_skb(skb);
-	sk_free(sk);
+	kfree(data);
+	if (sk)
+		sk_free(sk);
 	kfree(ctx);
 	return ret;
 }
-- 
2.43.0

[PATCH bpf-next v5 2/5] bpf: Reorder bpf_prog_test_run_skb initialization

[Paul Chaignon]

This patch reorders the initialization of bpf_prog_test_run_skb to
simplify the subsequent patch. Program types are checked first, followed
by the ctx init, and finally the data init. With the subsequent patch,
program types and the ctx init provide information that is used in the
data init. Thus, we need the data init to happen last.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 net/bpf/test_run.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 4b52db9e6433..3425100b1e8c 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1004,19 +1004,6 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	if (size < ETH_HLEN)
 		return -EINVAL;
 
-	data = bpf_test_init(kattr, kattr->test.data_size_in,
-			     size, NET_SKB_PAD + NET_IP_ALIGN,
-			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
-	if (IS_ERR(data))
-		return PTR_ERR(data);
-
-	ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
-	if (IS_ERR(ctx)) {
-		ret = PTR_ERR(ctx);
-		ctx = NULL;
-		goto out;
-	}
-
 	switch (prog->type) {
 	case BPF_PROG_TYPE_SCHED_CLS:
 	case BPF_PROG_TYPE_SCHED_ACT:
@@ -1032,6 +1019,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		break;
 	}
 
+	ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
+	if (IS_ERR(ctx))
+		return PTR_ERR(ctx);
+
+	data = bpf_test_init(kattr, kattr->test.data_size_in,
+			     size, NET_SKB_PAD + NET_IP_ALIGN,
+			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
+	if (IS_ERR(data)) {
+		ret = PTR_ERR(data);
+		data = NULL;
+		goto out;
+	}
+
 	sk = sk_alloc(net, AF_UNSPEC, GFP_USER, &bpf_dummy_proto, 1);
 	if (!sk) {
 		ret = -ENOMEM;
-- 
2.43.0

[PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Paul Chaignon]

This patch adds support for crafting non-linear skbs in BPF test runs
for tc programs. The size of the linear area is given by ctx->data_end,
with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
ctx->data_end are null, a linear skb is used.

This is particularly useful to test support for non-linear skbs in large
codebases such as Cilium. We've had multiple bugs in the past few years
where we were missing calls to bpf_skb_pull_data(). This support in
BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
BPF tests.

In addition to the selftests introduced later in the series, this patch
was tested by setting enabling non-linear skbs for all tc selftests
programs and checking test failures were expected.

Tested-by: syzbot@syzkaller.appspotmail.com
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 6 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 3425100b1e8c..e4f4b423646a 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 	/* cb is allowed */
 
 	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
+			   offsetof(struct __sk_buff, data_end)))
+		return -EINVAL;
+
+	/* data_end is allowed, but not copied to skb */
+
+	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
 			   offsetof(struct __sk_buff, tstamp)))
 		return -EINVAL;
 
@@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
 int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 			  union bpf_attr __user *uattr)
 {
+	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 	bool is_l2 = false, is_direct_pkt_access = false;
 	struct net *net = current->nsproxy->net_ns;
 	struct net_device *dev = net->loopback_dev;
+	u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
+	u32 linear_sz = kattr->test.data_size_in;
 	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	struct __sk_buff *ctx = NULL;
@@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
-	data = bpf_test_init(kattr, kattr->test.data_size_in,
-			     size, NET_SKB_PAD + NET_IP_ALIGN,
-			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
+	if (ctx) {
+		if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {
+			ret = -EINVAL;
+			goto out;
+		}
+		if (ctx->data_end)
+			linear_sz = max(ETH_HLEN, ctx->data_end);
+	}
+
+	data = bpf_test_init(kattr, linear_sz, size, headroom, tailroom);
 	if (IS_ERR(data)) {
 		ret = PTR_ERR(data);
 		data = NULL;
@@ -1044,10 +1060,47 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		ret = -ENOMEM;
 		goto out;
 	}
+
 	skb->sk = sk;
 
 	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
-	__skb_put(skb, size);
+	__skb_put(skb, linear_sz);
+
+	if (unlikely(kattr->test.data_size_in > linear_sz)) {
+		void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
+		struct skb_shared_info *sinfo = skb_shinfo(skb);
+
+		size = linear_sz;
+		while (size < kattr->test.data_size_in) {
+			struct page *page;
+			u32 data_len;
+
+			if (sinfo->nr_frags == MAX_SKB_FRAGS) {
+				ret = -ENOMEM;
+				goto out;
+			}
+
+			page = alloc_page(GFP_KERNEL);
+			if (!page) {
+				ret = -ENOMEM;
+				goto out;
+			}
+
+			data_len = min_t(u32, kattr->test.data_size_in - size,
+					 PAGE_SIZE);
+			skb_fill_page_desc(skb, sinfo->nr_frags, page, 0, data_len);
+
+			if (copy_from_user(page_address(page), data_in + size,
+					   data_len)) {
+				ret = -EFAULT;
+				goto out;
+			}
+			skb->data_len += data_len;
+			skb->truesize += PAGE_SIZE;
+			skb->len += data_len;
+			size += data_len;
+		}
+	}
 
 	data = NULL; /* data released via kfree_skb */
 
@@ -1130,9 +1183,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	convert_skb_to___skb(skb, ctx);
 
 	size = skb->len;
-	/* bpf program can never convert linear skb to non-linear */
-	if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
+	if (skb_is_nonlinear(skb)) {
+		/* bpf program can never convert linear skb to non-linear */
+		WARN_ON_ONCE(linear_sz == size);
 		size = skb_headlen(skb);
+	}
 	ret = bpf_test_finish(kattr, uattr, skb->data, NULL, size, retval,
 			      duration);
 	if (!ret)
-- 
2.43.0

Re: [PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Amery Hung]

On Thu, Oct 2, 2025 at 3:07 AM Paul Chaignon <paul.chaignon@gmail.com> wrote:
>
> This patch adds support for crafting non-linear skbs in BPF test runs
> for tc programs. The size of the linear area is given by ctx->data_end,
> with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
> ctx->data_end are null, a linear skb is used.
>
> This is particularly useful to test support for non-linear skbs in large
> codebases such as Cilium. We've had multiple bugs in the past few years
> where we were missing calls to bpf_skb_pull_data(). This support in
> BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
> BPF tests.
>
> In addition to the selftests introduced later in the series, this patch
> was tested by setting enabling non-linear skbs for all tc selftests
> programs and checking test failures were expected.
>
> Tested-by: syzbot@syzkaller.appspotmail.com
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
>  net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 3425100b1e8c..e4f4b423646a 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
>         /* cb is allowed */
>
>         if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
> +                          offsetof(struct __sk_buff, data_end)))
> +               return -EINVAL;
> +
> +       /* data_end is allowed, but not copied to skb */
> +
> +       if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
>                            offsetof(struct __sk_buff, tstamp)))
>                 return -EINVAL;
>
> @@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
>  int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>                           union bpf_attr __user *uattr)
>  {
> +       u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>         bool is_l2 = false, is_direct_pkt_access = false;
>         struct net *net = current->nsproxy->net_ns;
>         struct net_device *dev = net->loopback_dev;
> +       u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
> +       u32 linear_sz = kattr->test.data_size_in;
>         u32 size = kattr->test.data_size_in;
>         u32 repeat = kattr->test.repeat;
>         struct __sk_buff *ctx = NULL;
> @@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>         if (IS_ERR(ctx))
>                 return PTR_ERR(ctx);
>
> -       data = bpf_test_init(kattr, kattr->test.data_size_in,
> -                            size, NET_SKB_PAD + NET_IP_ALIGN,
> -                            SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> +       if (ctx) {
> +               if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {
> +                       ret = -EINVAL;
> +                       goto out;
> +               }
> +               if (ctx->data_end)
> +                       linear_sz = max(ETH_HLEN, ctx->data_end);
> +       }
> +
> +       data = bpf_test_init(kattr, linear_sz, size, headroom, tailroom);
>         if (IS_ERR(data)) {
>                 ret = PTR_ERR(data);
>                 data = NULL;
> @@ -1044,10 +1060,47 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>                 ret = -ENOMEM;
>                 goto out;
>         }
> +
>         skb->sk = sk;
>
>         skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> -       __skb_put(skb, size);
> +       __skb_put(skb, linear_sz);
> +
> +       if (unlikely(kattr->test.data_size_in > linear_sz)) {
> +               void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
> +               struct skb_shared_info *sinfo = skb_shinfo(skb);
> +
> +               size = linear_sz;
> +               while (size < kattr->test.data_size_in) {
> +                       struct page *page;
> +                       u32 data_len;
> +
> +                       if (sinfo->nr_frags == MAX_SKB_FRAGS) {
> +                               ret = -ENOMEM;
> +                               goto out;
> +                       }
> +
> +                       page = alloc_page(GFP_KERNEL);
> +                       if (!page) {
> +                               ret = -ENOMEM;
> +                               goto out;
> +                       }
> +
> +                       data_len = min_t(u32, kattr->test.data_size_in - size,
> +                                        PAGE_SIZE);
> +                       skb_fill_page_desc(skb, sinfo->nr_frags, page, 0, data_len);
> +
> +                       if (copy_from_user(page_address(page), data_in + size,
> +                                          data_len)) {
> +                               ret = -EFAULT;
> +                               goto out;
> +                       }
> +                       skb->data_len += data_len;
> +                       skb->truesize += PAGE_SIZE;
> +                       skb->len += data_len;
> +                       size += data_len;
> +               }
> +       }
>
>         data = NULL; /* data released via kfree_skb */

It seems that it can still double free if we error out when building
fragments. Maybe data need to be set to NULL right after
slab_build_skb() succeeds.

>
> @@ -1130,9 +1183,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>         convert_skb_to___skb(skb, ctx);
>
>         size = skb->len;
> -       /* bpf program can never convert linear skb to non-linear */
> -       if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
> +       if (skb_is_nonlinear(skb)) {
> +               /* bpf program can never convert linear skb to non-linear */
> +               WARN_ON_ONCE(linear_sz == size);
>                 size = skb_headlen(skb);
> +       }
>         ret = bpf_test_finish(kattr, uattr, skb->data, NULL, size, retval,
>                               duration);
>         if (!ret)
> --
> 2.43.0
>

Re: [PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Amery Hung]

On Thu, Oct 2, 2025 at 9:07 AM Amery Hung <ameryhung@gmail.com> wrote:
>
> On Thu, Oct 2, 2025 at 3:07 AM Paul Chaignon <paul.chaignon@gmail.com> wrote:
> >
> > This patch adds support for crafting non-linear skbs in BPF test runs
> > for tc programs. The size of the linear area is given by ctx->data_end,
> > with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
> > ctx->data_end are null, a linear skb is used.
> >
> > This is particularly useful to test support for non-linear skbs in large
> > codebases such as Cilium. We've had multiple bugs in the past few years
> > where we were missing calls to bpf_skb_pull_data(). This support in
> > BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
> > BPF tests.
> >
> > In addition to the selftests introduced later in the series, this patch
> > was tested by setting enabling non-linear skbs for all tc selftests
> > programs and checking test failures were expected.
> >
> > Tested-by: syzbot@syzkaller.appspotmail.com
> > Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> >  net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 61 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 3425100b1e8c..e4f4b423646a 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
> >         /* cb is allowed */
> >
> >         if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
> > +                          offsetof(struct __sk_buff, data_end)))
> > +               return -EINVAL;
> > +
> > +       /* data_end is allowed, but not copied to skb */
> > +
> > +       if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
> >                            offsetof(struct __sk_buff, tstamp)))
> >                 return -EINVAL;
> >
> > @@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
> >  int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >                           union bpf_attr __user *uattr)
> >  {
> > +       u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> >         bool is_l2 = false, is_direct_pkt_access = false;
> >         struct net *net = current->nsproxy->net_ns;
> >         struct net_device *dev = net->loopback_dev;
> > +       u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
> > +       u32 linear_sz = kattr->test.data_size_in;
> >         u32 size = kattr->test.data_size_in;
> >         u32 repeat = kattr->test.repeat;
> >         struct __sk_buff *ctx = NULL;
> > @@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >         if (IS_ERR(ctx))
> >                 return PTR_ERR(ctx);
> >
> > -       data = bpf_test_init(kattr, kattr->test.data_size_in,
> > -                            size, NET_SKB_PAD + NET_IP_ALIGN,
> > -                            SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> > +       if (ctx) {
> > +               if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {

Maybe we should reject non-zero ctx->data and ctx->data_meta to
prevent confusion from the user space.

> > +                       ret = -EINVAL;
> > +                       goto out;
> > +               }
> > +               if (ctx->data_end)
> > +                       linear_sz = max(ETH_HLEN, ctx->data_end);
> > +       }
> > +
> > +       data = bpf_test_init(kattr, linear_sz, size, headroom, tailroom);
> >         if (IS_ERR(data)) {
> >                 ret = PTR_ERR(data);
> >                 data = NULL;
> > @@ -1044,10 +1060,47 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >                 ret = -ENOMEM;
> >                 goto out;
> >         }
> > +
> >         skb->sk = sk;
> >
> >         skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> > -       __skb_put(skb, size);
> > +       __skb_put(skb, linear_sz);
> > +
> > +       if (unlikely(kattr->test.data_size_in > linear_sz)) {
> > +               void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
> > +               struct skb_shared_info *sinfo = skb_shinfo(skb);
> > +
> > +               size = linear_sz;
> > +               while (size < kattr->test.data_size_in) {
> > +                       struct page *page;
> > +                       u32 data_len;
> > +
> > +                       if (sinfo->nr_frags == MAX_SKB_FRAGS) {
> > +                               ret = -ENOMEM;
> > +                               goto out;
> > +                       }
> > +
> > +                       page = alloc_page(GFP_KERNEL);
> > +                       if (!page) {
> > +                               ret = -ENOMEM;
> > +                               goto out;
> > +                       }
> > +
> > +                       data_len = min_t(u32, kattr->test.data_size_in - size,
> > +                                        PAGE_SIZE);
> > +                       skb_fill_page_desc(skb, sinfo->nr_frags, page, 0, data_len);
> > +
> > +                       if (copy_from_user(page_address(page), data_in + size,
> > +                                          data_len)) {
> > +                               ret = -EFAULT;
> > +                               goto out;
> > +                       }
> > +                       skb->data_len += data_len;
> > +                       skb->truesize += PAGE_SIZE;
> > +                       skb->len += data_len;
> > +                       size += data_len;
> > +               }
> > +       }
> >
> >         data = NULL; /* data released via kfree_skb */
>
> It seems that it can still double free if we error out when building
> fragments. Maybe data need to be set to NULL right after
> slab_build_skb() succeeds.
>
> >
> > @@ -1130,9 +1183,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >         convert_skb_to___skb(skb, ctx);
> >
> >         size = skb->len;
> > -       /* bpf program can never convert linear skb to non-linear */
> > -       if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
> > +       if (skb_is_nonlinear(skb)) {
> > +               /* bpf program can never convert linear skb to non-linear */
> > +               WARN_ON_ONCE(linear_sz == size);
> >                 size = skb_headlen(skb);
> > +       }
> >         ret = bpf_test_finish(kattr, uattr, skb->data, NULL, size, retval,
> >                               duration);
> >         if (!ret)
> > --
> > 2.43.0
> >

Re: [PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Martin KaFai Lau]

On 10/2/25 3:07 AM, Paul Chaignon wrote:
> This patch adds support for crafting non-linear skbs in BPF test runs
> for tc programs. The size of the linear area is given by ctx->data_end,
> with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
> ctx->data_end are null, a linear skb is used.
> 
> This is particularly useful to test support for non-linear skbs in large
> codebases such as Cilium. We've had multiple bugs in the past few years
> where we were missing calls to bpf_skb_pull_data(). This support in
> BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
> BPF tests.
> 
> In addition to the selftests introduced later in the series, this patch
> was tested by setting enabling non-linear skbs for all tc selftests
> programs and checking test failures were expected.
> 
> Tested-by: syzbot@syzkaller.appspotmail.com
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
>   net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 3425100b1e8c..e4f4b423646a 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
>   	/* cb is allowed */
>   
>   	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
> +			   offsetof(struct __sk_buff, data_end)))
> +		return -EINVAL;
> +
> +	/* data_end is allowed, but not copied to skb */
> +
> +	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
>   			   offsetof(struct __sk_buff, tstamp)))
>   		return -EINVAL;
>   
> @@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
>   int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>   			  union bpf_attr __user *uattr)
>   {
> +	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>   	bool is_l2 = false, is_direct_pkt_access = false;
>   	struct net *net = current->nsproxy->net_ns;
>   	struct net_device *dev = net->loopback_dev;
> +	u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
> +	u32 linear_sz = kattr->test.data_size_in;
>   	u32 size = kattr->test.data_size_in;
>   	u32 repeat = kattr->test.repeat;
>   	struct __sk_buff *ctx = NULL;
> @@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>   	if (IS_ERR(ctx))
>   		return PTR_ERR(ctx);
>   
> -	data = bpf_test_init(kattr, kattr->test.data_size_in,
> -			     size, NET_SKB_PAD + NET_IP_ALIGN,
> -			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> +	if (ctx) {
> +		if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {

What is the need for the "!is_l2" test?

> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		if (ctx->data_end)
> +			linear_sz = max(ETH_HLEN, ctx->data_end);
> +	}
> +
> +	data = bpf_test_init(kattr, linear_sz, size, headroom, tailroom);

Instead of passing "size", should linear_sz be passed instead? Unlike xdp, 
allocating exactly linear_sz should be enough considering bpf_skb_pull_data can 
allocate new data if needed.

Should linear_sz be limited to "PAGE_SIZE - headroom..." like how test_run_xdp() 
does it ?

>   	if (IS_ERR(data)) {
>   		ret = PTR_ERR(data);
>   		data = NULL;
> @@ -1044,10 +1060,47 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>   		ret = -ENOMEM;
>   		goto out;
>   	}
> +
>   	skb->sk = sk;
>   
>   	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> -	__skb_put(skb, size);
> +	__skb_put(skb, linear_sz);
> +
> +	if (unlikely(kattr->test.data_size_in > linear_sz)) {
> +		void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
> +		struct skb_shared_info *sinfo = skb_shinfo(skb);
> +
> +		size = linear_sz;
> +		while (size < kattr->test.data_size_in) {
> +			struct page *page;
> +			u32 data_len;
> +
> +			if (sinfo->nr_frags == MAX_SKB_FRAGS) {
> +				ret = -ENOMEM;
> +				goto out;
> +			}
> +
> +			page = alloc_page(GFP_KERNEL);
> +			if (!page) {
> +				ret = -ENOMEM;
> +				goto out;
> +			}
> +
> +			data_len = min_t(u32, kattr->test.data_size_in - size,
> +					 PAGE_SIZE);
> +			skb_fill_page_desc(skb, sinfo->nr_frags, page, 0, data_len);
> +
> +			if (copy_from_user(page_address(page), data_in + size,
> +					   data_len)) {
> +				ret = -EFAULT;
> +				goto out;
> +			}
> +			skb->data_len += data_len;
> +			skb->truesize += PAGE_SIZE;
> +			skb->len += data_len;
> +			size += data_len;
> +		}
> +	}
>   
>   	data = NULL; /* data released via kfree_skb */
>   
> @@ -1130,9 +1183,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>   	convert_skb_to___skb(skb, ctx);
>   
>   	size = skb->len;
> -	/* bpf program can never convert linear skb to non-linear */
> -	if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
> +	if (skb_is_nonlinear(skb)) {
> +		/* bpf program can never convert linear skb to non-linear */
> +		WARN_ON_ONCE(linear_sz == size);
>   		size = skb_headlen(skb);
> +	}
>   	ret = bpf_test_finish(kattr, uattr, skb->data, NULL, size, retval,
>   			      duration);
>   	if (!ret)

Re: [PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Paul Chaignon]

On Thu, Oct 02, 2025 at 11:27:52AM -0700, Martin KaFai Lau wrote:
> On 10/2/25 3:07 AM, Paul Chaignon wrote:
> > This patch adds support for crafting non-linear skbs in BPF test runs
> > for tc programs. The size of the linear area is given by ctx->data_end,
> > with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
> > ctx->data_end are null, a linear skb is used.
> > 
> > This is particularly useful to test support for non-linear skbs in large
> > codebases such as Cilium. We've had multiple bugs in the past few years
> > where we were missing calls to bpf_skb_pull_data(). This support in
> > BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
> > BPF tests.
> > 
> > In addition to the selftests introduced later in the series, this patch
> > was tested by setting enabling non-linear skbs for all tc selftests
> > programs and checking test failures were expected.
> > 
> > Tested-by: syzbot@syzkaller.appspotmail.com
> > Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> >   net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 61 insertions(+), 6 deletions(-)
> > 
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 3425100b1e8c..e4f4b423646a 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
> >   	/* cb is allowed */
> >   	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
> > +			   offsetof(struct __sk_buff, data_end)))
> > +		return -EINVAL;
> > +
> > +	/* data_end is allowed, but not copied to skb */
> > +
> > +	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
> >   			   offsetof(struct __sk_buff, tstamp)))
> >   		return -EINVAL;
> > @@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
> >   int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >   			  union bpf_attr __user *uattr)
> >   {
> > +	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> >   	bool is_l2 = false, is_direct_pkt_access = false;
> >   	struct net *net = current->nsproxy->net_ns;
> >   	struct net_device *dev = net->loopback_dev;
> > +	u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
> > +	u32 linear_sz = kattr->test.data_size_in;
> >   	u32 size = kattr->test.data_size_in;
> >   	u32 repeat = kattr->test.repeat;
> >   	struct __sk_buff *ctx = NULL;
> > @@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> >   	if (IS_ERR(ctx))
> >   		return PTR_ERR(ctx);
> > -	data = bpf_test_init(kattr, kattr->test.data_size_in,
> > -			     size, NET_SKB_PAD + NET_IP_ALIGN,
> > -			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> > +	if (ctx) {
> > +		if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {
> 
> What is the need for the "!is_l2" test?

There's nothing limiting us to only tc program types, but I was
wondering if it makes sense to open this (non-linear skbs) to all
program types. For example, cgroup_skb programs cannot call
bpf_skb_pull_data to deal with non-linear skbs.

Even the LWT program types would require special care because ex. the
bpf_clone_redirect helper can end up calling eth_type_trans which
assumes we have at least ETH_HLEN in the linear area. I wasn't sure it
was worth opening this capability to these program types without a clear
use case.

> 
> > +			ret = -EINVAL;
> > +			goto out;
> > +		}
> > +		if (ctx->data_end)
> > +			linear_sz = max(ETH_HLEN, ctx->data_end);
> > +	}
> > +
> > +	data = bpf_test_init(kattr, linear_sz, size, headroom, tailroom);
> 
> Instead of passing "size", should linear_sz be passed instead? Unlike xdp,
> allocating exactly linear_sz should be enough considering bpf_skb_pull_data
> can allocate new data if needed.

Indeed. Thanks!

> 
> Should linear_sz be limited to "PAGE_SIZE - headroom..." like how
> test_run_xdp() does it ?

That changes a bit the current behavior. Currently, we will return
EINVAL if a user try to pass more than "PAGE_SIZE - headroom..." as
data_size_in. With the test_run_xdp approach, we'll end up silently
switching to non-linear mode if they do that.

I'm not against it given it brings consistency with the XDP counterpart,
but it could also be a bit surprising.

[...]

Re: [PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Martin KaFai Lau]

On 10/6/25 7:04 AM, Paul Chaignon wrote:
> On Thu, Oct 02, 2025 at 11:27:52AM -0700, Martin KaFai Lau wrote:
>> On 10/2/25 3:07 AM, Paul Chaignon wrote:
>>> This patch adds support for crafting non-linear skbs in BPF test runs
>>> for tc programs. The size of the linear area is given by ctx->data_end,
>>> with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
>>> ctx->data_end are null, a linear skb is used.
>>>
>>> This is particularly useful to test support for non-linear skbs in large
>>> codebases such as Cilium. We've had multiple bugs in the past few years
>>> where we were missing calls to bpf_skb_pull_data(). This support in
>>> BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
>>> BPF tests.
>>>
>>> In addition to the selftests introduced later in the series, this patch
>>> was tested by setting enabling non-linear skbs for all tc selftests
>>> programs and checking test failures were expected.
>>>
>>> Tested-by: syzbot@syzkaller.appspotmail.com
>>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>>> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
>>> ---
>>>    net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
>>>    1 file changed, 61 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>>> index 3425100b1e8c..e4f4b423646a 100644
>>> --- a/net/bpf/test_run.c
>>> +++ b/net/bpf/test_run.c
>>> @@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
>>>    	/* cb is allowed */
>>>    	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
>>> +			   offsetof(struct __sk_buff, data_end)))
>>> +		return -EINVAL;
>>> +
>>> +	/* data_end is allowed, but not copied to skb */
>>> +
>>> +	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
>>>    			   offsetof(struct __sk_buff, tstamp)))
>>>    		return -EINVAL;
>>> @@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
>>>    int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>>>    			  union bpf_attr __user *uattr)
>>>    {
>>> +	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>>>    	bool is_l2 = false, is_direct_pkt_access = false;
>>>    	struct net *net = current->nsproxy->net_ns;
>>>    	struct net_device *dev = net->loopback_dev;
>>> +	u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
>>> +	u32 linear_sz = kattr->test.data_size_in;
>>>    	u32 size = kattr->test.data_size_in;
>>>    	u32 repeat = kattr->test.repeat;
>>>    	struct __sk_buff *ctx = NULL;
>>> @@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>>>    	if (IS_ERR(ctx))
>>>    		return PTR_ERR(ctx);
>>> -	data = bpf_test_init(kattr, kattr->test.data_size_in,
>>> -			     size, NET_SKB_PAD + NET_IP_ALIGN,
>>> -			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
>>> +	if (ctx) {
>>> +		if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {
>>
>> What is the need for the "!is_l2" test?
> 
> There's nothing limiting us to only tc program types, but I was
> wondering if it makes sense to open this (non-linear skbs) to all
> program types. For example, cgroup_skb programs cannot call
> bpf_skb_pull_data to deal with non-linear skbs.

The bpf_dynptr_* and the bpf_load_* can still be used to handle the a non-linear 
skb.

One thing is all "!is_l2" program will get an -EINVAL now after this patch if 
either ctx_in or ctx_out is specified. Am I reading it right?

> 
> Even the LWT program types would require special care because ex. the
> bpf_clone_redirect helper can end up calling eth_type_trans which
> assumes we have at least ETH_HLEN in the linear area. I wasn't sure it
> was worth opening this capability to these program types without a clear
> use case.

I thought the linear_sz has been taken care of below such that it must be at 
least ETH_HLEN anyway. Why "!is_l2" still needs to be rejected?

I am not familiar with the LWT but I recalled we have fixed some cases on the 
ETH_HLEN and they are now tested by the prog_tests/empty_skb.c.

I think I am missing something on how is the non-linear skb different from the 
linear skb here for lwt if it has ensured there is ETH_HLEN in the linear. I am 
not sure how active is the lwt/bpf usage now. If it is hard to support, I think 
it is fine to exclude it.

CGROUP_SKB will be good to be supported at the beginning though.

> 
>>
>>> +			ret = -EINVAL;
>>> +			goto out;
>>> +		}
>>> +		if (ctx->data_end)
>>> +			linear_sz = max(ETH_HLEN, ctx->data_end);Here. linear_sz must be at least ETH_HLEN.

>>> +	}
>>> +
>>> +	data = bpf_test_init(kattr, linear_sz, size, headroom, tailroom);
>>
>> Instead of passing "size", should linear_sz be passed instead? Unlike xdp,
>> allocating exactly linear_sz should be enough considering bpf_skb_pull_data
>> can allocate new data if needed.
> 
> Indeed. Thanks!
> 
>>
>> Should linear_sz be limited to "PAGE_SIZE - headroom..." like how
>> test_run_xdp() does it ?
> 
> That changes a bit the current behavior. Currently, we will return
> EINVAL if a user try to pass more than "PAGE_SIZE - headroom..." as
> data_size_in. With the test_run_xdp approach, we'll end up silently
> switching to non-linear mode if they do that.
> 
> I'm not against it given it brings consistency with the XDP counterpart,
> but it could also be a bit surprising.

I think this is fine to accept what was previously rejected because of lacking 
non-linear skb support.


I would prefer to have similar expectation as the test_run_xdp for the parts 
that make sense.

Re: [PATCH bpf-next v5 3/5] bpf: Craft non-linear skbs in BPF_PROG_TEST_RUN

[Paul Chaignon]

On Mon, Oct 06, 2025 at 11:58:41AM -0700, Martin KaFai Lau wrote:
> On 10/6/25 7:04 AM, Paul Chaignon wrote:
> > On Thu, Oct 02, 2025 at 11:27:52AM -0700, Martin KaFai Lau wrote:
> > > On 10/2/25 3:07 AM, Paul Chaignon wrote:
> > > > This patch adds support for crafting non-linear skbs in BPF test runs
> > > > for tc programs. The size of the linear area is given by ctx->data_end,
> > > > with a minimum of ETH_HLEN always pulled in the linear area. If ctx or
> > > > ctx->data_end are null, a linear skb is used.
> > > > 
> > > > This is particularly useful to test support for non-linear skbs in large
> > > > codebases such as Cilium. We've had multiple bugs in the past few years
> > > > where we were missing calls to bpf_skb_pull_data(). This support in
> > > > BPF_PROG_TEST_RUN would allow us to automatically cover this case in our
> > > > BPF tests.
> > > > 
> > > > In addition to the selftests introduced later in the series, this patch
> > > > was tested by setting enabling non-linear skbs for all tc selftests
> > > > programs and checking test failures were expected.
> > > > 
> > > > Tested-by: syzbot@syzkaller.appspotmail.com
> > > > Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> > > > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > > > ---
> > > >    net/bpf/test_run.c | 67 +++++++++++++++++++++++++++++++++++++++++-----
> > > >    1 file changed, 61 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > > > index 3425100b1e8c..e4f4b423646a 100644
> > > > --- a/net/bpf/test_run.c
> > > > +++ b/net/bpf/test_run.c
> > > > @@ -910,6 +910,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
> > > >    	/* cb is allowed */
> > > >    	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, cb),
> > > > +			   offsetof(struct __sk_buff, data_end)))
> > > > +		return -EINVAL;
> > > > +
> > > > +	/* data_end is allowed, but not copied to skb */
> > > > +
> > > > +	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, data_end),
> > > >    			   offsetof(struct __sk_buff, tstamp)))
> > > >    		return -EINVAL;
> > > > @@ -984,9 +990,12 @@ static struct proto bpf_dummy_proto = {
> > > >    int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> > > >    			  union bpf_attr __user *uattr)
> > > >    {
> > > > +	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> > > >    	bool is_l2 = false, is_direct_pkt_access = false;
> > > >    	struct net *net = current->nsproxy->net_ns;
> > > >    	struct net_device *dev = net->loopback_dev;
> > > > +	u32 headroom = NET_SKB_PAD + NET_IP_ALIGN;
> > > > +	u32 linear_sz = kattr->test.data_size_in;
> > > >    	u32 size = kattr->test.data_size_in;
> > > >    	u32 repeat = kattr->test.repeat;
> > > >    	struct __sk_buff *ctx = NULL;
> > > > @@ -1023,9 +1032,16 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> > > >    	if (IS_ERR(ctx))
> > > >    		return PTR_ERR(ctx);
> > > > -	data = bpf_test_init(kattr, kattr->test.data_size_in,
> > > > -			     size, NET_SKB_PAD + NET_IP_ALIGN,
> > > > -			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> > > > +	if (ctx) {
> > > > +		if (!is_l2 || ctx->data_end > kattr->test.data_size_in) {
> > > 
> > > What is the need for the "!is_l2" test?
> > 
> > There's nothing limiting us to only tc program types, but I was
> > wondering if it makes sense to open this (non-linear skbs) to all
> > program types. For example, cgroup_skb programs cannot call
> > bpf_skb_pull_data to deal with non-linear skbs.
> 
> The bpf_dynptr_* and the bpf_load_* can still be used to handle the a
> non-linear skb.
> 
> One thing is all "!is_l2" program will get an -EINVAL now after this patch
> if either ctx_in or ctx_out is specified. Am I reading it right?

Yes, that needs to be fixed either way.

> 
> > 
> > Even the LWT program types would require special care because ex. the
> > bpf_clone_redirect helper can end up calling eth_type_trans which
> > assumes we have at least ETH_HLEN in the linear area. I wasn't sure it
> > was worth opening this capability to these program types without a clear
> > use case.
> 
> I thought the linear_sz has been taken care of below such that it must be at
> least ETH_HLEN anyway. Why "!is_l2" still needs to be rejected?

In case of !is_l2, we will still call eth_type_trans() before running
the BPF program. That pulls the L2 header via eth_skb_pull_mac(). Then,
if we run an LWT program calling bpf_clone_redirect, it calls
eth_type_trans() again, trying to pull the L2 header a second time.

IIRC I hit this exact issue on an earlier version, before adding the
!is_l2 check, when I enabled non-linear on all programs in test_loader.
empty_skb.c was then triggering kernel BUGs.

I couldn't find a case where this is an actual issue in LWT because it
seems we always at least linearize the L3 header (for route lookups)
before running the LWT programs. And the L3 header is bigger than
ETH_HLEN so eth_type_trans() doesn't throw a kernel BUG.

> 
> I am not familiar with the LWT but I recalled we have fixed some cases on
> the ETH_HLEN and they are now tested by the prog_tests/empty_skb.c.
> 
> I think I am missing something on how is the non-linear skb different from
> the linear skb here for lwt if it has ensured there is ETH_HLEN in the
> linear. I am not sure how active is the lwt/bpf usage now. If it is hard to
> support, I think it is fine to exclude it.

I'm sure it can be done, but not sure it's worth the hassle. As you say,
it's unclear how used they are. Even for Cilium, which I believe was the
initial use case, we don't use them.

> 
> CGROUP_SKB will be good to be supported at the beginning though.

Ack. I'll send a v6 excluding only the LWT types.

[...]

> > > Should linear_sz be limited to "PAGE_SIZE - headroom..." like how
> > > test_run_xdp() does it ?
> > 
> > That changes a bit the current behavior. Currently, we will return
> > EINVAL if a user try to pass more than "PAGE_SIZE - headroom..." as
> > data_size_in. With the test_run_xdp approach, we'll end up silently
> > switching to non-linear mode if they do that.
> > 
> > I'm not against it given it brings consistency with the XDP counterpart,
> > but it could also be a bit surprising.
> 
> I think this is fine to accept what was previously rejected because of
> lacking non-linear skb support.
> 
> 
> I would prefer to have similar expectation as the test_run_xdp for the parts
> that make sense.

Ack.

[PATCH bpf-next v5 4/5] selftests/bpf: Support non-linear flag in test loader

[Paul Chaignon]

This patch adds support for a new tag __linear_size in the test loader,
to specify the size of the linear area in case of non-linear skbs. If
the tag is absent or null, a linear skb is crafted.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 tools/testing/selftests/bpf/progs/bpf_misc.h  |  4 ++++
 .../bpf/progs/verifier_direct_packet_access.c |  1 +
 tools/testing/selftests/bpf/test_loader.c     | 19 +++++++++++++++++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
index a7a1a684eed1..c9bfbe1bafc1 100644
--- a/tools/testing/selftests/bpf/progs/bpf_misc.h
+++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
@@ -126,6 +126,9 @@
  *                   Several __arch_* annotations could be specified at once.
  *                   When test case is not run on current arch it is marked as skipped.
  * __caps_unpriv     Specify the capabilities that should be set when running the test.
+ *
+ * __linear_size     Specify the size of the linear area of non-linear skbs, or
+ *                   0 for linear skbs.
  */
 #define __msg(msg)		__attribute__((btf_decl_tag("comment:test_expect_msg=" XSTR(__COUNTER__) "=" msg)))
 #define __not_msg(msg)		__attribute__((btf_decl_tag("comment:test_expect_not_msg=" XSTR(__COUNTER__) "=" msg)))
@@ -159,6 +162,7 @@
 #define __stderr_unpriv(msg)	__attribute__((btf_decl_tag("comment:test_expect_stderr_unpriv=" XSTR(__COUNTER__) "=" msg)))
 #define __stdout(msg)		__attribute__((btf_decl_tag("comment:test_expect_stdout=" XSTR(__COUNTER__) "=" msg)))
 #define __stdout_unpriv(msg)	__attribute__((btf_decl_tag("comment:test_expect_stdout_unpriv=" XSTR(__COUNTER__) "=" msg)))
+#define __linear_size(sz)	__attribute__((btf_decl_tag("comment:test_linear_size=" XSTR(sz))))
 
 /* Define common capabilities tested using __caps_unpriv */
 #define CAP_NET_ADMIN		12
diff --git a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
index 28b602ac9cbe..a61897e01a50 100644
--- a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
+++ b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Converted from tools/testing/selftests/bpf/verifier/direct_packet_access.c */
 
+#include <linux/if_ether.h>
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
index 74ecc281bb8c..690181617f04 100644
--- a/tools/testing/selftests/bpf/test_loader.c
+++ b/tools/testing/selftests/bpf/test_loader.c
@@ -43,6 +43,7 @@
 #define TEST_TAG_EXPECT_STDERR_PFX_UNPRIV "comment:test_expect_stderr_unpriv="
 #define TEST_TAG_EXPECT_STDOUT_PFX "comment:test_expect_stdout="
 #define TEST_TAG_EXPECT_STDOUT_PFX_UNPRIV "comment:test_expect_stdout_unpriv="
+#define TEST_TAG_LINEAR_SIZE "comment:test_linear_size="
 
 /* Warning: duplicated in bpf_misc.h */
 #define POINTER_VALUE	0xbadcafe
@@ -89,6 +90,7 @@ struct test_spec {
 	int mode_mask;
 	int arch_mask;
 	int load_mask;
+	int linear_sz;
 	bool auxiliary;
 	bool valid;
 };
@@ -633,6 +635,11 @@ static int parse_test_spec(struct test_loader *tester,
 					      &spec->unpriv.stdout);
 			if (err)
 				goto cleanup;
+		} else if (str_has_pfx(s, TEST_TAG_LINEAR_SIZE)) {
+			val = s + sizeof(TEST_TAG_LINEAR_SIZE) - 1;
+			err = parse_int(val, &spec->linear_sz, "test linear size");
+			if (err)
+				goto cleanup;
 		}
 	}
 
@@ -1007,10 +1014,11 @@ static bool is_unpriv_capable_map(struct bpf_map *map)
 	}
 }
 
-static int do_prog_test_run(int fd_prog, int *retval, bool empty_opts)
+static int do_prog_test_run(int fd_prog, int *retval, bool empty_opts, int linear_sz)
 {
 	__u8 tmp_out[TEST_DATA_LEN << 2] = {};
 	__u8 tmp_in[TEST_DATA_LEN] = {};
+	struct __sk_buff ctx = {};
 	int err, saved_errno;
 	LIBBPF_OPTS(bpf_test_run_opts, topts,
 		.data_in = tmp_in,
@@ -1020,6 +1028,12 @@ static int do_prog_test_run(int fd_prog, int *retval, bool empty_opts)
 		.repeat = 1,
 	);
 
+	if (linear_sz) {
+		ctx.data_end = linear_sz;
+		topts.ctx_in = &ctx;
+		topts.ctx_size_in = sizeof(ctx);
+	}
+
 	if (empty_opts) {
 		memset(&topts, 0, sizeof(struct bpf_test_run_opts));
 		topts.sz = sizeof(struct bpf_test_run_opts);
@@ -1269,7 +1283,8 @@ void run_subtest(struct test_loader *tester,
 		}
 
 		err = do_prog_test_run(bpf_program__fd(tprog), &retval,
-				       bpf_program__type(tprog) == BPF_PROG_TYPE_SYSCALL ? true : false);
+				       bpf_program__type(tprog) == BPF_PROG_TYPE_SYSCALL ? true : false,
+				       spec->linear_sz);
 		if (!err && retval != subspec->retval && subspec->retval != POINTER_VALUE) {
 			PRINT_FAIL("Unexpected retval: %d != %d\n", retval, subspec->retval);
 			goto tobj_cleanup;
-- 
2.43.0

[PATCH bpf-next v5 5/5] selftests/bpf: Test direct packet access on non-linear skbs

[Paul Chaignon]

This patch adds new selftests in the direct packet access suite, to
cover the non-linear case. The three first tests cover the behavior of
the bounds check with a non-linear skb (first two with min. linear
size, third with long enough linear size). The last test adds a call to
bpf_skb_pull_data() to be able to access the packet.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 .../bpf/progs/verifier_direct_packet_access.c | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
index a61897e01a50..c5745a4ae0fd 100644
--- a/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
+++ b/tools/testing/selftests/bpf/progs/verifier_direct_packet_access.c
@@ -801,4 +801,57 @@ l0_%=:	/* exit(0) */					\
 	: __clobber_all);
 }
 
+#define access_test_non_linear(name, desc, retval, linear_sz)				\
+	SEC("tc")									\
+	__description("direct packet access: " #name " (non-linear, " desc ")")		\
+	__success __retval(retval)							\
+	__linear_size(linear_sz)							\
+	__naked void access_##name(void)						\
+	{										\
+		asm volatile ("								\
+		r2 = *(u32*)(r1 + %[skb_data]);						\
+		r3 = *(u32*)(r1 + %[skb_data_end]);					\
+		r0 = r2;								\
+		r0 += 22;								\
+		if r0 > r3 goto l0_%=;							\
+		r0 = *(u8*)(r0 - 1);							\
+		exit;									\
+	l0_%=:	r0 = 1;									\
+		exit;									\
+	"	:									\
+		: __imm_const(skb_data, offsetof(struct __sk_buff, data)),		\
+		  __imm_const(skb_data_end, offsetof(struct __sk_buff, data_end))	\
+		: __clobber_all);							\
+	}
+
+access_test_non_linear(test31, "too short eth", 1, ETH_HLEN);
+access_test_non_linear(test32, "too short 1", 1, 1);
+access_test_non_linear(test33, "long enough", 0, 22);
+
+SEC("tc")
+__description("direct packet access: test34 (non-linear, linearized)")
+__success __retval(0)
+__linear_size(ETH_HLEN)
+__naked void access_test34_non_linear_linearized(void)
+{
+	asm volatile ("					\
+	r6 = r1;					\
+	r2 = 22;					\
+	call %[bpf_skb_pull_data];			\
+	r2 = *(u32*)(r6 + %[skb_data]);			\
+	r3 = *(u32*)(r6 + %[skb_data_end]);		\
+	r0 = r2;					\
+	r0 += 22;					\
+	if r0 > r3 goto l0_%=;				\
+	r0 = *(u8*)(r0 - 1);				\
+	exit;						\
+l0_%=:	r0 = 1;						\
+	exit;						\
+"	:
+	: __imm(bpf_skb_pull_data),
+	  __imm_const(skb_data, offsetof(struct __sk_buff, data)),
+	  __imm_const(skb_data_end, offsetof(struct __sk_buff, data_end))
+	: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.43.0