From 80e70fdcf8861ab757b6bfc8f0fbaec31cde57bf Mon Sep 17 00:00:00 2001 From: Vadim Kochan Date: Fri, 13 Jan 2017 13:06:07 +0200 Subject: [PATCH] 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 Signed-off-by: Tobias Klauser --- flowtop.c | 148 ++++++++++++++++++++------------------------------------------ 1 file changed, 47 insertions(+), 101 deletions(-) diff --git a/flowtop.c b/flowtop.c index f48f5c80..3cee0ecf 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); -- 2.11.4.GIT