| Message ID | 20250512205815.870519-5-mmichels@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ales Musil |
| Headers | show |
| Series | Datapath and Port Sync Refactor | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Mon, May 12, 2025 at 10:58 PM Mark Michelson via dev < ovs-dev@openvswitch.org> wrote: > The "check packet length" test creates two unorthodox > MAC_Bindings. The two MAC Bindings' datapath values refer to > southbound Datapath_Bindings for northbound Logical_Switches. Meanwhile, > the MAC_Bindings' logical_port values refer to northbound > Logical_Router_Ports. > > OVN makes the assumption that all MAC_Bindings' will refer to > Datapath_Bindings corresponding to northbound Logical_Router_Ports. > Therefore, if ovn-northd thinks it should clean up stale MAC Bindings, > then it will delete any MAC_Binding whose datapath column refers to a > Logical_Switch Datapath_Binding. > > The test passes because it never happens to trigger ovn-northd's MAC > binding cleanup code. An upcoming patch is going to change the > circumstances under which ovn-northd will clean up stale MAC Bindings, > though. > > With this change, the test creates the MAC_Binding using the logical > router Datapath_Binding instead. The test still passes, and now it will > not have its MAC_Binding removed by ovn-northd when cleaning up stale > MAC_Bindings. > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > v4 -> v5: > * Rebased. > > v3 -> v4: > * Rebased. > > v3: This is the first version with this patch present. > --- > tests/ovn.at | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 596a7be2b..9dd602adf 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -22206,7 +22206,7 @@ AT_CAPTURE_FILE([sbflows]) > AT_CHECK([as hv1 ovs-ofctl dump-flows br-int \ > | grep "check_pkt_larger" | wc -l], [0], [[0 > ]]) > -dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep _uuid | \ > +dp_uuid=$(ovn-sbctl find datapath_binding | grep lr0 -B2 | grep _uuid | \ > awk '{print $3}') > check_uuid ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \ > logical_port=lr0-public mac="00\:00\:00\:12\:af\:11" > @@ -22277,7 +22277,7 @@ check ovn-nbctl lr-nat-add lr1 snat 172.168.0.100 > 10.0.0.0/24 > check ovn-nbctl lr-nat-add lr1 snat 2000::1 1000::/64 > check ovn-nbctl --wait=sb sync > > -dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep _uuid | \ > +dp_uuid=$(ovn-sbctl find datapath_binding | grep lr1 -B2 | grep _uuid | \ > awk '{print $3}') > check_uuid ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \ > logical_port=lr1-public mac="00\:00\:00\:12\:af\:11" > -- > 2.47.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
diff --git a/tests/ovn.at b/tests/ovn.at index 596a7be2b..9dd602adf 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22206,7 +22206,7 @@ AT_CAPTURE_FILE([sbflows]) AT_CHECK([as hv1 ovs-ofctl dump-flows br-int \ | grep "check_pkt_larger" | wc -l], [0], [[0 ]]) -dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep _uuid | \ +dp_uuid=$(ovn-sbctl find datapath_binding | grep lr0 -B2 | grep _uuid | \ awk '{print $3}') check_uuid ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \ logical_port=lr0-public mac="00\:00\:00\:12\:af\:11" @@ -22277,7 +22277,7 @@ check ovn-nbctl lr-nat-add lr1 snat 172.168.0.100 10.0.0.0/24 check ovn-nbctl lr-nat-add lr1 snat 2000::1 1000::/64 check ovn-nbctl --wait=sb sync -dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep _uuid | \ +dp_uuid=$(ovn-sbctl find datapath_binding | grep lr1 -B2 | grep _uuid | \ awk '{print $3}') check_uuid ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \ logical_port=lr1-public mac="00\:00\:00\:12\:af\:11"
The "check packet length" test creates two unorthodox MAC_Bindings. The two MAC Bindings' datapath values refer to southbound Datapath_Bindings for northbound Logical_Switches. Meanwhile, the MAC_Bindings' logical_port values refer to northbound Logical_Router_Ports. OVN makes the assumption that all MAC_Bindings' will refer to Datapath_Bindings corresponding to northbound Logical_Router_Ports. Therefore, if ovn-northd thinks it should clean up stale MAC Bindings, then it will delete any MAC_Binding whose datapath column refers to a Logical_Switch Datapath_Binding. The test passes because it never happens to trigger ovn-northd's MAC binding cleanup code. An upcoming patch is going to change the circumstances under which ovn-northd will clean up stale MAC Bindings, though. With this change, the test creates the MAC_Binding using the logical router Datapath_Binding instead. The test still passes, and now it will not have its MAC_Binding removed by ovn-northd when cleaning up stale MAC_Bindings. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- v4 -> v5: * Rebased. v3 -> v4: * Rebased. v3: This is the first version with this patch present. --- tests/ovn.at | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)