| Message ID | 20220324135653.2189-2-xiangxia.m.yue@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | BPF |
| Headers | show |
| Series | fix bpf_redirect to ifb netdev | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | fail | Patch does not apply to net |
On Thu, Mar 24, 2022 at 6:57 AM <xiangxia.m.yue@gmail.com> wrote: > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > 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 <kafai@fb.com> > Cc: Song Liu <songliubraving@fb.com> > Cc: Yonghong Song <yhs@fb.com> > 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 <alexandr.lobakin@intel.com> > Cc: Wei Wang <weiwan@google.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > --- > 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.
On Thu, Mar 24, 2022 at 11:16 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, Mar 24, 2022 at 6:57 AM <xiangxia.m.yue@gmail.com> wrote: > > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > > > 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 <kafai@fb.com> > > Cc: Song Liu <songliubraving@fb.com> > > Cc: Yonghong Song <yhs@fb.com> > > 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 <alexandr.lobakin@intel.com> > > Cc: Wei Wang <weiwan@google.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > --- > > 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. Hi Daniel, can you review this patch ?
On 3/25/22 1:56 AM, Tonghao Zhang wrote: > On Thu, Mar 24, 2022 at 11:16 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: >> >> On Thu, Mar 24, 2022 at 6:57 AM <xiangxia.m.yue@gmail.com> wrote: >>> >>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com> >>> >>> 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 <kafai@fb.com> >>> Cc: Song Liu <songliubraving@fb.com> >>> Cc: Yonghong Song <yhs@fb.com> >>> 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 <alexandr.lobakin@intel.com> >>> Cc: Wei Wang <weiwan@google.com> >>> Cc: Arnd Bergmann <arnd@arndb.de> >>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> >>> --- >>> 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
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 dev_xmit_recursion_inc(); ret = dev_queue_xmit(skb);