* [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook
@ 2020-01-17 7:05 Yoshiki Komachi
2020-01-17 7:05 ` [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for " Yoshiki Komachi
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Yoshiki Komachi @ 2020-01-17 7:05 UTC (permalink / raw)
To: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Petar Penkov
Cc: Yoshiki Komachi, netdev, bpf
When I tried a test based on the selftest program for BPF flow dissector
(test_flow_dissector.sh), I observed unexpected result as below:
$ tc filter add dev lo parent ffff: protocol ip pref 1337 flower ip_proto \
udp src_port 8-10 action drop
$ tools/testing/selftests/bpf/test_flow_dissector -i 4 -f 9 -F
inner.dest4: 127.0.0.1
inner.source4: 127.0.0.3
pkts: tx=10 rx=10
The last rx means the number of received packets. I expected rx=0 in this
test (i.e., all received packets should have been dropped), but it resulted
in acceptance.
Although the previous commit 8ffb055beae5 ("cls_flower: Fix the behavior
using port ranges with hw-offload") added new flag and field toward filtering
based on port ranges with hw-offload, it missed applying for BPF flow dissector
then. As a result, BPF flow dissector currently stores data extracted from
packets in incorrect field used for exact match whenever packets are classified
by filters based on port ranges. Thus, they never match rules in such cases
because flow dissector gives rise to generating incorrect flow keys.
This series fixes the issue by replacing incorrect flag and field with new
ones in BPF flow dissector, and adds a test for filtering based on specified
port ranges to the existing selftest program.
Changes in v2:
- set key_ports to NULL at the top of __skb_flow_bpf_to_target()
Yoshiki Komachi (2):
flow_dissector: Fix to use new variables for port ranges in bpf hook
selftests/bpf: Add test based on port range for BPF flow dissector
net/core/flow_dissector.c | 9 ++++++++-
tools/testing/selftests/bpf/test_flow_dissector.sh | 14 ++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for port ranges in bpf hook
2020-01-17 7:05 [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Yoshiki Komachi
@ 2020-01-17 7:05 ` Yoshiki Komachi
2020-01-17 17:00 ` Petar Penkov
2020-01-18 6:59 ` John Fastabend
2020-01-17 7:05 ` [PATCH v2 bpf 2/2] selftests/bpf: Add test based on port range for BPF flow dissector Yoshiki Komachi
2020-01-22 10:29 ` [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Daniel Borkmann
2 siblings, 2 replies; 7+ messages in thread
From: Yoshiki Komachi @ 2020-01-17 7:05 UTC (permalink / raw)
To: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Petar Penkov
Cc: Yoshiki Komachi, netdev, bpf
This patch applies new flag (FLOW_DISSECTOR_KEY_PORTS_RANGE) and
field (tp_range) to BPF flow dissector to generate appropriate flow
keys when classified by specified port ranges.
Fixes: 8ffb055beae5 ("cls_flower: Fix the behavior using port ranges with hw-offload")
Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
---
net/core/flow_dissector.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 2dbbb03..cc32d1d 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -834,10 +834,10 @@ static void __skb_flow_bpf_to_target(const struct bpf_flow_keys *flow_keys,
struct flow_dissector *flow_dissector,
void *target_container)
{
+ struct flow_dissector_key_ports *key_ports = NULL;
struct flow_dissector_key_control *key_control;
struct flow_dissector_key_basic *key_basic;
struct flow_dissector_key_addrs *key_addrs;
- struct flow_dissector_key_ports *key_ports;
struct flow_dissector_key_tags *key_tags;
key_control = skb_flow_dissector_target(flow_dissector,
@@ -876,10 +876,17 @@ static void __skb_flow_bpf_to_target(const struct bpf_flow_keys *flow_keys,
key_control->addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS;
}
- if (dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_PORTS)) {
+ if (dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_PORTS))
key_ports = skb_flow_dissector_target(flow_dissector,
FLOW_DISSECTOR_KEY_PORTS,
target_container);
+ else if (dissector_uses_key(flow_dissector,
+ FLOW_DISSECTOR_KEY_PORTS_RANGE))
+ key_ports = skb_flow_dissector_target(flow_dissector,
+ FLOW_DISSECTOR_KEY_PORTS_RANGE,
+ target_container);
+
+ if (key_ports) {
key_ports->src = flow_keys->sport;
key_ports->dst = flow_keys->dport;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 bpf 2/2] selftests/bpf: Add test based on port range for BPF flow dissector
2020-01-17 7:05 [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Yoshiki Komachi
2020-01-17 7:05 ` [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for " Yoshiki Komachi
@ 2020-01-17 7:05 ` Yoshiki Komachi
2020-01-18 7:05 ` John Fastabend
2020-01-22 10:29 ` [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Daniel Borkmann
2 siblings, 1 reply; 7+ messages in thread
From: Yoshiki Komachi @ 2020-01-17 7:05 UTC (permalink / raw)
To: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Petar Penkov
Cc: Yoshiki Komachi, netdev, bpf, Petar Penkov
Add a simple test to make sure that a filter based on specified port
range classifies packets correctly.
Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
Acked-by: Petar Penkov <ppenkov@google.com>
---
tools/testing/selftests/bpf/test_flow_dissector.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.sh b/tools/testing/selftests/bpf/test_flow_dissector.sh
index a8485ae..174b72a 100755
--- a/tools/testing/selftests/bpf/test_flow_dissector.sh
+++ b/tools/testing/selftests/bpf/test_flow_dissector.sh
@@ -139,6 +139,20 @@ echo "Testing IPv4 + GRE..."
tc filter del dev lo ingress pref 1337
+echo "Testing port range..."
+# Drops all IP/UDP packets coming from port 8-10
+tc filter add dev lo parent ffff: protocol ip pref 1337 flower ip_proto \
+ udp src_port 8-10 action drop
+
+# Send 10 IPv4/UDP packets from port 7. Filter should not drop any.
+./test_flow_dissector -i 4 -f 7
+# Send 10 IPv4/UDP packets from port 9. Filter should drop all.
+./test_flow_dissector -i 4 -f 9 -F
+# Send 10 IPv4/UDP packets from port 11. Filter should not drop any.
+./test_flow_dissector -i 4 -f 11
+
+tc filter del dev lo ingress pref 1337
+
echo "Testing IPv6..."
# Drops all IPv6/UDP packets coming from port 9
tc filter add dev lo parent ffff: protocol ipv6 pref 1337 flower ip_proto \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for port ranges in bpf hook
2020-01-17 7:05 ` [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for " Yoshiki Komachi
@ 2020-01-17 17:00 ` Petar Penkov
2020-01-18 6:59 ` John Fastabend
1 sibling, 0 replies; 7+ messages in thread
From: Petar Penkov @ 2020-01-17 17:00 UTC (permalink / raw)
To: Yoshiki Komachi
Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Networking, bpf
On Thu, Jan 16, 2020 at 11:09 PM Yoshiki Komachi
<komachi.yoshiki@gmail.com> wrote:
>
> This patch applies new flag (FLOW_DISSECTOR_KEY_PORTS_RANGE) and
> field (tp_range) to BPF flow dissector to generate appropriate flow
> keys when classified by specified port ranges.
>
> Fixes: 8ffb055beae5 ("cls_flower: Fix the behavior using port ranges with hw-offload")
> Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
Acked-by: Petar Penkov <ppenkov@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for port ranges in bpf hook
2020-01-17 7:05 ` [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for " Yoshiki Komachi
2020-01-17 17:00 ` Petar Penkov
@ 2020-01-18 6:59 ` John Fastabend
1 sibling, 0 replies; 7+ messages in thread
From: John Fastabend @ 2020-01-18 6:59 UTC (permalink / raw)
To: Yoshiki Komachi, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Petar Penkov
Cc: Yoshiki Komachi, netdev, bpf
Yoshiki Komachi wrote:
> This patch applies new flag (FLOW_DISSECTOR_KEY_PORTS_RANGE) and
> field (tp_range) to BPF flow dissector to generate appropriate flow
> keys when classified by specified port ranges.
>
> Fixes: 8ffb055beae5 ("cls_flower: Fix the behavior using port ranges with hw-offload")
> Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
> ---
> net/core/flow_dissector.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 2dbbb03..cc32d1d 100644
LGTM
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2 bpf 2/2] selftests/bpf: Add test based on port range for BPF flow dissector
2020-01-17 7:05 ` [PATCH v2 bpf 2/2] selftests/bpf: Add test based on port range for BPF flow dissector Yoshiki Komachi
@ 2020-01-18 7:05 ` John Fastabend
0 siblings, 0 replies; 7+ messages in thread
From: John Fastabend @ 2020-01-18 7:05 UTC (permalink / raw)
To: Yoshiki Komachi, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
Andrii Nakryiko, Petar Penkov
Cc: Yoshiki Komachi, netdev, bpf, Petar Penkov
Yoshiki Komachi wrote:
> Add a simple test to make sure that a filter based on specified port
> range classifies packets correctly.
>
> Signed-off-by: Yoshiki Komachi <komachi.yoshiki@gmail.com>
> Acked-by: Petar Penkov <ppenkov@google.com>
> ---
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook
2020-01-17 7:05 [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Yoshiki Komachi
2020-01-17 7:05 ` [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for " Yoshiki Komachi
2020-01-17 7:05 ` [PATCH v2 bpf 2/2] selftests/bpf: Add test based on port range for BPF flow dissector Yoshiki Komachi
@ 2020-01-22 10:29 ` Daniel Borkmann
2 siblings, 0 replies; 7+ messages in thread
From: Daniel Borkmann @ 2020-01-22 10:29 UTC (permalink / raw)
To: Yoshiki Komachi, David S. Miller, Alexei Starovoitov,
Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
Petar Penkov
Cc: netdev, bpf
On 1/17/20 8:05 AM, Yoshiki Komachi wrote:
> When I tried a test based on the selftest program for BPF flow dissector
> (test_flow_dissector.sh), I observed unexpected result as below:
>
> $ tc filter add dev lo parent ffff: protocol ip pref 1337 flower ip_proto \
> udp src_port 8-10 action drop
> $ tools/testing/selftests/bpf/test_flow_dissector -i 4 -f 9 -F
> inner.dest4: 127.0.0.1
> inner.source4: 127.0.0.3
> pkts: tx=10 rx=10
>
> The last rx means the number of received packets. I expected rx=0 in this
> test (i.e., all received packets should have been dropped), but it resulted
> in acceptance.
>
> Although the previous commit 8ffb055beae5 ("cls_flower: Fix the behavior
> using port ranges with hw-offload") added new flag and field toward filtering
> based on port ranges with hw-offload, it missed applying for BPF flow dissector
> then. As a result, BPF flow dissector currently stores data extracted from
> packets in incorrect field used for exact match whenever packets are classified
> by filters based on port ranges. Thus, they never match rules in such cases
> because flow dissector gives rise to generating incorrect flow keys.
>
> This series fixes the issue by replacing incorrect flag and field with new
> ones in BPF flow dissector, and adds a test for filtering based on specified
> port ranges to the existing selftest program.
>
> Changes in v2:
> - set key_ports to NULL at the top of __skb_flow_bpf_to_target()
>
> Yoshiki Komachi (2):
> flow_dissector: Fix to use new variables for port ranges in bpf hook
> selftests/bpf: Add test based on port range for BPF flow dissector
>
> net/core/flow_dissector.c | 9 ++++++++-
> tools/testing/selftests/bpf/test_flow_dissector.sh | 14 ++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-22 10:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 7:05 [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Yoshiki Komachi
2020-01-17 7:05 ` [PATCH v2 bpf 1/2] flow_dissector: Fix to use new variables for " Yoshiki Komachi
2020-01-17 17:00 ` Petar Penkov
2020-01-18 6:59 ` John Fastabend
2020-01-17 7:05 ` [PATCH v2 bpf 2/2] selftests/bpf: Add test based on port range for BPF flow dissector Yoshiki Komachi
2020-01-18 7:05 ` John Fastabend
2020-01-22 10:29 ` [PATCH v2 bpf 0/2] Fix the classification based on port ranges in bpf hook Daniel Borkmann
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).