Re: [PATCH bpf-next 1/2] net: bpf: Always call BPF cgroup filters for egress.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 12, 2023 at 12:16 PM Kui-Feng Lee <thinker.li@xxxxxxxxx> wrote:
>
> Always call BPF filters if CGROUP BPF is enabled for EGRESS without
> checking skb->sk against sk.
>
> The filters were called only if sk_buff is owned by the sock that the
> sk_buff is sent out through.  In another words, sk_buff::sk should point to

What is "sk_buff::sk" ? Did you mean skb->sk ?

> the sock that it is sending through its egress.  However, the filters would
> miss SYNACK sk_buffs that they are owned by a request_sock but sent through
> the listening sock, that is the socket listening incoming connections.
> This is an unnecessary restrict.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxxxx>
> ---
>  include/linux/bpf-cgroup.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 57e9e109257e..e656da531f9f 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -199,7 +199,7 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
>  #define BPF_CGROUP_RUN_PROG_INET_EGRESS(sk, skb)                              \
>  ({                                                                            \
>         int __ret = 0;                                                         \
> -       if (cgroup_bpf_enabled(CGROUP_INET_EGRESS) && sk && sk == skb->sk) { \
> +       if (cgroup_bpf_enabled(CGROUP_INET_EGRESS) && sk) {


I did a bit of git-archeology.
That check was there since the beginning of cgroup-bpf and
came as a suggestion to use 'sk' instead of 'skb->sk':
https://lore.kernel.org/all/58193E9D.7040201@xxxxxxxxxxxxx/

Using sk is certainly correct. It looks to me that the check
was added just for a "piece of mind".





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux