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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Sat, 15 Apr 2023 00:57:44 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        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
Subject: Re: [PATCH net-next] bpf, net: Support redirecting to ifb with bpf

On 4/14/23 6:07 PM, Toke Høiland-Jørgensen wrote:
> Daniel Borkmann <daniel@...earbox.net> 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@...earbox.net>
From: Daniel Borkmann <daniel@...earbox.net>
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@...hat.com>
Reported-by: Yafang Shao <laoar.shao@...il.com>
Reported-by: Tonghao Zhang <xiangxia.m.yue@...il.com>
Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
Link: https://git.openwrt.org/?p=project/qosify.git;a=blob;f=README
Link: https://lore.kernel.org/bpf/875y9yzbuy.fsf@toke.dk
---
  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