| Message ID | 160650040800.2890576.9811290366501747109.stgit@firesoul (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: New approach for BPF MTU handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 10 this patch: 10 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 21 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 10 this patch: 10 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 11/27/20 7:06 PM, Jesper Dangaard Brouer wrote: > This change makes it possible to identify SKBs that have been redirected > by TC-BPF (cls_act). This is needed for a number of cases. > > (1) For collaborating with driver ifb net_devices. > (2) For avoiding starting generic-XDP prog on TC ingress redirect. > > It is most important to fix XDP case(2), because this can break userspace > when a driver gets support for native-XDP. Imagine userspace loads XDP > prog on eth0, which fallback to generic-XDP, and it process TC-redirected > packets. When kernel is updated with native-XDP support for eth0, then the > program no-longer see the TC-redirected packets. Therefore it is important > to keep the order intact; that XDP runs before TC-BPF. I don't follow the statement in the very last sentence here.. the order is still intact just that these rediected packets are invoked for generic XDP as well, so from an ingress path perspective it's still generic XDP and then tc ingress. What this aims to achieve is to bypass the generic XDP instead. > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > net/core/dev.c | 2 ++ > net/sched/Kconfig | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 6ceb6412ee97..26b40f8005ae 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3872,6 +3872,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev) > return NULL; > case TC_ACT_REDIRECT: > /* No need to push/pop skb's mac_header here on egress! */ > + skb_set_redirected(skb, false); > skb_do_redirect(skb); > *ret = NET_XMIT_SUCCESS; > return NULL; > @@ -4963,6 +4964,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret, > * redirecting to another netdev > */ > __skb_push(skb, skb->mac_len); > + skb_set_redirected(skb, true); > if (skb_do_redirect(skb) == -EAGAIN) { > __skb_pull(skb, skb->mac_len); > *another = true; I'm not sure I follow the logic in the two cases here.. why mark it in one but not the other? Both are used in practice to recircle back to ingress. Also, this is not BPF specific.. same goes for other subsystems that recircle from TX into RX where they will still go into generic XDP (think of action attached to actual qdisc as an example). > diff --git a/net/sched/Kconfig b/net/sched/Kconfig > index a3b37d88800e..a1bbaa8fd054 100644 > --- a/net/sched/Kconfig > +++ b/net/sched/Kconfig > @@ -384,6 +384,7 @@ config NET_SCH_INGRESS > depends on NET_CLS_ACT > select NET_INGRESS > select NET_EGRESS > + select NET_REDIRECT > help > Say Y here if you want to use classifiers for incoming and/or outgoing > packets. This qdisc doesn't do anything else besides running classifiers, > >
diff --git a/net/core/dev.c b/net/core/dev.c index 6ceb6412ee97..26b40f8005ae 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3872,6 +3872,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev) return NULL; case TC_ACT_REDIRECT: /* No need to push/pop skb's mac_header here on egress! */ + skb_set_redirected(skb, false); skb_do_redirect(skb); *ret = NET_XMIT_SUCCESS; return NULL; @@ -4963,6 +4964,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret, * redirecting to another netdev */ __skb_push(skb, skb->mac_len); + skb_set_redirected(skb, true); if (skb_do_redirect(skb) == -EAGAIN) { __skb_pull(skb, skb->mac_len); *another = true; diff --git a/net/sched/Kconfig b/net/sched/Kconfig index a3b37d88800e..a1bbaa8fd054 100644 --- a/net/sched/Kconfig +++ b/net/sched/Kconfig @@ -384,6 +384,7 @@ config NET_SCH_INGRESS depends on NET_CLS_ACT select NET_INGRESS select NET_EGRESS + select NET_REDIRECT help Say Y here if you want to use classifiers for incoming and/or outgoing packets. This qdisc doesn't do anything else besides running classifiers,
This change makes it possible to identify SKBs that have been redirected by TC-BPF (cls_act). This is needed for a number of cases. (1) For collaborating with driver ifb net_devices. (2) For avoiding starting generic-XDP prog on TC ingress redirect. It is most important to fix XDP case(2), because this can break userspace when a driver gets support for native-XDP. Imagine userspace loads XDP prog on eth0, which fallback to generic-XDP, and it process TC-redirected packets. When kernel is updated with native-XDP support for eth0, then the program no-longer see the TC-redirected packets. Therefore it is important to keep the order intact; that XDP runs before TC-BPF. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- net/core/dev.c | 2 ++ net/sched/Kconfig | 1 + 2 files changed, 3 insertions(+)