summaryrefslogtreecommitdiffstats
path: root/ospfd/ospf_packet.c
diff options
context:
space:
mode:
authorDonald Sharp <sharpd@cumulusnetworks.com>2019-08-30 12:03:09 +0200
committerDonald Sharp <sharpd@cumulusnetworks.com>2019-08-30 22:48:07 +0200
commit4173cc8e23c19d9b901750413c790a1292b3f5fc (patch)
tree5ae4c0d3200ce407d038d25f3c3a12188bfd22c3 /ospfd/ospf_packet.c
parentMerge pull request #4794 from opensourcerouting/isis_nexthop_v4v6 (diff)
downloadfrr-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.c22
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);