[ovs-dev] [PATCH v4 5/7] keepalive: Retrieve PMD status periodically.

[Fischetti, Antonio]

Comments inline.

> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org]
> On Behalf Of Bhanuprakash Bodireddy
> Sent: Tuesday, August 22, 2017 10:41 AM
> To: dev at openvswitch.org
> Cc: i.maximets at samsung.com
> Subject: [ovs-dev] [PATCH v4 5/7] keepalive: Retrieve PMD status periodically.
> 
> This commit implements APIs to retrieve the PMD thread status and return
> the status in the below format for each PMD thread.
> 
>   Format: pmdid="status,core id,last_seen_timestamp(epoch)"
>   eg:     pmd62="ALIVE,2,1503333332575"
>           pmd63="GONE,3,1503333332525"
> 
> The status is periodically retrieved by keepalive thread and stored in
> keepalive_stats struc which later shall be retrieved by vswitchd thread.
> In case of four PMD threads the status is as below:
> 
>    "pmd62"="ALIVE,0,1503333332575"
>    "pmd63"="ALIVE,1,1503333332575"
>    "pmd64"="ALIVE,2,1503333332575"
>    "pmd65"="ALIVE,3,1503333332575"
> 
> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
> ---
>  lib/dpif-netdev.c |  1 +
>  lib/keepalive.c   | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/keepalive.h   |  1 +
>  3 files changed, 71 insertions(+)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 67ee424..8475a24 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -990,6 +990,7 @@ ovs_keepalive(void *f_)
>          int n_pmds = cmap_count(&dp->poll_threads) - 1;
>          if (n_pmds > 0) {
>              dispatch_heartbeats();
> +            get_ka_stats();
>          }
> 
>          xusleep(get_ka_interval() * 1000);
> diff --git a/lib/keepalive.c b/lib/keepalive.c
> index 4ee89c0..9fd71b2 100644
> --- a/lib/keepalive.c
> +++ b/lib/keepalive.c
> @@ -23,6 +23,7 @@
>  #include "keepalive.h"
>  #include "lib/vswitch-idl.h"
>  #include "openvswitch/vlog.h"
> +#include "ovs-thread.h"
>  #include "process.h"
>  #include "timeval.h"
> 
> @@ -33,6 +34,9 @@ static bool ka_init_status = ka_init_failure; /* Keepalive
> initialization */
>  static uint32_t keepalive_timer_interval;     /* keepalive timer interval */
>  static struct keepalive_info *ka_info = NULL;
> 
> +static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> +static struct smap *keepalive_stats OVS_GUARDED_BY(mutex);
> +
>  inline bool
>  ka_is_enabled(void)
>  {
> @@ -211,6 +215,71 @@ ka_get_timer_interval(const struct smap *ovs_other_config
> OVS_UNUSED)
>      return ka_interval;
>  }
> 
> +static void
> +get_pmd_status(struct smap *ka_pmd_stats)
> +    OVS_REQUIRES(ka_info->proclist_mutex)
> +{
> +    if (OVS_UNLIKELY(!ka_info)) {
> +        return;
> +    }
> +
> +    struct ka_process_info *pinfo, *pinfo_next;
> +    HMAP_FOR_EACH_SAFE (pinfo, pinfo_next, node, &ka_info->process_list) {
> +        int core_id = pinfo->core_id;
> +        char *state = NULL;
> +        if (pinfo->core_state == KA_STATE_UNUSED) {
> +            continue;
> +        }
> +
> +        switch (pinfo->core_state) {
> +        case KA_STATE_ALIVE:
> +            state = "ALIVE";
> +            break;
> +        case KA_STATE_MISSING:
> +            state = "MISSING";
> +            break;
> +        case KA_STATE_DEAD:
> +            state = "DEAD";
> +            break;
> +        case KA_STATE_GONE:
> +            state = "GONE";
> +            break;
> +        case KA_STATE_DOZING:
> +            state = "DOZING";
> +            break;
> +        case KA_STATE_SLEEP:
> +            state = "SLEEP";
> +            break;
> +        case KA_STATE_UNUSED:
> +            break;
> +        }

[Antonio]
Quite similarly to comment in patch #2, I'd add to the switch
statement at the end something like?
+    default:
+        VLOG_DBG("Unexpected %d value for core_state.", pinfo->core_state);


> +
> +        smap_add_format(ka_pmd_stats, pinfo->name, "%s,%d,%ld",
> +                        state, core_id, pinfo->core_last_seen_times);
> +    }
> +}
> +
> +void
> +get_ka_stats(void)
> +{
> +    struct smap *ka_pmd_stats;
> +    ka_pmd_stats = xmalloc(sizeof *ka_pmd_stats);
> +    smap_init(ka_pmd_stats);
> +
> +    ovs_mutex_lock(&ka_info->proclist_mutex);
> +    get_pmd_status(ka_pmd_stats);
> +    ovs_mutex_unlock(&ka_info->proclist_mutex);
> +
> +    ovs_mutex_lock(&mutex);
> +    if (keepalive_stats) {
> +        smap_destroy(keepalive_stats);
> +        free(keepalive_stats);
> +        keepalive_stats = NULL;
> +    }
> +    keepalive_stats = ka_pmd_stats;
> +    ovs_mutex_unlock(&mutex);
> +}
> +
>  /* Dispatch heartbeats. */
>  void
>  dispatch_heartbeats(void)
> diff --git a/lib/keepalive.h b/lib/keepalive.h
> index a344006..f5da460 100644
> --- a/lib/keepalive.h
> +++ b/lib/keepalive.h
> @@ -100,6 +100,7 @@ uint32_t get_ka_interval(void);
>  int get_ka_init_status(void);
>  int ka_alloc_portstats(unsigned, int);
>  void ka_destroy_portstats(void);
> +void get_ka_stats(void);
> 
>  void dispatch_heartbeats(void);
>  #endif /* keepalive.h */
> --
> 2.4.11
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev