mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 14:25:26 +00:00
Code Issues Releases Wiki Activity

meta-flow: Fix nw_frag mask while parsing from string.

Browse Source 
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: 9e44d71563 ("Don't overload IP TOS with the frag matching bits.")
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Ilya Maximets
2024-10-23 19:06:08 +02:00
parent c30ff22b5d
commit a119828ea6
2 changed files with 47 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
lib/meta-flow.c
View File

@@ -2654,7 +2654,8 @@ mf_set(const struct mf_field *mf,
break;
case MFF_IP_FRAG:
match_set_nw_frag_masked(match, value->u8, mask->u8);
match_set_nw_frag_masked(match, value->u8,
mask->u8 & FLOW_NW_FRAG_MASK);
break;
case MFF_ARP_SPA:

45
tests/ovs-ofctl.at
View File

@@ -3100,6 +3100,51 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLO
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ovs-ofctl replace-flows with fragments])
OVS_VSWITCHD_START
AT_DATA([frag_flows.txt], [dnl
ip,nw_frag=first actions=drop
ip,nw_frag=later actions=drop
ip,nw_frag=no actions=NORMAL
ip,nw_frag=not_later actions=NORMAL
ip,nw_frag=yes actions=LOCAL
])
AT_DATA([replace_flows.txt], [dnl
ip,nw_frag=first actions=NORMAL
ip,nw_frag=later actions=LOCAL
ip,nw_frag=no actions=drop
ip,nw_frag=not_later actions=drop
ip,nw_frag=yes actions=drop
])
AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 frag_flows.txt])
on_exit 'ovs-ofctl -O OpenFlow13 dump-flows br0'
dnl Check that flow replacement works.
AT_CHECK([ovs-ofctl -vvconn:console:dbg -O OpenFlow13 \
replace-flows br0 replace_flows.txt 2>&1 | grep FLOW_MOD \
| sed 's/.*\(OFPT_FLOW_MOD.*\)/\1/' | strip_xids | sort], [0], [dnl
OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=first actions=NORMAL
OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=later actions=LOCAL
OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=no actions=drop
OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=not_later actions=drop
OFPT_FLOW_MOD (OF1.3): ADD ip,nw_frag=yes actions=drop
])
dnl Check that replacement to the same set doesn't cause flow modifications.
AT_CHECK([ovs-ofctl -vvconn:console:dbg -O OpenFlow13 \
replace-flows br0 replace_flows.txt 2>&1 | grep FLOW_MOD \
| sed 's/.*\(OFPT_FLOW_MOD.*\)/\1/' | strip_xids | sort], [0], [])
dnl Compare the flow dump against the expected set.
cat replace_flows.txt > expout
AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 \
| ofctl_strip | sed '/OFPST_FLOW/d' | sort], [0], [expout])
OVS_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([ovs-ofctl replace-flows with --bundle])
OVS_VSWITCHD_START