On 4/30/26 8:00 PM, Naveen Yerramneni wrote:
> clone_xlate_actions() has two code paths. The non-reversible path
> saves and restores ctx->conntracked across the inner actions, but
> the reversible path does not.
>
> This is a problem for flows like:
> clone(ct_clear, <actions>), ct_clear, resubmit(,N)
>
> The ct_clear inside the clone sets ctx->conntracked to false. After
> the clone returns, the flag stays false but flow.ct_state is
> restored by xretain_state_restore_and_free(). The ct_clear that
> follows the clone then does nothing, because OFPACT_CT_CLEAR runs
> only when ctx->conntracked is true. The original packet keeps its old
> ct_state, and the ct_clear did not take effect.
Good catch!
>
> Save and restore ctx->conntracked in the reversible path too, so it
> behaves the same as the non-reversible path.
We shouldn't do that. The real problem here is that ct_clear was reversible
in the past, but it's no longer reversible since:
1fe178d251c8 ("dpif: Add support for OVS_ACTION_ATTR_CT_CLEAR")
Because after that commit we generate the datapath action and we can't
reverse that without passing a packet through conntrack again, which we
can't do as it will break the state. So, we should just not take the
reversible path, because it isn't.
>
> Add a kernel datapath system test to cover this scenario.
This issue is not related to the conntrack implementation in the datapath,
so it should not be a system test. We'll need a test for this in
tests/ofproto-dpif.at instead. There are some tests for reversible actions
in there already.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
