On 4/14/23 6:07 PM, Toke Høiland-Jørgensen wrote:
Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes:
[...]
https://git.openwrt.org/?p=project/qosify.git;a=blob;f=README
Thanks for the explanation, that sounds reasonable and this should ideally be part of the commit msg! Yafang, Toke, how about we craft it the following way then to support this case: From f6c83e5e55c5eb9da8acd19369c688acf53951db Mon Sep 17 00:00:00 2001 Message-Id: <f6c83e5e55c5eb9da8acd19369c688acf53951db.1681512637.git.daniel@xxxxxxxxxxxxx> From: Daniel Borkmann <daniel@xxxxxxxxxxxxx> Date: Sat, 15 Apr 2023 00:30:27 +0200 Subject: [PATCH bpf-next] bpf: Set skb redirect and from_ingress info in __bpf_tx_skb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are some use-cases where it is desirable to use bpf_redirect() in combination with ifb device, which currently is not supported, for example, around filtering inbound traffic with BPF to then push it to ifb which holds the qdisc for shaping in contrast to doing that on the egress device. Toke mentions the following case related to OpenWrt: Because there's not always a single egress on the other side. These are mainly home routers, which tend to have one or more WiFi devices bridged to one or more ethernet ports on the LAN side, and a single upstream WAN port. And the objective is to control the total amount of traffic going over the WAN link (in both directions), to deal with bufferbloat in the ISP network (which is sadly still all too prevalent). In this setup, the traffic can be split arbitrarily between the links on the LAN side, and the only "single bottleneck" is the WAN link. So we install both egress and ingress shapers on this, configured to something like 95-98% of the true link bandwidth, thus moving the queues into the qdisc layer in the router. It's usually necessary to set the ingress bandwidth shaper a bit lower than the egress due to being "downstream" of the bottleneck link, but it does work surprisingly well. We usually use something like a matchall filter to put all ingress traffic on the ifb, so doing the redirect from BPF has not been an immediate requirement thus far. However, it does seem a bit odd that this is not possible, and we do have a BPF-based filter that layers on top of this kind of setup, which currently uses u32 as the ingress filter and so it could presumably be improved to use BPF instead if that was available. Reported-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> Reported-by: Yafang Shao <laoar.shao@xxxxxxxxx> Reported-by: Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx> Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx> Link: https://git.openwrt.org/?p=project/qosify.git;a=blob;f=README Link: https://lore.kernel.org/bpf/875y9yzbuy.fsf@xxxxxxx --- include/linux/skbuff.h | 9 +++++++++ net/core/filter.c | 1 + 2 files changed, 10 insertions(+) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index ff7ad331fb82..2bbf9245640a 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -5049,6 +5049,15 @@ static inline void skb_reset_redirect(struct sk_buff *skb) skb->redirected = 0; } +static inline void skb_set_redirected_noclear(struct sk_buff *skb, + bool from_ingress) +{ + skb->redirected = 1; +#ifdef CONFIG_NET_REDIRECT + skb->from_ingress = from_ingress; +#endif +} + static inline bool skb_csum_is_sctp(struct sk_buff *skb) { return skb->csum_not_inet; diff --git a/net/core/filter.c b/net/core/filter.c index 1d6f165923bf..27ba616aaa1a 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2111,6 +2111,7 @@ static inline int __bpf_tx_skb(struct net_device *dev, struct sk_buff *skb) } skb->dev = dev; + skb_set_redirected_noclear(skb, skb->tc_at_ingress); skb_clear_tstamp(skb); dev_xmit_recursion_inc(); -- 2.21.0
