Re: [ovs-dev] [PATCH v6 3/8] dpif-netdev: Enable heartbeats for DPDK datapath.
On 12/08/2017 12:04 PM, Bhanuprakash Bodireddy wrote: > This commit adds heartbeat mechanism support for DPDK datapath. Heartbeats > are sent to registered PMD threads at predefined intervals (as set in ovsdb > with 'keepalive-interval'). > > The heartbeats are only enabled when there is atleast one port added to > the bridge and with active PMD thread polling the port. > > Signed-off-by: Bhanuprakash Bodireddy> --- > lib/dpif-netdev.c | 14 +- > lib/keepalive.c | 42 ++ > lib/keepalive.h | 1 + > 3 files changed, 56 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index c978a76..9021906 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -1021,14 +1021,26 @@ sorted_poll_thread_list(struct dp_netdev *dp, > } > > static void * > -ovs_keepalive(void *f_ OVS_UNUSED) > +ovs_keepalive(void *f_) > { > +struct dp_netdev *dp = f_; > + > pthread_detach(pthread_self()); > > for (;;) { > +bool hb_enable; > +int n_pmds; > uint64_t interval; I don't think you need any of these variables > > interval = get_ka_interval(); > +n_pmds = cmap_count(>poll_threads) - 1; > +hb_enable = (n_pmds > 0) ? true : false; > + > +/* Dispatch heartbeats only if pmd[s] exist. */ > +if (hb_enable) { Why is this needed? it's not atomic, so that suggests it wouldn't be a problem to just call the dispatch_heartbeats() anyway. > +dispatch_heartbeats(); > +} > + > xnanosleep(interval * 1000 * 1000); > } > > diff --git a/lib/keepalive.c b/lib/keepalive.c > index b04877f..0e4b2b6 100644 > --- a/lib/keepalive.c > +++ b/lib/keepalive.c > @@ -284,6 +284,48 @@ ka_mark_pmd_thread_sleep(int tid) > } > } > > +/* Dispatch heartbeats from 'ovs_keepalive' thread. */ > +void > +dispatch_heartbeats(void) > +{ > +struct ka_process_info *pinfo, *pinfo_next; > + > +/* Iterates over the list of processes in 'cached_process_list' map. */ > +HMAP_FOR_EACH_SAFE (pinfo, pinfo_next, node, > +_info.cached_process_list) { > +if (pinfo->state == KA_STATE_UNUSED) { > +continue; > +} > + Is the pinfo state atomic - you could be writing it as alive/sleep at the same time as calling this, no? > +switch (pinfo->state) { > +case KA_STATE_UNUSED: > +break; > +case KA_STATE_ALIVE: > +pinfo->state = KA_STATE_MISSING; > +pinfo->last_seen_time = time_wall_msec(); > +break; > +case KA_STATE_MISSING: > +pinfo->state = KA_STATE_DEAD; > +break; > +case KA_STATE_DEAD: > +pinfo->state = KA_STATE_GONE; > +break; > +case KA_STATE_GONE: > +break; > +case KA_STATE_SLEEP: > +pinfo->state = KA_STATE_SLEEP; > +pinfo->last_seen_time = time_wall_msec(); I don't think last_seen_time should be updated here > +break; > +default: > +OVS_NOT_REACHED(); > +} > + > +/* Invoke 'ka_update_thread_state' cb function to update state info > + * in to 'ka_info.process_list' map. */ > +ka_info.relay_cb(pinfo->tid, pinfo->state, pinfo->last_seen_time); > +} > +} > + > void > ka_init(const struct smap *ovs_other_config) > { > diff --git a/lib/keepalive.h b/lib/keepalive.h > index 7674ea3..cbc2387 100644 > --- a/lib/keepalive.h > +++ b/lib/keepalive.h > @@ -100,6 +100,7 @@ void ka_free_cached_threads(void); > void ka_cache_registered_threads(void); > void ka_mark_pmd_thread_alive(int); > void ka_mark_pmd_thread_sleep(int); > +void dispatch_heartbeats(void); > void ka_init(const struct smap *); > void ka_destroy(void); > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 3/8] dpif-netdev: Enable heartbeats for DPDK datapath.
LGTM, Tested-by: Antonio Fischetti <antonio.fische...@intel.com> Acked-by: Antonio Fischetti <antonio.fische...@intel.com> > -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Bhanuprakash Bodireddy > Sent: Friday, December 8, 2017 12:04 PM > To: d...@openvswitch.org > Subject: [ovs-dev] [PATCH v6 3/8] dpif-netdev: Enable heartbeats for > DPDK datapath. > > This commit adds heartbeat mechanism support for DPDK datapath. > Heartbeats > are sent to registered PMD threads at predefined intervals (as set in > ovsdb > with 'keepalive-interval'). > > The heartbeats are only enabled when there is atleast one port added to > the bridge and with active PMD thread polling the port. > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodire...@intel.com> > --- > lib/dpif-netdev.c | 14 +- > lib/keepalive.c | 42 ++ > lib/keepalive.h | 1 + > 3 files changed, 56 insertions(+), 1 deletion(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index c978a76..9021906 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -1021,14 +1021,26 @@ sorted_poll_thread_list(struct dp_netdev *dp, > } > > static void * > -ovs_keepalive(void *f_ OVS_UNUSED) > +ovs_keepalive(void *f_) > { > +struct dp_netdev *dp = f_; > + > pthread_detach(pthread_self()); > > for (;;) { > +bool hb_enable; > +int n_pmds; > uint64_t interval; > > interval = get_ka_interval(); > +n_pmds = cmap_count(>poll_threads) - 1; > +hb_enable = (n_pmds > 0) ? true : false; > + > +/* Dispatch heartbeats only if pmd[s] exist. */ > +if (hb_enable) { > +dispatch_heartbeats(); > +} > + > xnanosleep(interval * 1000 * 1000); > } > > diff --git a/lib/keepalive.c b/lib/keepalive.c > index b04877f..0e4b2b6 100644 > --- a/lib/keepalive.c > +++ b/lib/keepalive.c > @@ -284,6 +284,48 @@ ka_mark_pmd_thread_sleep(int tid) > } > } > > +/* Dispatch heartbeats from 'ovs_keepalive' thread. */ > +void > +dispatch_heartbeats(void) > +{ > +struct ka_process_info *pinfo, *pinfo_next; > + > +/* Iterates over the list of processes in 'cached_process_list' > map. */ > +HMAP_FOR_EACH_SAFE (pinfo, pinfo_next, node, > +_info.cached_process_list) { > +if (pinfo->state == KA_STATE_UNUSED) { > +continue; > +} > + > +switch (pinfo->state) { > +case KA_STATE_UNUSED: > +break; > +case KA_STATE_ALIVE: > +pinfo->state = KA_STATE_MISSING; > +pinfo->last_seen_time = time_wall_msec(); > +break; > +case KA_STATE_MISSING: > +pinfo->state = KA_STATE_DEAD; > +break; > +case KA_STATE_DEAD: > +pinfo->state = KA_STATE_GONE; > +break; > +case KA_STATE_GONE: > +break; > +case KA_STATE_SLEEP: > +pinfo->state = KA_STATE_SLEEP; > +pinfo->last_seen_time = time_wall_msec(); > +break; > +default: > +OVS_NOT_REACHED(); > +} > + > +/* Invoke 'ka_update_thread_state' cb function to update state > info > + * in to 'ka_info.process_list' map. */ > +ka_info.relay_cb(pinfo->tid, pinfo->state, pinfo- > >last_seen_time); > +} > +} > + > void > ka_init(const struct smap *ovs_other_config) > { > diff --git a/lib/keepalive.h b/lib/keepalive.h > index 7674ea3..cbc2387 100644 > --- a/lib/keepalive.h > +++ b/lib/keepalive.h > @@ -100,6 +100,7 @@ void ka_free_cached_threads(void); > void ka_cache_registered_threads(void); > void ka_mark_pmd_thread_alive(int); > void ka_mark_pmd_thread_sleep(int); > +void dispatch_heartbeats(void); > void ka_init(const struct smap *); > void ka_destroy(void); > > -- > 2.4.11 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev