BPF Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run
@ 2020-08-02 18:26 Dmitry Yakunin
  2020-08-02 18:26 ` [PATCH bpf-next v4 1/2] bpf: setup socket family and addresses in bpf_prog_test_run_skb Dmitry Yakunin
  2020-08-02 18:26 ` [PATCH bpf-next v4 2/2] bpf: allow to specify ifindex for skb " Dmitry Yakunin
  0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Yakunin @ 2020-08-02 18:26 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, netdev, bpf; +Cc: sdf

This patchset contains some improvements for testing cgroup/skb programs
through BPF_PROG_TEST_RUN command.

v2:
  - fix build without CONFIG_CGROUP_BPF (kernel test robot <lkp@intel.com>)

v3:
  - fix build without CONFIG_IPV6 (kernel test robot <lkp@intel.com>)

v4:
  - remove cgroup storage related commits for future rework (Daniel Borkmann)

Dmitry Yakunin (2):
  bpf: setup socket family and addresses in bpf_prog_test_run_skb
  bpf: allow to specify ifindex for skb in bpf_prog_test_run_skb

 net/bpf/test_run.c                               | 39 ++++++++++++++++++++++--
 tools/testing/selftests/bpf/prog_tests/skb_ctx.c |  5 +++
 2 files changed, 42 insertions(+), 2 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v4 1/2] bpf: setup socket family and addresses in bpf_prog_test_run_skb
  2020-08-02 18:26 [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run Dmitry Yakunin
@ 2020-08-02 18:26 ` Dmitry Yakunin
  2020-08-02 20:19   ` Eric Dumazet
  2020-08-02 18:26 ` [PATCH bpf-next v4 2/2] bpf: allow to specify ifindex for skb " Dmitry Yakunin
  1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Yakunin @ 2020-08-02 18:26 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, netdev, bpf; +Cc: sdf

Now it's impossible to test all branches of cgroup_skb bpf program which
accesses skb->family and skb->{local,remote}_ip{4,6} fields because they
are zeroed during socket allocation. This commit fills socket family and
addresses from related fields in constructed skb.

v2:
  - fix build without CONFIG_IPV6 (kernel test robot <lkp@intel.com>)

Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
 net/bpf/test_run.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index b03c469..2521b27 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -449,6 +449,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);
 	skb_reset_network_header(skb);
 
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		sk->sk_family = AF_INET;
+		sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
+		sk->sk_daddr = ip_hdr(skb)->daddr;
+		break;
+#if IS_ENABLED(CONFIG_IPV6)
+	case htons(ETH_P_IPV6):
+		sk->sk_family = AF_INET6;
+		sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
+		sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
+		break;
+#endif
+	default:
+		break;
+	}
+
 	if (is_l2)
 		__skb_push(skb, hh_len);
 	if (is_direct_pkt_access)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v4 2/2] bpf: allow to specify ifindex for skb in bpf_prog_test_run_skb
  2020-08-02 18:26 [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run Dmitry Yakunin
  2020-08-02 18:26 ` [PATCH bpf-next v4 1/2] bpf: setup socket family and addresses in bpf_prog_test_run_skb Dmitry Yakunin
@ 2020-08-02 18:26 ` Dmitry Yakunin
  1 sibling, 0 replies; 6+ messages in thread
From: Dmitry Yakunin @ 2020-08-02 18:26 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, netdev, bpf; +Cc: sdf

Now skb->dev is unconditionally set to the loopback device in current net
namespace. But if we want to test bpf program which contains code branch
based on ifindex condition (eg filters out localhost packets) it is useful
to allow specifying of ifindex from userspace. This patch adds such option
through ctx_in (__sk_buff) parameter.

Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
 net/bpf/test_run.c                               | 22 ++++++++++++++++++++--
 tools/testing/selftests/bpf/prog_tests/skb_ctx.c |  5 +++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 2521b27..99cdd3c 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -327,6 +327,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 	/* priority is allowed */
 
 	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, priority),
+			   offsetof(struct __sk_buff, ifindex)))
+		return -EINVAL;
+
+	/* ifindex is allowed */
+
+	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, ifindex),
 			   offsetof(struct __sk_buff, cb)))
 		return -EINVAL;
 
@@ -381,6 +387,7 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
 
 	__skb->mark = skb->mark;
 	__skb->priority = skb->priority;
+	__skb->ifindex = skb->dev->ifindex;
 	__skb->tstamp = skb->tstamp;
 	memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
 	__skb->wire_len = cb->pkt_len;
@@ -391,6 +398,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 			  union bpf_attr __user *uattr)
 {
 	bool is_l2 = false, is_direct_pkt_access = false;
+	struct net *net = current->nsproxy->net_ns;
+	struct net_device *dev = net->loopback_dev;
 	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	struct __sk_buff *ctx = NULL;
@@ -432,7 +441,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		kfree(ctx);
 		return -ENOMEM;
 	}
-	sock_net_set(sk, current->nsproxy->net_ns);
+	sock_net_set(sk, net);
 	sock_init_data(NULL, sk);
 
 	skb = build_skb(data, 0);
@@ -446,7 +455,14 @@ 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);
-	skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);
+	if (ctx && ctx->ifindex > 1) {
+		dev = dev_get_by_index(net, ctx->ifindex);
+		if (!dev) {
+			ret = -ENODEV;
+			goto out;
+		}
+	}
+	skb->protocol = eth_type_trans(skb, dev);
 	skb_reset_network_header(skb);
 
 	switch (skb->protocol) {
@@ -498,6 +514,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		ret = bpf_ctx_finish(kattr, uattr, ctx,
 				     sizeof(struct __sk_buff));
 out:
+	if (dev && dev != net->loopback_dev)
+		dev_put(dev);
 	kfree_skb(skb);
 	bpf_sk_storage_free(sk);
 	kfree(sk);
diff --git a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
index 7021b92..25de86a 100644
--- a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
+++ b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
@@ -11,6 +11,7 @@ void test_skb_ctx(void)
 		.cb[3] = 4,
 		.cb[4] = 5,
 		.priority = 6,
+		.ifindex = 1,
 		.tstamp = 7,
 		.wire_len = 100,
 		.gso_segs = 8,
@@ -92,6 +93,10 @@ void test_skb_ctx(void)
 		   "ctx_out_priority",
 		   "skb->priority == %d, expected %d\n",
 		   skb.priority, 7);
+	CHECK_ATTR(skb.ifindex != 1,
+		   "ctx_out_ifindex",
+		   "skb->ifindex == %d, expected %d\n",
+		   skb.ifindex, 1);
 	CHECK_ATTR(skb.tstamp != 8,
 		   "ctx_out_tstamp",
 		   "skb->tstamp == %lld, expected %d\n",
-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v4 1/2] bpf: setup socket family and addresses in bpf_prog_test_run_skb
  2020-08-02 18:26 ` [PATCH bpf-next v4 1/2] bpf: setup socket family and addresses in bpf_prog_test_run_skb Dmitry Yakunin
@ 2020-08-02 20:19   ` Eric Dumazet
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2020-08-02 20:19 UTC (permalink / raw)
  To: Dmitry Yakunin, alexei.starovoitov, daniel, netdev, bpf; +Cc: sdf



On 8/2/20 11:26 AM, Dmitry Yakunin wrote:
> Now it's impossible to test all branches of cgroup_skb bpf program which
> accesses skb->family and skb->{local,remote}_ip{4,6} fields because they
> are zeroed during socket allocation. This commit fills socket family and
> addresses from related fields in constructed skb.
> 
> v2:
>   - fix build without CONFIG_IPV6 (kernel test robot <lkp@intel.com>)
> 
> Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
> ---
>  net/bpf/test_run.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index b03c469..2521b27 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -449,6 +449,23 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>  	skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);
>  	skb_reset_network_header(skb);
>  

At this point, there is no guarantee the skb contains these headers.

You will have to add safety checks against skb->len

> +	switch (skb->protocol) {
> +	case htons(ETH_P_IP):
> +		sk->sk_family = AF_INET;
> +		sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
> +		sk->sk_daddr = ip_hdr(skb)->daddr;
> +		break;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case htons(ETH_P_IPV6):
> +		sk->sk_family = AF_INET6;
> +		sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
> +		sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
> +		break;
> +#endif
> +	default:
> +		break;
> +	}
> +
>  	if (is_l2)
>  		__skb_push(skb, hh_len);
>  	if (is_direct_pkt_access)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run
  2020-08-02 21:30 [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run Dmitry Yakunin
@ 2020-08-02 21:35 ` Dmitry Yakunin
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Yakunin @ 2020-08-02 21:35 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, netdev, bpf; +Cc: sdf

Sorry, forgot to bump version in cover letter subject. I will resend it.

03.08.2020, 00:30, "Dmitry Yakunin" <zeil@yandex-team.ru>:
> This patchset contains some improvements for testing cgroup/skb programs
> through BPF_PROG_TEST_RUN command.
>
> v2:
>   - fix build without CONFIG_CGROUP_BPF (kernel test robot <lkp@intel.com>)
>
> v3:
>   - fix build without CONFIG_IPV6 (kernel test robot <lkp@intel.com>)
>
> v4:
>   - remove cgroup storage related commits for future rework (Daniel Borkmann)
>
> v5:
>   - check skb length before access to inet headers (Eric Dumazet)
>
> Dmitry Yakunin (2):
>   bpf: setup socket family and addresses in bpf_prog_test_run_skb
>   bpf: allow to specify ifindex for skb in bpf_prog_test_run_skb
>
>  net/bpf/test_run.c | 39 ++++++++++++++++++++++--
>  tools/testing/selftests/bpf/prog_tests/skb_ctx.c | 5 +++
>  2 files changed, 42 insertions(+), 2 deletions(-)
>
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run
@ 2020-08-02 21:30 Dmitry Yakunin
  2020-08-02 21:35 ` Dmitry Yakunin
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Yakunin @ 2020-08-02 21:30 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, netdev, bpf; +Cc: sdf

This patchset contains some improvements for testing cgroup/skb programs
through BPF_PROG_TEST_RUN command.

v2:
  - fix build without CONFIG_CGROUP_BPF (kernel test robot <lkp@intel.com>)

v3:
  - fix build without CONFIG_IPV6 (kernel test robot <lkp@intel.com>)

v4:
  - remove cgroup storage related commits for future rework (Daniel Borkmann)

v5:
  - check skb length before access to inet headers (Eric Dumazet)

Dmitry Yakunin (2):
  bpf: setup socket family and addresses in bpf_prog_test_run_skb
  bpf: allow to specify ifindex for skb in bpf_prog_test_run_skb

 net/bpf/test_run.c                               | 39 ++++++++++++++++++++++--
 tools/testing/selftests/bpf/prog_tests/skb_ctx.c |  5 +++
 2 files changed, 42 insertions(+), 2 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-02 18:26 [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run Dmitry Yakunin
2020-08-02 18:26 ` [PATCH bpf-next v4 1/2] bpf: setup socket family and addresses in bpf_prog_test_run_skb Dmitry Yakunin
2020-08-02 20:19   ` Eric Dumazet
2020-08-02 18:26 ` [PATCH bpf-next v4 2/2] bpf: allow to specify ifindex for skb " Dmitry Yakunin
2020-08-02 21:30 [PATCH bpf-next v4 0/2] bpf: cgroup skb improvements for bpf_prog_test_run Dmitry Yakunin
2020-08-02 21:35 ` Dmitry Yakunin

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git