Aaron Conole <[email protected]> writes: > [email protected] writes: > >> From: wenxu <[email protected]> >> >> Now, the default timeout policy for netdev datapath is hard codeing. In >> some case show or modify is needed. >> Add command for get/set default timeout policy. Using like this: >> >> ovs-appctl dpctl/ct-get-default-tp [dp] >> ovs-appctl dpctl/ct-set-default-tp [dp] policies >> >> Signed-off-by: wenxu <[email protected]> >> --- > > So far looks good. Just one minor comment (and one conflict) that might be > able to be addressed
After over an hour of loops, I did manage to produce one failure with your test case: --- - 2022-01-17 10:31:12.740151293 -0500 +++ /home/aconole/git/ovs2/ovs/tests/system-userspace-testsuite.dir/at-groups/94/stdout 2022-01-17 10:31:12.739245956 -0500 @@ -1,3 +1,2 @@ -icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0),zone=5 udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),zone=5 I think the test suite is "stable" but it might be better to use something like 'ovs-ofctl monitor' to watch for events. Maybe you have tried this already? The errant 'sleep's can result in failures like this. WDYT? >> NEWS | 4 +++ >> lib/conntrack-tp.c | 11 +++++++ >> lib/conntrack-tp.h | 2 ++ >> lib/ct-dpif.c | 56 ++++++++++++++++++++++++++++++++ >> lib/ct-dpif.h | 9 ++++++ >> lib/dpctl.c | 69 >> ++++++++++++++++++++++++++++++++++++++++ >> lib/dpif-netdev.c | 25 +++++++++++++++ >> lib/dpif-netlink.c | 2 ++ >> lib/dpif-provider.h | 8 +++++ >> tests/system-kmod-macros.at | 10 ++++++ >> tests/system-traffic.at | 67 ++++++++++++++++++++++++++++++++++++++ >> tests/system-userspace-macros.at | 7 ++++ >> 12 files changed, 270 insertions(+) >> >> diff --git a/NEWS b/NEWS >> index 553a57d..f9fc04a 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -39,6 +39,10 @@ Post-v2.16.0 >> is mainly for the benefit of OVN load balancing configurations. >> - Ingress policing on Linux now uses 'matchall' classifier instead of >> 'basic', if available. >> + - ovs-appctl dpctl/: >> + * New commands 'ct-set-default-tp' and >> + 'ct-set-default-tp' that allows to get or configure >> + netdev datapath ct default timeout policy. > > I had a conflict with this section on apply. It isn't a huge deal. > >> diff --git a/lib/conntrack-tp.h b/lib/conntrack-tp.h >> index 4d411d1..07dcb4e 100644 >> --- a/lib/conntrack-tp.h >> +++ b/lib/conntrack-tp.h >> @@ -710,6 +729,43 @@ ct_dpif_free_zone_limits(struct ovs_list *zone_limits) >> } >> } >> >> + >> +/* Parses a specification of a timeout policy from 's' into '*tp' >> + * . Returns true on success. Otherwise, returns false and >> + * and puts the error message in 'ds'. */ > > The '.' in the above is ending the sentence on the previous line, and > the world 'and' appears twice in a row. > > I think it should read: > > /* Parses a specification of a timeout policy from 's' into '*tp'. > * Returns true on success. Otherwise, returns false and puts the > * error message in 'ds'. */ _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
