On 1/8/21 10:56 PM, Mark Michelson wrote: > Hi Dumitru, > Hi Mark,
Thanks for the review! > I noticed in patch 2 that your unit tests will not run if > ENABLE_UNIT_TESTS is set to no. This appears to mimic what I've done in > my IPAM refactor patchset. However, neither of us needs this any longer. > This is a holdover from when I had created a unit test framework, and > had created a configure-time option to enable unit tests. We can both > remove this check from our tests. You're right, I inspired myself from your IPAM refactor patch set (sorry I should've explicitly mentioned that in my commit log; I'll fix it in v3), and ENABLE_UNIT_TESTS is not needed. > > The other general suggestion I have is to add some tests to patch 3 to > ensure that ovs interface external-id gets set as expected. Makes sense, I'll add more tests in the next revision. Regards, Dumitru > > On 1/6/21 2:37 PM, Dumitru Ceara wrote: >> This series adds support to ovn-controller to notify northd that all OVS >> flows and OVS interface corresponding to an OVN logical port have been >> installed in OVS. >> >> The series extends the ofctrl barrier implementation and makes it generic >> so that multiple modules can use it. For now these are: >> - main ovn-controller for nb_cfg propagation. >> - binding module for marking Port_Bindings as installed. >> >> NOTE: With these patches applied one of the unit tests fails more often. >> A separate patch has been submitted for that: >> http://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ >> >> >> V2: >> - Patch 3: >> - Fix check in binding_seqno_install() to take into account deleted >> port bindings. >> >> Dumitru Ceara (3): >> ofctrl: Rename 'nb_cfg' to 'req_cfg'. >> controller: Implement a generic barrier based on ofctrl cur_cfg >> sync. >> binding: Set Logical_Switch_Port.up when all OVS flows are >> installed. >> >> >> NEWS | 6 + >> controller/automake.mk | 2 >> controller/binding.c | 173 ++++++++++++++++++++++++++++ >> controller/binding.h | 5 + >> controller/ofctrl-seqno.c | 245 >> ++++++++++++++++++++++++++++++++++++++++ >> controller/ofctrl-seqno.h | 50 ++++++++ >> controller/ofctrl.c | 70 ++++++----- >> controller/ofctrl.h | 4 - >> controller/ovn-controller.c | 53 +++++++-- >> controller/test-ofctrl-seqno.c | 194 ++++++++++++++++++++++++++++++++ >> northd/ovn-northd.c | 4 - >> ovn-sb.ovsschema | 5 - >> ovn-sb.xml | 8 + >> tests/automake.mk | 8 + >> tests/ovn-ofctrl-seqno.at | 228 >> +++++++++++++++++++++++++++++++++++++ >> tests/testsuite.at | 1 >> utilities/ovn-sbctl.c | 4 + >> 17 files changed, 1008 insertions(+), 52 deletions(-) >> create mode 100644 controller/ofctrl-seqno.c >> create mode 100644 controller/ofctrl-seqno.h >> create mode 100644 controller/test-ofctrl-seqno.c >> create mode 100644 tests/ovn-ofctrl-seqno.at >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
