| Message ID | 1548976201-25288-2-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,1/2] dpif-netlink: Fix a bug that causes duplicate key error in datapath | expand |
Minor comments about the commit message Yifeng On Thu, Jan 31, 2019 at 3:10 PM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > Test "flow resume with geneve tun_metadata" failed because there is > no controller running to send controller(pause) message. The controller receives a 'pause' related to the continuation. The controller sends back a 'resume' related to the continuation. s/no controller running to send controller(pause) message./no controller running to handle the continuation./ > A previous > commit deleted the line that starts ovs-ofctl as a controller in > order to avoid a race condition on monitor log. This patch adds > back this line but omits the log file because this test doesn't > dpends on the log file. > s/dpends/depend/ > > Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition > on monitor log") > CC: David Marchand <david.marchand@redhat.com> > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > tests/system-traffic.at | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index ffe508dd61f7..84c2af4170a3 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -538,6 +538,8 @@ OVS_CHECK_GENEVE() > OVS_TRAFFIC_VSWITCHD_START() > ADD_BR([br-underlay]) > > +AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile 2> > /dev/null]) > + > ADD_NAMESPACES(at_ns0) > > dnl Set up underlay link from host into the namespace using veth pair. > @@ -567,6 +569,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 | > FORMAT_PING], [0], [dnl > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > > +OVS_APP_EXIT_AND_WAIT([ovs-ofctl]) > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > -- > 2.7.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks Darrell, I will send a new version tomorrow. On Thu, Jan 31, 2019 at 5:19 PM Darrell Ball <dlu998@gmail.com> wrote: > Minor comments about the commit message Yifeng > > On Thu, Jan 31, 2019 at 3:10 PM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > >> Test "flow resume with geneve tun_metadata" failed because there is >> no controller running to send controller(pause) message. > > > The controller receives a 'pause' related to the continuation. > The controller sends back a 'resume' related to the continuation. > > s/no controller running to send controller(pause) message./no controller > running to handle the continuation./ > > >> A previous >> commit deleted the line that starts ovs-ofctl as a controller in >> order to avoid a race condition on monitor log. This patch adds >> back this line but omits the log file because this test doesn't >> dpends on the log file. >> > > s/dpends/depend/ > > >> >> Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition >> on monitor log") >> CC: David Marchand <david.marchand@redhat.com> >> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> >> --- >> tests/system-traffic.at | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >> index ffe508dd61f7..84c2af4170a3 100644 >> --- a/tests/system-traffic.at >> +++ b/tests/system-traffic.at >> @@ -538,6 +538,8 @@ OVS_CHECK_GENEVE() >> OVS_TRAFFIC_VSWITCHD_START() >> ADD_BR([br-underlay]) >> >> +AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile 2> >> /dev/null]) >> + >> ADD_NAMESPACES(at_ns0) >> >> dnl Set up underlay link from host into the namespace using veth pair. >> @@ -567,6 +569,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 | >> FORMAT_PING], [0], [dnl >> 3 packets transmitted, 3 received, 0% packet loss, time 0ms >> ]) >> >> +OVS_APP_EXIT_AND_WAIT([ovs-ofctl]) >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> >> -- >> 2.7.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
On Fri, Feb 1, 2019 at 12:10 AM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > Test "flow resume with geneve tun_metadata" failed because there is > no controller running to send controller(pause) message. A previous > commit deleted the line that starts ovs-ofctl as a controller in > order to avoid a race condition on monitor log. This patch adds > back this line but omits the log file because this test doesn't > dpends on the log file. > > Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition > on monitor log") > CC: David Marchand <david.marchand@redhat.com> > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > tests/system-traffic.at | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index ffe508dd61f7..84c2af4170a3 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -538,6 +538,8 @@ OVS_CHECK_GENEVE() > OVS_TRAFFIC_VSWITCHD_START() > ADD_BR([br-underlay]) > > +AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile 2> > /dev/null]) > + > ADD_NAMESPACES(at_ns0) > > dnl Set up underlay link from host into the namespace using veth pair. > @@ -567,6 +569,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 | > FORMAT_PING], [0], [dnl > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > > +OVS_APP_EXIT_AND_WAIT([ovs-ofctl]) > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > Oops. Thanks for fixing.
diff --git a/tests/system-traffic.at b/tests/system-traffic.at index ffe508dd61f7..84c2af4170a3 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -538,6 +538,8 @@ OVS_CHECK_GENEVE() OVS_TRAFFIC_VSWITCHD_START() ADD_BR([br-underlay]) +AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile 2> /dev/null]) + ADD_NAMESPACES(at_ns0) dnl Set up underlay link from host into the namespace using veth pair. @@ -567,6 +569,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 10.1.1.100 | FORMAT_PING], [0], [dnl 3 packets transmitted, 3 received, 0% packet loss, time 0ms ]) +OVS_APP_EXIT_AND_WAIT([ovs-ofctl]) OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP
Test "flow resume with geneve tun_metadata" failed because there is no controller running to send controller(pause) message. A previous commit deleted the line that starts ovs-ofctl as a controller in order to avoid a race condition on monitor log. This patch adds back this line but omits the log file because this test doesn't dpends on the log file. Fixes: e8833217914f9c071c49 ("system-traffic.at: avoid a race condition on monitor log") CC: David Marchand <david.marchand@redhat.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> --- tests/system-traffic.at | 3 +++ 1 file changed, 3 insertions(+)