From 665b8e5283df4f64eb44d8fb434489be1474b5d4 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Tue, 22 Mar 2016 13:35:40 +0100 Subject: Birdlib: Do cleanups after remove/free To avoid byzantine behavior in case of some errors, linked lists are cleared after rem_node() and resource headers are cleared after rfree(). --- lib/lists.c | 19 +------------------ lib/lists.h | 1 - lib/resource.c | 12 +++--------- 3 files changed, 4 insertions(+), 28 deletions(-) (limited to 'lib') diff --git a/lib/lists.c b/lib/lists.c index d323a4b6..20a9a072 100644 --- a/lib/lists.c +++ b/lib/lists.c @@ -88,7 +88,7 @@ insert_node(node *n, node *after) * rem_node - remove a node from a list * @n: node to be removed * - * Removes a node @n from the list it's linked in. + * Removes a node @n from the list it's linked in. Afterwards, node @n is cleared. */ LIST_INLINE void rem_node(node *n) @@ -96,23 +96,6 @@ rem_node(node *n) node *z = n->prev; node *x = n->next; - z->next = x; - x->prev = z; -} - -/** - * rem2_node - remove a node from a list, with cleanup - * @n: node to be removed - * - * Removes a node @n from the list it's linked in and resets its pointers to NULL. - * Useful if you want to distinguish between linked and unlinked nodes. - */ -LIST_INLINE void -rem2_node(node *n) -{ - node *z = n->prev; - node *x = n->next; - z->next = x; x->prev = z; n->next = NULL; diff --git a/lib/lists.h b/lib/lists.h index 80a4dc93..4204cbc5 100644 --- a/lib/lists.h +++ b/lib/lists.h @@ -61,7 +61,6 @@ typedef struct list { /* In fact two overlayed nodes */ void add_tail(list *, node *); void add_head(list *, node *); void rem_node(node *); -void rem2_node(node *); void add_tail_list(list *, list *); void init_list(list *); void insert_node(node *, node *); diff --git a/lib/resource.c b/lib/resource.c index 64f9a39c..68718dfb 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -163,6 +163,7 @@ rfree(void *res) if (r->n.next) rem_node(&r->n); r->class->free(r); + r->class = NULL; xfree(r); } @@ -383,16 +384,9 @@ mb_allocz(pool *p, unsigned size) void * mb_realloc(void *m, unsigned size) { - struct mblock *ob = NULL; - - if (m) - { - ob = SKIP_BACK(struct mblock, data, m); - if (ob->r.n.next) - rem_node(&ob->r.n); - } + struct mblock *b = SKIP_BACK(struct mblock, data, m); - struct mblock *b = xrealloc(ob, sizeof(struct mblock) + size); + b = xrealloc(b, sizeof(struct mblock) + size); replace_node(&b->r.n, &b->r.n); b->size = size; return b->data; -- cgit v1.2.3 From 54bb032d21d25a2221877e15325e79add10278ec Mon Sep 17 00:00:00 2001 From: Jan Moskyto Matejka Date: Wed, 23 Mar 2016 01:45:37 +0100 Subject: Birdlib: Modify lists to avoid problems with pointer aliasing rules The old linked list implementation used some wild typecasts and required GCC option -fno-strict-aliasing to work properly. This patch fixes that. However, we still keep the option due to other potential problems. (Commited by Ondrej Santiago Zajicek) --- lib/lists.c | 10 +++++----- lib/lists.h | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/lists.c b/lib/lists.c index 20a9a072..12ef3cc6 100644 --- a/lib/lists.c +++ b/lib/lists.c @@ -41,7 +41,7 @@ add_tail(list *l, node *n) { node *z = l->tail; - n->next = (node *) &l->null; + n->next = &l->tail_node; n->prev = z; z->next = n; l->tail = n; @@ -60,7 +60,7 @@ add_head(list *l, node *n) node *z = l->head; n->next = z; - n->prev = (node *) &l->head; + n->prev = &l->head_node; z->prev = n; l->head = n; } @@ -133,9 +133,9 @@ replace_node(node *old, node *new) LIST_INLINE void init_list(list *l) { - l->head = (node *) &l->null; + l->head = &l->tail_node; l->null = NULL; - l->tail = (node *) &l->head; + l->tail = &l->head_node; } /** @@ -155,6 +155,6 @@ add_tail_list(list *to, list *l) p->next = q; q->prev = p; q = l->tail; - q->next = (node *) &to->null; + q->next = &to->tail_node; to->tail = q; } diff --git a/lib/lists.h b/lib/lists.h index 4204cbc5..51856b05 100644 --- a/lib/lists.h +++ b/lib/lists.h @@ -26,10 +26,23 @@ typedef struct node { struct node *next, *prev; } node; -typedef struct list { /* In fact two overlayed nodes */ - struct node *head, *null, *tail; +typedef union list { /* In fact two overlayed nodes */ + struct { /* Head node */ + struct node head_node; + void *head_padding; + }; + struct { /* Tail node */ + void *tail_padding; + struct node tail_node; + }; + struct { /* Split to separate pointers */ + struct node *head; + struct node *null; + struct node *tail; + }; } list; + #define NODE (node *) #define HEAD(list) ((void *)((list).head)) #define TAIL(list) ((void *)((list).tail)) -- cgit v1.2.3 From 9e7b3ebdf9556d7464911dd39e862b1c003319b3 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 6 Apr 2016 11:49:34 +0200 Subject: IO: Replace RX priority heuristic with explicit mark In BIRD, RX has lower priority than TX with the exception of RX from control socket. The patch replaces heuristic based on socket type with explicit mark and uses it for both control socket and BGP session waiting to be established. This should avoid an issue when during heavy load, outgoing connection could connect (TX event), send open, but then failed to receive OPEN / establish in time, not sending notifications between and therefore got hold timer expired error from the neighbor immediately after it finally established the connection. --- lib/socket.h | 1 + proto/bgp/bgp.c | 3 +++ sysdep/unix/io.c | 4 ++-- sysdep/unix/main.c | 2 ++ 4 files changed, 8 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/socket.h b/lib/socket.h index fbea92aa..0327e9e5 100644 --- a/lib/socket.h +++ b/lib/socket.h @@ -28,6 +28,7 @@ typedef struct birdsock { struct iface *iface; /* Interface; specify this for broad/multicast sockets */ byte *rbuf, *rpos; /* NULL=allocate automatically */ + uint fast_rx; /* RX has higher priority in event loop */ uint rbsize; int (*rx_hook)(struct birdsock *, int size); /* NULL=receiving turned off, returns 1 to clear rx buffer */ diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 11f33b14..7328cb79 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -374,6 +374,8 @@ bgp_conn_enter_established_state(struct bgp_conn *conn) if (ipa_zero(p->source_addr)) p->source_addr = conn->sk->saddr; + conn->sk->fast_rx = 0; + p->conn = conn; p->last_error_class = 0; p->last_error_code = 0; @@ -696,6 +698,7 @@ bgp_setup_sk(struct bgp_conn *conn, sock *s) { s->data = conn; s->err_hook = bgp_sock_err; + s->fast_rx = 1; conn->sk = s; } diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c index 112ebe91..078fe73c 100644 --- a/sysdep/unix/io.c +++ b/sysdep/unix/io.c @@ -2152,7 +2152,7 @@ io_loop(void) int steps; steps = MAX_STEPS; - if ((s->type >= SK_MAGIC) && (pfd[s->index].revents & (POLLIN | POLLHUP | POLLERR)) && s->rx_hook) + if (s->fast_rx && (pfd[s->index].revents & (POLLIN | POLLHUP | POLLERR)) && s->rx_hook) do { steps--; @@ -2197,7 +2197,7 @@ io_loop(void) goto next2; } - if ((s->type < SK_MAGIC) && (pfd[s->index].revents & (POLLIN | POLLHUP | POLLERR)) && s->rx_hook) + if (!s->fast_rx && (pfd[s->index].revents & (POLLIN | POLLHUP | POLLERR)) && s->rx_hook) { count++; io_log_event(s->rx_hook, s->data); diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index 24d34f60..5d5586a0 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -450,6 +450,7 @@ cli_connect(sock *s, int size UNUSED) s->err_hook = cli_err; s->data = c = cli_new(s); s->pool = c->pool; /* We need to have all the socket buffers allocated in the cli pool */ + s->fast_rx = 1; c->rx_pos = c->rx_buf; c->rx_aux = NULL; rmove(s, c->pool); @@ -466,6 +467,7 @@ cli_init_unix(uid_t use_uid, gid_t use_gid) s->type = SK_UNIX_PASSIVE; s->rx_hook = cli_connect; s->rbsize = 1024; + s->fast_rx = 1; /* Return value intentionally ignored */ unlink(path_control_socket); -- cgit v1.2.3