On 1/18/24 14:00, David Marchand wrote:
> Seen in GHA recently.
> Unit tests are checking conntracks relating to a destination ip address
> but the FORMAT_CT macro is not strict enough and would match unrelated
> conntracks too.
>
> Example:
> 148. system-traffic.at:6432: testing conntrack - DNAT with
> additional SNAT ...
> [...]
> ./system-traffic.at:6460: ovs-appctl dpctl/dump-conntrack |
> grep "dst=10.1.1.1" |
> sed -e 's/port=[0-9]*/port=<cleared>/g'
> -e 's/id=[0-9]*/id=<cleared>/g'
> -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort | uniq
> [...]
> @@ -1,2 +1,7 @@
> tcp,orig=(src=10.1.1.1,dst=172.1.1.2,sport=<cleared>,...
> +tcp,...,reply=(src=13.107.42.16,dst=10.1.1.10,sport=<cleared>,...
> +tcp,...,reply=(src=168.63.129.16,dst=10.1.1.10,sport=<cleared>,...
> +tcp,...,reply=(src=20.242.161.191,dst=10.1.1.10,sport=<cleared>,...
> +tcp,orig=(src=13.107.42.16,dst=10.1.1.10,sport=<cleared>,...
> +tcp,orig=(src=20.242.161.191,dst=10.1.1.10,sport=<cleared>,...
>
> Fixes: 07659514c3c1 ("Add support for connection tracking.")
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> tests/system-common-macros.at | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
> index 01ebe364ee..07be29f673 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -256,7 +256,7 @@ m4_define([STRIP_MONITOR_CSUM], [grep "csum:" | sed
> 's/csum:.*/csum: <skip>/'])
> # and limit the output to the rows containing 'ip-addr'.
> #
> m4_define([FORMAT_CT],
> - [[grep "dst=$1" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e
> 's/id=[0-9]*/id=<cleared>/g' -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort
> | uniq]])
> + [[grep "dst=$1\>" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e
> 's/id=[0-9]*/id=<cleared>/g' -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort
> | uniq]])
>
> # NETNS_DAEMONIZE([namespace], [command], [pidfile])
> #
I remembered why the macro is loose. We wanted to be able
to match on "subnets" by supplying only part of the address.
There was at least one test that used this functionality.
Eelco removed it though here:
https://github.com/openvswitch/ovs/commit/a80883f7682158c7a6955360ee852e8279f748e9
Did you check if have any more instances of such tests?
They can be tricky to find, as we can supply 10.1.1.2 in order
to match 10.1.1.240, for example.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
