On 8/14/23 22:17, Dumitru Ceara wrote: > On 8/9/23 20:01, Mark Michelson wrote: >> Thanks Mohammad and Ales. I have pushed the change to main and all >> branches back to 22.03. >> > > Hi Mark, Mohammad, > > It seems this change broke 22.03: > > https://github.com/ovn-org/ovn/actions/runs/5812479357 > >> On 8/3/23 04:46, Ales Musil wrote: >>> 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, > > This part triggers the ct_zones_runtime_data_handler() I-P handler to be > called. But on 22.03 this aborts on TRACKED_RESOURCE_UPDATED: > > https://github.com/ovn-org/ovn/blob/915c556f0c50e81b1dbc05a68d754cf4cb7d4551/controller/ovn-controller.c#L2154 > > Do you maybe have some time to look into this? Or should we just revert > the backport on 22.03? I don't think ofport updates are common. >
I went ahead and reverted this on branch 22.03 as that's our LTS and I wanted to make sure we have green CI on it. Mohammad, please post a custom backport patch if you want this fixed on 22.03 too. Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
