| Message ID | 20e30dcf3a69fe123b78da6095e0455a2738995f.1531996402.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] Introduce ovs-appctl command to monitor HVs sb connection status | expand |
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Too many signoffs; are you missing Co-authored-by lines? ERROR: Co-authored-by/Signed-off-by corruption Lines checked: 144, Warnings: 0, Errors: 2 Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
0-day Robot <robot@bytheb.org> writes: > Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have > tried out your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > checkpatch: > ERROR: Too many signoffs; are you missing Co-authored-by lines? > ERROR: Co-authored-by/Signed-off-by corruption > Lines checked: 144, Warnings: 0, Errors: 2 Probably we can improve this log to be a bit more informative. In the meantime, from the submitting-patches.rst: All co-authors must also sign off. So, you'll need to get the sign-off tag. I think patchwork can automatically record it from a reply, so it would probably be sufficient to just have aginwala (CCd) to reply to the patch with the appropriate tag. Thanks, Lorenzo and aginwala! -Aaron > Please check this out. If you feel there has been an error, please > email aconole@bytheb.org > > Thanks, > 0-day Robot > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Signed-off-by: aginwala <aginwala@ebay.com> On Thu, Jul 19, 2018 at 4:01 AM Lorenzo Bianconi < lorenzo.bianconi@redhat.com> wrote: > Add 'connection-status' command to ovs-appctl utility in order to check > if a given chassis is currently connected to SB db > > Co-authored-by: aginwala <aginwala@ebay.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > lib/jsonrpc.c | 6 ++++++ > lib/jsonrpc.h | 1 + > lib/ovsdb-idl.c | 6 ++++++ > lib/ovsdb-idl.h | 1 + > ovn/controller/ovn-controller.8.xml | 5 +++++ > ovn/controller/ovn-controller.c | 18 ++++++++++++++++++ > 6 files changed, 37 insertions(+) > > diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c > index 4c2c1ba84..21de06003 100644 > --- a/lib/jsonrpc.c > +++ b/lib/jsonrpc.c > @@ -1162,6 +1162,12 @@ jsonrpc_session_is_connected(const struct > jsonrpc_session *s) > return s->rpc != NULL; > } > > +bool > +jsonrpc_session_is_rconnected(const struct jsonrpc_session *s) > +{ > + return reconnect_is_connected(s->reconnect); > +} > + > /* Returns a sequence number for 's'. The sequence number increments > every > * time 's' connects or disconnects. Thus, a caller can use the change > (or > * lack of change) in the sequence number to figure out whether the > underlying > diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h > index a44114e8d..cf8351aaf 100644 > --- a/lib/jsonrpc.h > +++ b/lib/jsonrpc.h > @@ -125,6 +125,7 @@ void jsonrpc_session_recv_wait(struct jsonrpc_session > *); > > bool jsonrpc_session_is_alive(const struct jsonrpc_session *); > bool jsonrpc_session_is_connected(const struct jsonrpc_session *); > +bool jsonrpc_session_is_rconnected(const struct jsonrpc_session *); > unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *); > int jsonrpc_session_get_status(const struct jsonrpc_session *); > int jsonrpc_session_get_last_error(const struct jsonrpc_session *); > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 9ab5d6723..2c057a0ba 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl) > idl->state != IDL_S_ERROR; > } > > +bool > +ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl) > +{ > + return jsonrpc_session_is_rconnected(idl->session); > +} > + > /* Returns the last error reported on a connection by 'idl'. The return > value > * is 0 only if no connection made by 'idl' has ever encountered an error > and > * a negative response to a schema request has never been received. See > diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h > index ea18b22f5..e657829c7 100644 > --- a/lib/ovsdb-idl.h > +++ b/lib/ovsdb-idl.h > @@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *); > void ovsdb_idl_verify_write_only(struct ovsdb_idl *); > > bool ovsdb_idl_is_alive(const struct ovsdb_idl *); > +bool ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl); > int ovsdb_idl_get_last_error(const struct ovsdb_idl *); > > void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int > probe_interval); > diff --git a/ovn/controller/ovn-controller.8.xml > b/ovn/controller/ovn-controller.8.xml > index 0eff2113f..0861edbc4 100644 > --- a/ovn/controller/ovn-controller.8.xml > +++ b/ovn/controller/ovn-controller.8.xml > @@ -388,6 +388,11 @@ > 0x800</code>. > </p> > </dd> > + > + <dt><code>connection-status</code></dt> > + <dd> > + Show OVN SBDB connection status for the chassis. > + </dd> > </dl> > </p> > > diff --git a/ovn/controller/ovn-controller.c > b/ovn/controller/ovn-controller.c > index 6ee72a9fa..a05098e9b 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -60,12 +60,14 @@ > #include "timeval.h" > #include "timer.h" > #include "stopwatch.h" > +#include "lib/reconnect.h" > > VLOG_DEFINE_THIS_MODULE(main); > > static unixctl_cb_func ovn_controller_exit; > static unixctl_cb_func ct_zone_list; > static unixctl_cb_func inject_pkt; > +static unixctl_cb_func ovn_controller_rconn_show; > > #define DEFAULT_BRIDGE_NAME "br-int" > #define DEFAULT_PROBE_INTERVAL_MSEC 5000 > @@ -588,6 +590,9 @@ main(int argc, char *argv[]) > ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true)); > ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false); > > + unixctl_command_register("connection-status", "", 0, 0, > + ovn_controller_rconn_show, > ovnsb_idl_loop.idl); > + > struct ovsdb_idl_index *sbrec_chassis_by_name > = chassis_index_create(ovnsb_idl_loop.idl); > struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath > @@ -1061,3 +1066,16 @@ update_probe_interval(const struct > ovsrec_open_vswitch_table *ovs_table, > > ovsdb_idl_set_probe_interval(ovnsb_idl, interval); > } > + > +static void > +ovn_controller_rconn_show(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *idl_) > +{ > + char *result = "not connected"; > + struct ovsdb_idl *idl = idl_; > + > + if (idl && ovsdb_idl_is_rconnected(idl)) { > + result = "connected"; > + } > + unixctl_command_reply(conn, result); > +} > -- > 2.17.1 > >
Hi Lorenzo, I have some comments inline below. Also, I think there should be a test for this. You could start OVN, run `ovn-appctl -t ovn-controller connection-status` and make sure it returns "connected". Then you could run `ovn-sbctl set-connection <some random TCP port>`, and then ensure that when you check the connection status it reports "not connected". If you really wanted to get fancy, you could set the connection back again and ensure that the connection status returns back to "connected". On 07/19/2018 07:01 AM, Lorenzo Bianconi wrote: > Add 'connection-status' command to ovs-appctl utility in order to check > if a given chassis is currently connected to SB db > > Co-authored-by: aginwala <aginwala@ebay.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > lib/jsonrpc.c | 6 ++++++ > lib/jsonrpc.h | 1 + > lib/ovsdb-idl.c | 6 ++++++ > lib/ovsdb-idl.h | 1 + > ovn/controller/ovn-controller.8.xml | 5 +++++ > ovn/controller/ovn-controller.c | 18 ++++++++++++++++++ > 6 files changed, 37 insertions(+) > > diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c > index 4c2c1ba84..21de06003 100644 > --- a/lib/jsonrpc.c > +++ b/lib/jsonrpc.c > @@ -1162,6 +1162,12 @@ jsonrpc_session_is_connected(const struct jsonrpc_session *s) > return s->rpc != NULL; > } > > +bool > +jsonrpc_session_is_rconnected(const struct jsonrpc_session *s) > +{ > + return reconnect_is_connected(s->reconnect); > +} > + Is this function necessary? There is already a jsonrpc_session_is_connected() function that returns true if s->rpc is non-NULL. jsonrpc_session_run() will set s->rpc to NULL if reconnect_run() says that the session is disconnected. So therefore, I think you could just use jsonrpc_session_is_connected(). > /* Returns a sequence number for 's'. The sequence number increments every > * time 's' connects or disconnects. Thus, a caller can use the change (or > * lack of change) in the sequence number to figure out whether the underlying > diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h > index a44114e8d..cf8351aaf 100644 > --- a/lib/jsonrpc.h > +++ b/lib/jsonrpc.h > @@ -125,6 +125,7 @@ void jsonrpc_session_recv_wait(struct jsonrpc_session *); > > bool jsonrpc_session_is_alive(const struct jsonrpc_session *); > bool jsonrpc_session_is_connected(const struct jsonrpc_session *); > +bool jsonrpc_session_is_rconnected(const struct jsonrpc_session *); > unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *); > int jsonrpc_session_get_status(const struct jsonrpc_session *); > int jsonrpc_session_get_last_error(const struct jsonrpc_session *); > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 9ab5d6723..2c057a0ba 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl) > idl->state != IDL_S_ERROR; > } > > +bool > +ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl) > +{ > + return jsonrpc_session_is_rconnected(idl->session); > +} > + I suggested calling this ovsdb_idl_is_connected(). I also suggest adding a comment that explains the difference between this and ovsdb_idl_is_alive(). > /* Returns the last error reported on a connection by 'idl'. The return value > * is 0 only if no connection made by 'idl' has ever encountered an error and > * a negative response to a schema request has never been received. See > diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h > index ea18b22f5..e657829c7 100644 > --- a/lib/ovsdb-idl.h > +++ b/lib/ovsdb-idl.h > @@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *); > void ovsdb_idl_verify_write_only(struct ovsdb_idl *); > > bool ovsdb_idl_is_alive(const struct ovsdb_idl *); > +bool ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl); > int ovsdb_idl_get_last_error(const struct ovsdb_idl *); > > void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int probe_interval); > diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml > index 0eff2113f..0861edbc4 100644 > --- a/ovn/controller/ovn-controller.8.xml > +++ b/ovn/controller/ovn-controller.8.xml > @@ -388,6 +388,11 @@ > 0x800</code>. > </p> > </dd> > + > + <dt><code>connection-status</code></dt> > + <dd> > + Show OVN SBDB connection status for the chassis. > + </dd> > </dl> > </p> > > diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c > index 6ee72a9fa..a05098e9b 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -60,12 +60,14 @@ > #include "timeval.h" > #include "timer.h" > #include "stopwatch.h" > +#include "lib/reconnect.h" > > VLOG_DEFINE_THIS_MODULE(main); > > static unixctl_cb_func ovn_controller_exit; > static unixctl_cb_func ct_zone_list; > static unixctl_cb_func inject_pkt; > +static unixctl_cb_func ovn_controller_rconn_show; > > #define DEFAULT_BRIDGE_NAME "br-int" > #define DEFAULT_PROBE_INTERVAL_MSEC 5000 > @@ -588,6 +590,9 @@ main(int argc, char *argv[]) > ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true)); > ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false); > > + unixctl_command_register("connection-status", "", 0, 0, > + ovn_controller_rconn_show, ovnsb_idl_loop.idl); > + > struct ovsdb_idl_index *sbrec_chassis_by_name > = chassis_index_create(ovnsb_idl_loop.idl); > struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath > @@ -1061,3 +1066,16 @@ update_probe_interval(const struct ovsrec_open_vswitch_table *ovs_table, > > ovsdb_idl_set_probe_interval(ovnsb_idl, interval); > } > + > +static void > +ovn_controller_rconn_show(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *idl_) > +{ > + char *result = "not connected"; > + struct ovsdb_idl *idl = idl_; > + > + if (idl && ovsdb_idl_is_rconnected(idl)) { > + result = "connected"; > + } > + unixctl_command_reply(conn, result); > +} >
diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c index 4c2c1ba84..21de06003 100644 --- a/lib/jsonrpc.c +++ b/lib/jsonrpc.c @@ -1162,6 +1162,12 @@ jsonrpc_session_is_connected(const struct jsonrpc_session *s) return s->rpc != NULL; } +bool +jsonrpc_session_is_rconnected(const struct jsonrpc_session *s) +{ + return reconnect_is_connected(s->reconnect); +} + /* Returns a sequence number for 's'. The sequence number increments every * time 's' connects or disconnects. Thus, a caller can use the change (or * lack of change) in the sequence number to figure out whether the underlying diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h index a44114e8d..cf8351aaf 100644 --- a/lib/jsonrpc.h +++ b/lib/jsonrpc.h @@ -125,6 +125,7 @@ void jsonrpc_session_recv_wait(struct jsonrpc_session *); bool jsonrpc_session_is_alive(const struct jsonrpc_session *); bool jsonrpc_session_is_connected(const struct jsonrpc_session *); +bool jsonrpc_session_is_rconnected(const struct jsonrpc_session *); unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *); int jsonrpc_session_get_status(const struct jsonrpc_session *); int jsonrpc_session_get_last_error(const struct jsonrpc_session *); diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 9ab5d6723..2c057a0ba 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl) idl->state != IDL_S_ERROR; } +bool +ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl) +{ + return jsonrpc_session_is_rconnected(idl->session); +} + /* Returns the last error reported on a connection by 'idl'. The return value * is 0 only if no connection made by 'idl' has ever encountered an error and * a negative response to a schema request has never been received. See diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index ea18b22f5..e657829c7 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *); void ovsdb_idl_verify_write_only(struct ovsdb_idl *); bool ovsdb_idl_is_alive(const struct ovsdb_idl *); +bool ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl); int ovsdb_idl_get_last_error(const struct ovsdb_idl *); void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int probe_interval); diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml index 0eff2113f..0861edbc4 100644 --- a/ovn/controller/ovn-controller.8.xml +++ b/ovn/controller/ovn-controller.8.xml @@ -388,6 +388,11 @@ 0x800</code>. </p> </dd> + + <dt><code>connection-status</code></dt> + <dd> + Show OVN SBDB connection status for the chassis. + </dd> </dl> </p> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 6ee72a9fa..a05098e9b 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -60,12 +60,14 @@ #include "timeval.h" #include "timer.h" #include "stopwatch.h" +#include "lib/reconnect.h" VLOG_DEFINE_THIS_MODULE(main); static unixctl_cb_func ovn_controller_exit; static unixctl_cb_func ct_zone_list; static unixctl_cb_func inject_pkt; +static unixctl_cb_func ovn_controller_rconn_show; #define DEFAULT_BRIDGE_NAME "br-int" #define DEFAULT_PROBE_INTERVAL_MSEC 5000 @@ -588,6 +590,9 @@ main(int argc, char *argv[]) ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true)); ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false); + unixctl_command_register("connection-status", "", 0, 0, + ovn_controller_rconn_show, ovnsb_idl_loop.idl); + struct ovsdb_idl_index *sbrec_chassis_by_name = chassis_index_create(ovnsb_idl_loop.idl); struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath @@ -1061,3 +1066,16 @@ update_probe_interval(const struct ovsrec_open_vswitch_table *ovs_table, ovsdb_idl_set_probe_interval(ovnsb_idl, interval); } + +static void +ovn_controller_rconn_show(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[] OVS_UNUSED, void *idl_) +{ + char *result = "not connected"; + struct ovsdb_idl *idl = idl_; + + if (idl && ovsdb_idl_is_rconnected(idl)) { + result = "connected"; + } + unixctl_command_reply(conn, result); +}
Add 'connection-status' command to ovs-appctl utility in order to check if a given chassis is currently connected to SB db Co-authored-by: aginwala <aginwala@ebay.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- lib/jsonrpc.c | 6 ++++++ lib/jsonrpc.h | 1 + lib/ovsdb-idl.c | 6 ++++++ lib/ovsdb-idl.h | 1 + ovn/controller/ovn-controller.8.xml | 5 +++++ ovn/controller/ovn-controller.c | 18 ++++++++++++++++++ 6 files changed, 37 insertions(+)