[ovs-dev] [PATCH ovn] ovn-controller: Fix I-P for SB Port_Binding and OVS Interface.
Dumitru Ceara
dceara at redhat.com
Thu Jun 11 08:31:23 UTC 2020
On 6/10/20 7:58 PM, Numan Siddique wrote:
>
>
> On Wed, Jun 10, 2020 at 10:16 PM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
>
> The commit that introduced incremental processing for Port_Binding and
> OVS Interface in the I-P runtime_data node covered most cases but two
> were missed:
>
> 1. If a Port_Binding was already claimed by the local hypervisor when
> ovn-controller starts, binding_handle_port_binding_changes doesn't
> correctly set the "changed" variable causing en_runtime_data node to
> go to EN_VALID instead of EN_UPDATED. Due to this update_sb_monitors()
> is skipped in that run and ovn-controller does not register for
> updates regarding the datapath containing the Port_Binding.
>
> 2. If a Port_Binding was already claimed by the local hypervisor when
> ovn-controller starts, but the underlying OVS interface was removed in
> the meantime, handle_updated_vif_lport() would fail the assertion that a
> local_binding should exist in memory.
>
> To address the first issue, we now explicitly track changes to the
> binding
> context local_lport and local_lport_ids sets. If these change during
> incremental processing of the runtime_data OVS_Interface and
> SB_Port_Binding input nodes then the runtime_data node should change
> state to EN_UPDATED.
>
> For the second issue, we now allow the case when a stale port_binding is
> released.
>
> Also, added an explicit non_vif_ports_changed variable to
> binding_ctx_out to track if other types of Port_Bindings
> have been changed in the current run. This kind of update should also
> cause runtime_data to move to EN_UPDATED such that update_sb_monitors()
> gets executed.
>
>
> Thanks Dumitru for fixing this. I applied this patch to master and
> branch-20.06.
>
Thanks Numan!
> When other types of Port_Bindings get updated, I don't think there is a
> need to
> call update_sb_monitors(). Because we don't do conditional monitoring on
> other type of Port_Bindings and the fact that 'non_vif_ports_changed'
> gets set
> to true, indicate that the ovn-controller received the updates for other
> type of
> Port_Bindings.
Actually, at least for some non-vif port bindings like LP_PATCH we need
to call update_sb_monitors() because local_datapaths might be updated.
Some SB records (e.g., Logical_Flow) are filtered on datapath id and if
we don't call update_sb_monitors() SB won't send them to us.
It's true that we might have to refine in which cases we need to set
non_vif_ports_changed. For LP_PATCH for example we don't really need to
because patch port bindings are also added to local_lport_ids so
local_lport_ids_changed will be true. But for now I just decided to set
it in all places where we had "*changed = true" for non-vif port binding
updates.
Regards,
Dumitru
>
> I'll rebase my I-P patch series and submit v12.
>
> Thanks
> Numan
>
More information about the dev
mailing list