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".
