On Tue, Nov 22, 2022 at 9:24 PM Xavier Simonart <[email protected]> wrote:
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2144084 > > Signed-off-by: Xavier Simonart <[email protected]> > --- > lib/features.c | 16 ++++++++++------ > tests/ovn.at | 18 ++++++++++++++++++ > 2 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/lib/features.c b/lib/features.c > index f15ec42bb..571e24ded 100644 > --- a/lib/features.c > +++ b/lib/features.c > @@ -26,6 +26,7 @@ > #include "openvswitch/rconn.h" > #include "openvswitch/ofp-msgs.h" > #include "openvswitch/ofp-meter.h" > +#include "openvswitch/ofp-util.h" > #include "ovn/features.h" > > VLOG_DEFINE_THIS_MODULE(features); > @@ -90,6 +91,8 @@ ovs_feature_rconn_setup(const char *br_name) > static bool > ovs_feature_get_openflow_cap(const char *br_name) > { > + struct ofpbuf *msg; > + > if (!br_name) { > return false; > } > @@ -102,15 +105,14 @@ ovs_feature_get_openflow_cap(const char *br_name) > } > > /* send new requests just after reconnect. */ > - if (conn_seq_no == rconn_get_connection_seqno(swconn)) { > - return false; > + if (conn_seq_no != rconn_get_connection_seqno(swconn)) { > + /* dump datapath meter capabilities. */ > + msg = ofpraw_alloc(OFPRAW_OFPST13_METER_FEATURES_REQUEST, > + rconn_get_version(swconn), 0); > + rconn_send(swconn, msg, NULL); > } > > bool ret = false; > - /* dump datapath meter capabilities. */ > - struct ofpbuf *msg = > ofpraw_alloc(OFPRAW_OFPST13_METER_FEATURES_REQUEST, > - rconn_get_version(swconn), 0); > - rconn_send(swconn, msg, NULL); > for (int i = 0; i < 50; i++) { > msg = rconn_recv(swconn); > if (!msg) { > @@ -137,6 +139,8 @@ ovs_feature_get_openflow_cap(const char *br_name) > } > } > conn_seq_no = rconn_get_connection_seqno(swconn); > + } else if (type == OFPTYPE_ECHO_REQUEST) { > + rconn_send(swconn, ofputil_encode_echo_reply(oh), NULL); > } > ofpbuf_delete(msg); > } > diff --git a/tests/ovn.at b/tests/ovn.at > index cf1ea991d..3a1c231f5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -33069,3 +33069,21 @@ check ovn-nbctl --wait=hv sync > OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([feature inactivity probe]) > +ovn_start > +net_add n1 > + > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +# Wait for more than 2x 60 seconds > +sleep 125 > + > +AT_CHECK([test -z "`grep disconnecting hv1/ovs-vswitchd.log`"]) > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > -- > 2.31.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Hi Xavier, the change itself looks good, however I'm very concerned about the test. I wonder if we can roll with confirmation that this just works, without actually having test that sleeps for more than 2 minutes. Dumitru, Mark, Han any thoughts? -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
