On 4/12/21 1:01 PM, Ilya Maximets wrote:
> On 4/8/21 8:45 PM, Dumitru Ceara wrote:
>> Considering two logical datapaths (DP1 and DP2) and a logical flow that
>> is shared between the two, ovn-northd will create the following SB
>> records:
>>
>> Datapaths: DP1, DP2
>> Datapath_Group: DPG1 {dps=[DP1, DP2]}
>
> s/Datapath_Group/Logical_DP_Group/g
>
> Not a big deal, but just for consistency for future readers.
>
>> Logical_Flow: LF {dp_group=DPG1}
>>
>> If an ovn-controller is conditionally monitoring DP1 and DP2 its
>> monitor conditions look like this:
>>
>> Datapath_Group table when:
>> - Datapath_Group.datapaths in {DP1._uuid, DP2._uuid}
>
> 'in' doesn't sound right as it's more an opposite relation.
> Again, it's a nitpick, but maybe worth changing to something
> like:
> - Logical_DP_Group.datapaths includes [DP1._uuid or DP2._uuid]
>
>> Logical_Flow table when:
>> - Logical_Flow.logical_datapath in {DP1._uuid, DP2._uuid}
>> - Logical_Flow.logical_dp_group in {DPG1._uuid}
>>
>> If at this point a new logical datapath DP3 is created (e.g., a LS is
>> added) and the logical flow LF is shared with this third datapath, then
>> ovn-northd populates the SB with the following contents, replacing
>> old DPG1 with DPG1-new:
>>
>> Datapaths: DP1, DP2, DP3
>> Datapath_Group: DPG1-new {dps=[DP1, DP2, DP3]}
>> Logical_Flow: LF {dp_group=DPG1-new}
>>
>> At this point, the SB ovsdb-server will send the following updates to
>> ovn-controller, to match the current monitor condition it requested:
>>
>> Datapaths:
>> - "insert" DP3
>>
>> Datapath_Group:
>> - "delete" DPG1
>> - "insert" DPG1-new {dps=[DP1, DP2, DP3]}
>
> We can clarify here that this new group received because it has
> DP1 and DP2 and we have this condition set.
>
>>
>> Logical_Flow:
>> - "delete" LF
>>
>> The logical flow is deleted from the client's view of the database
>> because the monitor condition for Logical_Flow records doesn't include
>> DPG1-new._uuid.
>>
>> Now ovn-controller will:
>> - delete all openflows corresponding to LF, including the ones generated
>> for datapaths DP1 and DP2.
>> - compute new set of monitor conditions and send the monitor_cond_change
>> request to the SB:
>>
>> Datapath_Group table when Datapath_Group.uuid in {DPG1-new._uuid}
>
> This should look like this (we're not monitoring groups by group uuid):
>
> - Logical_DP_Group.datapaths includes
> [DP1._uuid or DP2._uuid or DP3._uuid]
>
>> Logical_Flow table when:
>> - Logical_Flow.logical_datapath in {DP1._uuid, DP2._uuid, DP3._uuid}
>> - Logical_Flow.logical_dp_group in {DPG1-new._uuid}
>>
>> Upon processing this new set of conditions, the SB sends the following
>> update:
>> Logical_Flow:
>> - "insert" LF <--- now LF.logical_dp_group matches the condition.
>>
>> Finally, ovn-controller will add all openflows that correspond to LF, on
>> all datapaths, DP1, DP2, DP3.
>>
>> There is therefore a window in which traffic on DP1 and DP2 might be
>> dropped because openflows corresponding to LF1 on DP1 and DP2 have been
>> removed but not yet reinstalled.
>>
>> To avoid this we now unconditionally monitor all logical flows that
>> refer to datapath groups.
>>
>> Fixes: 4b946b366c4c ("controller: Add support for Logical Datapath Groups.")
>> Reported-at: https://bugzilla.redhat.com/1947056
>> Suggested-by: Ilya Maximets <[email protected]>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>
>
> Beside some small nitpicks above, LGTM.
>
> Acked-by: Ilya Maximets <[email protected]>
>
Thanks, Ilya, for the review!
I sent a v2 with the rephrased commit log:
http://patchwork.ozlabs.org/project/ovn/list/?series=238728
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
