diff options
author | Donald Sharp <sharpd@cumulusnetworks.com> | 2019-08-30 12:03:09 +0200 |
---|---|---|
committer | Donald Sharp <sharpd@cumulusnetworks.com> | 2019-08-30 22:48:07 +0200 |
commit | 4173cc8e23c19d9b901750413c790a1292b3f5fc (patch) | |
tree | 5ae4c0d3200ce407d038d25f3c3a12188bfd22c3 /ospfd/ospf_packet.c | |
parent | Merge pull request #4794 from opensourcerouting/isis_nexthop_v4v6 (diff) | |
download | frr-4173cc8e23c19d9b901750413c790a1292b3f5fc.tar.xz frr-4173cc8e23c19d9b901750413c790a1292b3f5fc.zip |
ospfd: Do not turn on write thread unless we have something in it
I am rarely seeing this crash:
r2: ospfd crashed. Core file found - Backtrace follows:
[New LWP 32748]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/ospfd'.
Program terminated with signal SIGABRT, Aborted.
2019-08-29 15:59:36,149 ERROR: assert failed at "test_ospf_sr_topo1/test_memory_leak":
Which translates to this code:
node = listhead(ospf->oi_write_q);
assert(node);
oi = listgetdata(node);
assert(oi);
So if we get into ospf_write without anything on the oi_write_q
we are stopping the program.
This is happening because in ospf_ls_upd_queue_send we are calling
ospf_write. Imagine that we have a interface already on the on_write_q
and then ospf_write handles the packet send for all functions. We
are not clearing the t_write thread and we are popping and causing
a crash.
Additionally modify OSPF_ISM_WRITE_ON(O) to not just blindly
turn on the t_write thread. Only do so if we have data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
ospfd: Remove redundant asserts
assert(oi) is impossible all listgetdata(node) directly proceeding
it already asserts here, besides a node cannot be created
with a null pointer!
If list_isempty is called directly before the listhead call
it is impossilbe that we do not have a valid pointer here.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Diffstat (limited to '')
-rw-r--r-- | ospfd/ospf_packet.c | 22 |
1 files changed, 17 insertions, 5 deletions
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 62b044479..c69218fb5 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -698,12 +698,9 @@ static int ospf_write(struct thread *thread) return -1; } - ospf->t_write = NULL; - node = listhead(ospf->oi_write_q); assert(node); oi = listgetdata(node); - assert(oi); #ifdef WANT_OSPF_WRITE_FRAGMENT /* seed ipid static with low order bits of time */ @@ -906,9 +903,7 @@ static int ospf_write(struct thread *thread) /* Setup to service from the head of the queue again */ if (!list_isempty(ospf->oi_write_q)) { node = listhead(ospf->oi_write_q); - assert(node); oi = listgetdata(node); - assert(oi); } } @@ -4062,6 +4057,23 @@ static void ospf_ls_upd_queue_send(struct ospf_interface *oi, oi->on_write_q = 1; } ospf_write(&os_packet_thd); + /* + * We are fake calling ospf_write with a fake + * thread. Imagine that we have oi_a already + * enqueued and we have turned on the write + * thread(t_write). + * Now this function calls this for oi_b + * so the on_write_q has oi_a and oi_b on + * it, ospf_write runs and clears the packets + * for both oi_a and oi_b. Removing them from + * the on_write_q. After this thread of execution + * finishes we will execute the t_write thread + * with nothing in the on_write_q causing an + * assert. So just make sure that the t_write + * is actually turned off. + */ + if (list_isempty(oi->ospf->oi_write_q)) + OSPF_TIMER_OFF(oi->ospf->t_write); } else { /* Hook thread to write packet. */ OSPF_ISM_WRITE_ON(oi->ospf); |