On Tue, Jan 19, 2021 at 6:45 AM Aaron Conole <[email protected]> wrote:
>
> The fixes tag for this should be:
>
> Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
Yes, commit a867c010ee91 ("conntrack: Fix conntrack new state") is the
right one to fix. Commit a867c010ee91 only takes care of TCP, UDP
(conntrack-others) but does not address ICMP properly.
I checked Tonghao's patch and it does address the ICMP case properly.
> Since that was the patch which attempted to make the behavior between
> kernel-userspace consistent.
>
> Please submit a v2 with the following diff (and run 'make
> check-system-userspace' and 'make check-kernel' to ensure proper
> coverage of the tests). This way, we don't break the behavior in the
> future (and can guarantee consistency). I did try this out and it
> worked for me.
>
> 122: conntrack - Multiple ICMP traverse ok
>
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -5898,6 +5898,58 @@ ovs-appctl dpif/dump-flows br0
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> +# OFPROTO_CLEAR_DURATION_IDLE([])
> +#
> +# Clear the duration from the piped input which would differ from test to
> test
> +#
> +m4_define([OFPROTO_CLEAR_DURATION_IDLE], [[sed -e
> 's/duration=.*s,/duration=<cleared>,/g' -e
> 's/idle_age=[0-9]*,/idle_age=<cleared>,/g']])
Thanks Aaron for adding this new system traffic test. It does work on
my machine for both kernel and userspace conntrack.
One suggestion for the test is that usually we add the testing m4
marco for system traffic test on system-common-macros.at. The rest of
the test case looks good to me.
Thanks,
-Yi-Hung
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
