Re: [net v6 1/2] net: core: set skb useful vars in __bpf_tx_skb

On 3/25/22 1:56 AM, Tonghao Zhang wrote:

On Thu, Mar 24, 2022 at 11:16 PM Alexei Starovoitov
[off-list ref] wrote:

quoted

On Thu, Mar 24, 2022 at 6:57 AM [off-list ref] wrote:

quoted

From: Tonghao Zhang <redacted>

We may use bpf_redirect to redirect the packets to other
netdevice (e.g. ifb) in ingress or egress path.

The target netdevice may check the *skb_iif, *redirected
and *from_ingress. For example, if skb_iif or redirected
is 0, ifb will drop the packets.

Fixes: a70b506efe89 ("bpf: enforce recursion limit on redirects")
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <redacted>
Cc: Song Liu <redacted>
Cc: Yonghong Song <redacted>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Alexander Lobakin <redacted>
Cc: Wei Wang <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Tonghao Zhang <redacted>
---
  net/core/filter.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index a7044e98765e..c1f45d2e6b0a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c

@@ -2107,7 +2107,15 @@ static inline int __bpf_tx_skb(struct net_device *dev, struct sk_buff *skb)
         }

         skb->dev = dev;
+       /* The target netdevice (e.g. ifb) may use the:
+        * - redirected
+        * - from_ingress
+        */
+#ifdef CONFIG_NET_CLS_ACT
+       skb_set_redirected(skb, skb->tc_at_ingress);
+#else
         skb_clear_tstamp(skb);
+#endif

I thought Daniel Nacked it a couple times already.
Please stop this spam.

Hi
Daniel rejected the 2/3 patch,
https://patchwork.kernel.org/project/netdevbpf/patch/20211208145459.9590-3-xiangxia.m.yue@gmail.com/
The reasons are as follows.
* 2/3 patch adds a check in fastpath.
* on egress, redirect skb to ifb is not useful.
but this patch fixes  redirect skb to ifb on ingress. I think it is
useful for us.

Daniel, can you review this patch ?

Still nack, above makes tc forwarding path for BPF less predictable and could break
existing setups, e.g. if someone is relying on generic XDP. I'm certain you can
resolve this without ifb hack, ifb usage is really discouraged in general.

Thanks,
Daniel