| Message ID | 5ac3e6uwvhdujq6tywb6b5bh5flqln6d7kedmcbvhyp55jp4yo@65pnej6e2ub6 (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [v3,bpf-next] net: remove check in __cgroup_bpf_run_filter_skb | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/apply | fail | Patch does not apply to bpf-next |
On Fri, Feb 09, 2024 at 04:01:21PM -0800, Stanislav Fomichev wrote: > The bot still can't git-am it. And I can't either. Did you somehow > manually mangle that part above? The original line has less trailing spaces > than what your diff source has, look at: I literally saw the bot issue and the spaces just before you sent this. > Can you drop this part? Let the idents stay broken :-) Just did. Didn't manually edit the patch so I don't exactly know how that happened, but thanks for pointing it out.
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> > > v2->v3: Sent to bpf-next instead of generic patch > 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(-) > > 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); \ The bot still can't git-am it. And I can't either. Did you somehow manually mangle that part above? The original line has less trailing spaces than what your diff source has, look at: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/include/linux/bpf-cgroup.h#n201 Can you drop this part? Let the idents stay broken :-)
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> v2->v3: Sent to bpf-next instead of generic patch 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(-)