linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
@ 2021-11-02  5:59 Di Zhu
  2021-11-02  5:59 ` [PATCH bpf-next v4 2/2] selftests: bpf: test " Di Zhu
  0 siblings, 1 reply; 8+ messages in thread
From: Di Zhu @ 2021-11-02  5:59 UTC (permalink / raw)
  To: davem, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, jakub
  Cc: bpf, linux-kernel, zhudi2

Right now there is no way to query whether BPF programs are
attached to a sockmap or not.

we can use the standard interface in libbpf to query, such as:
bpf_prog_query(mapFd, BPF_SK_SKB_STREAM_PARSER, 0, NULL, ...);
the mapFd is the fd of sockmap.

Signed-off-by: Di Zhu <zhudi2@huawei.com>
---
 include/linux/bpf.h  |  9 +++++
 kernel/bpf/syscall.c |  5 +++
 net/core/sock_map.c  | 88 ++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d604c8251d88..db7d0e5115b7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1961,6 +1961,9 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
 int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog *prog);
 int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype);
 int sock_map_update_elem_sys(struct bpf_map *map, void *key, void *value, u64 flags);
+int sockmap_bpf_prog_query(const union bpf_attr *attr,
+				 union bpf_attr __user *uattr);
+
 void sock_map_unhash(struct sock *sk);
 void sock_map_close(struct sock *sk, long timeout);
 #else
@@ -2014,6 +2017,12 @@ static inline int sock_map_update_elem_sys(struct bpf_map *map, void *key, void
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int sockmap_bpf_prog_query(const union bpf_attr *attr,
+					       union bpf_attr __user *uattr)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_BPF_SYSCALL */
 #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4e50c0bfdb7d..17faeff8f85f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3275,6 +3275,11 @@ static int bpf_prog_query(const union bpf_attr *attr,
 	case BPF_FLOW_DISSECTOR:
 	case BPF_SK_LOOKUP:
 		return netns_bpf_prog_query(attr, uattr);
+	case BPF_SK_SKB_STREAM_PARSER:
+	case BPF_SK_SKB_STREAM_VERDICT:
+	case BPF_SK_MSG_VERDICT:
+	case BPF_SK_SKB_VERDICT:
+		return sockmap_bpf_prog_query(attr, uattr);
 	default:
 		return -EINVAL;
 	}
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index e252b8ec2b85..ca65ed0004d3 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -1412,38 +1412,50 @@ static struct sk_psock_progs *sock_map_progs(struct bpf_map *map)
 	return NULL;
 }
 
-static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
-				struct bpf_prog *old, u32 which)
+static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog **pprog[],
+				u32 which)
 {
 	struct sk_psock_progs *progs = sock_map_progs(map);
-	struct bpf_prog **pprog;
 
 	if (!progs)
 		return -EOPNOTSUPP;
 
 	switch (which) {
 	case BPF_SK_MSG_VERDICT:
-		pprog = &progs->msg_parser;
+		*pprog = &progs->msg_parser;
 		break;
 #if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
 	case BPF_SK_SKB_STREAM_PARSER:
-		pprog = &progs->stream_parser;
+		*pprog = &progs->stream_parser;
 		break;
 #endif
 	case BPF_SK_SKB_STREAM_VERDICT:
 		if (progs->skb_verdict)
 			return -EBUSY;
-		pprog = &progs->stream_verdict;
+		*pprog = &progs->stream_verdict;
 		break;
 	case BPF_SK_SKB_VERDICT:
 		if (progs->stream_verdict)
 			return -EBUSY;
-		pprog = &progs->skb_verdict;
+		*pprog = &progs->skb_verdict;
 		break;
 	default:
 		return -EOPNOTSUPP;
 	}
 
+	return 0;
+}
+
+static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
+				struct bpf_prog *old, u32 which)
+{
+	struct bpf_prog **pprog;
+	int ret;
+
+	ret = sock_map_prog_lookup(map, &pprog, which);
+	if (ret)
+		return ret;
+
 	if (old)
 		return psock_replace_prog(pprog, prog, old);
 
@@ -1451,6 +1463,68 @@ static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
 	return 0;
 }
 
+int sockmap_bpf_prog_query(const union bpf_attr *attr,
+			   union bpf_attr __user *uattr)
+{
+	__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
+	u32 prog_cnt = 0, flags = 0;
+	u32 ufd = attr->target_fd;
+	struct bpf_prog **pprog;
+	struct bpf_prog *prog;
+	struct bpf_map *map;
+	struct fd f;
+	int ret;
+	u32 id = 0;
+
+	if (attr->query.query_flags)
+		return -EINVAL;
+
+	f = fdget(ufd);
+	map = __bpf_map_get(f);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	rcu_read_lock();
+
+	ret = sock_map_prog_lookup(map, &pprog, attr->query.attach_type);
+	if (ret)
+		goto end;
+
+	prog = *pprog;
+	prog_cnt = (!prog) ? 0 : 1;
+
+	if (!attr->query.prog_cnt || !prog_ids || !prog_cnt)
+		goto end;
+
+	prog = bpf_prog_inc_not_zero(prog);
+	if (IS_ERR(prog)) {
+		ret = PTR_ERR(prog);
+		goto end;
+	}
+	id = prog->aux->id;
+	bpf_prog_put(prog);
+
+end:
+	rcu_read_unlock();
+
+	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
+		ret = -EFAULT;
+		goto err;
+	}
+	if (id != 0 && copy_to_user(prog_ids, &id, sizeof(u32))) {
+		ret = -EFAULT;
+		goto err;
+	}
+	if (copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt))) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+err:
+	fdput(f);
+	return ret;
+}
+
 static void sock_map_unlink(struct sock *sk, struct sk_psock_link *link)
 {
 	switch (link->map->map_type) {
-- 
2.27.0


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

* [PATCH bpf-next v4 2/2] selftests: bpf: test BPF_PROG_QUERY for progs attached to sockmap
  2021-11-02  5:59 [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap Di Zhu
@ 2021-11-02  5:59 ` Di Zhu
  0 siblings, 0 replies; 8+ messages in thread
From: Di Zhu @ 2021-11-02  5:59 UTC (permalink / raw)
  To: davem, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, jakub
  Cc: bpf, linux-kernel, zhudi2

Add test for querying progs attached to sockmap. we use an existing
libbpf query interface to query prog cnt before and after progs
attaching to sockmap and check whether the queried prog id is right.

Signed-off-by: Di Zhu <zhudi2@huawei.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 85 +++++++++++++++++++
 .../bpf/progs/test_sockmap_progs_query.c      | 24 ++++++
 2 files changed, 109 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_progs_query.c

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 1352ec104149..3ef4a7341e56 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -8,6 +8,7 @@
 #include "test_sockmap_update.skel.h"
 #include "test_sockmap_invalid_update.skel.h"
 #include "test_sockmap_skb_verdict_attach.skel.h"
+#include "test_sockmap_progs_query.skel.h"
 #include "bpf_iter_sockmap.skel.h"
 
 #define TCP_REPAIR		19	/* TCP sock is under repair right now */
@@ -315,6 +316,84 @@ static void test_sockmap_skb_verdict_attach(enum bpf_attach_type first,
 	test_sockmap_skb_verdict_attach__destroy(skel);
 }
 
+static __u32 query_prog_id(int prog)
+{
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
+	int err;
+
+	err = bpf_obj_get_info_by_fd(prog, &info, &info_len);
+	if (CHECK_FAIL(err || info_len != sizeof(info))) {
+		perror("bpf_obj_get_info_by_fd");
+		return 0;
+	}
+
+	return info.id;
+}
+
+static void test_sockmap_progs_query(enum bpf_attach_type attach_type)
+{
+	struct test_sockmap_progs_query *skel;
+	int err, map, verdict, duration = 0;
+	__u32 attach_flags = 0;
+	__u32 prog_ids[3] = {};
+	__u32 prog_cnt = 3;
+
+	skel = test_sockmap_progs_query__open_and_load();
+	if (CHECK_FAIL(!skel)) {
+		perror("test_sockmap_progs_query__open_and_load");
+		return;
+	}
+
+	map = bpf_map__fd(skel->maps.sock_map);
+
+	if (attach_type == BPF_SK_MSG_VERDICT)
+		verdict = bpf_program__fd(skel->progs.prog_skmsg_verdict);
+	else
+		verdict = bpf_program__fd(skel->progs.prog_skb_verdict);
+
+	err = bpf_prog_query(map, attach_type, 0 /* query flags */,
+			     &attach_flags, prog_ids, &prog_cnt);
+	if (CHECK(err, "bpf_prog_query", "failed\n"))
+		goto out;
+
+	if (CHECK(attach_flags != 0, "bpf_prog_query",
+		  "wrong attach_flags on query: %u", attach_flags))
+		goto out;
+
+	if (CHECK(prog_cnt != 0, "bpf_prog_query",
+		  "wrong program count on query: %u", prog_cnt))
+		goto out;
+
+	err = bpf_prog_attach(verdict, map, attach_type, 0);
+	if (CHECK(err, "bpf_prog_attach", "failed\n"))
+		goto out;
+
+	prog_cnt = 1;
+	err = bpf_prog_query(map, attach_type, 0 /* query flags */,
+			     &attach_flags, prog_ids, &prog_cnt);
+	if (CHECK(err, "bpf_prog_query", "failed\n"))
+		goto detach;
+
+	if (CHECK(attach_flags != 0, "bpf_prog_query attach_flags",
+		  "wrong attach_flags on query: %u", attach_flags))
+		goto detach;
+
+	if (CHECK(prog_cnt != 1, "bpf_prog_query prog_cnt",
+		  "wrong program count on query: %u", prog_cnt))
+		goto detach;
+
+	if (CHECK(prog_ids[0] != query_prog_id(verdict), "bpf_prog_query",
+		  "wrong prog_ids on query: %u", prog_ids[0]))
+		goto detach;
+
+detach:
+	bpf_prog_detach2(verdict, map, attach_type);
+out:
+	test_sockmap_progs_query__destroy(skel);
+
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
@@ -341,4 +420,10 @@ void test_sockmap_basic(void)
 		test_sockmap_skb_verdict_attach(BPF_SK_SKB_STREAM_VERDICT,
 						BPF_SK_SKB_VERDICT);
 	}
+	if (test__start_subtest("sockmap progs query")) {
+		test_sockmap_progs_query(BPF_SK_MSG_VERDICT);
+		test_sockmap_progs_query(BPF_SK_SKB_STREAM_PARSER);
+		test_sockmap_progs_query(BPF_SK_SKB_STREAM_VERDICT);
+		test_sockmap_progs_query(BPF_SK_SKB_VERDICT);
+	}
 }
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_progs_query.c b/tools/testing/selftests/bpf/progs/test_sockmap_progs_query.c
new file mode 100644
index 000000000000..9d58d61c0dee
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_progs_query.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKMAP);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u64);
+} sock_map SEC(".maps");
+
+SEC("sk_skb")
+int prog_skb_verdict(struct __sk_buff *skb)
+{
+	return SK_PASS;
+}
+
+SEC("sk_msg")
+int prog_skmsg_verdict(struct sk_msg_md *msg)
+{
+	return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.27.0


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

* Re: [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
  2021-11-03  2:23 zhudi (E)
@ 2021-11-03  2:37 ` Yonghong Song
  0 siblings, 0 replies; 8+ messages in thread
From: Yonghong Song @ 2021-11-03  2:37 UTC (permalink / raw)
  To: zhudi (E),
	davem, ast, daniel, andrii, kafai, songliubraving,
	john.fastabend, kpsingh, jakub
  Cc: bpf, linux-kernel



On 11/2/21 7:23 PM, zhudi (E) wrote:
>> On 11/2/21 1:48 AM, Di Zhu wrote:
>>> Right now there is no way to query whether BPF programs are
>>> attached to a sockmap or not.
>>>
>>> we can use the standard interface in libbpf to query, such as:
>>> bpf_prog_query(mapFd, BPF_SK_SKB_STREAM_PARSER, 0, NULL, ...);
>>> the mapFd is the fd of sockmap.
>>>
>>> Signed-off-by: Di Zhu <zhudi2@huawei.com>
>>> ---
>>>    include/linux/bpf.h  |  9 +++++
>>>    kernel/bpf/syscall.c |  5 +++
>>>    net/core/sock_map.c  | 88
>> ++++++++++++++++++++++++++++++++++++++++----
>>>    3 files changed, 95 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>> index d604c8251d88..594ca91992db 100644
>>> --- a/include/linux/bpf.h
>>> +++ b/include/linux/bpf.h
>>> @@ -1961,6 +1961,9 @@ int bpf_prog_test_run_syscall(struct bpf_prog
>> *prog,
>>>    int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog
>> *prog);
>>>    int sock_map_prog_detach(const union bpf_attr *attr, enum
>> bpf_prog_type ptype);
>>>    int sock_map_update_elem_sys(struct bpf_map *map, void *key, void
>> *value, u64 flags);
>>> +int sockmap_bpf_prog_query(const union bpf_attr *attr,
>>> +			   union bpf_attr __user *uattr);
>>
>> All previous functions are with prefix "sock_map". Why you choose
>> a different prefix "sockmap"?
>>
> 
> Thanks for all your suggestions, I will make changes to the inappropriate code.
> 
>>> +
>>>    void sock_map_unhash(struct sock *sk);
>>>    void sock_map_close(struct sock *sk, long timeout);
>>>    #else
>>> @@ -2014,6 +2017,12 @@ static inline int
>> sock_map_update_elem_sys(struct bpf_map *map, void *key, void
>>>    {
>>>    	return -EOPNOTSUPP;
>>>    }
>>> +
>>> +static inline int sockmap_bpf_prog_query(const union bpf_attr *attr,
>>> +					 union bpf_attr __user *uattr)
>>> +{
>>> +	return -EINVAL;
>>> +}
>>>    #endif /* CONFIG_BPF_SYSCALL */
>>>    #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
>>>
>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>> index 4e50c0bfdb7d..17faeff8f85f 100644
>>> --- a/kernel/bpf/syscall.c
>>> +++ b/kernel/bpf/syscall.c
>>> @@ -3275,6 +3275,11 @@ static int bpf_prog_query(const union bpf_attr
>> *attr,
>>>    	case BPF_FLOW_DISSECTOR:
>>>    	case BPF_SK_LOOKUP:
>>>    		return netns_bpf_prog_query(attr, uattr);
>>> +	case BPF_SK_SKB_STREAM_PARSER:
>>> +	case BPF_SK_SKB_STREAM_VERDICT:
>>> +	case BPF_SK_MSG_VERDICT:
>>> +	case BPF_SK_SKB_VERDICT:
>>> +		return sockmap_bpf_prog_query(attr, uattr);
>>>    	default:
>>>    		return -EINVAL;
>>>    	}
>>> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
>>> index e252b8ec2b85..ca65ed0004d3 100644
>>> --- a/net/core/sock_map.c
>>> +++ b/net/core/sock_map.c
>>> @@ -1412,38 +1412,50 @@ static struct sk_psock_progs
>> *sock_map_progs(struct bpf_map *map)
>>>    	return NULL;
>>>    }
>>>
>>> -static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog
>> *prog,
>>> -				struct bpf_prog *old, u32 which)
>>> +static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog
>> **pprog[],
>>
>> Can we just change "**pprog[]" to "***pprog"? In the code, you really
>> just pass the address of the decl "struct bpf_prog **pprog;" to the
>> function.
>>
>>> +				u32 which)
>>
>> Some format issue here?
> 
> 
> Format is right, passed the checkpatch script check.

Sorry about this. I guess my reply formating cheated me:

 >>> +static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog
 >> **pprog[],
 >>> +				u32 which)

I see a larger misalignment between "struct bpf_map *map" and
"u32 which" in the reply email. But looking at original
patch, there are no issues.

> 
> 
>>
>>>    {
>>>    	struct sk_psock_progs *progs = sock_map_progs(map);
>>> -	struct bpf_prog **pprog;
>>>
>>>    	if (!progs)
>>>    		return -EOPNOTSUPP;
[...]

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

* Re: [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
@ 2021-11-03  2:25 zhudi (E)
  0 siblings, 0 replies; 8+ messages in thread
From: zhudi (E) @ 2021-11-03  2:25 UTC (permalink / raw)
  To: Alexei Starovoitov, Yonghong Song
  Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, John Fastabend,
	KP Singh, Jakub Sitnicki, bpf, LKML

> On Tue, Nov 2, 2021 at 1:11 PM Yonghong Song <yhs@fb.com> wrote:
> > >
> > > -static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog
> *prog,
> > > -                             struct bpf_prog *old, u32 which)
> > > +static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog
> **pprog[],
> >
> > Can we just change "**pprog[]" to "***pprog"? In the code, you really
> > just pass the address of the decl "struct bpf_prog **pprog;" to the
> > function.
> 
> Di,
> 
> this feedback was given twice already.
> You also didn't address several other points from the earlier reviews.
> Please do not resubmit until you address all points.

Maybe i miss something...
I will recheck the review comments.

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

* Re: [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
@ 2021-11-03  2:23 zhudi (E)
  2021-11-03  2:37 ` Yonghong Song
  0 siblings, 1 reply; 8+ messages in thread
From: zhudi (E) @ 2021-11-03  2:23 UTC (permalink / raw)
  To: Yonghong Song, davem, ast, daniel, andrii, kafai, songliubraving,
	john.fastabend, kpsingh, jakub
  Cc: bpf, linux-kernel

> On 11/2/21 1:48 AM, Di Zhu wrote:
> > Right now there is no way to query whether BPF programs are
> > attached to a sockmap or not.
> >
> > we can use the standard interface in libbpf to query, such as:
> > bpf_prog_query(mapFd, BPF_SK_SKB_STREAM_PARSER, 0, NULL, ...);
> > the mapFd is the fd of sockmap.
> >
> > Signed-off-by: Di Zhu <zhudi2@huawei.com>
> > ---
> >   include/linux/bpf.h  |  9 +++++
> >   kernel/bpf/syscall.c |  5 +++
> >   net/core/sock_map.c  | 88
> ++++++++++++++++++++++++++++++++++++++++----
> >   3 files changed, 95 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index d604c8251d88..594ca91992db 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1961,6 +1961,9 @@ int bpf_prog_test_run_syscall(struct bpf_prog
> *prog,
> >   int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog
> *prog);
> >   int sock_map_prog_detach(const union bpf_attr *attr, enum
> bpf_prog_type ptype);
> >   int sock_map_update_elem_sys(struct bpf_map *map, void *key, void
> *value, u64 flags);
> > +int sockmap_bpf_prog_query(const union bpf_attr *attr,
> > +			   union bpf_attr __user *uattr);
> 
> All previous functions are with prefix "sock_map". Why you choose
> a different prefix "sockmap"?
> 

Thanks for all your suggestions, I will make changes to the inappropriate code.

> > +
> >   void sock_map_unhash(struct sock *sk);
> >   void sock_map_close(struct sock *sk, long timeout);
> >   #else
> > @@ -2014,6 +2017,12 @@ static inline int
> sock_map_update_elem_sys(struct bpf_map *map, void *key, void
> >   {
> >   	return -EOPNOTSUPP;
> >   }
> > +
> > +static inline int sockmap_bpf_prog_query(const union bpf_attr *attr,
> > +					 union bpf_attr __user *uattr)
> > +{
> > +	return -EINVAL;
> > +}
> >   #endif /* CONFIG_BPF_SYSCALL */
> >   #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 4e50c0bfdb7d..17faeff8f85f 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3275,6 +3275,11 @@ static int bpf_prog_query(const union bpf_attr
> *attr,
> >   	case BPF_FLOW_DISSECTOR:
> >   	case BPF_SK_LOOKUP:
> >   		return netns_bpf_prog_query(attr, uattr);
> > +	case BPF_SK_SKB_STREAM_PARSER:
> > +	case BPF_SK_SKB_STREAM_VERDICT:
> > +	case BPF_SK_MSG_VERDICT:
> > +	case BPF_SK_SKB_VERDICT:
> > +		return sockmap_bpf_prog_query(attr, uattr);
> >   	default:
> >   		return -EINVAL;
> >   	}
> > diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> > index e252b8ec2b85..ca65ed0004d3 100644
> > --- a/net/core/sock_map.c
> > +++ b/net/core/sock_map.c
> > @@ -1412,38 +1412,50 @@ static struct sk_psock_progs
> *sock_map_progs(struct bpf_map *map)
> >   	return NULL;
> >   }
> >
> > -static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog
> *prog,
> > -				struct bpf_prog *old, u32 which)
> > +static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog
> **pprog[],
> 
> Can we just change "**pprog[]" to "***pprog"? In the code, you really
> just pass the address of the decl "struct bpf_prog **pprog;" to the
> function.
> 
> > +				u32 which)
> 
> Some format issue here?


Format is right, passed the checkpatch script check.


> 
> >   {
> >   	struct sk_psock_progs *progs = sock_map_progs(map);
> > -	struct bpf_prog **pprog;
> >
> >   	if (!progs)
> >   		return -EOPNOTSUPP;
> >
> >   	switch (which) {
> >   	case BPF_SK_MSG_VERDICT:
> > -		pprog = &progs->msg_parser;
> > +		*pprog = &progs->msg_parser;
> >   		break;
> >   #if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
> >   	case BPF_SK_SKB_STREAM_PARSER:
> > -		pprog = &progs->stream_parser;
> > +		*pprog = &progs->stream_parser;
> >   		break;
> >   #endif
> >   	case BPF_SK_SKB_STREAM_VERDICT:
> >   		if (progs->skb_verdict)
> >   			return -EBUSY;
> > -		pprog = &progs->stream_verdict;
> > +		*pprog = &progs->stream_verdict;
> >   		break;
> >   	case BPF_SK_SKB_VERDICT:
> >   		if (progs->stream_verdict)
> >   			return -EBUSY;
> > -		pprog = &progs->skb_verdict;
> > +		*pprog = &progs->skb_verdict;
> >   		break;
> >   	default:
> >   		return -EOPNOTSUPP;
> >   	}
> >
> > +	return 0;
> > +}
> > +
> > +static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog
> *prog,
> > +				struct bpf_prog *old, u32 which)
> 
> Some format issue here?
> 
> > +{
> > +	struct bpf_prog **pprog;
> > +	int ret;
> > +
> > +	ret = sock_map_prog_lookup(map, &pprog, which);
> > +	if (ret)
> > +		return ret;
> > +
> >   	if (old)
> >   		return psock_replace_prog(pprog, prog, old);
> >
> > @@ -1451,6 +1463,68 @@ static int sock_map_prog_update(struct
> bpf_map *map, struct bpf_prog *prog,
> >   	return 0;
> >   }
> >
> > +int sockmap_bpf_prog_query(const union bpf_attr *attr,
> > +			   union bpf_attr __user *uattr)
> 
> Format issue here?
> 
> > +{
> > +	__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
> 
> Typically we use u32 in the kernel code. But I know there are __u32
> usage as well, esp. with __user attributes. I put a comment here just
> in case that somebody else has a different opinion.
> 
> > +	u32 prog_cnt = 0, flags = 0;
> > +	u32 ufd = attr->target_fd;
> 
> You can merge the above u32 together.
> 
> > +	struct bpf_prog **pprog;
> > +	struct bpf_prog *prog;
> > +	struct bpf_map *map;
> > +	struct fd f;
> > +	int ret;
> > +	u32 id = 0;
> 
> to maintain reverse christmas tree?
> 
> > +
> > +	if (attr->query.query_flags)
> > +		return -EINVAL;
> > +
> > +	f = fdget(ufd);
> > +	map = __bpf_map_get(f);
> > +	if (IS_ERR(map))
> > +		return PTR_ERR(map);
> > +
> > +	rcu_read_lock();
> > +
> > +	ret = sock_map_prog_lookup(map, &pprog, attr->query.attach_type);
> > +	if (ret)
> > +		goto end;
> > +
> > +	prog = *pprog;
> > +	prog_cnt = (!prog) ? 0 : 1;
> > +
> > +	if (!attr->query.prog_cnt || !prog_ids || !prog_cnt)
> > +		goto end;
> > +
> > +	prog = bpf_prog_inc_not_zero(prog);
> 
> Could you explain why we need bpf_prog_inc_not_zero here?
> We are inside rcu_read_lock/unlock region. We got a program
> from *pprog. If this program is not NULL, this program should
> not disappear since we are in rcu read lock region, right?
> Maybe I missed something, it would be good you can explain
> the scenario you try to pretect here.


bpf_prog_inc_not_zero() return a failure indicating that the program is
being released and prog->aux->id will be set to 0.
Yes, it is just ok for accessing prog->aux->id directly.


> > +	if (IS_ERR(prog)) {
> > +		ret = PTR_ERR(prog);
> > +		goto end;
> > +	}
> > +	id = prog->aux->id;
> > +	bpf_prog_put(prog);
> > +
> > +end:
> > +	rcu_read_unlock();
> > +
> > +	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
> > +		ret = -EFAULT;
> > +		goto err;
> > +	}
> > +	if (id != 0 && copy_to_user(prog_ids, &id, sizeof(u32))) {
> > +		ret = -EFAULT;
> > +		goto err;
> > +	}
> > +	if (copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt))) {
> > +		ret = -EFAULT;
> > +		goto err;
> > +	}
> 
> You can do
> 
> 	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)) ||
> 	    (id != 0 && copy_to_user(prog_ids, &id, sizeof(u32))) ||
> 	    copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt)))
> 		ret = -EFAULT;
> 
> to make code a little bit concise.
> 
> > +
> > +err:
> > +	fdput(f);
> > +	return ret;
> > +}
> > +
> >   static void sock_map_unlink(struct sock *sk, struct sk_psock_link *link)
> >   {
> >   	switch (link->map->map_type) {
> >

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

* Re: [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
  2021-11-02 20:11 ` Yonghong Song
@ 2021-11-02 21:16   ` Alexei Starovoitov
  0 siblings, 0 replies; 8+ messages in thread
From: Alexei Starovoitov @ 2021-11-02 21:16 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Di Zhu, David S. Miller, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, John Fastabend,
	KP Singh, Jakub Sitnicki, bpf, LKML

On Tue, Nov 2, 2021 at 1:11 PM Yonghong Song <yhs@fb.com> wrote:
> >
> > -static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
> > -                             struct bpf_prog *old, u32 which)
> > +static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog **pprog[],
>
> Can we just change "**pprog[]" to "***pprog"? In the code, you really
> just pass the address of the decl "struct bpf_prog **pprog;" to the
> function.

Di,

this feedback was given twice already.
You also didn't address several other points from the earlier reviews.
Please do not resubmit until you address all points.

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

* Re: [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
  2021-11-02  8:48 [PATCH bpf-next v4 1/2] bpf: support " Di Zhu
@ 2021-11-02 20:11 ` Yonghong Song
  2021-11-02 21:16   ` Alexei Starovoitov
  0 siblings, 1 reply; 8+ messages in thread
From: Yonghong Song @ 2021-11-02 20:11 UTC (permalink / raw)
  To: Di Zhu, davem, ast, daniel, andrii, kafai, songliubraving,
	john.fastabend, kpsingh, jakub
  Cc: bpf, linux-kernel



On 11/2/21 1:48 AM, Di Zhu wrote:
> Right now there is no way to query whether BPF programs are
> attached to a sockmap or not.
> 
> we can use the standard interface in libbpf to query, such as:
> bpf_prog_query(mapFd, BPF_SK_SKB_STREAM_PARSER, 0, NULL, ...);
> the mapFd is the fd of sockmap.
> 
> Signed-off-by: Di Zhu <zhudi2@huawei.com>
> ---
>   include/linux/bpf.h  |  9 +++++
>   kernel/bpf/syscall.c |  5 +++
>   net/core/sock_map.c  | 88 ++++++++++++++++++++++++++++++++++++++++----
>   3 files changed, 95 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d604c8251d88..594ca91992db 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1961,6 +1961,9 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
>   int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog *prog);
>   int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype);
>   int sock_map_update_elem_sys(struct bpf_map *map, void *key, void *value, u64 flags);
> +int sockmap_bpf_prog_query(const union bpf_attr *attr,
> +			   union bpf_attr __user *uattr);

All previous functions are with prefix "sock_map". Why you choose
a different prefix "sockmap"?

> +
>   void sock_map_unhash(struct sock *sk);
>   void sock_map_close(struct sock *sk, long timeout);
>   #else
> @@ -2014,6 +2017,12 @@ static inline int sock_map_update_elem_sys(struct bpf_map *map, void *key, void
>   {
>   	return -EOPNOTSUPP;
>   }
> +
> +static inline int sockmap_bpf_prog_query(const union bpf_attr *attr,
> +					 union bpf_attr __user *uattr)
> +{
> +	return -EINVAL;
> +}
>   #endif /* CONFIG_BPF_SYSCALL */
>   #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
>   
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 4e50c0bfdb7d..17faeff8f85f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3275,6 +3275,11 @@ static int bpf_prog_query(const union bpf_attr *attr,
>   	case BPF_FLOW_DISSECTOR:
>   	case BPF_SK_LOOKUP:
>   		return netns_bpf_prog_query(attr, uattr);
> +	case BPF_SK_SKB_STREAM_PARSER:
> +	case BPF_SK_SKB_STREAM_VERDICT:
> +	case BPF_SK_MSG_VERDICT:
> +	case BPF_SK_SKB_VERDICT:
> +		return sockmap_bpf_prog_query(attr, uattr);
>   	default:
>   		return -EINVAL;
>   	}
> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index e252b8ec2b85..ca65ed0004d3 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c
> @@ -1412,38 +1412,50 @@ static struct sk_psock_progs *sock_map_progs(struct bpf_map *map)
>   	return NULL;
>   }
>   
> -static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
> -				struct bpf_prog *old, u32 which)
> +static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog **pprog[],

Can we just change "**pprog[]" to "***pprog"? In the code, you really 
just pass the address of the decl "struct bpf_prog **pprog;" to the 
function.

> +				u32 which)

Some format issue here?

>   {
>   	struct sk_psock_progs *progs = sock_map_progs(map);
> -	struct bpf_prog **pprog;
>   
>   	if (!progs)
>   		return -EOPNOTSUPP;
>   
>   	switch (which) {
>   	case BPF_SK_MSG_VERDICT:
> -		pprog = &progs->msg_parser;
> +		*pprog = &progs->msg_parser;
>   		break;
>   #if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
>   	case BPF_SK_SKB_STREAM_PARSER:
> -		pprog = &progs->stream_parser;
> +		*pprog = &progs->stream_parser;
>   		break;
>   #endif
>   	case BPF_SK_SKB_STREAM_VERDICT:
>   		if (progs->skb_verdict)
>   			return -EBUSY;
> -		pprog = &progs->stream_verdict;
> +		*pprog = &progs->stream_verdict;
>   		break;
>   	case BPF_SK_SKB_VERDICT:
>   		if (progs->stream_verdict)
>   			return -EBUSY;
> -		pprog = &progs->skb_verdict;
> +		*pprog = &progs->skb_verdict;
>   		break;
>   	default:
>   		return -EOPNOTSUPP;
>   	}
>   
> +	return 0;
> +}
> +
> +static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
> +				struct bpf_prog *old, u32 which)

Some format issue here?

> +{
> +	struct bpf_prog **pprog;
> +	int ret;
> +
> +	ret = sock_map_prog_lookup(map, &pprog, which);
> +	if (ret)
> +		return ret;
> +
>   	if (old)
>   		return psock_replace_prog(pprog, prog, old);
>   
> @@ -1451,6 +1463,68 @@ static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
>   	return 0;
>   }
>   
> +int sockmap_bpf_prog_query(const union bpf_attr *attr,
> +			   union bpf_attr __user *uattr)

Format issue here?

> +{
> +	__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);

Typically we use u32 in the kernel code. But I know there are __u32 
usage as well, esp. with __user attributes. I put a comment here just
in case that somebody else has a different opinion.

> +	u32 prog_cnt = 0, flags = 0;
> +	u32 ufd = attr->target_fd;

You can merge the above u32 together.

> +	struct bpf_prog **pprog;
> +	struct bpf_prog *prog;
> +	struct bpf_map *map;
> +	struct fd f;
> +	int ret;
> +	u32 id = 0;

to maintain reverse christmas tree?

> +
> +	if (attr->query.query_flags)
> +		return -EINVAL;
> +
> +	f = fdget(ufd);
> +	map = __bpf_map_get(f);
> +	if (IS_ERR(map))
> +		return PTR_ERR(map);
> +
> +	rcu_read_lock();
> +
> +	ret = sock_map_prog_lookup(map, &pprog, attr->query.attach_type);
> +	if (ret)
> +		goto end;
> +
> +	prog = *pprog;
> +	prog_cnt = (!prog) ? 0 : 1;
> +
> +	if (!attr->query.prog_cnt || !prog_ids || !prog_cnt)
> +		goto end;
> +
> +	prog = bpf_prog_inc_not_zero(prog);

Could you explain why we need bpf_prog_inc_not_zero here?
We are inside rcu_read_lock/unlock region. We got a program
from *pprog. If this program is not NULL, this program should
not disappear since we are in rcu read lock region, right?
Maybe I missed something, it would be good you can explain
the scenario you try to pretect here.

> +	if (IS_ERR(prog)) {
> +		ret = PTR_ERR(prog);
> +		goto end;
> +	}
> +	id = prog->aux->id;
> +	bpf_prog_put(prog);
> +
> +end:
> +	rcu_read_unlock();
> +
> +	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	if (id != 0 && copy_to_user(prog_ids, &id, sizeof(u32))) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	if (copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt))) {
> +		ret = -EFAULT;
> +		goto err;
> +	}

You can do

	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)) ||
	    (id != 0 && copy_to_user(prog_ids, &id, sizeof(u32))) ||
	    copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt)))
		ret = -EFAULT;

to make code a little bit concise.

> +
> +err:
> +	fdput(f);
> +	return ret;
> +}
> +
>   static void sock_map_unlink(struct sock *sk, struct sk_psock_link *link)
>   {
>   	switch (link->map->map_type) {
> 

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

* [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap
@ 2021-11-02  8:48 Di Zhu
  2021-11-02 20:11 ` Yonghong Song
  0 siblings, 1 reply; 8+ messages in thread
From: Di Zhu @ 2021-11-02  8:48 UTC (permalink / raw)
  To: davem, ast, daniel, andrii, kafai, songliubraving, yhs,
	john.fastabend, kpsingh, jakub
  Cc: bpf, linux-kernel, zhudi2

Right now there is no way to query whether BPF programs are
attached to a sockmap or not.

we can use the standard interface in libbpf to query, such as:
bpf_prog_query(mapFd, BPF_SK_SKB_STREAM_PARSER, 0, NULL, ...);
the mapFd is the fd of sockmap.

Signed-off-by: Di Zhu <zhudi2@huawei.com>
---
 include/linux/bpf.h  |  9 +++++
 kernel/bpf/syscall.c |  5 +++
 net/core/sock_map.c  | 88 ++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d604c8251d88..594ca91992db 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1961,6 +1961,9 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
 int sock_map_get_from_fd(const union bpf_attr *attr, struct bpf_prog *prog);
 int sock_map_prog_detach(const union bpf_attr *attr, enum bpf_prog_type ptype);
 int sock_map_update_elem_sys(struct bpf_map *map, void *key, void *value, u64 flags);
+int sockmap_bpf_prog_query(const union bpf_attr *attr,
+			   union bpf_attr __user *uattr);
+
 void sock_map_unhash(struct sock *sk);
 void sock_map_close(struct sock *sk, long timeout);
 #else
@@ -2014,6 +2017,12 @@ static inline int sock_map_update_elem_sys(struct bpf_map *map, void *key, void
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int sockmap_bpf_prog_query(const union bpf_attr *attr,
+					 union bpf_attr __user *uattr)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_BPF_SYSCALL */
 #endif /* CONFIG_NET && CONFIG_BPF_SYSCALL */
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4e50c0bfdb7d..17faeff8f85f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3275,6 +3275,11 @@ static int bpf_prog_query(const union bpf_attr *attr,
 	case BPF_FLOW_DISSECTOR:
 	case BPF_SK_LOOKUP:
 		return netns_bpf_prog_query(attr, uattr);
+	case BPF_SK_SKB_STREAM_PARSER:
+	case BPF_SK_SKB_STREAM_VERDICT:
+	case BPF_SK_MSG_VERDICT:
+	case BPF_SK_SKB_VERDICT:
+		return sockmap_bpf_prog_query(attr, uattr);
 	default:
 		return -EINVAL;
 	}
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index e252b8ec2b85..ca65ed0004d3 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -1412,38 +1412,50 @@ static struct sk_psock_progs *sock_map_progs(struct bpf_map *map)
 	return NULL;
 }
 
-static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
-				struct bpf_prog *old, u32 which)
+static int sock_map_prog_lookup(struct bpf_map *map, struct bpf_prog **pprog[],
+				u32 which)
 {
 	struct sk_psock_progs *progs = sock_map_progs(map);
-	struct bpf_prog **pprog;
 
 	if (!progs)
 		return -EOPNOTSUPP;
 
 	switch (which) {
 	case BPF_SK_MSG_VERDICT:
-		pprog = &progs->msg_parser;
+		*pprog = &progs->msg_parser;
 		break;
 #if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
 	case BPF_SK_SKB_STREAM_PARSER:
-		pprog = &progs->stream_parser;
+		*pprog = &progs->stream_parser;
 		break;
 #endif
 	case BPF_SK_SKB_STREAM_VERDICT:
 		if (progs->skb_verdict)
 			return -EBUSY;
-		pprog = &progs->stream_verdict;
+		*pprog = &progs->stream_verdict;
 		break;
 	case BPF_SK_SKB_VERDICT:
 		if (progs->stream_verdict)
 			return -EBUSY;
-		pprog = &progs->skb_verdict;
+		*pprog = &progs->skb_verdict;
 		break;
 	default:
 		return -EOPNOTSUPP;
 	}
 
+	return 0;
+}
+
+static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
+				struct bpf_prog *old, u32 which)
+{
+	struct bpf_prog **pprog;
+	int ret;
+
+	ret = sock_map_prog_lookup(map, &pprog, which);
+	if (ret)
+		return ret;
+
 	if (old)
 		return psock_replace_prog(pprog, prog, old);
 
@@ -1451,6 +1463,68 @@ static int sock_map_prog_update(struct bpf_map *map, struct bpf_prog *prog,
 	return 0;
 }
 
+int sockmap_bpf_prog_query(const union bpf_attr *attr,
+			   union bpf_attr __user *uattr)
+{
+	__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
+	u32 prog_cnt = 0, flags = 0;
+	u32 ufd = attr->target_fd;
+	struct bpf_prog **pprog;
+	struct bpf_prog *prog;
+	struct bpf_map *map;
+	struct fd f;
+	int ret;
+	u32 id = 0;
+
+	if (attr->query.query_flags)
+		return -EINVAL;
+
+	f = fdget(ufd);
+	map = __bpf_map_get(f);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	rcu_read_lock();
+
+	ret = sock_map_prog_lookup(map, &pprog, attr->query.attach_type);
+	if (ret)
+		goto end;
+
+	prog = *pprog;
+	prog_cnt = (!prog) ? 0 : 1;
+
+	if (!attr->query.prog_cnt || !prog_ids || !prog_cnt)
+		goto end;
+
+	prog = bpf_prog_inc_not_zero(prog);
+	if (IS_ERR(prog)) {
+		ret = PTR_ERR(prog);
+		goto end;
+	}
+	id = prog->aux->id;
+	bpf_prog_put(prog);
+
+end:
+	rcu_read_unlock();
+
+	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
+		ret = -EFAULT;
+		goto err;
+	}
+	if (id != 0 && copy_to_user(prog_ids, &id, sizeof(u32))) {
+		ret = -EFAULT;
+		goto err;
+	}
+	if (copy_to_user(&uattr->query.prog_cnt, &prog_cnt, sizeof(prog_cnt))) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+err:
+	fdput(f);
+	return ret;
+}
+
 static void sock_map_unlink(struct sock *sk, struct sk_psock_link *link)
 {
 	switch (link->map->map_type) {
-- 
2.27.0


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

end of thread, other threads:[~2021-11-03  2:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02  5:59 [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap Di Zhu
2021-11-02  5:59 ` [PATCH bpf-next v4 2/2] selftests: bpf: test " Di Zhu
2021-11-02  8:48 [PATCH bpf-next v4 1/2] bpf: support " Di Zhu
2021-11-02 20:11 ` Yonghong Song
2021-11-02 21:16   ` Alexei Starovoitov
2021-11-03  2:23 zhudi (E)
2021-11-03  2:37 ` Yonghong Song
2021-11-03  2:25 zhudi (E)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).