Ilya Maximets <[email protected]> writes: > 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]> > ---
Clearly no one noticed it for a while :) Also, thanks for fixing it in the `mf_set` rather than in `match_set_nw_frag_masked`. The original `cls_rule_set_frag_masked` was a bit messy due to the overload. Your fix continues to look clean to me. Acked-by: Aaron Conole <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
