On Fri, Feb 19, 2021 at 3:59 PM Mark Gray <[email protected]> wrote: > > On 18/02/2021 18:49, Dumitru Ceara wrote: > > Depending on the logical flow matches, multiple SB flows can point to > > the same desired flow. If it happens that the desired flow conflicts > > with a more restrictive (already installed) flow, then we shouldn't try > > to add the desired flow multiple times to the list maintained for the > > installed flow. > > > > Fixes: 6f0b1e02d9ab ("ofctrl: Incremental processing for flow installation > > by tracking.") > > Signed-off-by: Dumitru Ceara <[email protected]> > > --- > > Note: I'm quite sure about the "Fixes" tag above, however, I couldn't > > reproduce the issue without the following commit: > > dadae4f800cc ("ofctrl.c: Only merge actions for conjunctive flows.") > > > > v2: > > - Address Mark's comments: > > - readd log message removed by accident. > > - add comments to the new test. > > --- > > controller/ofctrl.c | 2 +- > > tests/ovn.at | 23 +++++++++++++++++++++++ > > 2 files changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > > index 1c9694a..415d9b7 100644 > > --- a/controller/ofctrl.c > > +++ b/controller/ofctrl.c > > @@ -1989,7 +1989,7 @@ update_installed_flows_by_track(struct > > ovn_desired_flow_table *flow_table, > > * tracked, so it must have been modified. */ > > installed_flow_mod(&i->flow, &f->flow, msgs); > > ovn_flow_log(&i->flow, "updating installed (tracked)"); > > - } else { > > + } else if (!f->installed_flow) { > > /* Adding a new flow that conflicts with an existing > > installed > > * flow, so add it to the link. If this flow becomes > > active, > > * e.g., it is less restrictive than the previous active > > flow > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 344e6bf..3d027c4 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -13955,6 +13955,29 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int > > table=45 | ofctl_strip_all | \ > > table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 > > actions=conjunction() > > ]) > > > > +# Add another ACL that overlaps with the existing less restrictive ones. > > +check ovn-nbctl acl-add ls1 to-lport 3 'udp || ((ip4.src==10.0.0.1 || > > ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4))' allow > > +check ovn-nbctl --wait=hv sync > > + > > +# Check OVS flows, the same conjunctive flows as above should still be > > there, > > +# with an additional conjunction action. > > +# > > +# New non-conjunctive flows should be added to match on 'udp'. > > +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ > > + grep "priority=1003" | \ > > + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl > > + table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) > > + table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) > > + table=45, priority=1003,conj_id=4,ip,metadata=0x1 actions=resubmit(,46) > > + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 > > actions=conjunction(),conjunction(),conjunction() > > + table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 > > actions=conjunction(),conjunction(),conjunction() > > + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 > > actions=resubmit(,46) > > + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 > > actions=conjunction(),conjunction() > > + table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 > > actions=conjunction() > > + table=45, priority=1003,udp,metadata=0x1 actions=resubmit(,46) > > + table=45, priority=1003,udp6,metadata=0x1 actions=resubmit(,46) > > +]) > > + > > OVN_CLEANUP([hv1]) > > AT_CLEANUP > > > > > Acked-by: Mark Gray <[email protected]> > > Also tested it.
Thanks Dumitru and Mark. I applied this patch to master and branch-20.12. I'm not sure if it is required for 20.09 or not. It doesn't apply cleanly though. Numan > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
