On 23 Oct 2024, at 19:06, Ilya Maximets wrote:
> mf_from_frag_string() sets all the upper bits of the nw_frag to make
> sure the exact match check will pass. This was not taken into account
> while splitting nw_frag and IP TOS bits into separate fields and the
> mask clean up was removed from the cls_rule_set_frag_masked() which is
> now called match_set_nw_frag_masked(). This leads to the case where
> the match parsed from the string is not considered equal to the
> match parsed from the OpenFlow, due to difference in masks. And that
> is causing ovs-ofctl replace-flows to always replace flows that match
> on nw_frag, even if they are exactly the same triggering unnecessary
> flow table updates and revalidation.
>
> $ cat frag_flows.txt
> ip,in_port=1,nw_frag=yes actions=drop
>
> $ ovs-ofctl dump-flows --no-stat --no-names br0
> ip,in_port=1,nw_frag=yes actions=drop
>
> $ ovs-ofctl -vvconn replace-flows br0 frag_flows.txt 2>&1 | grep MOD
> NXT_FLOW_MOD: DEL_STRICT ip,in_port=1,nw_frag=yes actions=drop
> NXT_FLOW_MOD: ADD ip,in_port=1,nw_frag=yes actions=drop
>
> Clear the extra mask bits while setting match/flow structure from the
> field to avoid the issue.
>
> The issue mainly affects non-exact matches 'non_later' and 'yes', since
> exact matches are special-handled in many places / considered equal to
> not having a mask at all.
>
> Note: ideally we would not use byte-sized is_all_ones() for exact match
> checking, but use field-specific checks instead. However, this leads
> to a much larger change throughout OVS code base and would be much
> harder to backport. For now, fixing the issue in the way the code was
> originally implemented.
>
> Fixes: 9e44d715638a ("Don't overload IP TOS with the frag matching bits.")
> Signed-off-by: Ilya Maximets <[email protected]>
Thanks for fixing this. The changes look fine to me.
Acked-by: Eelco Chaudron <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
