diff options
| -rw-r--r-- | bgpd/bgp_io.c | 14 | ||||
| -rw-r--r-- | bgpd/bgp_keepalives.c | 4 | ||||
| -rw-r--r-- | bgpd/bgpd.c | 23 | ||||
| -rw-r--r-- | bgpd/bgpd.h | 8 | ||||
| -rw-r--r-- | lib/frr_pthread.c | 144 | ||||
| -rw-r--r-- | lib/frr_pthread.h | 24 | ||||
| -rw-r--r-- | tests/bgpd/test_aspath.c | 6 | ||||
| -rw-r--r-- | tests/bgpd/test_capability.c | 2 | ||||
| -rwxr-xr-x | tools/checkpatch.pl | 1 | ||||
| -rw-r--r-- | zebra/zserv.c | 1 |
10 files changed, 69 insertions, 158 deletions
diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c index c3bfbe4a90..95c3f15a66 100644 --- a/bgpd/bgp_io.c +++ b/bgpd/bgp_io.c @@ -23,7 +23,7 @@ #include <zebra.h> #include <pthread.h> // for pthread_mutex_unlock, pthread_mutex_lock -#include "frr_pthread.h" // for frr_pthread_get, frr_pthread +#include "frr_pthread.h" #include "linklist.h" // for list_delete, list_delete_all_node, lis... #include "log.h" // for zlog_debug, safe_strerror, zlog_err #include "memory.h" // for MTYPE_TMP, XCALLOC, XFREE @@ -56,7 +56,7 @@ static bool validate_header(struct peer *); void bgp_writes_on(struct peer *peer) { - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_IO); + struct frr_pthread *fpt = bgp_pth_io; assert(fpt->running); assert(peer->status != Deleted); @@ -74,7 +74,7 @@ void bgp_writes_on(struct peer *peer) void bgp_writes_off(struct peer *peer) { - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_IO); + struct frr_pthread *fpt = bgp_pth_io; assert(fpt->running); thread_cancel_async(fpt->master, &peer->t_write, NULL); @@ -85,7 +85,7 @@ void bgp_writes_off(struct peer *peer) void bgp_reads_on(struct peer *peer) { - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_IO); + struct frr_pthread *fpt = bgp_pth_io; assert(fpt->running); assert(peer->status != Deleted); @@ -105,7 +105,7 @@ void bgp_reads_on(struct peer *peer) void bgp_reads_off(struct peer *peer) { - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_IO); + struct frr_pthread *fpt = bgp_pth_io; assert(fpt->running); thread_cancel_async(fpt->master, &peer->t_read, NULL); @@ -130,7 +130,7 @@ static int bgp_process_writes(struct thread *thread) if (peer->fd < 0) return -1; - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_IO); + struct frr_pthread *fpt = bgp_pth_io; pthread_mutex_lock(&peer->io_mtx); { @@ -182,7 +182,7 @@ static int bgp_process_reads(struct thread *thread) if (peer->fd < 0 || bm->terminating) return -1; - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_IO); + struct frr_pthread *fpt = bgp_pth_io; pthread_mutex_lock(&peer->io_mtx); { diff --git a/bgpd/bgp_keepalives.c b/bgpd/bgp_keepalives.c index aeb95f91ba..91fa8fa373 100644 --- a/bgpd/bgp_keepalives.c +++ b/bgpd/bgp_keepalives.c @@ -229,7 +229,7 @@ void bgp_keepalives_on(struct peer *peer) if (CHECK_FLAG(peer->thread_flags, PEER_THREAD_KEEPALIVES_ON)) return; - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_KEEPALIVES); + struct frr_pthread *fpt = bgp_pth_ka; assert(fpt->running); /* placeholder bucket data to use for fast key lookups */ @@ -259,7 +259,7 @@ void bgp_keepalives_off(struct peer *peer) if (!CHECK_FLAG(peer->thread_flags, PEER_THREAD_KEEPALIVES_ON)) return; - struct frr_pthread *fpt = frr_pthread_get(PTHREAD_KEEPALIVES); + struct frr_pthread *fpt = bgp_pth_ka; assert(fpt->running); /* placeholder bucket data to use for fast key lookups */ diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index fd3a6c9443..e4dedc2420 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -7750,35 +7750,36 @@ static const struct cmd_variable_handler bgp_viewvrf_var_handlers[] = { {.completions = NULL}, }; +struct frr_pthread *bgp_pth_io; +struct frr_pthread *bgp_pth_ka; + static void bgp_pthreads_init() { + assert(!bgp_pth_io); + assert(!bgp_pth_ka); + frr_pthread_init(); struct frr_pthread_attr io = { - .id = PTHREAD_IO, .start = frr_pthread_attr_default.start, .stop = frr_pthread_attr_default.stop, }; struct frr_pthread_attr ka = { - .id = PTHREAD_KEEPALIVES, .start = bgp_keepalives_start, .stop = bgp_keepalives_stop, }; - frr_pthread_new(&io, "BGP I/O thread", "bgpd_io"); - frr_pthread_new(&ka, "BGP Keepalives thread", "bgpd_ka"); + bgp_pth_io = frr_pthread_new(&io, "BGP I/O thread", "bgpd_io"); + bgp_pth_ka = frr_pthread_new(&ka, "BGP Keepalives thread", "bgpd_ka"); } void bgp_pthreads_run() { - struct frr_pthread *io = frr_pthread_get(PTHREAD_IO); - struct frr_pthread *ka = frr_pthread_get(PTHREAD_KEEPALIVES); - - frr_pthread_run(io, NULL); - frr_pthread_run(ka, NULL); + frr_pthread_run(bgp_pth_io, NULL); + frr_pthread_run(bgp_pth_ka, NULL); /* Wait until threads are ready. */ - frr_pthread_wait_running(io); - frr_pthread_wait_running(ka); + frr_pthread_wait_running(bgp_pth_io); + frr_pthread_wait_running(bgp_pth_ka); } void bgp_pthreads_finish() diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index ebcb9b5ea2..861435c036 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -24,6 +24,7 @@ #include "qobj.h" #include <pthread.h> +#include "frr_pthread.h" #include "lib/json.h" #include "vrf.h" #include "vty.h" @@ -97,6 +98,9 @@ enum bgp_af_index { for (afi = AFI_IP; afi < AFI_MAX; afi++) \ for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) +extern struct frr_pthread *bgp_pth_io; +extern struct frr_pthread *bgp_pth_ka; + /* BGP master for system wide configurations and variables. */ struct bgp_master { /* BGP instance list. */ @@ -105,10 +109,6 @@ struct bgp_master { /* BGP thread master. */ struct thread_master *master; -/* BGP pthreads. */ -#define PTHREAD_IO (1 << 1) -#define PTHREAD_KEEPALIVES (1 << 2) - /* work queues */ struct work_queue *process_main_queue; diff --git a/lib/frr_pthread.c b/lib/frr_pthread.c index d48b23f38a..c1ce57e24e 100644 --- a/lib/frr_pthread.c +++ b/lib/frr_pthread.c @@ -26,108 +26,77 @@ #include "frr_pthread.h" #include "memory.h" -#include "hash.h" +#include "linklist.h" DEFINE_MTYPE(LIB, FRR_PTHREAD, "FRR POSIX Thread"); DEFINE_MTYPE(LIB, PTHREAD_PRIM, "POSIX synchronization primitives"); -/* id for next created pthread */ -static _Atomic uint32_t next_id = 0; - /* default frr_pthread start/stop routine prototypes */ static void *fpt_run(void *arg); static int fpt_halt(struct frr_pthread *fpt, void **res); /* default frr_pthread attributes */ struct frr_pthread_attr frr_pthread_attr_default = { - .id = 0, .start = fpt_run, .stop = fpt_halt, }; -/* hash table to keep track of all frr_pthreads */ -static struct hash *frr_pthread_hash; -static pthread_mutex_t frr_pthread_hash_mtx = PTHREAD_MUTEX_INITIALIZER; - -/* frr_pthread_hash->hash_cmp */ -static int frr_pthread_hash_cmp(const void *value1, const void *value2) -{ - const struct frr_pthread *tq1 = value1; - const struct frr_pthread *tq2 = value2; - - return (tq1->attr.id == tq2->attr.id); -} - -/* frr_pthread_hash->hash_key */ -static unsigned int frr_pthread_hash_key(void *value) -{ - return ((struct frr_pthread *)value)->attr.id; -} +/* list to keep track of all frr_pthreads */ +static pthread_mutex_t frr_pthread_list_mtx = PTHREAD_MUTEX_INITIALIZER; +static struct list *frr_pthread_list; /* ------------------------------------------------------------------------ */ void frr_pthread_init() { - pthread_mutex_lock(&frr_pthread_hash_mtx); + pthread_mutex_lock(&frr_pthread_list_mtx); { - frr_pthread_hash = hash_create(frr_pthread_hash_key, - frr_pthread_hash_cmp, NULL); + frr_pthread_list = list_new(); + frr_pthread_list->del = (void (*)(void *))&frr_pthread_destroy; } - pthread_mutex_unlock(&frr_pthread_hash_mtx); + pthread_mutex_unlock(&frr_pthread_list_mtx); } void frr_pthread_finish() { - pthread_mutex_lock(&frr_pthread_hash_mtx); + pthread_mutex_lock(&frr_pthread_list_mtx); { - hash_clean(frr_pthread_hash, - (void (*)(void *))frr_pthread_destroy); - hash_free(frr_pthread_hash); + list_delete_and_null(&frr_pthread_list); } - pthread_mutex_unlock(&frr_pthread_hash_mtx); + pthread_mutex_unlock(&frr_pthread_list_mtx); } struct frr_pthread *frr_pthread_new(struct frr_pthread_attr *attr, const char *name, const char *os_name) { - static struct frr_pthread holder = {}; struct frr_pthread *fpt = NULL; attr = attr ? attr : &frr_pthread_attr_default; - pthread_mutex_lock(&frr_pthread_hash_mtx); + fpt = XCALLOC(MTYPE_FRR_PTHREAD, sizeof(struct frr_pthread)); + /* initialize mutex */ + pthread_mutex_init(&fpt->mtx, NULL); + /* create new thread master */ + fpt->master = thread_master_create(name); + /* set attributes */ + fpt->attr = *attr; + name = (name ? name : "Anonymous thread"); + fpt->name = XSTRDUP(MTYPE_FRR_PTHREAD, name); + if (os_name) + snprintf(fpt->os_name, OS_THREAD_NAMELEN, "%s", os_name); + /* initialize startup synchronization primitives */ + fpt->running_cond_mtx = XCALLOC( + MTYPE_PTHREAD_PRIM, sizeof(pthread_mutex_t)); + fpt->running_cond = XCALLOC(MTYPE_PTHREAD_PRIM, + sizeof(pthread_cond_t)); + pthread_mutex_init(fpt->running_cond_mtx, NULL); + pthread_cond_init(fpt->running_cond, NULL); + + pthread_mutex_lock(&frr_pthread_list_mtx); { - holder.attr.id = attr->id; - - if (!hash_lookup(frr_pthread_hash, &holder)) { - fpt = XCALLOC(MTYPE_FRR_PTHREAD, - sizeof(struct frr_pthread)); - /* initialize mutex */ - pthread_mutex_init(&fpt->mtx, NULL); - /* create new thread master */ - fpt->master = thread_master_create(name); - /* set attributes */ - fpt->attr = *attr; - name = (name ? name : "Anonymous thread"); - fpt->name = XSTRDUP(MTYPE_FRR_PTHREAD, name); - if (os_name) - snprintf(fpt->os_name, OS_THREAD_NAMELEN, - "%s", os_name); - if (attr == &frr_pthread_attr_default) - fpt->attr.id = frr_pthread_get_id(); - /* initialize startup synchronization primitives */ - fpt->running_cond_mtx = XCALLOC( - MTYPE_PTHREAD_PRIM, sizeof(pthread_mutex_t)); - fpt->running_cond = XCALLOC(MTYPE_PTHREAD_PRIM, - sizeof(pthread_cond_t)); - pthread_mutex_init(fpt->running_cond_mtx, NULL); - pthread_cond_init(fpt->running_cond, NULL); - - /* insert into global thread hash */ - hash_get(frr_pthread_hash, fpt, hash_alloc_intern); - } + listnode_add(frr_pthread_list, fpt); } - pthread_mutex_unlock(&frr_pthread_hash_mtx); + pthread_mutex_unlock(&frr_pthread_list_mtx); return fpt; } @@ -180,21 +149,6 @@ int frr_pthread_set_name(struct frr_pthread *fpt, const char *name, return ret; } -struct frr_pthread *frr_pthread_get(uint32_t id) -{ - static struct frr_pthread holder = {}; - struct frr_pthread *fpt; - - pthread_mutex_lock(&frr_pthread_hash_mtx); - { - holder.attr.id = id; - fpt = hash_lookup(frr_pthread_hash, &holder); - } - pthread_mutex_unlock(&frr_pthread_hash_mtx); - - return fpt; -} - int frr_pthread_run(struct frr_pthread *fpt, const pthread_attr_t *attr) { int ret; @@ -239,36 +193,16 @@ int frr_pthread_stop(struct frr_pthread *fpt, void **result) return ret; } -/* - * Callback for hash_iterate to stop all frr_pthread's. - */ -static void frr_pthread_stop_all_iter(struct hash_backet *hb, void *arg) -{ - struct frr_pthread *fpt = hb->data; - frr_pthread_stop(fpt, NULL); -} - void frr_pthread_stop_all() { - pthread_mutex_lock(&frr_pthread_hash_mtx); + pthread_mutex_lock(&frr_pthread_list_mtx); { - hash_iterate(frr_pthread_hash, frr_pthread_stop_all_iter, NULL); + struct listnode *n; + struct frr_pthread *fpt; + for (ALL_LIST_ELEMENTS_RO(frr_pthread_list, n, fpt)) + frr_pthread_stop(fpt, NULL); } - pthread_mutex_unlock(&frr_pthread_hash_mtx); -} - -uint32_t frr_pthread_get_id(void) -{ - _Atomic uint32_t nxid; - nxid = atomic_fetch_add_explicit(&next_id, 1, memory_order_seq_cst); - /* just a sanity check, this should never happen */ - assert(nxid <= (UINT32_MAX - 1)); - return nxid; -} - -void frr_pthread_yield(void) -{ - (void)sched_yield(); + pthread_mutex_unlock(&frr_pthread_list_mtx); } /* diff --git a/lib/frr_pthread.h b/lib/frr_pthread.h index 732e2925fe..b9e60511d5 100644 --- a/lib/frr_pthread.h +++ b/lib/frr_pthread.h @@ -34,7 +34,6 @@ struct frr_pthread; struct frr_pthread_attr; struct frr_pthread_attr { - _Atomic uint32_t id; void *(*start)(void *); int (*stop)(struct frr_pthread *, void **); }; @@ -155,13 +154,6 @@ int frr_pthread_set_name(struct frr_pthread *fpt, const char *name, void frr_pthread_destroy(struct frr_pthread *fpt); /* - * Gets an existing frr_pthread by its id. - * - * @return frr_thread associated with the provided id, or NULL on error - */ -struct frr_pthread *frr_pthread_get(uint32_t id); - -/* * Creates a new pthread and binds it to a frr_pthread. * * This function is a wrapper for pthread_create. The first parameter is the @@ -218,22 +210,6 @@ int frr_pthread_stop(struct frr_pthread *fpt, void **result); /* Stops all frr_pthread's. */ void frr_pthread_stop_all(void); -/* Yields the current thread of execution */ -void frr_pthread_yield(void); - -/* - * Returns a unique identifier for use with frr_pthread_new(). - * - * Internally, this is an integer that increments after each call to this - * function. Because the number of pthreads created should never exceed INT_MAX - * during the life of the program, there is no overflow protection. If by - * chance this function returns an ID which is already in use, - * frr_pthread_new() will fail when it is provided. - * - * @return unique identifier - */ -uint32_t frr_pthread_get_id(void); - #ifndef HAVE_PTHREAD_CONDATTR_SETCLOCK #define pthread_condattr_setclock(A, B) #endif diff --git a/tests/bgpd/test_aspath.c b/tests/bgpd/test_aspath.c index 247591580a..d2d960f278 100644 --- a/tests/bgpd/test_aspath.c +++ b/tests/bgpd/test_aspath.c @@ -1273,9 +1273,6 @@ static int handle_attr_test(struct aspath_tests *t) struct aspath *asp; size_t datalen; - bgp_pthreads_init(); - frr_pthread_get(PTHREAD_KEEPALIVES)->running = true; - asp = make_aspath(t->segment->asdata, t->segment->len, 0); peer.curr = stream_new(BGP_MAX_PACKET_SIZE); @@ -1382,6 +1379,9 @@ int main(void) i = 0; + bgp_pthreads_init(); + bgp_pth_ka->running = true; + while (aspath_tests[i].desc) { printf("aspath_attr test %d\n", i); attr_test(&aspath_tests[i++]); diff --git a/tests/bgpd/test_capability.c b/tests/bgpd/test_capability.c index 83af5e9c6d..968f9ac445 100644 --- a/tests/bgpd/test_capability.c +++ b/tests/bgpd/test_capability.c @@ -917,7 +917,7 @@ int main(void) bgp_option_set(BGP_OPT_NO_LISTEN); bgp_pthreads_init(); - frr_pthread_get(PTHREAD_KEEPALIVES)->running = true; + bgp_pth_ka->running = true; if (fileno(stdout) >= 0) tty = isatty(fileno(stdout)); diff --git a/tools/checkpatch.pl b/tools/checkpatch.pl index 55b3e1e564..3b861e4f87 100755 --- a/tools/checkpatch.pl +++ b/tools/checkpatch.pl @@ -359,6 +359,7 @@ our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeIni # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + _Atomic| __percpu| __nocast| __safe| diff --git a/zebra/zserv.c b/zebra/zserv.c index 2f4bb22ffe..8cc462577a 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -703,7 +703,6 @@ static struct zserv *zserv_client_create(int sock) listnode_add(zebrad.client_list, client); struct frr_pthread_attr zclient_pthr_attrs = { - .id = frr_pthread_get_id(), .start = frr_pthread_attr_default.start, .stop = frr_pthread_attr_default.stop }; |