Messages in this thread | | | Date | Tue, 2 Nov 2021 19:37:16 -0700 | Subject | Re: [PATCH bpf-next v4 1/2] bpf: support BPF_PROG_QUERY for progs attached to sockmap | From | Yonghong Song <> |
| |
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; [...]
| |