| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <25844618-b63d-251b-f8e1-1f0c045b87f3@mojatatu.com>
Date: Wed, 10 Nov 2021 09:39:40 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Petr Machata <petrm@...dia.com>, netdev@...r.kernel.org
Cc: Jiri Pirko <jiri@...nulli.us>,
Cong Wang <xiyou.wangcong@...il.com>,
David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, komachi.yoshiki@...il.com,
Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
Subject: Re: [PATCH net] selftests: forwarding: Fix packet matching in
mirroring selftests
TBH, the bigger question is why that patch was even applied to begin
with and not much of a discussion that happened.
While it makes sense to look at outer header - every other script
out in the wild (including your selftests) assumed inner header.
Now we have to deal with fallout - which is frustrating.
Or the patch could be simply reverted.
IMO: We could have introduced a construct like outer_ip_src/dst/proto
on tc to handle the new semantics while keeping the old assumptions
in place.
cheers,
jamal
On 2021-11-09 11:17, Petr Machata wrote:
> In commit 6de6e46d27ef ("cls_flower: Fix inability to match GRE/IPIP
> packets"), cls_flower was fixed to match an outer packet of a tunneled
> packet as would be expected, rather than dissecting to the inner packet and
> matching on that.
>
> This fix uncovered several issues in packet matching in mirroring
> selftests:
>
> - in mirror_gre_bridge_1d_vlan.sh and mirror_gre_vlan_bridge_1q.sh, the
> vlan_ethtype match is copied around as "ip", even as some of the tests
> are running over ip6gretap. This is fixed by using an "ipv6" for
> vlan_ethtype in the ip6gretap tests.
>
> - in mirror_gre_changes.sh, a filter to count GRE packets is set up to
> match TTL of 50. This used to trigger in the offloaded datapath, where
> the envelope TTL was matched, but not in the software datapath, which
> considered TTL of the inner packet. Now that both match consistently, all
> the packets were double-counted. This is fixed by marking the filter as
> skip_hw, leaving only the SW datapath component active.
>
> Fixes: 6de6e46d27ef ("cls_flower: Fix inability to match GRE/IPIP packets")
> Signed-off-by: Petr Machata <petrm@...dia.com>
> ---
> .../net/forwarding/mirror_gre_bridge_1d_vlan.sh | 2 +-
> .../selftests/net/forwarding/mirror_gre_changes.sh | 2 +-
> .../net/forwarding/mirror_gre_vlan_bridge_1q.sh | 13 +++++++------
> .../testing/selftests/net/forwarding/mirror_lib.sh | 3 ++-
> .../testing/selftests/net/forwarding/mirror_vlan.sh | 4 ++--
> 5 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh
> index f8cda822c1ce..1b27f2b0f196 100755
> --- a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh
> +++ b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh
> @@ -80,7 +80,7 @@ test_gretap()
>
> test_ip6gretap()
> {
> - test_vlan_match gt6 'skip_hw vlan_id 555 vlan_ethtype ip' \
> + test_vlan_match gt6 'skip_hw vlan_id 555 vlan_ethtype ipv6' \
> "mirror to ip6gretap"
> }
>
> diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh b/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh
> index 472bd023e2a5..aff88f78e339 100755
> --- a/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh
> +++ b/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh
> @@ -74,7 +74,7 @@ test_span_gre_ttl()
>
> mirror_install $swp1 ingress $tundev "matchall $tcflags"
> tc filter add dev $h3 ingress pref 77 prot $prot \
> - flower ip_ttl 50 action pass
> + flower skip_hw ip_ttl 50 action pass
>
> mirror_test v$h1 192.0.2.1 192.0.2.2 $h3 77 0
>
> diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
> index 880e3ab9d088..c8a9b5bd841f 100755
> --- a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
> +++ b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
> @@ -141,7 +141,7 @@ test_gretap()
>
> test_ip6gretap()
> {
> - test_vlan_match gt6 'skip_hw vlan_id 555 vlan_ethtype ip' \
> + test_vlan_match gt6 'skip_hw vlan_id 555 vlan_ethtype ipv6' \
> "mirror to ip6gretap"
> }
>
> @@ -218,6 +218,7 @@ test_ip6gretap_forbidden_egress()
> test_span_gre_untagged_egress()
> {
> local tundev=$1; shift
> + local ul_proto=$1; shift
> local what=$1; shift
>
> RET=0
> @@ -225,7 +226,7 @@ test_span_gre_untagged_egress()
> mirror_install $swp1 ingress $tundev "matchall $tcflags"
>
> quick_test_span_gre_dir $tundev ingress
> - quick_test_span_vlan_dir $h3 555 ingress
> + quick_test_span_vlan_dir $h3 555 ingress "$ul_proto"
>
> h3_addr_add_del del $h3.555
> bridge vlan add dev $swp3 vid 555 pvid untagged
> @@ -233,7 +234,7 @@ test_span_gre_untagged_egress()
> sleep 5
>
> quick_test_span_gre_dir $tundev ingress
> - fail_test_span_vlan_dir $h3 555 ingress
> + fail_test_span_vlan_dir $h3 555 ingress "$ul_proto"
>
> h3_addr_add_del del $h3
> bridge vlan add dev $swp3 vid 555
> @@ -241,7 +242,7 @@ test_span_gre_untagged_egress()
> sleep 5
>
> quick_test_span_gre_dir $tundev ingress
> - quick_test_span_vlan_dir $h3 555 ingress
> + quick_test_span_vlan_dir $h3 555 ingress "$ul_proto"
>
> mirror_uninstall $swp1 ingress
>
> @@ -250,12 +251,12 @@ test_span_gre_untagged_egress()
>
> test_gretap_untagged_egress()
> {
> - test_span_gre_untagged_egress gt4 "mirror to gretap"
> + test_span_gre_untagged_egress gt4 ip "mirror to gretap"
> }
>
> test_ip6gretap_untagged_egress()
> {
> - test_span_gre_untagged_egress gt6 "mirror to ip6gretap"
> + test_span_gre_untagged_egress gt6 ipv6 "mirror to ip6gretap"
> }
>
> test_span_gre_fdb_roaming()
> diff --git a/tools/testing/selftests/net/forwarding/mirror_lib.sh b/tools/testing/selftests/net/forwarding/mirror_lib.sh
> index 6406cd76a19d..3e8ebeff3019 100644
> --- a/tools/testing/selftests/net/forwarding/mirror_lib.sh
> +++ b/tools/testing/selftests/net/forwarding/mirror_lib.sh
> @@ -115,13 +115,14 @@ do_test_span_vlan_dir_ips()
> local dev=$1; shift
> local vid=$1; shift
> local direction=$1; shift
> + local ul_proto=$1; shift
> local ip1=$1; shift
> local ip2=$1; shift
>
> # Install the capture as skip_hw to avoid double-counting of packets.
> # The traffic is meant for local box anyway, so will be trapped to
> # kernel.
> - vlan_capture_install $dev "skip_hw vlan_id $vid vlan_ethtype ip"
> + vlan_capture_install $dev "skip_hw vlan_id $vid vlan_ethtype $ul_proto"
> mirror_test v$h1 $ip1 $ip2 $dev 100 $expect
> mirror_test v$h2 $ip2 $ip1 $dev 100 $expect
> vlan_capture_uninstall $dev
> diff --git a/tools/testing/selftests/net/forwarding/mirror_vlan.sh b/tools/testing/selftests/net/forwarding/mirror_vlan.sh
> index 9ab2ce77b332..0b44e148235e 100755
> --- a/tools/testing/selftests/net/forwarding/mirror_vlan.sh
> +++ b/tools/testing/selftests/net/forwarding/mirror_vlan.sh
> @@ -85,9 +85,9 @@ test_tagged_vlan_dir()
> RET=0
>
> mirror_install $swp1 $direction $swp3.555 "matchall $tcflags"
> - do_test_span_vlan_dir_ips 10 "$h3.555" 111 "$direction" \
> + do_test_span_vlan_dir_ips 10 "$h3.555" 111 "$direction" ip \
> 192.0.2.17 192.0.2.18
> - do_test_span_vlan_dir_ips 0 "$h3.555" 555 "$direction" \
> + do_test_span_vlan_dir_ips 0 "$h3.555" 555 "$direction" ip \
> 192.0.2.17 192.0.2.18
> mirror_uninstall $swp1 $direction
>
>
Powered by blists - more mailing lists