On Mon, Jul 31, 2023 at 7:18 PM Mohammad Heib <mh...@redhat.com> wrote:
> Currently when ovs interface ofport is updated > after setting external_ids:iface_id, the ovn-controller > will see this change but will not do much if it handles > this change incrementally. > > This behavior leads to a mismatch between the ovs openflow > flows in table=0 (inaccurate in_port) and the ofport number > that the packet was received at which will lead to packets > drop in table=0. > > This patch will resolve the above issue by triggering > flows recompute during the I-P processing only if the > affected port are associated with lport and has flows > that need to be updated. > > Reported-at: https://issues.redhat.com/browse/FD-3063 > Signed-off-by: Mohammad Heib <mh...@redhat.com> > --- > controller/binding.c | 15 ++++++++++++++ > tests/ovn-controller.at | 45 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/controller/binding.c b/controller/binding.c > index 9aa3fc6c4..cc4c2b0bb 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -2360,6 +2360,21 @@ consider_iface_claim(const struct ovsrec_interface > *iface_rec, > /* Get the (updated) b_lport again for the lbinding. */ > b_lport = local_binding_get_primary_lport(lbinding); > > + /* > + * Update the tracked_dp_bindings whenever an ofport > + * on a specific ovs port changes. > + * This update will trigger flow recomputation during > + * the incremental processing run which updates the local > + * flows in_port filed. > + */ > + if (b_lport && ovsrec_interface_is_updated(iface_rec, > + OVSREC_INTERFACE_COL_OFPORT)) { > + tracked_datapath_lport_add(b_lport->pb, TRACKED_RESOURCE_UPDATED, > + b_ctx_out->tracked_dp_bindings); > + b_ctx_out->local_lports_changed = true; > + } > + > + > /* Update the child local_binding's iface (if any children) and try to > * claim the container lbindings. */ > LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) { > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index e1b6491b3..f2216d245 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -2606,3 +2606,48 @@ AT_CHECK([ovn-sbctl get chassis $chassis_id > other_config:unsupported], [1], [ign > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn-controller - ovs iface change ofport]) > +AT_KEYWORDS([ovn]) > +ovn_start > + > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > +ovs-vsctl -- add-port br-int hv1-vif1 -- \ > + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap \ > + ofport-request=1 > + > + > +ovn-nbctl ls-add sw0 > + > +check ovn-nbctl lsp-add sw0 sw0-p1 > +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3 > 1000::3" > + > +wait_for_ports_up > +ovn-nbctl --wait=hv sync > + > +OVS_WAIT_FOR_OUTPUT([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c > in_port=1], [0],[dnl > +1 > +]) > + > +# update the ovs interface ofport from 1 to 24 > +check as hv1 ovs-vsctl set Interface hv1-vif1 ofport-request=24 > +OVS_WAIT_UNTIL([test x`as hv1 ovs-vsctl get Interface hv1-vif1 ofport` = > x24]) > + > +OVS_WAIT_FOR_OUTPUT([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c > in_port=24], [0],[dnl > +1 > +]) > +OVS_WAIT_FOR_OUTPUT([as hv1 ovs-ofctl dump-flows br-int table=0 | grep -c > in_port=1], [1],[dnl > +0 > +]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > -- > 2.34.3 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amu...@redhat.com> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> amu...@redhat.com IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
