Hi Lorenzo,
As a general note on this changeset, I think that more tests should be
added. At this point, the only test here is the augmentation of the
existing ECMP symmetric reply tests. This is great since it gives us a
good end-to-end test. However, there are two issues here:
1) It proves the results, but it doesn't prove that the results happen
for the reasons we might expect.
2) If something causes the test to break, trying to figure out what is
causing the failure is going to be difficult.
I suggest that several more tests be added
1) Add tests to the "action parsing" tests that ensure that the new
actions that have been added to OVN encode as expected.
2) Add a test to ensure that the flows added in table 76 and 77 by the
learn() action are what we expect.
3) Ensure that the flows in table 76 and 77 are removed after the hard
timeout. You can probably just add this onto the ECMP symmetric replies
test in system-ovn.at.
On 7/13/22 08:44, Lorenzo Bianconi wrote:
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2096233
Lorenzo Bianconi (3):
actions: introduce commit_ecmp_nh action
actions: introduce chk_ecmp_nh and chk_ecmp_nh_mac actions
northd: rely on new actions for ecmp-symmetric routing
controller/lflow.h | 2 +
include/ovn/actions.h | 9 ++
include/ovn/logical-fields.h | 3 +
lib/actions.c | 230 +++++++++++++++++++++++++++++++++++
northd/northd.c | 46 +++++--
northd/ovn-northd.8.xml | 25 ++--
ovn-sb.xml | 54 ++++++++
tests/ovn-northd.at | 4 +-
tests/ovn.at | 4 +-
tests/system-ovn.at | 52 ++++++++
utilities/ovn-trace.c | 6 +
11 files changed, 410 insertions(+), 25 deletions(-)
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
