On 19 Jan 2024, at 13:49, Ilya Maximets wrote:
> 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 <[email protected]>
>> ---
>> 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
>
I guess the subnet mask part will work if we add the subnet dot ., i,e.
FORMAT_CT(10.1.1.)]
But this also requires some more change, i.e. making the dot not being a
wildcard. Something like this:
- [[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=$(echo $1 | sed -e 's/\./\\./g')\>" | \
> 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
