netdev - Re: [PATCH net-next] bpf, net: Support redirecting to ifb with bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Apr 2023 13:47:34 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Yafang Shao <laoar.shao@...il.com>, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        ast@...nel.org, hawk@...nel.org, john.fastabend@...il.com
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Tonghao Zhang <xiangxia.m.yue@...il.com>, martin.lau@...ux.dev,
        toke@...hat.com
Subject: Re: [PATCH net-next] bpf, net: Support redirecting to ifb with bpf

On 4/13/23 4:53 AM, Yafang Shao wrote:
> In our container environment, we are using EDT-bpf to limit the egress
> bandwidth. EDT-bpf can be used to limit egress only, but can't be used
> to limit ingress. Some of our users also want to limit the ingress
> bandwidth. But after applying EDT-bpf, which is based on clsact qdisc,
> it is impossible to limit the ingress bandwidth currently, due to some
> reasons,
> 1). We can't add ingress qdisc
> The ingress qdisc can't coexist with clsact qdisc as clsact has both
> ingress and egress handler. So our traditional method to limit ingress
> bandwidth can't work any more.

I'm not following, the latter is a super set of the former, why do you
need it to co-exist?

> 2). We can't redirect ingress packet to ifb with bpf
> By trying to analyze if it is possible to redirect the ingress packet to
> ifb with a bpf program, we find that the ifb device is not supported by
> bpf redirect yet.

You actually can: Just let BPF program return TC_ACT_UNSPEC for this
case and then add a matchall with higher prio (so it runs after bpf)
that contains an action with mirred egress redirect that pushes to ifb
dev - there is no change needed.

> This patch tries to resolve it by supporting redirecting to ifb with bpf
> program.
> 
> Ingress bandwidth limit is useful in some scenarios, for example, for the
> TCP-based service, there may be lots of clients connecting it, so it is
> not wise to limit the clients' egress. After limiting the server-side's
> ingress, it will lower the send rate of the client by lowering the TCP
> cwnd if the ingress bandwidth limit is reached. If we don't limit it,
> the clients will continue sending requests at a high rate.

Adding artificial queueing for the inbound traffic, aren't you worried
about DoS'ing your node? If you need to tell the sender to slow down,
have you looked at hbm (https://lpc.events/event/4/contributions/486/,
samples/bpf/hbm_out_kern.c) which uses ECN CE marking to tell the TCP
sender to slow down? (Fwiw, for UDP https://github.com/cloudflare/rakelimit
would be an option.)

Thanks,
Daniel