[ovs-dev] [PATCH] Introduce ovs-appctl command to monitor HVs sb connection status

Mark Michelson mmichels at redhat.com
Thu Jul 26 18:42:45 UTC 2018


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 at ebay.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at 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);
> +}
> 



More information about the dev mailing list