| Message ID | 1512734667-23202-4-git-send-email-bhanuprakash.bodireddy@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add OVS DPDK keep-alive functionality. | expand |
LGTM, Tested-by: Antonio Fischetti <antonio.fischetti@intel.com> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> > -----Original Message----- > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev- > bounces@openvswitch.org] On Behalf Of Bhanuprakash Bodireddy > Sent: Friday, December 8, 2017 12:04 PM > To: dev@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.bodireddy@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(&dp->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, > + &ka_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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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 <bhanuprakash.bodireddy@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; I don't think you need any of these variables > > interval = get_ka_interval(); > + n_pmds = cmap_count(&dp->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, > + &ka_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); > >
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(&dp->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, + &ka_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);
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.bodireddy@intel.com> --- lib/dpif-netdev.c | 14 +++++++++++++- lib/keepalive.c | 42 ++++++++++++++++++++++++++++++++++++++++++ lib/keepalive.h | 1 + 3 files changed, 56 insertions(+), 1 deletion(-)