> Hi Lorenzo, Hi Mark,
thx for the review. > > 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. sorry, I did not get you here, can you please elaborate on this? > > 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. ack, I will do > > 2) Add a test to ensure that the flows added in table 76 and 77 by the > learn() action are what we expect. ack, I will do > > 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. ack, I will do Regards, Lorenzo > > 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
