Hi Ilya, Thanks for the review. I have addressed the comments and sent v2.
Thanks, Naveen > On 30 Apr 2026, at 11:58 PM, Ilya Maximets <[email protected]> wrote: > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > 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
