On 6/21/21 11:20, Eelco Chaudron wrote:
> For patch ports, the is_last_action value is not propagated and is
> always set to true. This causes non-reversible actions to modify the
> packet, and the original content is not preserved when processing
> the remaining actions.
>
> This patch propagates the is_last_action flag for patch port related
> actions. In addition, it also fixes a general last action propagation
> to the individual actions.
>
> Fixed check_pkt_larger as last action, as it is a valid case for the
> drop action, so it should not be skipped.
>
> Fixes: feee58b95 ("ofproto-dpif-xlate: Keep track of the last action")
> Fixes: 5b34f8fc3 ("Add a new OVS action check_pkt_larger")
> Signed-off-by: Eelco Chaudron <[email protected]>
>
> ---
> v2: Fixed additional last action propagation to individual actions.
> Fixed problem with the check_pkt_larger now that that the correct
> last action state is passed.
>
> ofproto/ofproto-dpif-xlate.c | 27 ++++++++++++---------------
> tests/ofproto-dpif.at | 28 ++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+), 15 deletions(-
<snip>
> @@ -6742,7 +6744,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
> ofpacts_len,
> const struct ofpact_set_field *set_field;
> const struct mf_field *mf;
> bool last = is_last_action && ofpact_last(a, ofpacts, ofpacts_len)
> - && ctx->action_set.size;
> + && !ctx->action_set.size;
This part was weird indeed and I'm not really sure if we need to check
for the action set being non-empty here at all. But yeah, this code
is tricky.
Anyway, this version seems to work fine. I additionally re-checked
OVN tests, they also work as expected.
Thanks for the fix! Applied and backported down to 2.13.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
