* [dpdk-dev] [PATCH v1] net/tap: fix eBPF handling of non-RSS flows @ 2018-02-03 23:34 Ophir Munk 2018-02-05 14:40 ` [dpdk-dev] [PATCH v2] fix eBPF handling for non-RSS rules Ophir Munk 0 siblings, 1 reply; 5+ messages in thread From: Ophir Munk @ 2018-02-03 23:34 UTC (permalink / raw) To: dev, Pascal Mazon; +Cc: Thomas Monjalon, Olga Shern, Ophir Munk eBPF classifier (section "cls_q" in tap_bpf_program.c) is looking for marked packets whose skb->cb[1] contains an RSS queue number and redirects those packets to the matched queue. It is expected that skb->cb[1] has been previously set with a valid RSS queue number during an eBPF action (section "l3_l4" in tap_bpf_program.c), however for non-RSS flows skb->cb[1] may contain a random unset value which can falsely be interpreted as a valid RSS queue. To avoid this potential error, tap_bpf_program.c has been updated as follows: 1. After calculating the RSS queue number it is added a unique offset in order to uniquely identify it as a valid RSS queue number. 2. After matching an RSS queue to a packet - skb->cb[1] is set to 0 Fixes: cdc07e83bb24 ("net/tap: add eBPF program file") Fixes: aabe70df73a3 ("net/tap: add eBPF bytes code") Signed-off-by: Ophir Munk <ophirmu@mellanox.com> --- drivers/net/tap/tap_bpf_insns.h | 23 +++++++++++++---------- drivers/net/tap/tap_bpf_program.c | 5 ++++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/drivers/net/tap/tap_bpf_insns.h b/drivers/net/tap/tap_bpf_insns.h index c406f78..89873b6 100644 --- a/drivers/net/tap/tap_bpf_insns.h +++ b/drivers/net/tap/tap_bpf_insns.h @@ -6,17 +6,20 @@ /* bpf_insn array matching cls_q section. See tap_bpf_program.c file */ struct bpf_insn cls_q_insns[] = { - {0x61, 1, 1, 52, 0x00000000}, - {0x18, 2, 0, 0, 0xdeadbeef}, + {0x61, 2, 1, 52, 0x00000000}, + {0x18, 3, 0, 0, 0xdeadbeef}, {0x00, 0, 0, 0, 0x00000000}, - {0x63, 10, 2, -4, 0x00000000}, - {0x61, 2, 10, -4, 0x00000000}, - {0x07, 2, 0, 0, 0x00000001}, - {0x67, 2, 0, 0, 0x00000020}, - {0x77, 2, 0, 0, 0x00000020}, - {0xb7, 0, 0, 0, 0xffffffff}, - {0x1d, 1, 2, 1, 0x00000000}, + {0x63, 10, 3, -4, 0x00000000}, {0xb7, 0, 0, 0, 0x00000000}, + {0x61, 3, 10, -4, 0x00000000}, + {0x07, 3, 0, 0, 0x7cafe800}, + {0x67, 3, 0, 0, 0x00000020}, + {0x77, 3, 0, 0, 0x00000020}, + {0x5d, 2, 3, 4, 0x00000000}, + {0xb7, 2, 0, 0, 0x00000000}, + {0x63, 1, 2, 52, 0x00000000}, + {0x18, 0, 0, 0, 0xffffffff}, + {0x00, 0, 0, 0, 0x00000000}, {0x95, 0, 0, 0, 0x00000000}, }; @@ -1685,7 +1688,7 @@ struct bpf_insn l3_l4_hash_insns[] = { {0x4f, 3, 2, 0, 0x00000000}, {0x67, 3, 0, 0, 0x00000010}, {0x4f, 3, 1, 0, 0x00000000}, - {0x07, 3, 0, 0, 0x00000001}, + {0x07, 3, 0, 0, 0x7cafe800}, {0x63, 5, 3, 52, 0x00000000}, {0xb7, 7, 0, 0, 0x00000001}, {0xbf, 0, 7, 0, 0x00000000}, diff --git a/drivers/net/tap/tap_bpf_program.c b/drivers/net/tap/tap_bpf_program.c index 848c50b..64dcdb8 100644 --- a/drivers/net/tap/tap_bpf_program.c +++ b/drivers/net/tap/tap_bpf_program.c @@ -31,7 +31,7 @@ * The queue number is offset by 1, to distinguish packets that have * gone through this rule (skb->cb[1] != 0) from others. */ -#define QUEUE_OFFSET 1 +#define QUEUE_OFFSET 0x7cafe800 #define PIN_GLOBAL_NS 2 #define KEY_IDX 0 @@ -63,6 +63,9 @@ match_q(struct __sk_buff *skb) if (queue != match_queue) return TC_ACT_OK; + + /* queue match */ + skb->cb[1] = 0; return TC_ACT_UNSPEC; } -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-dev] [PATCH v2] fix eBPF handling for non-RSS rules 2018-02-03 23:34 [dpdk-dev] [PATCH v1] net/tap: fix eBPF handling of non-RSS flows Ophir Munk @ 2018-02-05 14:40 ` Ophir Munk 2018-02-05 14:40 ` [dpdk-dev] [PATCH v2] net/tap: fix eBPF handling of non-RSS flows Ophir Munk 0 siblings, 1 reply; 5+ messages in thread From: Ophir Munk @ 2018-02-05 14:40 UTC (permalink / raw) To: dev, Pascal Mazon; +Cc: Thomas Monjalon, Olga Shern, Ophir Munk v2: * Minor commit message updates * Update a comment in code Ophir Munk (1): net/tap: fix eBPF handling of non-RSS flows drivers/net/tap/tap_bpf_insns.h | 23 +++++++++++++---------- drivers/net/tap/tap_bpf_program.c | 9 ++++++--- 2 files changed, 19 insertions(+), 13 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-dev] [PATCH v2] net/tap: fix eBPF handling of non-RSS flows 2018-02-05 14:40 ` [dpdk-dev] [PATCH v2] fix eBPF handling for non-RSS rules Ophir Munk @ 2018-02-05 14:40 ` Ophir Munk 2018-02-05 14:51 ` Pascal Mazon 0 siblings, 1 reply; 5+ messages in thread From: Ophir Munk @ 2018-02-05 14:40 UTC (permalink / raw) To: dev, Pascal Mazon; +Cc: Thomas Monjalon, Olga Shern, Ophir Munk The eBPF classifier (section "cls_q" in tap_bpf_program.c) is tracing marked packets in which skb->cb[1] contains an RSS queue number, and redirects those packets to the matched queue. It is expected that skb->cb[1] has been previously set with a valid RSS queue number during an eBPF action (section "l3_l4" in tap_bpf_program.c). However, for non-RSS flows, skb->cb[1] may contain a random unset value, which could falsely be interpreted as a valid RSS queue. To avoid this potential error, tap_bpf_program.c has been updated as follows: 1. After calculating the RSS queue number, it is added a unique offset in order to uniquely identify it as a valid RSS queue number. 2. After matching an RSS queue to a packet, skb->cb[1] is set to 0. Fixes: cdc07e83bb24 ("net/tap: add eBPF program file") Fixes: aabe70df73a3 ("net/tap: add eBPF bytes code") Signed-off-by: Ophir Munk <ophirmu@mellanox.com> --- drivers/net/tap/tap_bpf_insns.h | 23 +++++++++++++---------- drivers/net/tap/tap_bpf_program.c | 9 ++++++--- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/net/tap/tap_bpf_insns.h b/drivers/net/tap/tap_bpf_insns.h index c406f78..89873b6 100644 --- a/drivers/net/tap/tap_bpf_insns.h +++ b/drivers/net/tap/tap_bpf_insns.h @@ -6,17 +6,20 @@ /* bpf_insn array matching cls_q section. See tap_bpf_program.c file */ struct bpf_insn cls_q_insns[] = { - {0x61, 1, 1, 52, 0x00000000}, - {0x18, 2, 0, 0, 0xdeadbeef}, + {0x61, 2, 1, 52, 0x00000000}, + {0x18, 3, 0, 0, 0xdeadbeef}, {0x00, 0, 0, 0, 0x00000000}, - {0x63, 10, 2, -4, 0x00000000}, - {0x61, 2, 10, -4, 0x00000000}, - {0x07, 2, 0, 0, 0x00000001}, - {0x67, 2, 0, 0, 0x00000020}, - {0x77, 2, 0, 0, 0x00000020}, - {0xb7, 0, 0, 0, 0xffffffff}, - {0x1d, 1, 2, 1, 0x00000000}, + {0x63, 10, 3, -4, 0x00000000}, {0xb7, 0, 0, 0, 0x00000000}, + {0x61, 3, 10, -4, 0x00000000}, + {0x07, 3, 0, 0, 0x7cafe800}, + {0x67, 3, 0, 0, 0x00000020}, + {0x77, 3, 0, 0, 0x00000020}, + {0x5d, 2, 3, 4, 0x00000000}, + {0xb7, 2, 0, 0, 0x00000000}, + {0x63, 1, 2, 52, 0x00000000}, + {0x18, 0, 0, 0, 0xffffffff}, + {0x00, 0, 0, 0, 0x00000000}, {0x95, 0, 0, 0, 0x00000000}, }; @@ -1685,7 +1688,7 @@ struct bpf_insn l3_l4_hash_insns[] = { {0x4f, 3, 2, 0, 0x00000000}, {0x67, 3, 0, 0, 0x00000010}, {0x4f, 3, 1, 0, 0x00000000}, - {0x07, 3, 0, 0, 0x00000001}, + {0x07, 3, 0, 0, 0x7cafe800}, {0x63, 5, 3, 52, 0x00000000}, {0xb7, 7, 0, 0, 0x00000001}, {0xbf, 0, 7, 0, 0x00000000}, diff --git a/drivers/net/tap/tap_bpf_program.c b/drivers/net/tap/tap_bpf_program.c index 848c50b..8abb3b7 100644 --- a/drivers/net/tap/tap_bpf_program.c +++ b/drivers/net/tap/tap_bpf_program.c @@ -28,10 +28,10 @@ ((b) & 0xff)) /* - * The queue number is offset by 1, to distinguish packets that have - * gone through this rule (skb->cb[1] != 0) from others. + * The queue number is offset by a unique QUEUE_OFFSET, to distinguish + * packets that have gone through this rule (skb->cb[1] != 0) from others. */ -#define QUEUE_OFFSET 1 +#define QUEUE_OFFSET 0x7cafe800 #define PIN_GLOBAL_NS 2 #define KEY_IDX 0 @@ -63,6 +63,9 @@ match_q(struct __sk_buff *skb) if (queue != match_queue) return TC_ACT_OK; + + /* queue match */ + skb->cb[1] = 0; return TC_ACT_UNSPEC; } -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: fix eBPF handling of non-RSS flows 2018-02-05 14:40 ` [dpdk-dev] [PATCH v2] net/tap: fix eBPF handling of non-RSS flows Ophir Munk @ 2018-02-05 14:51 ` Pascal Mazon 2018-02-05 18:10 ` Ferruh Yigit 0 siblings, 1 reply; 5+ messages in thread From: Pascal Mazon @ 2018-02-05 14:51 UTC (permalink / raw) To: Ophir Munk, dev; +Cc: Thomas Monjalon, Olga Shern The mitigation is good enough, random packets are more likely to have cb[1] == 0 than something above 7cafe800. Acked-by: Pascal Mazon <pascal.mazon@6wind.com> On 05/02/2018 15:40, Ophir Munk wrote: > The eBPF classifier (section "cls_q" in tap_bpf_program.c) is tracing > marked packets in which skb->cb[1] contains an RSS queue number, and > redirects those packets to the matched queue. > It is expected that skb->cb[1] has been previously set with a valid RSS > queue number during an eBPF action (section "l3_l4" in tap_bpf_program.c). > However, for non-RSS flows, skb->cb[1] may contain a random unset value, > which could falsely be interpreted as a valid RSS queue. > To avoid this potential error, tap_bpf_program.c has been updated as > follows: > 1. After calculating the RSS queue number, it is added a unique offset in > order to uniquely identify it as a valid RSS queue number. > 2. After matching an RSS queue to a packet, skb->cb[1] is set to 0. > > Fixes: cdc07e83bb24 ("net/tap: add eBPF program file") > Fixes: aabe70df73a3 ("net/tap: add eBPF bytes code") > > Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > --- > drivers/net/tap/tap_bpf_insns.h | 23 +++++++++++++---------- > drivers/net/tap/tap_bpf_program.c | 9 ++++++--- > 2 files changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/tap/tap_bpf_insns.h b/drivers/net/tap/tap_bpf_insns.h > index c406f78..89873b6 100644 > --- a/drivers/net/tap/tap_bpf_insns.h > +++ b/drivers/net/tap/tap_bpf_insns.h > @@ -6,17 +6,20 @@ > > /* bpf_insn array matching cls_q section. See tap_bpf_program.c file */ > struct bpf_insn cls_q_insns[] = { > - {0x61, 1, 1, 52, 0x00000000}, > - {0x18, 2, 0, 0, 0xdeadbeef}, > + {0x61, 2, 1, 52, 0x00000000}, > + {0x18, 3, 0, 0, 0xdeadbeef}, > {0x00, 0, 0, 0, 0x00000000}, > - {0x63, 10, 2, -4, 0x00000000}, > - {0x61, 2, 10, -4, 0x00000000}, > - {0x07, 2, 0, 0, 0x00000001}, > - {0x67, 2, 0, 0, 0x00000020}, > - {0x77, 2, 0, 0, 0x00000020}, > - {0xb7, 0, 0, 0, 0xffffffff}, > - {0x1d, 1, 2, 1, 0x00000000}, > + {0x63, 10, 3, -4, 0x00000000}, > {0xb7, 0, 0, 0, 0x00000000}, > + {0x61, 3, 10, -4, 0x00000000}, > + {0x07, 3, 0, 0, 0x7cafe800}, > + {0x67, 3, 0, 0, 0x00000020}, > + {0x77, 3, 0, 0, 0x00000020}, > + {0x5d, 2, 3, 4, 0x00000000}, > + {0xb7, 2, 0, 0, 0x00000000}, > + {0x63, 1, 2, 52, 0x00000000}, > + {0x18, 0, 0, 0, 0xffffffff}, > + {0x00, 0, 0, 0, 0x00000000}, > {0x95, 0, 0, 0, 0x00000000}, > }; > > @@ -1685,7 +1688,7 @@ struct bpf_insn l3_l4_hash_insns[] = { > {0x4f, 3, 2, 0, 0x00000000}, > {0x67, 3, 0, 0, 0x00000010}, > {0x4f, 3, 1, 0, 0x00000000}, > - {0x07, 3, 0, 0, 0x00000001}, > + {0x07, 3, 0, 0, 0x7cafe800}, > {0x63, 5, 3, 52, 0x00000000}, > {0xb7, 7, 0, 0, 0x00000001}, > {0xbf, 0, 7, 0, 0x00000000}, > diff --git a/drivers/net/tap/tap_bpf_program.c b/drivers/net/tap/tap_bpf_program.c > index 848c50b..8abb3b7 100644 > --- a/drivers/net/tap/tap_bpf_program.c > +++ b/drivers/net/tap/tap_bpf_program.c > @@ -28,10 +28,10 @@ > ((b) & 0xff)) > > /* > - * The queue number is offset by 1, to distinguish packets that have > - * gone through this rule (skb->cb[1] != 0) from others. > + * The queue number is offset by a unique QUEUE_OFFSET, to distinguish > + * packets that have gone through this rule (skb->cb[1] != 0) from others. > */ > -#define QUEUE_OFFSET 1 > +#define QUEUE_OFFSET 0x7cafe800 > #define PIN_GLOBAL_NS 2 > > #define KEY_IDX 0 > @@ -63,6 +63,9 @@ match_q(struct __sk_buff *skb) > > if (queue != match_queue) > return TC_ACT_OK; > + > + /* queue match */ > + skb->cb[1] = 0; > return TC_ACT_UNSPEC; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/tap: fix eBPF handling of non-RSS flows 2018-02-05 14:51 ` Pascal Mazon @ 2018-02-05 18:10 ` Ferruh Yigit 0 siblings, 0 replies; 5+ messages in thread From: Ferruh Yigit @ 2018-02-05 18:10 UTC (permalink / raw) To: Pascal Mazon, Ophir Munk, dev; +Cc: Thomas Monjalon, Olga Shern On 2/5/2018 2:51 PM, Pascal Mazon wrote: > On 05/02/2018 15:40, Ophir Munk wrote: >> The eBPF classifier (section "cls_q" in tap_bpf_program.c) is tracing >> marked packets in which skb->cb[1] contains an RSS queue number, and >> redirects those packets to the matched queue. >> It is expected that skb->cb[1] has been previously set with a valid RSS >> queue number during an eBPF action (section "l3_l4" in tap_bpf_program.c). >> However, for non-RSS flows, skb->cb[1] may contain a random unset value, >> which could falsely be interpreted as a valid RSS queue. >> To avoid this potential error, tap_bpf_program.c has been updated as >> follows: >> 1. After calculating the RSS queue number, it is added a unique offset in >> order to uniquely identify it as a valid RSS queue number. >> 2. After matching an RSS queue to a packet, skb->cb[1] is set to 0. >> >> Fixes: cdc07e83bb24 ("net/tap: add eBPF program file") >> Fixes: aabe70df73a3 ("net/tap: add eBPF bytes code") >> >> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > The mitigation is good enough, random packets are more likely to have > cb[1] == 0 than something above 7cafe800. > > Acked-by: Pascal Mazon <pascal.mazon@6wind.com> Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-02-05 18:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-03 23:34 [dpdk-dev] [PATCH v1] net/tap: fix eBPF handling of non-RSS flows Ophir Munk 2018-02-05 14:40 ` [dpdk-dev] [PATCH v2] fix eBPF handling for non-RSS rules Ophir Munk 2018-02-05 14:40 ` [dpdk-dev] [PATCH v2] net/tap: fix eBPF handling of non-RSS flows Ophir Munk 2018-02-05 14:51 ` Pascal Mazon 2018-02-05 18:10 ` Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).