flowtop: Replace single linked list by list_head from list.h

list.h provides generic Linux-like linked list API which also supports
RCU list operations.

Also additionally was removed the spinlock which is not needed for
RCU-list operations, for the list_del_rcu(...) case it is needed
additionally call call_rcu(...) before free the flow entry.

Because of full RCU support now flows are freed after grace-period
(after presenter leaves RCU lock) via calling call_rcu(), because
of that for the new entries we return NFCT_CB_STOLEN to tell conntrack
API do not automatically free received nfct_conntrack object, it will be
freed by us via call_rcu(...) therefor no need to use nfct_clone(n).

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

1 files changed, 47 insertions, 101 deletions

diff --git a/flowtop.c b/flowtop.c
index f48f5c8..3cee0ec 100644
--- a/flowtop.c
+++ b/flowtop.c
@@ -36,7 +36,6 @@
 #include "lookup.h"
 #include "geoip.h"
 #include "built_in.h"
-#include "locking.h"
 #include "pkt_buff.h"
 #include "screen.h"
 #include "proc.h"
@@ -51,6 +50,9 @@
 #endif
 
 struct flow_entry {
+	struct list_head entry;
+	struct rcu_head rcu;
+
 	uint32_t flow_id, use, status;
 	uint8_t  l3_proto, l4_proto;
 	uint32_t ip4_src_addr, ip4_dst_addr;
@@ -65,7 +67,6 @@ struct flow_entry {
 	char city_src[128], city_dst[128];
 	char rev_dns_src[256], rev_dns_dst[256];
 	char procname[256];
-	struct flow_entry *next;
 	int inode;
 	unsigned int procnum;
 	bool is_visible;
@@ -78,8 +79,7 @@ struct flow_entry {
 };
 
 struct flow_list {
-	struct flow_entry *head;
-	struct spinlock lock;
+	struct list_head head;
 };
 
 enum flow_direction {
@@ -360,10 +360,16 @@ static inline void flow_entry_xfree(struct flow_entry *n)
 	xfree(n);
 }
 
+static void flow_entry_xfree_rcu(struct rcu_head *head)
+{
+	struct flow_entry *n = container_of(head, struct flow_entry, rcu);
+
+	flow_entry_xfree(n);
+}
+
 static inline void flow_list_init(struct flow_list *fl)
 {
-	fl->head = NULL;
-	spinlock_init(&fl->lock);
+	INIT_LIST_HEAD(&fl->head);
 }
 
 static inline bool nfct_is_dns(const struct nf_conntrack *ct)
@@ -374,7 +380,7 @@ static inline bool nfct_is_dns(const struct nf_conntrack *ct)
 	return ntohs(port_src) == 53 || ntohs(port_dst) == 53;
 }
 
-static void flow_list_new_entry(struct flow_list *fl, const struct nf_conntrack *ct)
+static int flow_list_new_entry(struct flow_list *fl, struct nf_conntrack *ct)
 {
 	struct flow_entry *n;
 
@@ -382,94 +388,56 @@ static void flow_list_new_entry(struct flow_list *fl, const struct nf_conntrack
 	 * use it to resolve reverse dns.
 	 */
 	if (nfct_is_dns(ct))
-		return;
+		return NFCT_CB_CONTINUE;
 
 	n = flow_entry_xalloc();
 
-	n->ct = nfct_clone(ct);
+	n->ct = ct;
 
 	flow_entry_update_time(n);
 	flow_entry_from_ct(n, ct);
 	flow_entry_get_extended(n);
 
-	rcu_assign_pointer(n->next, fl->head);
-	rcu_assign_pointer(fl->head, n);
+	list_add_rcu(&n->entry, &fl->head);
 
 	n->is_visible = true;
+
+	return NFCT_CB_STOLEN;
 }
 
-static struct flow_entry *flow_list_find_id(struct flow_list *fl,
-					    uint32_t id)
+static struct flow_entry *flow_list_find_id(struct flow_list *fl, uint32_t id)
 {
-	struct flow_entry *n = rcu_dereference(fl->head);
+	struct flow_entry *n;
 
-	while (n != NULL) {
+	list_for_each_entry_rcu(n, &fl->head, entry) {
 		if (n->flow_id == id)
 			return n;
-
-		n = rcu_dereference(n->next);
 	}
 
 	return NULL;
 }
 
-static struct flow_entry *flow_list_find_prev_id(const struct flow_list *fl,
-						 uint32_t id)
+static int flow_list_del_entry(struct flow_list *fl, const struct nf_conntrack *ct)
 {
-	struct flow_entry *prev = rcu_dereference(fl->head), *next;
-
-	if (prev->flow_id == id)
-		return NULL;
-
-	while ((next = rcu_dereference(prev->next)) != NULL) {
-		if (next->flow_id == id)
-			return prev;
+	struct flow_entry *n;
 
-		prev = next;
+	n = flow_list_find_id(fl, nfct_get_attr_u32(ct, ATTR_ID));
+	if (n) {
+		list_del_rcu(&n->entry);
+		call_rcu(&n->rcu, flow_entry_xfree_rcu);
 	}
 
-	return NULL;
-}
-
-static void flow_list_destroy_entry(struct flow_list *fl,
-				    const struct nf_conntrack *ct)
-{
-	struct flow_entry *n1, *n2;
-	uint32_t id = nfct_get_attr_u32(ct, ATTR_ID);
-
-	n1 = flow_list_find_id(fl, id);
-	if (n1) {
-		n2 = flow_list_find_prev_id(fl, id);
-		if (n2) {
-			rcu_assign_pointer(n2->next, n1->next);
-			n1->next = NULL;
-
-			flow_entry_xfree(n1);
-		} else {
-			struct flow_entry *next = fl->head->next;
-
-			flow_entry_xfree(fl->head);
-			fl->head = next;
-		}
-	}
+	return NFCT_CB_CONTINUE;
 }
 
 static void flow_list_destroy(struct flow_list *fl)
 {
-	struct flow_entry *n;
-
-	synchronize_rcu();
-	spinlock_lock(&flow_list.lock);
+	struct flow_entry *n, *tmp;
 
-	while (fl->head != NULL) {
-		n = rcu_dereference(fl->head->next);
-		fl->head->next = NULL;
-
-		flow_entry_xfree(fl->head);
-		rcu_assign_pointer(fl->head, n);
+	list_for_each_entry_safe(n, tmp, &fl->head, entry) {
+		list_del_rcu(&n->entry);
+		call_rcu(&n->rcu, flow_entry_xfree_rcu);
 	}
-
-	spinlock_unlock(&flow_list.lock);
 }
 
 static void flow_entry_find_process(struct flow_entry *n)
@@ -1044,15 +1012,14 @@ static void draw_flows(WINDOW *screen, struct flow_list *fl,
 
 	rcu_read_lock();
 
-	n = rcu_dereference(fl->head);
-	if (!n)
+	if (list_empty(&fl->head))
 		mvwprintw(screen, line, 2, "(No sessions! "
 			  "Is netfilter running?)");
 
 	ui_table_clear(&flows_tbl);
 	ui_table_header_print(&flows_tbl);
 
-	for (; n; n = rcu_dereference(n->next)) {
+	list_for_each_entry_rcu(n, &fl->head, entry) {
 		if (!n->is_visible)
 			continue;
 		if (presenter_flow_wrong_state(n))
@@ -1070,6 +1037,8 @@ static void draw_flows(WINDOW *screen, struct flow_list *fl,
 		line += row_width;
 	}
 
+	rcu_read_unlock();
+
 	mvwprintw(screen, 1, 0, "%*s", COLS - 1, " ");
 	mvwprintw(screen, 1, 2, "Kernel netfilter flows(%u) for ", flows);
 
@@ -1096,8 +1065,6 @@ static void draw_flows(WINDOW *screen, struct flow_list *fl,
 
 	if (is_flow_collecting)
 		printw(" [Collecting flows ...]");
-
-	rcu_read_unlock();
 }
 
 static void draw_help(void)
@@ -1415,37 +1382,23 @@ static int flow_event_cb(enum nf_conntrack_msg_type type,
 	if (sigint)
 		return NFCT_CB_STOP;
 
-	synchronize_rcu();
-	spinlock_lock(&flow_list.lock);
-
 	switch (type) {
 	case NFCT_T_NEW:
-		flow_list_new_entry(&flow_list, ct);
-		break;
+		return flow_list_new_entry(&flow_list, ct);
 	case NFCT_T_UPDATE:
-		flow_list_update_entry(&flow_list, ct);
-		break;
+		return flow_list_update_entry(&flow_list, ct);
 	case NFCT_T_DESTROY:
-		flow_list_destroy_entry(&flow_list, ct);
-		break;
+		return flow_list_del_entry(&flow_list, ct);
 	default:
-		break;
+		return NFCT_CB_CONTINUE;
 	}
-
-	spinlock_unlock(&flow_list.lock);
-
-	if (sigint)
-		return NFCT_CB_STOP;
-
-	return NFCT_CB_CONTINUE;
 }
 
 static void collector_refresh_flows(struct nfct_handle *handle)
 {
 	struct flow_entry *n;
 
-	n = rcu_dereference(flow_list.head);
-	for (; n; n = rcu_dereference(n->next))
+	list_for_each_entry_rcu(n, &flow_list.head, entry)
 		nfct_query(handle, NFCT_Q_GET, n->ct);
 }
 
@@ -1500,9 +1453,6 @@ static int flow_dump_cb(enum nf_conntrack_msg_type type __maybe_unused,
 	if (sigint)
 		return NFCT_CB_STOP;
 
-	synchronize_rcu();
-	spinlock_lock(&flow_list.lock);
-
 	if (!(what & ~(INCLUDE_IPV4 | INCLUDE_IPV6)))
 		goto check_addr;
 
@@ -1555,10 +1505,9 @@ check_addr:
 			goto skip_flow;
 	}
 
-	flow_list_new_entry(&flow_list, ct);
+	return flow_list_new_entry(&flow_list, ct);
 
 skip_flow:
-	spinlock_unlock(&flow_list.lock);
 	return NFCT_CB_CONTINUE;
 }
 
@@ -1635,18 +1584,15 @@ static void *collector(void *null __maybe_unused)
 				continue;
 
 			panic("Error while polling: %s\n", strerror(errno));
-		} else if (status == 0) {
-			continue;
+		} else if (status != 0) {
+			if (poll_fd[0].revents & POLLIN)
+				nfct_catch(ct_event);
 		}
-
-		if (poll_fd[0].revents & POLLIN)
-			nfct_catch(ct_event);
 	}
 
-	rcu_unregister_thread();
-
 	flow_list_destroy(&flow_list);
-	spinlock_destroy(&flow_list.lock);
+
+	rcu_unregister_thread();
 
 	nfct_close(ct_event);