| Message ID | adwssgplvtrgagjw5ftcc5ogpq2nz4pp722wzn3yt2jnql6odf@peiwrqaoyil2 (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [v2] net: remove check in __cgroup_bpf_run_filter_skb | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-PR | fail | merge-conflict |
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 02/09, Oliver Crumrine wrote: > Originally, this patch removed a redundant check in > BPF_CGROUP_RUN_PROG_INET_EGRESS, as the check was already being done in > the function it called, __cgroup_bpf_run_filter_skb. For v2, it was > reccomended that I remove the check from __cgroup_bpf_run_filter_skb, > and add the checks to the other macro that calls that function, > BPF_CGROUP_RUN_PROG_INET_INGRESS. > > To sum it up, checking that the socket exists and that it is a full > socket is now part of both macros BPF_CGROUP_RUN_PROG_INET_EGRESS and > BPF_CGROUP_RUN_PROG_INET_INGRESS, and it is no longer part of the > function they call, __cgroup_bpf_run_filter_skb. > > Signed-off-by: Oliver Crumrine <ozlinuxc@gmail.com> > > v1->v2: Addressed feedback about where check should be removed. Can you please repost with [PATCH bpf-next] subj? I think bot is having problem applying your changes otherwise..
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h index a789266feac3..b28dc0ff4218 100644 --- a/include/linux/bpf-cgroup.h +++ b/include/linux/bpf-cgroup.h @@ -195,10 +195,11 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk, #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk, skb) \ ({ \ int __ret = 0; \ - if (cgroup_bpf_enabled(CGROUP_INET_INGRESS) && \ - cgroup_bpf_sock_enabled(sk, CGROUP_INET_INGRESS)) \ + if (cgroup_bpf_enabled(CGROUP_INET_INGRESS) && \ + cgroup_bpf_sock_enabled(sk, CGROUP_INET_INGRESS) && sk && \ + sk_fullsock(sk)) \ __ret = __cgroup_bpf_run_filter_skb(sk, skb, \ - CGROUP_INET_INGRESS); \ + CGROUP_INET_INGRESS); \ \ __ret; \ }) diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 491d20038cbe..644bfb39cf9d 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -1364,9 +1364,6 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk, struct cgroup *cgrp; int ret; - if (!sk || !sk_fullsock(sk)) - return 0; - if (sk->sk_family != AF_INET && sk->sk_family != AF_INET6) return 0;
Originally, this patch removed a redundant check in BPF_CGROUP_RUN_PROG_INET_EGRESS, as the check was already being done in the function it called, __cgroup_bpf_run_filter_skb. For v2, it was reccomended that I remove the check from __cgroup_bpf_run_filter_skb, and add the checks to the other macro that calls that function, BPF_CGROUP_RUN_PROG_INET_INGRESS. To sum it up, checking that the socket exists and that it is a full socket is now part of both macros BPF_CGROUP_RUN_PROG_INET_EGRESS and BPF_CGROUP_RUN_PROG_INET_INGRESS, and it is no longer part of the function they call, __cgroup_bpf_run_filter_skb. Signed-off-by: Oliver Crumrine <ozlinuxc@gmail.com> v1->v2: Addressed feedback about where check should be removed. --- include/linux/bpf-cgroup.h | 7 ++++--- kernel/bpf/cgroup.c | 3 --- 2 files changed, 4 insertions(+), 6 deletions(-)