code was so difficult to read, that I had to rewrite it from scratch. Original code was using list structure designed only for promisc code, and that means it has to have its own adding / removing / traversing code. Not smart idea. Also I removed the code which stores and loads from disk state of promosic mode. Code does:
init path init_promisc_list(&promisc_list); save_promisc_list(promisc_list); srpromisc(1, promisc_list); destroy_promisc_list(&promisc_list);
... ...
exit/error path load_promisc_list(&promisc_list); srpromisc(0, promisc_list); destroy_promisc_list(&promisc_list);
now it does
init path init_promisc_list(&promisc_list); promisc_set(promisc_list);
... ...
exit/error path promisc_restore(promisc_list); destroy_promisc_list(&promisc_list);
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- src/detstats.c | 15 ++-- src/hostmon.c | 16 ++-- src/ifaces.c | 2 +- src/ifaces.h | 4 +- src/ifstats.c | 16 ++-- src/iptraf-ng-compat.h | 3 + src/itrafmon.c | 17 ++-- src/list.h | 6 ++ src/pktsize.c | 14 ++-- src/promisc.c | 212 +++++++++++++++---------------------------------- src/promisc.h | 30 +++---- src/serv.c | 14 ++-- 12 files changed, 121 insertions(+), 228 deletions(-)
diff --git a/src/detstats.c b/src/detstats.c index c1e415f..43b442a 100644 --- a/src/detstats.c +++ b/src/detstats.c @@ -291,7 +291,6 @@ void detstats(char *iface, time_t facilitytime) unsigned long peakpps_in = 0; unsigned long peakpps_out = 0;
- struct promisc_states *promisc_list; int fd;
/* @@ -312,11 +311,10 @@ void detstats(char *iface, time_t facilitytime) return; }
- if (first_active_facility() && options.promisc) { - init_promisc_list(&promisc_list); - save_promisc_list(promisc_list); - srpromisc(1, promisc_list); - destroy_promisc_list(&promisc_list); + LIST_HEAD(promisc); + if (options.promisc && first_active_facility()) { + promisc_init(&promisc, iface); + promisc_set_list(&promisc); }
adjust_instance_count(PROCCOUNTFILE, 1); @@ -594,9 +592,8 @@ err: rate_destroy(&rate);
if (options.promisc && is_last_instance()) { - load_promisc_list(&promisc_list); - srpromisc(0, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_restore_list(&promisc); + promisc_destroy(&promisc); }
adjust_instance_count(PROCCOUNTFILE, -1); diff --git a/src/hostmon.c b/src/hostmon.c index 21b1797..b807926 100644 --- a/src/hostmon.c +++ b/src/hostmon.c @@ -778,8 +778,6 @@ void hostmon(time_t facilitytime, char *ifptr)
int fd;
- struct promisc_states *promisc_list; - if (!facility_active(LANMONIDFILE, ifptr)) mark_facility(LANMONIDFILE, "LAN monitor", ifptr); else { @@ -796,11 +794,10 @@ void hostmon(time_t facilitytime, char *ifptr) } }
- if (first_active_facility() && options.promisc) { - init_promisc_list(&promisc_list); - save_promisc_list(promisc_list); - srpromisc(1, promisc_list); - destroy_promisc_list(&promisc_list); + LIST_HEAD(promisc); + if (options.promisc && first_active_facility()) { + promisc_init(&promisc, ifptr); + promisc_set_list(&promisc); }
adjust_instance_count(PROCCOUNTFILE, 1); @@ -1031,9 +1028,8 @@ err_close:
err: if (options.promisc && is_last_instance()) { - load_promisc_list(&promisc_list); - srpromisc(0, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_restore_list(&promisc); + promisc_destroy(&promisc); }
adjust_instance_count(PROCCOUNTFILE, -1); diff --git a/src/ifaces.c b/src/ifaces.c index 88151ef..437d5bb 100644 --- a/src/ifaces.c +++ b/src/ifaces.c @@ -211,7 +211,7 @@ err: /* need to preserve errno across call to close() */ return ir; }
-int dev_set_promisc(char *ifname) +int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); } diff --git a/src/ifaces.h b/src/ifaces.h index d753cac..f2eee94 100644 --- a/src/ifaces.h +++ b/src/ifaces.h @@ -17,8 +17,8 @@ int dev_get_mtu(const char *iface); int dev_get_flags(const char *iface); int dev_set_flags(const char *iface, int flags); int dev_clear_flags(const char *iface, int flags); -int dev_set_promisc(char *ifname); -int dev_clear_promisc(char *ifname); +int dev_set_promisc(const char *ifname); +int dev_clear_promisc(const char *ifname); int dev_get_ifname(int ifindex, char *ifname); int dev_bind_ifindex(const int fd, const int ifindex); int dev_bind_ifname(const int fd, const char const *ifname); diff --git a/src/ifstats.c b/src/ifstats.c index 8214838..f8943bc 100644 --- a/src/ifstats.c +++ b/src/ifstats.c @@ -451,8 +451,6 @@ void ifstats(time_t facilitytime) time_t startlog = 0; struct timeval updtime;
- struct promisc_states *promisc_list; - if (!facility_active(GSTATIDFILE, "")) mark_facility(GSTATIDFILE, "general interface statistics", ""); else { @@ -469,11 +467,10 @@ void ifstats(time_t facilitytime)
initiftab(&table);
- if (first_active_facility() && options.promisc) { - init_promisc_list(&promisc_list); - save_promisc_list(promisc_list); - srpromisc(1, promisc_list); - destroy_promisc_list(&promisc_list); + LIST_HEAD(promisc); + if (options.promisc && first_active_facility()) { + promisc_init(&promisc, NULL); + promisc_set_list(&promisc); }
adjust_instance_count(PROCCOUNTFILE, 1); @@ -630,9 +627,8 @@ void ifstats(time_t facilitytime)
err: if (options.promisc && is_last_instance()) { - load_promisc_list(&promisc_list); - srpromisc(0, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_restore_list(&promisc); + promisc_destroy(&promisc); }
adjust_instance_count(PROCCOUNTFILE, -1); diff --git a/src/iptraf-ng-compat.h b/src/iptraf-ng-compat.h index 64b3db0..1e1ef3f 100644 --- a/src/iptraf-ng-compat.h +++ b/src/iptraf-ng-compat.h @@ -86,6 +86,9 @@ } \ } while (0)
+#define likely(x) __builtin_expect(!!(x), 1) +#define unlikely(x) __builtin_expect(!!(x), 0) + extern int is_first_instance; /* in iptraf.c */ extern int daemonized; extern int exitloop; diff --git a/src/itrafmon.c b/src/itrafmon.c index b35468f..a2cbc52 100644 --- a/src/itrafmon.c +++ b/src/itrafmon.c @@ -599,8 +599,6 @@ void ipmon(time_t facilitytime, char *ifptr) int keymode = 0; char msgstring[80];
- struct promisc_states *promisc_list; - int rvnfd = 0;
int instance_id; @@ -628,12 +626,10 @@ void ipmon(time_t facilitytime, char *ifptr) } }
- if (options.promisc) { - if (first_active_facility()) { - init_promisc_list(&promisc_list); - save_promisc_list(promisc_list); - srpromisc(1, promisc_list); - } + LIST_HEAD(promisc); + if (options.promisc && first_active_facility()) { + promisc_init(&promisc, ifptr); + promisc_set_list(&promisc); }
/* @@ -1188,9 +1184,8 @@ err: close_rvn_socket(rvnfd);
if (options.promisc && is_last_instance()) { - load_promisc_list(&promisc_list); - srpromisc(0, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_restore_list(&promisc); + promisc_destroy(&promisc); }
attrset(STDATTR); diff --git a/src/list.h b/src/list.h index daa7628..a7b495e 100644 --- a/src/list.h +++ b/src/list.h @@ -80,4 +80,10 @@ static inline int list_empty(const struct list_head *head) &pos->member != (head); \ pos = list_entry(pos->member.next, typeof(*pos), member))
+#define list_for_each_entry_safe(pos, n, head, member) \ + for (pos = list_entry((head)->next, typeof(*pos), member), \ + n = list_entry(pos->member.next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = n, n = list_entry(n->member.next, typeof(*n), member)) + #endif /* IPTRAF_NG_LIST_H */ diff --git a/src/pktsize.c b/src/pktsize.c index 44bc61f..b54d79b 100644 --- a/src/pktsize.c +++ b/src/pktsize.c @@ -161,8 +161,6 @@ void packet_size_breakdown(char *ifname, time_t facilitytime) int logging = options.logging; FILE *logfile = NULL;
- struct promisc_states *promisc_list; - int fd;
if (!facility_active(PKTSIZEIDFILE, ifname)) @@ -241,11 +239,10 @@ void packet_size_breakdown(char *ifname, time_t facilitytime) updtime = tv; now = starttime = startlog = timeint = tv.tv_sec;
+ LIST_HEAD(promisc); if (first_active_facility() && options.promisc) { - init_promisc_list(&promisc_list); - save_promisc_list(promisc_list); - srpromisc(1, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_init(&promisc, ifname); + promisc_set_list(&promisc); }
adjust_instance_count(PROCCOUNTFILE, 1); @@ -337,9 +334,8 @@ err: }
if (options.promisc && is_last_instance()) { - load_promisc_list(&promisc_list); - srpromisc(0, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_restore_list(&promisc); + promisc_destroy(&promisc); }
adjust_instance_count(PROCCOUNTFILE, -1); diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags) { - FILE *fd; - char buf[IFNAMSIZ]; - struct promisc_states *ptmp; - struct promisc_states *tail = NULL; - - *list = NULL; - fd = open_procnetdev(); - - while (get_next_iface(fd, buf, sizeof(buf))) { - if (strcmp(buf, "") != 0) { - ptmp = xmalloc(sizeof(struct promisc_states)); - strcpy(ptmp->params.ifname, buf); - - if (*list == NULL) { - *list = ptmp; - } else - tail->next_entry = ptmp; - - tail = ptmp; - ptmp->next_entry = NULL; - - /* - * Retrieve and save interface flags - */ - - if ((strncmp(buf, "eth", 3) == 0) - || (strncmp(buf, "ra", 2) == 0) - || (strncmp(buf, "fddi", 4) == 0) - || (strncmp(buf, "tr", 2) == 0) - || (strncmp(buf, "ath", 3) == 0) - || (strncmp(buf, "bnep", 4) == 0) - || (strncmp(buf, "ni", 2) == 0) - || (strncmp(buf, "tap", 3) == 0) - || (strncmp(buf, "dummy", 5) == 0) - || (strncmp(buf, "br", 2) == 0) - || (strncmp(buf, "vmnet", 5) == 0) - || (strncmp(ptmp->params.ifname, "wvlan", 4) == 0) - || (strncmp(ptmp->params.ifname, "lec", 3) == 0)) { - int flags = dev_get_flags(buf); - - if (flags < 0) { - write_error("Unable to obtain interface parameters for %s", - buf); - ptmp->params.state_valid = 0; - } else { - ptmp->params.saved_state = flags; - ptmp->params.state_valid = 1; - } - } - } - } -} + struct promisc_list *p = xmallocz(sizeof(*p)); + strcpy(p->ifname, dev_name); + INIT_LIST_HEAD(&p->list);
-/* - * Save interfaces and their states to a temporary file. Used only by the - * first IPTraf instance. Needed in case there are subsequent, simultaneous - * instances of IPTraf, which may still need promiscuous mode even after - * the first instance exits. These subsequent instances will need to restore - * the promiscuous state from this file. - */ + p->flags = flags; + list_add_tail(&p->list, promisc); +}
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name) { - int fd; - struct promisc_states *ptmp = list; - - fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR); - - if (fd < 0) { - write_error("Unable to save interface flags"); - return; + int flags = dev_get_flags(dev_name); + if (flags < 0) { + write_error("Unable to obtain interface parameters for %s", + dev_name); + return -1; }
- while (ptmp != NULL) { - write(fd, &(ptmp->params), sizeof(struct promisc_params)); - ptmp = ptmp->next_entry; - } + if (flags & IFF_PROMISC) + return -1;
- close(fd); + return flags; }
-/* - * Load promiscuous states into list - */ - -void load_promisc_list(struct promisc_states **list) +void promisc_init(struct list_head *promisc, const char *device_name) { - int fd; - struct promisc_states *ptmp = NULL; - struct promisc_states *tail = NULL; - int br; + if (device_name) { + int flags = dev_promisc_flag(device_name); + if (flags < 0) + return;
- fd = open(PROMISCLISTFILE, O_RDONLY); + add_dev(promisc, device_name, flags);
- if (fd < 0) { - write_error("Unable to retrieve saved interface flags"); - *list = NULL; return; }
- do { - ptmp = xmalloc(sizeof(struct promisc_states)); - br = read(fd, &(ptmp->params), sizeof(struct promisc_params)); + FILE *fp = open_procnetdev(); + if (!fp) + die_errno("%s: open_procnetdev", __func__); + + char dev_name[IFNAMSIZ]; + while (get_next_iface(fp, dev_name, sizeof(dev_name))) { + if (!strcmp(dev_name, "")) + continue;
- if (br > 0) { - if (tail != NULL) - tail->next_entry = ptmp; - else - *list = ptmp; + int flags = dev_promisc_flag(dev_name); + if (flags < 0) + continue;
- ptmp->next_entry = NULL; - tail = ptmp; - } else - free(ptmp); - } while (br > 0); + add_dev(promisc, dev_name, flags); + }
- close(fd); + fclose(fp); }
-/* - * Set/restore interface promiscuous mode. - */ +static void promisc_set(const char *dev_name) +{ + int r = dev_set_promisc(dev_name); + if (r < 0) + write_error("Failed to set promiscuous mode on %s", dev_name); +}
-void srpromisc(int mode, struct promisc_states *list) +void promisc_set_list(struct list_head *promisc) { - struct promisc_states *ptmp; - - ptmp = list; - - while (ptmp != NULL) { - if (((strncmp(ptmp->params.ifname, "eth", 3) == 0) - || (strncmp(ptmp->params.ifname, "fddi", 4) == 0) - || (strncmp(ptmp->params.ifname, "tr", 2) == 0) - || (strncmp(ptmp->params.ifname, "ra", 2) == 0) - || (strncmp(ptmp->params.ifname, "ath", 3) == 0) - || (strncmp(ptmp->params.ifname, "wvlan", 4) == 0) - || (strncmp(ptmp->params.ifname, "lec", 3) == 0)) - && (ptmp->params.state_valid)) { - if (mode) { - /* set promiscuous */ - int r = dev_set_promisc(ptmp->params.ifname); - if(r < 0) - write_error("Failed to set promiscuous mode on %s", ptmp->params.ifname); - } else { - /* restore saved state */ - if (ptmp->params.saved_state & IFF_PROMISC) - /* was promisc, so leave it as is */ - continue; - /* wasn't promisc, clear it */ - int r = dev_clear_promisc(ptmp->params.ifname); - if(r < 0) - write_error("Failed to clear promiscuous mode on %s", ptmp->params.ifname); - } - } - ptmp = ptmp->next_entry; - } + struct promisc_list *entry = NULL; + list_for_each_entry(entry, promisc, list) + promisc_set(entry->ifname); }
-void destroy_promisc_list(struct promisc_states **list) +static void promisc_restore(const char *dev_name) { - struct promisc_states *ptmp = *list; - struct promisc_states *ctmp; + int r = dev_clear_promisc(dev_name); + if (r < 0) + write_error("Failed to clear promiscuous mode on %s", dev_name); +}
- if (ptmp != NULL) - ctmp = ptmp->next_entry; +void promisc_restore_list(struct list_head *promisc) +{ + struct promisc_list *entry = NULL; + list_for_each_entry(entry, promisc, list) { + if (unlikely(entry->flags & IFF_PROMISC)) + continue; + promisc_restore(entry->ifname); + } +}
- while (ptmp != NULL) { - free(ptmp); - ptmp = ctmp; - if (ctmp != NULL) - ctmp = ctmp->next_entry; +void promisc_destroy(struct list_head *promisc) +{ + struct promisc_list *entry, *tmp; + list_for_each_entry_safe(entry, tmp, promisc, list) { + list_del(&entry->list); + free(entry); } } diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/* - * promisc.h - definitions for promiscuous state save/recovery - * - * Thanks to Holger Friese - * evildead@bs-pc5.et-inf.fho-emden.de for the base patch. - * Applied it, but then additional issues came up and I ended up doing more - * than slight modifications. struct iflist is becoming way too large for - * comfort and for something as little as this. - */ +#include "list.h"
-struct promisc_params { +struct promisc_list { + struct list_head list; char ifname[IFNAMSIZ]; - int saved_state; - int state_valid; + int flags; };
-struct promisc_states { - struct promisc_params params; - struct promisc_states *next_entry; -};
-void init_promisc_list(struct promisc_states **list); -void save_promisc_list(struct promisc_states *list); -void load_promisc_list(struct promisc_states **list); -void srpromisc(int mode, struct promisc_states *promisc_list); -void destroy_promisc_list(struct promisc_states **list); +void promisc_init(struct list_head *promisc, const char *device_name); +void promisc_destroy(struct list_head *promisc); + +void promisc_set_list(struct list_head *promisc); +void promisc_restore_list(struct list_head *promisc);
#endif /* IPTRAF_NG_PROMISC_H */ diff --git a/src/serv.c b/src/serv.c index 513cc94..84e4e94 100644 --- a/src/serv.c +++ b/src/serv.c @@ -779,8 +779,6 @@ void servmon(char *ifname, time_t facilitytime)
FILE *logfile = NULL;
- struct promisc_states *promisc_list; - WINDOW *sortwin; PANEL *sortpanel;
@@ -812,11 +810,10 @@ void servmon(char *ifname, time_t facilitytime)
loadaddports(&ports);
+ LIST_HEAD(promisc); if (first_active_facility() && options.promisc) { - init_promisc_list(&promisc_list); - save_promisc_list(promisc_list); - srpromisc(1, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_init(&promisc, ifname); + promisc_set_list(&promisc); }
adjust_instance_count(PROCCOUNTFILE, 1); @@ -1091,9 +1088,8 @@ err: endservent();
if (options.promisc && is_last_instance()) { - load_promisc_list(&promisc_list); - srpromisc(0, promisc_list); - destroy_promisc_list(&promisc_list); + promisc_restore_list(&promisc); + promisc_destroy(&promisc); }
adjust_instance_count(PROCCOUNTFILE, -1);
we don't need it any more. the idea will be to have only one running iptraf-ng instance
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- Makefile | 2 - src/detstats.c | 23 +-------- src/fltedit.c | 34 ++----------- src/fltmgr.c | 21 +------- src/fltmgr.h | 2 - src/fltselect.c | 11 ---- src/hostmon.c | 31 ++--------- src/ifstats.c | 20 ++------ src/instances.c | 136 ------------------------------------------------- src/instances.h | 18 ------- src/iptraf-ng-compat.h | 1 - src/iptraf.c | 35 ------------- src/itrafmon.c | 36 +++---------- src/options.c | 8 --- src/pktsize.c | 17 +------ src/serv.c | 22 +------- 16 files changed, 25 insertions(+), 392 deletions(-) delete mode 100644 src/instances.c delete mode 100644 src/instances.h
diff --git a/Makefile b/Makefile index c72ff7a..c8711db 100644 --- a/Makefile +++ b/Makefile @@ -93,7 +93,6 @@ iptraf-h += src/fltmgr.h iptraf-h += src/ipfrag.h iptraf-h += src/serv.h iptraf-h += src/servname.h -iptraf-h += src/instances.h iptraf-h += src/timer.h iptraf-h += src/ifaces.h iptraf-h += src/error.h @@ -133,7 +132,6 @@ iptraf-o += src/fltmgr.o iptraf-o += src/ipfrag.o iptraf-o += src/serv.o iptraf-o += src/servname.o -iptraf-o += src/instances.o iptraf-o += src/timer.o iptraf-o += src/revname.o iptraf-o += src/pktsize.o diff --git a/src/detstats.c b/src/detstats.c index 43b442a..37c43bb 100644 --- a/src/detstats.c +++ b/src/detstats.c @@ -22,7 +22,6 @@ detstats.c - the interface statistics module #include "attrs.h" #include "serv.h" #include "timer.h" -#include "instances.h" #include "logvars.h" #include "promisc.h" #include "error.h" @@ -293,32 +292,17 @@ void detstats(char *iface, time_t facilitytime)
int fd;
- /* - * Mark this facility - */ - - if (!facility_active(DSTATIDFILE, iface)) - mark_facility(DSTATIDFILE, "detailed interface statistics", - iface); - else { - write_error("Detailed interface stats already monitoring %s", iface); - return; - } - if (!dev_up(iface)) { err_iface_down(); - unmark_facility(DSTATIDFILE, iface); return; }
LIST_HEAD(promisc); - if (options.promisc && first_active_facility()) { + if (options.promisc) { promisc_init(&promisc, iface); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1); - move(LINES - 1, 1); stdexitkeyhelp(); statwin = newwin(LINES - 2, COLS, 1, 0); @@ -591,13 +575,11 @@ err: rate_destroy(&rate_in); rate_destroy(&rate);
- if (options.promisc && is_last_instance()) { + if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1); - if (logging) { signal(SIGUSR1, SIG_DFL); writedstatlog(iface, @@ -612,7 +594,6 @@ err:
del_panel(statpanel); delwin(statwin); - unmark_facility(DSTATIDFILE, iface); strcpy(current_logfile, ""); pkt_cleanup(); update_panels(); diff --git a/src/fltedit.c b/src/fltedit.c index cd926f1..50801ec 100644 --- a/src/fltedit.c +++ b/src/fltedit.c @@ -104,7 +104,6 @@ void savefilter(char *filename, struct filterlist *fl)
if (bw < 0) { tui_error(ANYKEY_MSG, "Unable to save filter changes"); - clear_flt_tag(); return; } fe = fe->next_entry; @@ -505,19 +504,11 @@ void definefilter(int *aborted) int pfd; int bw;
- /* - * Lock facility - */ - - if (!mark_filter_change()) - return; - get_filter_description(ffile.desc, aborted, "");
- if (*aborted) { - clear_flt_tag(); + if (*aborted) return; - } + genname(time(NULL), fntemp);
pfd = @@ -526,7 +517,6 @@ void definefilter(int *aborted) if (pfd < 0) { tui_error(ANYKEY_MSG, "Cannot create filter data file"); *aborted = 1; - clear_flt_tag(); return; }
@@ -537,7 +527,6 @@ void definefilter(int *aborted)
if (pfd < 0) { listfileerr(1); - clear_flt_tag(); return; } strcpy(ffile.filename, fntemp); @@ -551,8 +540,6 @@ void definefilter(int *aborted) modify_host_parameters(&fl); savefilter(get_path(T_WORKDIR, fntemp), &fl); destroyfilter(&fl); - - clear_flt_tag(); }
/* @@ -566,21 +553,15 @@ void editfilter(int *aborted) struct ffnode *ffile; struct filterfileent *ffe;
- if (!mark_filter_change()) - return; - if (loadfilterlist(&flist) == 1) { listfileerr(1); destroyfilterlist(flist); - clear_flt_tag(); return; } pickafilter(flist, &ffile, aborted);
- clear_flt_tag(); if ((*aborted)) { destroyfilterlist(flist); - clear_flt_tag(); return; } ffe = &(ffile->ffe); @@ -589,7 +570,6 @@ void editfilter(int *aborted)
if (*aborted) { destroyfilterlist(flist); - clear_flt_tag(); return; } strncpy(filename, get_path(T_WORKDIR, ffe->filename), @@ -614,22 +594,17 @@ void delfilter(int *aborted) struct ffnode *fltfile; struct ffnode *fltlist;
- if (!mark_filter_change()) - return; - if (loadfilterlist(&fltlist) == 1) { *aborted = 1; listfileerr(1); destroyfilterlist(fltlist); - clear_flt_tag(); return; } pickafilter(fltlist, &fltfile, aborted);
- if (*aborted) { - clear_flt_tag(); + if (*aborted) return; - } + unlink(get_path(T_WORKDIR, fltfile->ffe.filename));
if (fltfile->prev_entry == NULL) { @@ -646,6 +621,5 @@ void delfilter(int *aborted) free(fltfile);
save_filterlist(fltlist); - clear_flt_tag(); *aborted = 0; } diff --git a/src/fltmgr.c b/src/fltmgr.c index 5771f89..ad194ee 100644 --- a/src/fltmgr.c +++ b/src/fltmgr.c @@ -21,7 +21,6 @@ fltmgr.c - filter list management routines #include "dirs.h" #include "fltdefs.h" #include "fltmgr.h" -#include "instances.h" #include "error.h"
void makestdfiltermenu(struct MENU *menu) @@ -50,23 +49,6 @@ void genname(unsigned long n, char *m) sprintf(m, "%lu", n); }
-int mark_filter_change(void) -{ - if (!facility_active(OTHIPFLTIDFILE, "")) - mark_facility(OTHIPFLTIDFILE, "IP filter change", ""); - else { - tui_error(ANYKEY_MSG, - "IP protocol data file in use; try again later"); - return 0; - } - return 1; -} - -void clear_flt_tag(void) -{ - unmark_facility(OTHIPFLTIDFILE, ""); -} - void listfileerr(int code) { if (code == 1) @@ -224,16 +206,15 @@ void save_filterlist(struct ffnode *fltlist)
if (fd < 0) { listfileerr(2); - clear_flt_tag(); return; } + fltfile = fltlist; while (fltfile != NULL) { bw = write(fd, &(fltfile->ffe), sizeof(struct filterfileent));
if (bw < 0) { listfileerr(2); - clear_flt_tag(); return; } ffntemp = fltfile; diff --git a/src/fltmgr.h b/src/fltmgr.h index c40b2ff..0477631 100644 --- a/src/fltmgr.h +++ b/src/fltmgr.h @@ -30,7 +30,5 @@ void get_filter_description(char *description, int *aborted, char *pre_edit); void genname(unsigned long n, char *m); unsigned long int nametoaddr(char *ascname, int *err); void listfileerr(int code); -int mark_filter_change(void); -void clear_flt_tag(void);
#endif /* IPTRAF_NG_FLTMGR_H */ diff --git a/src/fltselect.c b/src/fltselect.c index 698c5bd..4be6243 100644 --- a/src/fltselect.c +++ b/src/fltselect.c @@ -23,7 +23,6 @@ fltselect.c - a menu-based module that allows selection of #include "ipfilter.h" #include "deskman.h" #include "attrs.h" -#include "instances.h"
struct filterstate ofilter;
@@ -192,15 +191,6 @@ void savefilters(void) int pfd; int bw;
- if (!facility_active(FLTIDFILE, "")) - mark_facility(FLTIDFILE, "Filter configuration change", ""); - else { - tui_error(ANYKEY_MSG, - "Filter state file currently in use;" - " try again later"); - return; - } - pfd = open(FLTSTATEFILE, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR); bw = write(pfd, &ofilter, sizeof(struct filterstate)); @@ -210,5 +200,4 @@ void savefilters(void)
close(pfd);
- unmark_facility(FLTIDFILE, ""); } diff --git a/src/hostmon.c b/src/hostmon.c index b807926..de72163 100644 --- a/src/hostmon.c +++ b/src/hostmon.c @@ -24,7 +24,6 @@ Discovers LAN hosts and displays packet statistics for them #include "timer.h" #include "landesc.h" #include "options.h" -#include "instances.h" #include "logvars.h" #include "promisc.h" #include "error.h" @@ -774,35 +773,19 @@ void hostmon(time_t facilitytime, char *ifptr) PANEL *sortpanel; int keymode = 0;
- int instance_id; - int fd;
- if (!facility_active(LANMONIDFILE, ifptr)) - mark_facility(LANMONIDFILE, "LAN monitor", ifptr); - else { - write_error("LAN station monitor already running on %s", - gen_iface_msg(ifptr)); + if (ifptr && !dev_up(ifptr)) { + err_iface_down(); return; }
- if (ifptr != NULL) { - if (!dev_up(ifptr)) { - err_iface_down(); - unmark_facility(LANMONIDFILE, ifptr); - return; - } - } - LIST_HEAD(promisc); - if (options.promisc && first_active_facility()) { + if (options.promisc) { promisc_init(&promisc, ifptr); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1); - instance_id = adjust_instance_count(LANMONCOUNTFILE, 1); - hostmonhelp();
initethtab(&table); @@ -816,7 +799,7 @@ void hostmon(time_t facilitytime, char *ifptr) if (logging) { if (strcmp(current_logfile, "") == 0) { strncpy(current_logfile, - gen_instance_logname(LANLOG, instance_id), 80); + gen_instance_logname(LANLOG, getpid()), 80);
if (!daemonized) input_logfile(current_logfile, &logging); @@ -1027,14 +1010,11 @@ err_close: close(fd);
err: - if (options.promisc && is_last_instance()) { + if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1); - adjust_instance_count(LANMONCOUNTFILE, -1); - if (logging) { signal(SIGUSR1, SIG_DFL); writeethlog(table.head, time(NULL) - statbegin, logfile); @@ -1055,6 +1035,5 @@ err: free_eth_desc(elist); free_eth_desc(flist);
- unmark_facility(LANMONIDFILE, ifptr); strcpy(current_logfile, ""); } diff --git a/src/ifstats.c b/src/ifstats.c index f8943bc..e8eda20 100644 --- a/src/ifstats.c +++ b/src/ifstats.c @@ -24,7 +24,6 @@ ifstats.c - the interface statistics module #include "attrs.h" #include "serv.h" #include "timer.h" -#include "instances.h" #include "logvars.h" #include "promisc.h" #include "error.h" @@ -451,30 +450,20 @@ void ifstats(time_t facilitytime) time_t startlog = 0; struct timeval updtime;
- if (!facility_active(GSTATIDFILE, "")) - mark_facility(GSTATIDFILE, "general interface statistics", ""); - else { - write_error("General interface stats already active in another process"); - return; - } - initiflist(&(table.head)); - if (table.head == NULL) { + if (!table.head) { no_ifaces_error(); - unmark_facility(GSTATIDFILE, ""); return; }
initiftab(&table);
LIST_HEAD(promisc); - if (options.promisc && first_active_facility()) { + if (options.promisc) { promisc_init(&promisc, NULL); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1); - if (logging) { if (strcmp(current_logfile, "") == 0) { strcpy(current_logfile, GSTATLOG); @@ -626,13 +615,11 @@ void ifstats(time_t facilitytime) close(fd);
err: - if (options.promisc && is_last_instance()) { + if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1); - del_panel(table.statpanel); delwin(table.statwin); del_panel(table.borderpanel); @@ -649,7 +636,6 @@ err: } destroyiflist(table.head); pkt_cleanup(); - unmark_facility(GSTATIDFILE, ""); strcpy(current_logfile, ""); }
diff --git a/src/instances.c b/src/instances.c deleted file mode 100644 index a5b3af5..0000000 --- a/src/instances.c +++ /dev/null @@ -1,136 +0,0 @@ -/* For terms of usage/redistribution/modification see the LICENSE file */ -/* For authors and contributors see the AUTHORS file */ - -/*** - -instances.c - handler routines for multiple IPTraf instances - -***/ - -#include "iptraf-ng-compat.h" - -#include "error.h" -#include "dirs.h" -#include "instances.h" - -static void gen_lockfile_name(char *tagfile, char *iface, char *result) -{ - if (iface == NULL) - snprintf(result, 64, "%s.all", tagfile); - else - snprintf(result, 64, "%s.%s", tagfile, iface); -} - -void mark_facility(char *tagfile, char *facility, char *iface) -{ - int fd; - char lockfile[64]; - - gen_lockfile_name(tagfile, iface, lockfile); - fd = open(lockfile, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR); - if (fd < 0) - write_error("Warning: unable to lock %s on %s", facility, iface); - close(fd); -} - -void unmark_facility(char *tagfile, char *iface) -{ - char lockfile[64]; - - gen_lockfile_name(tagfile, iface, lockfile); - unlink(lockfile); -} - -int facility_active(char *tagfile, char *iface) -{ - int fd; - char lockfile[64]; - - gen_lockfile_name(tagfile, iface, lockfile); - fd = open(lockfile, O_RDONLY); - - if (fd < 0) - return 0; - else { - close(fd); - return 1; - } -} - -/* - * Increments or decrements the process count - */ - -int adjust_instance_count(char *countfile, int inc) -{ - int fd; - int proccount = 0; - int brw; - - fd = open(countfile, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR); - brw = read(fd, &proccount, sizeof(int)); - if ((brw == 0) || (brw == -1)) - proccount = 0; - - proccount += inc; - - if (proccount < 0) - proccount = 0; - - lseek(fd, 0, SEEK_SET); - brw = write(fd, &proccount, sizeof(int)); - close(fd); - - return proccount; -} - -int get_instance_count(char *countfile) -{ - int fd; - int proccount = 0; - int br; - - fd = open(countfile, O_RDONLY); - br = read(fd, &proccount, sizeof(int)); - if ((br == 0) || (br == -1)) - proccount = 0; - - close(fd); - return proccount; -} - -/* - * Returns TRUE if this is the last instance, and is therefore responsible - * for restoring the promicuous states saved by the first instance. - * - * Man, this is getting more complex by the minute :) - */ - -int is_last_instance(void) -{ - int fd; - int proccount = 0; - int br; - - fd = open(PROCCOUNTFILE, O_RDONLY); - br = read(fd, &proccount, sizeof(int)); - close(fd); - return ((proccount == 1) || (br < 0) || fd < 0); -} - -/* - * Returns TRUE if no facilities are currently running in other instances of - * IPTraf. Call this before the first invocation of adjust_process_count(1) - */ - -int first_active_facility(void) -{ - int fd; - int proccount = 0; - int br; - - fd = open(PROCCOUNTFILE, O_RDONLY); - br = read(fd, &proccount, sizeof(int)); - close(fd); - return ((proccount == 0) || (br < 0) || (fd < 0)); -} diff --git a/src/instances.h b/src/instances.h deleted file mode 100644 index 47d1080..0000000 --- a/src/instances.h +++ /dev/null @@ -1,18 +0,0 @@ -#ifndef IPTRAF_NG_INSTANCES_H -#define IPTRAF_NG_INSTANCES_H - -/*** - -instances.h - header file for instances.c - -***/ - -void mark_facility(char *tagfile, char *facility, char *ifptr); -void unmark_facility(char *tagfile, char *ifptr); -int facility_active(char *tagfile, char *ifptr); -int adjust_instance_count(char *countfile, int inc); -int get_instance_count(char *countfile); -int is_last_instance(void); -int first_active_facility(void); - -#endif /* IPTRAF_NG_INSTANCES_H */ diff --git a/src/iptraf-ng-compat.h b/src/iptraf-ng-compat.h index 1e1ef3f..0dfc4dc 100644 --- a/src/iptraf-ng-compat.h +++ b/src/iptraf-ng-compat.h @@ -89,7 +89,6 @@ #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0)
-extern int is_first_instance; /* in iptraf.c */ extern int daemonized; extern int exitloop;
diff --git a/src/iptraf.c b/src/iptraf.c index 65e9f2c..15789a9 100644 --- a/src/iptraf.c +++ b/src/iptraf.c @@ -52,7 +52,6 @@ struct cmd_struct { int exitloop = 0; int daemonized = 0; int facility_running = 0; -int is_first_instance;
static void press_enter_to_continue(void) { @@ -246,33 +245,6 @@ static void program_interface(void) tx_destroymenu(&menu); }
-static int first_instance(void) -{ - int fd; - - fd = open(IPTIDFILE, O_RDONLY); - - if (fd < 0) - return !0; - else { - close(fd); - return 0; - } -} - -static void mark_first_instance(void) -{ - int fd; - - fd = open(IPTIDFILE, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR); - if (fd < 0) { - fprintf(stderr, "\nWarning: unable to tag this process\r\n"); - press_enter_to_continue(); - return; - } - close(fd); -} - static const char *const iptraf_ng_usage[] = { IPTRAF_NAME " [options]", IPTRAF_NAME " [options] -B [-i <iface> | -d <iface> | -s <iface> | -z <iface> | -l <iface> | -g]", @@ -419,8 +391,6 @@ int main(int argc, char **argv) current_log_interval = I_opt; #endif
- is_first_instance = first_instance(); - if ((getenv("TERM") == NULL) && (!daemonized)) die("Your TERM variable is not set.\n" "Please set it to an appropriate value"); @@ -460,8 +430,6 @@ int main(int argc, char **argv) die("This program requires a screen size of at least 80 columns by 24 lines\n" "Please resize your window"); }
- mark_first_instance(); - signal(SIGTSTP, SIG_IGN); signal(SIGINT, SIG_IGN); signal(SIGUSR1, SIG_IGN); @@ -533,8 +501,5 @@ int main(int argc, char **argv) doupdate(); endwin();
- if (is_first_instance) - unlink(IPTIDFILE); - return (0); } diff --git a/src/itrafmon.c b/src/itrafmon.c index a2cbc52..670ab6d 100644 --- a/src/itrafmon.c +++ b/src/itrafmon.c @@ -28,7 +28,6 @@ itrafmon.c - the IP traffic monitor module #include "dirs.h" #include "timer.h" #include "ipfrag.h" -#include "instances.h" #include "logvars.h" #include "itrafmon.h" #include "sockaddr.h" @@ -601,7 +600,6 @@ void ipmon(time_t facilitytime, char *ifptr)
int rvnfd = 0;
- int instance_id; int revlook = options.revlook; int wasempty = 1;
@@ -611,34 +609,17 @@ void ipmon(time_t facilitytime, char *ifptr) * Mark this instance of the traffic monitor */
- if (!facility_active(IPMONIDFILE, ifptr)) - mark_facility(IPMONIDFILE, "IP traffic monitor", ifptr); - else { - write_error("IP Traffic Monitor already listening on %s", gen_iface_msg(ifptr)); + if (ifptr && !dev_up(ifptr)) { + err_iface_down(); return; }
- if (ifptr != NULL) { - if (!dev_up(ifptr)) { - err_iface_down(); - unmark_facility(IPMONIDFILE, ifptr); - return; - } - } - LIST_HEAD(promisc); - if (options.promisc && first_active_facility()) { + if (options.promisc) { promisc_init(&promisc, ifptr); promisc_set_list(&promisc); }
- /* - * Adjust instance counters - */ - - adjust_instance_count(PROCCOUNTFILE, 1); - instance_id = adjust_instance_count(ITRAFMONCOUNTFILE, 1); - init_tcp_table(&table); init_othp_table(&othptbl);
@@ -679,7 +660,7 @@ void ipmon(time_t facilitytime, char *ifptr) if (logging) { if (strcmp(current_logfile, "") == 0) { strncpy(current_logfile, - gen_instance_logname(IPMONLOG, instance_id), + gen_instance_logname(IPMONLOG, getpid()), 80);
if (!daemonized) @@ -1174,8 +1155,7 @@ void ipmon(time_t facilitytime, char *ifptr) err_close: close(fd); err: - if (get_instance_count(ITRAFMONCOUNTFILE) <= 1) - killrvnamed(); + killrvnamed();
if (options.servnames) endservent(); @@ -1183,7 +1163,7 @@ err: endprotoent(); close_rvn_socket(rvnfd);
- if (options.promisc && is_last_instance()) { + if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); } @@ -1213,8 +1193,4 @@ err: fclose(logfile); strcpy(current_logfile, ""); } - - adjust_instance_count(PROCCOUNTFILE, -1); - adjust_instance_count(ITRAFMONCOUNTFILE, -1); - unmark_facility(IPMONIDFILE, ifptr); } diff --git a/src/options.c b/src/options.c index d0ead45..a1cd4f8 100644 --- a/src/options.c +++ b/src/options.c @@ -21,7 +21,6 @@ options.c - implements the configuration section of the utility #include "landesc.h" #include "promisc.h" #include "dirs.h" -#include "instances.h"
#define ALLOW_ZERO 1 #define DONT_ALLOW_ZERO 0 @@ -262,13 +261,6 @@ void setoptions(void)
struct porttab *ports;
- if (!is_first_instance) { - tui_error(ANYKEY_MSG, - "Only the first instance of ipraf-ng" - " can configure"); - return; - } - loadaddports(&ports);
makeoptionmenu(&menu); diff --git a/src/pktsize.c b/src/pktsize.c index b54d79b..eecb2e5 100644 --- a/src/pktsize.c +++ b/src/pktsize.c @@ -21,7 +21,6 @@ pktsize.c - the packet size breakdown facility #include "pktsize.h" #include "options.h" #include "timer.h" -#include "instances.h" #include "log.h" #include "logvars.h" #include "promisc.h" @@ -163,13 +162,6 @@ void packet_size_breakdown(char *ifname, time_t facilitytime)
int fd;
- if (!facility_active(PKTSIZEIDFILE, ifname)) - mark_facility(PKTSIZEIDFILE, "Packet size breakdown", ifname); - else { - write_error("Packet sizes already being monitored on %s", ifname); - return; - } - if (!dev_up(ifname)) { err_iface_down(); goto err_unmark; @@ -240,13 +232,11 @@ void packet_size_breakdown(char *ifname, time_t facilitytime) now = starttime = startlog = timeint = tv.tv_sec;
LIST_HEAD(promisc); - if (first_active_facility() && options.promisc) { + if (options.promisc) { promisc_init(&promisc, ifname); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1); - fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); if(fd == -1) { write_error("Unable to obtain monitoring socket"); @@ -333,18 +323,15 @@ err: fclose(logfile); }
- if (options.promisc && is_last_instance()) { + if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1); - del_panel(panel); delwin(win); del_panel(borderpanel); delwin(borderwin); err_unmark: - unmark_facility(PKTSIZEIDFILE, ifname); strcpy(current_logfile, ""); } diff --git a/src/serv.c b/src/serv.c index 84e4e94..1bef335 100644 --- a/src/serv.c +++ b/src/serv.c @@ -27,7 +27,6 @@ serv.c - TCP/UDP port statistics module #include "timer.h" #include "promisc.h" #include "options.h" -#include "instances.h" #include "packet.h" #include "logvars.h" #include "error.h" @@ -791,33 +790,19 @@ void servmon(char *ifname, time_t facilitytime)
struct porttab *ports;
- /* - * Mark this facility - */ - - if (!facility_active(TCPUDPIDFILE, ifname)) - mark_facility(TCPUDPIDFILE, "TCP/UDP monitor", ifname); - else { - write_error("TCP/UDP monitor already running on %s", ifname); - return; - } - if (!dev_up(ifname)) { err_iface_down(); - unmark_facility(TCPUDPIDFILE, ifname); return; }
loadaddports(&ports);
LIST_HEAD(promisc); - if (first_active_facility() && options.promisc) { + if (options.promisc) { promisc_init(&promisc, ifname); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1); - initportlist(&list); statwin = newwin(1, COLS, LINES - 2, 0); statpanel = new_panel(statwin); @@ -1087,20 +1072,17 @@ err: if (options.servnames) endservent();
- if (options.promisc && is_last_instance()) { + if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1); - del_panel(list.panel); delwin(list.win); del_panel(list.borderpanel); delwin(list.borderwin); del_panel(statpanel); delwin(statwin); - unmark_facility(TCPUDPIDFILE, ifname); update_panels(); doupdate(); destroyportlist(&list);
On Mon, Sep 03, 2012 at 02:06:12PM +0200, Nikola Pajkovsky wrote:
we don't need it any more. the idea will be to have only one running iptraf-ng instance
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com
Signed-off-by: Vitezslav Samel vitezslav@samel.cz
Makefile | 2 - src/detstats.c | 23 +-------- src/fltedit.c | 34 ++----------- src/fltmgr.c | 21 +------- src/fltmgr.h | 2 - src/fltselect.c | 11 ---- src/hostmon.c | 31 ++--------- src/ifstats.c | 20 ++------ src/instances.c | 136 ------------------------------------------------- src/instances.h | 18 ------- src/iptraf-ng-compat.h | 1 - src/iptraf.c | 35 ------------- src/itrafmon.c | 36 +++---------- src/options.c | 8 --- src/pktsize.c | 17 +------ src/serv.c | 22 +------- 16 files changed, 25 insertions(+), 392 deletions(-) delete mode 100644 src/instances.c delete mode 100644 src/instances.h
diff --git a/Makefile b/Makefile index c72ff7a..c8711db 100644 --- a/Makefile +++ b/Makefile @@ -93,7 +93,6 @@ iptraf-h += src/fltmgr.h iptraf-h += src/ipfrag.h iptraf-h += src/serv.h iptraf-h += src/servname.h -iptraf-h += src/instances.h iptraf-h += src/timer.h iptraf-h += src/ifaces.h iptraf-h += src/error.h @@ -133,7 +132,6 @@ iptraf-o += src/fltmgr.o iptraf-o += src/ipfrag.o iptraf-o += src/serv.o iptraf-o += src/servname.o -iptraf-o += src/instances.o iptraf-o += src/timer.o iptraf-o += src/revname.o iptraf-o += src/pktsize.o diff --git a/src/detstats.c b/src/detstats.c index 43b442a..37c43bb 100644 --- a/src/detstats.c +++ b/src/detstats.c @@ -22,7 +22,6 @@ detstats.c - the interface statistics module #include "attrs.h" #include "serv.h" #include "timer.h" -#include "instances.h" #include "logvars.h" #include "promisc.h" #include "error.h" @@ -293,32 +292,17 @@ void detstats(char *iface, time_t facilitytime)
int fd;
/*
* Mark this facility*/if (!facility_active(DSTATIDFILE, iface))
mark_facility(DSTATIDFILE, "detailed interface statistics",iface);else {
write_error("Detailed interface stats already monitoring %s", iface);return;}
if (!dev_up(iface)) { err_iface_down();
unmark_facility(DSTATIDFILE, iface);return; }
LIST_HEAD(promisc);
if (options.promisc && first_active_facility()) {
- if (options.promisc) { promisc_init(&promisc, iface); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1);
- move(LINES - 1, 1); stdexitkeyhelp(); statwin = newwin(LINES - 2, COLS, 1, 0);
@@ -591,13 +575,11 @@ err: rate_destroy(&rate_in); rate_destroy(&rate);
- if (options.promisc && is_last_instance()) {
- if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1);
- if (logging) { signal(SIGUSR1, SIG_DFL); writedstatlog(iface,
@@ -612,7 +594,6 @@ err:
del_panel(statpanel); delwin(statwin);
- unmark_facility(DSTATIDFILE, iface); strcpy(current_logfile, ""); pkt_cleanup(); update_panels();
diff --git a/src/fltedit.c b/src/fltedit.c index cd926f1..50801ec 100644 --- a/src/fltedit.c +++ b/src/fltedit.c @@ -104,7 +104,6 @@ void savefilter(char *filename, struct filterlist *fl)
if (bw < 0) { tui_error(ANYKEY_MSG, "Unable to save filter changes");
clear_flt_tag(); return;@@ -505,19 +504,11 @@ void definefilter(int *aborted) int pfd; int bw;
/*
* Lock facility*/if (!mark_filter_change())
return;get_filter_description(ffile.desc, aborted, "");
if (*aborted) {
clear_flt_tag();
- if (*aborted) return;
- }
genname(time(NULL), fntemp);
pfd =
@@ -526,7 +517,6 @@ void definefilter(int *aborted) if (pfd < 0) { tui_error(ANYKEY_MSG, "Cannot create filter data file"); *aborted = 1;
clear_flt_tag();@@ -537,7 +527,6 @@ void definefilter(int *aborted)
if (pfd < 0) { listfileerr(1);
clear_flt_tag();@@ -551,8 +540,6 @@ void definefilter(int *aborted) modify_host_parameters(&fl); savefilter(get_path(T_WORKDIR, fntemp), &fl); destroyfilter(&fl);
- clear_flt_tag();
}
/* @@ -566,21 +553,15 @@ void editfilter(int *aborted) struct ffnode *ffile; struct filterfileent *ffe;
if (!mark_filter_change())
return;if (loadfilterlist(&flist) == 1) { listfileerr(1); destroyfilterlist(flist);
clear_flt_tag();return; } pickafilter(flist, &ffile, aborted);
clear_flt_tag(); if ((*aborted)) { destroyfilterlist(flist);
clear_flt_tag();return; } ffe = &(ffile->ffe);
@@ -589,7 +570,6 @@ void editfilter(int *aborted)
if (*aborted) { destroyfilterlist(flist);
clear_flt_tag();@@ -614,22 +594,17 @@ void delfilter(int *aborted) struct ffnode *fltfile; struct ffnode *fltlist;
if (!mark_filter_change())
return;if (loadfilterlist(&fltlist) == 1) { *aborted = 1; listfileerr(1); destroyfilterlist(fltlist);
clear_flt_tag();return; } pickafilter(fltlist, &fltfile, aborted);
if (*aborted) {
clear_flt_tag();
- if (*aborted) return;
- }
unlink(get_path(T_WORKDIR, fltfile->ffe.filename));
if (fltfile->prev_entry == NULL) {
@@ -646,6 +621,5 @@ void delfilter(int *aborted) free(fltfile);
save_filterlist(fltlist);
- clear_flt_tag(); *aborted = 0;
} diff --git a/src/fltmgr.c b/src/fltmgr.c index 5771f89..ad194ee 100644 --- a/src/fltmgr.c +++ b/src/fltmgr.c @@ -21,7 +21,6 @@ fltmgr.c - filter list management routines #include "dirs.h" #include "fltdefs.h" #include "fltmgr.h" -#include "instances.h" #include "error.h"
void makestdfiltermenu(struct MENU *menu) @@ -50,23 +49,6 @@ void genname(unsigned long n, char *m) sprintf(m, "%lu", n); }
-int mark_filter_change(void) -{
- if (!facility_active(OTHIPFLTIDFILE, ""))
mark_facility(OTHIPFLTIDFILE, "IP filter change", "");- else {
tui_error(ANYKEY_MSG,"IP protocol data file in use; try again later");return 0;- }
- return 1;
-}
-void clear_flt_tag(void) -{
- unmark_facility(OTHIPFLTIDFILE, "");
-}
void listfileerr(int code) { if (code == 1) @@ -224,16 +206,15 @@ void save_filterlist(struct ffnode *fltlist)
if (fd < 0) { listfileerr(2);
clear_flt_tag();
fltfile = fltlist; while (fltfile != NULL) { bw = write(fd, &(fltfile->ffe), sizeof(struct filterfileent));
if (bw < 0) { listfileerr(2);
clear_flt_tag(); return;diff --git a/src/fltmgr.h b/src/fltmgr.h index c40b2ff..0477631 100644 --- a/src/fltmgr.h +++ b/src/fltmgr.h @@ -30,7 +30,5 @@ void get_filter_description(char *description, int *aborted, char *pre_edit); void genname(unsigned long n, char *m); unsigned long int nametoaddr(char *ascname, int *err); void listfileerr(int code); -int mark_filter_change(void); -void clear_flt_tag(void);
#endif /* IPTRAF_NG_FLTMGR_H */ diff --git a/src/fltselect.c b/src/fltselect.c index 698c5bd..4be6243 100644 --- a/src/fltselect.c +++ b/src/fltselect.c @@ -23,7 +23,6 @@ fltselect.c - a menu-based module that allows selection of #include "ipfilter.h" #include "deskman.h" #include "attrs.h" -#include "instances.h"
struct filterstate ofilter;
@@ -192,15 +191,6 @@ void savefilters(void) int pfd; int bw;
- if (!facility_active(FLTIDFILE, ""))
mark_facility(FLTIDFILE, "Filter configuration change", "");- else {
tui_error(ANYKEY_MSG,"Filter state file currently in use;"" try again later");return;- }
- pfd = open(FLTSTATEFILE, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR); bw = write(pfd, &ofilter, sizeof(struct filterstate));
@@ -210,5 +200,4 @@ void savefilters(void)
close(pfd);
- unmark_facility(FLTIDFILE, "");
} diff --git a/src/hostmon.c b/src/hostmon.c index b807926..de72163 100644 --- a/src/hostmon.c +++ b/src/hostmon.c @@ -24,7 +24,6 @@ Discovers LAN hosts and displays packet statistics for them #include "timer.h" #include "landesc.h" #include "options.h" -#include "instances.h" #include "logvars.h" #include "promisc.h" #include "error.h" @@ -774,35 +773,19 @@ void hostmon(time_t facilitytime, char *ifptr) PANEL *sortpanel; int keymode = 0;
int instance_id;
int fd;
if (!facility_active(LANMONIDFILE, ifptr))
mark_facility(LANMONIDFILE, "LAN monitor", ifptr);else {
write_error("LAN station monitor already running on %s",gen_iface_msg(ifptr));
- if (ifptr && !dev_up(ifptr)) {
err_iface_down();
- if (ifptr != NULL) {
if (!dev_up(ifptr)) {err_iface_down();unmark_facility(LANMONIDFILE, ifptr);return;}- }
- LIST_HEAD(promisc);
- if (options.promisc && first_active_facility()) {
- if (options.promisc) { promisc_init(&promisc, ifptr); promisc_set_list(&promisc); }
adjust_instance_count(PROCCOUNTFILE, 1);
instance_id = adjust_instance_count(LANMONCOUNTFILE, 1);
hostmonhelp();
initethtab(&table);
@@ -816,7 +799,7 @@ void hostmon(time_t facilitytime, char *ifptr) if (logging) { if (strcmp(current_logfile, "") == 0) { strncpy(current_logfile,
gen_instance_logname(LANLOG, instance_id), 80);
gen_instance_logname(LANLOG, getpid()), 80); if (!daemonized) input_logfile(current_logfile, &logging);@@ -1027,14 +1010,11 @@ err_close: close(fd);
err:
- if (options.promisc && is_last_instance()) {
- if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1);
- adjust_instance_count(LANMONCOUNTFILE, -1);
- if (logging) { signal(SIGUSR1, SIG_DFL); writeethlog(table.head, time(NULL) - statbegin, logfile);
@@ -1055,6 +1035,5 @@ err: free_eth_desc(elist); free_eth_desc(flist);
- unmark_facility(LANMONIDFILE, ifptr); strcpy(current_logfile, "");
} diff --git a/src/ifstats.c b/src/ifstats.c index f8943bc..e8eda20 100644 --- a/src/ifstats.c +++ b/src/ifstats.c @@ -24,7 +24,6 @@ ifstats.c - the interface statistics module #include "attrs.h" #include "serv.h" #include "timer.h" -#include "instances.h" #include "logvars.h" #include "promisc.h" #include "error.h" @@ -451,30 +450,20 @@ void ifstats(time_t facilitytime) time_t startlog = 0; struct timeval updtime;
- if (!facility_active(GSTATIDFILE, ""))
mark_facility(GSTATIDFILE, "general interface statistics", "");- else {
write_error("General interface stats already active in another process");return;- }
- initiflist(&(table.head));
- if (table.head == NULL) {
- if (!table.head) { no_ifaces_error();
unmark_facility(GSTATIDFILE, "");return; }
initiftab(&table);
LIST_HEAD(promisc);
if (options.promisc && first_active_facility()) {
- if (options.promisc) { promisc_init(&promisc, NULL); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1);
- if (logging) { if (strcmp(current_logfile, "") == 0) { strcpy(current_logfile, GSTATLOG);
@@ -626,13 +615,11 @@ void ifstats(time_t facilitytime) close(fd);
err:
- if (options.promisc && is_last_instance()) {
- if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1);
- del_panel(table.statpanel); delwin(table.statwin); del_panel(table.borderpanel);
@@ -649,7 +636,6 @@ err: } destroyiflist(table.head); pkt_cleanup();
- unmark_facility(GSTATIDFILE, ""); strcpy(current_logfile, "");
}
diff --git a/src/instances.c b/src/instances.c deleted file mode 100644 index a5b3af5..0000000 --- a/src/instances.c +++ /dev/null @@ -1,136 +0,0 @@ -/* For terms of usage/redistribution/modification see the LICENSE file */ -/* For authors and contributors see the AUTHORS file */
-/***
-instances.c - handler routines for multiple IPTraf instances
-***/
-#include "iptraf-ng-compat.h"
-#include "error.h" -#include "dirs.h" -#include "instances.h"
-static void gen_lockfile_name(char *tagfile, char *iface, char *result) -{
- if (iface == NULL)
snprintf(result, 64, "%s.all", tagfile);- else
snprintf(result, 64, "%s.%s", tagfile, iface);-}
-void mark_facility(char *tagfile, char *facility, char *iface) -{
- int fd;
- char lockfile[64];
- gen_lockfile_name(tagfile, iface, lockfile);
- fd = open(lockfile, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR);
- if (fd < 0)
write_error("Warning: unable to lock %s on %s", facility, iface);- close(fd);
-}
-void unmark_facility(char *tagfile, char *iface) -{
- char lockfile[64];
- gen_lockfile_name(tagfile, iface, lockfile);
- unlink(lockfile);
-}
-int facility_active(char *tagfile, char *iface) -{
- int fd;
- char lockfile[64];
- gen_lockfile_name(tagfile, iface, lockfile);
- fd = open(lockfile, O_RDONLY);
- if (fd < 0)
return 0;- else {
close(fd);return 1;- }
-}
-/*
- Increments or decrements the process count
- */
-int adjust_instance_count(char *countfile, int inc) -{
- int fd;
- int proccount = 0;
- int brw;
- fd = open(countfile, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
- brw = read(fd, &proccount, sizeof(int));
- if ((brw == 0) || (brw == -1))
proccount = 0;- proccount += inc;
- if (proccount < 0)
proccount = 0;- lseek(fd, 0, SEEK_SET);
- brw = write(fd, &proccount, sizeof(int));
- close(fd);
- return proccount;
-}
-int get_instance_count(char *countfile) -{
- int fd;
- int proccount = 0;
- int br;
- fd = open(countfile, O_RDONLY);
- br = read(fd, &proccount, sizeof(int));
- if ((br == 0) || (br == -1))
proccount = 0;- close(fd);
- return proccount;
-}
-/*
- Returns TRUE if this is the last instance, and is therefore responsible
- for restoring the promicuous states saved by the first instance.
- Man, this is getting more complex by the minute :)
- */
-int is_last_instance(void) -{
- int fd;
- int proccount = 0;
- int br;
- fd = open(PROCCOUNTFILE, O_RDONLY);
- br = read(fd, &proccount, sizeof(int));
- close(fd);
- return ((proccount == 1) || (br < 0) || fd < 0);
-}
-/*
- Returns TRUE if no facilities are currently running in other instances of
- IPTraf. Call this before the first invocation of adjust_process_count(1)
- */
-int first_active_facility(void) -{
- int fd;
- int proccount = 0;
- int br;
- fd = open(PROCCOUNTFILE, O_RDONLY);
- br = read(fd, &proccount, sizeof(int));
- close(fd);
- return ((proccount == 0) || (br < 0) || (fd < 0));
-} diff --git a/src/instances.h b/src/instances.h deleted file mode 100644 index 47d1080..0000000 --- a/src/instances.h +++ /dev/null @@ -1,18 +0,0 @@ -#ifndef IPTRAF_NG_INSTANCES_H -#define IPTRAF_NG_INSTANCES_H
-/***
-instances.h - header file for instances.c
-***/
-void mark_facility(char *tagfile, char *facility, char *ifptr); -void unmark_facility(char *tagfile, char *ifptr); -int facility_active(char *tagfile, char *ifptr); -int adjust_instance_count(char *countfile, int inc); -int get_instance_count(char *countfile); -int is_last_instance(void); -int first_active_facility(void);
-#endif /* IPTRAF_NG_INSTANCES_H */ diff --git a/src/iptraf-ng-compat.h b/src/iptraf-ng-compat.h index 1e1ef3f..0dfc4dc 100644 --- a/src/iptraf-ng-compat.h +++ b/src/iptraf-ng-compat.h @@ -89,7 +89,6 @@ #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0)
-extern int is_first_instance; /* in iptraf.c */ extern int daemonized; extern int exitloop;
diff --git a/src/iptraf.c b/src/iptraf.c index 65e9f2c..15789a9 100644 --- a/src/iptraf.c +++ b/src/iptraf.c @@ -52,7 +52,6 @@ struct cmd_struct { int exitloop = 0; int daemonized = 0; int facility_running = 0; -int is_first_instance;
static void press_enter_to_continue(void) { @@ -246,33 +245,6 @@ static void program_interface(void) tx_destroymenu(&menu); }
-static int first_instance(void) -{
- int fd;
- fd = open(IPTIDFILE, O_RDONLY);
- if (fd < 0)
return !0;- else {
close(fd);return 0;- }
-}
-static void mark_first_instance(void) -{
- int fd;
- fd = open(IPTIDFILE, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR);
- if (fd < 0) {
fprintf(stderr, "\nWarning: unable to tag this process\r\n");press_enter_to_continue();return;- }
- close(fd);
-}
static const char *const iptraf_ng_usage[] = { IPTRAF_NAME " [options]", IPTRAF_NAME " [options] -B [-i <iface> | -d <iface> | -s <iface> | -z <iface> | -l <iface> | -g]", @@ -419,8 +391,6 @@ int main(int argc, char **argv) current_log_interval = I_opt; #endif
- is_first_instance = first_instance();
- if ((getenv("TERM") == NULL) && (!daemonized)) die("Your TERM variable is not set.\n" "Please set it to an appropriate value");
@@ -460,8 +430,6 @@ int main(int argc, char **argv) die("This program requires a screen size of at least 80 columns by 24 lines\n" "Please resize your window"); }
- mark_first_instance();
- signal(SIGTSTP, SIG_IGN); signal(SIGINT, SIG_IGN); signal(SIGUSR1, SIG_IGN);
@@ -533,8 +501,5 @@ int main(int argc, char **argv) doupdate(); endwin();
- if (is_first_instance)
unlink(IPTIDFILE);- return (0);
} diff --git a/src/itrafmon.c b/src/itrafmon.c index a2cbc52..670ab6d 100644 --- a/src/itrafmon.c +++ b/src/itrafmon.c @@ -28,7 +28,6 @@ itrafmon.c - the IP traffic monitor module #include "dirs.h" #include "timer.h" #include "ipfrag.h" -#include "instances.h" #include "logvars.h" #include "itrafmon.h" #include "sockaddr.h" @@ -601,7 +600,6 @@ void ipmon(time_t facilitytime, char *ifptr)
int rvnfd = 0;
- int instance_id; int revlook = options.revlook; int wasempty = 1;
@@ -611,34 +609,17 @@ void ipmon(time_t facilitytime, char *ifptr) * Mark this instance of the traffic monitor */
- if (!facility_active(IPMONIDFILE, ifptr))
mark_facility(IPMONIDFILE, "IP traffic monitor", ifptr);- else {
write_error("IP Traffic Monitor already listening on %s", gen_iface_msg(ifptr));
- if (ifptr && !dev_up(ifptr)) {
err_iface_down();
- if (ifptr != NULL) {
if (!dev_up(ifptr)) {err_iface_down();unmark_facility(IPMONIDFILE, ifptr);return;}- }
- LIST_HEAD(promisc);
- if (options.promisc && first_active_facility()) {
- if (options.promisc) { promisc_init(&promisc, ifptr); promisc_set_list(&promisc); }
- /*
* Adjust instance counters*/- adjust_instance_count(PROCCOUNTFILE, 1);
- instance_id = adjust_instance_count(ITRAFMONCOUNTFILE, 1);
- init_tcp_table(&table); init_othp_table(&othptbl);
@@ -679,7 +660,7 @@ void ipmon(time_t facilitytime, char *ifptr) if (logging) { if (strcmp(current_logfile, "") == 0) { strncpy(current_logfile,
gen_instance_logname(IPMONLOG, instance_id),
gen_instance_logname(IPMONLOG, getpid()), 80); if (!daemonized)@@ -1174,8 +1155,7 @@ void ipmon(time_t facilitytime, char *ifptr) err_close: close(fd); err:
- if (get_instance_count(ITRAFMONCOUNTFILE) <= 1)
killrvnamed();
killrvnamed();
if (options.servnames) endservent();
@@ -1183,7 +1163,7 @@ err: endprotoent(); close_rvn_socket(rvnfd);
- if (options.promisc && is_last_instance()) {
- if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
@@ -1213,8 +1193,4 @@ err: fclose(logfile); strcpy(current_logfile, ""); }
- adjust_instance_count(PROCCOUNTFILE, -1);
- adjust_instance_count(ITRAFMONCOUNTFILE, -1);
- unmark_facility(IPMONIDFILE, ifptr);
} diff --git a/src/options.c b/src/options.c index d0ead45..a1cd4f8 100644 --- a/src/options.c +++ b/src/options.c @@ -21,7 +21,6 @@ options.c - implements the configuration section of the utility #include "landesc.h" #include "promisc.h" #include "dirs.h" -#include "instances.h"
#define ALLOW_ZERO 1 #define DONT_ALLOW_ZERO 0 @@ -262,13 +261,6 @@ void setoptions(void)
struct porttab *ports;
if (!is_first_instance) {
tui_error(ANYKEY_MSG,"Only the first instance of ipraf-ng"" can configure");return;}
loadaddports(&ports);
makeoptionmenu(&menu);
diff --git a/src/pktsize.c b/src/pktsize.c index b54d79b..eecb2e5 100644 --- a/src/pktsize.c +++ b/src/pktsize.c @@ -21,7 +21,6 @@ pktsize.c - the packet size breakdown facility #include "pktsize.h" #include "options.h" #include "timer.h" -#include "instances.h" #include "log.h" #include "logvars.h" #include "promisc.h" @@ -163,13 +162,6 @@ void packet_size_breakdown(char *ifname, time_t facilitytime)
int fd;
- if (!facility_active(PKTSIZEIDFILE, ifname))
mark_facility(PKTSIZEIDFILE, "Packet size breakdown", ifname);- else {
write_error("Packet sizes already being monitored on %s", ifname);return;- }
- if (!dev_up(ifname)) { err_iface_down(); goto err_unmark;
@@ -240,13 +232,11 @@ void packet_size_breakdown(char *ifname, time_t facilitytime) now = starttime = startlog = timeint = tv.tv_sec;
LIST_HEAD(promisc);
- if (first_active_facility() && options.promisc) {
- if (options.promisc) { promisc_init(&promisc, ifname); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1);
- fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); if(fd == -1) { write_error("Unable to obtain monitoring socket");
@@ -333,18 +323,15 @@ err: fclose(logfile); }
- if (options.promisc && is_last_instance()) {
- if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1);
- del_panel(panel); delwin(win); del_panel(borderpanel); delwin(borderwin);
err_unmark:
- unmark_facility(PKTSIZEIDFILE, ifname); strcpy(current_logfile, "");
} diff --git a/src/serv.c b/src/serv.c index 84e4e94..1bef335 100644 --- a/src/serv.c +++ b/src/serv.c @@ -27,7 +27,6 @@ serv.c - TCP/UDP port statistics module #include "timer.h" #include "promisc.h" #include "options.h" -#include "instances.h" #include "packet.h" #include "logvars.h" #include "error.h" @@ -791,33 +790,19 @@ void servmon(char *ifname, time_t facilitytime)
struct porttab *ports;
/*
* Mark this facility*/if (!facility_active(TCPUDPIDFILE, ifname))
mark_facility(TCPUDPIDFILE, "TCP/UDP monitor", ifname);else {
write_error("TCP/UDP monitor already running on %s", ifname);return;}
if (!dev_up(ifname)) { err_iface_down();
unmark_facility(TCPUDPIDFILE, ifname);return; }
loadaddports(&ports);
LIST_HEAD(promisc);
if (first_active_facility() && options.promisc) {
- if (options.promisc) { promisc_init(&promisc, ifname); promisc_set_list(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, 1);
- initportlist(&list); statwin = newwin(1, COLS, LINES - 2, 0); statpanel = new_panel(statwin);
@@ -1087,20 +1072,17 @@ err: if (options.servnames) endservent();
- if (options.promisc && is_last_instance()) {
- if (options.promisc) { promisc_restore_list(&promisc); promisc_destroy(&promisc); }
- adjust_instance_count(PROCCOUNTFILE, -1);
- del_panel(list.panel); delwin(list.win); del_panel(list.borderpanel); delwin(list.borderwin); del_panel(statpanel); delwin(statwin);
- unmark_facility(TCPUDPIDFILE, ifname); update_panels(); doupdate(); destroyportlist(&list);
-- 1.7.11.5
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- src/iptraf.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/iptraf.c b/src/iptraf.c index 15789a9..9fe7138 100644 --- a/src/iptraf.c +++ b/src/iptraf.c @@ -34,6 +34,10 @@ An IP Network Statistics Utility #define WITHALL 1 #define WITHOUTALL 0
+#ifndef IPTRAF_PIDFILE +#define IPTRAF_PIDFILE "/var/run/iptraf-ng.pid" +#endif + const char *ALLSPEC = "all";
#define CMD(name, h) { .cmd = #name, .fn = cmd_##name, .help = h } @@ -283,6 +287,29 @@ static struct options iptraf_ng_options[] = { OPT_END() };
+static int create_pidfile(void) +{ + int fd = open(IPTRAF_PIDFILE, O_WRONLY|O_CREAT, 0644); + if (fd < 0) { + perror("can not open "IPTRAF_PIDFILE); + return -1; + } + + if (lockf(fd, F_TLOCK, 0) < 0) { + perror("can not lock file "IPTRAF_PIDFILE); + return -1; + } + + fcntl(fd, F_SETFD, FD_CLOEXEC); + + char buf[sizeof(long) * 3 + 2]; + int len = sprintf(buf, "%lu\n", (long) getpid()); + write(fd, buf, len); + ftruncate(fd, len); + /* we leak opened+locked fd intentionally */ + return 0; +} + static void sanitize_dir(const char *dir) { /* Check whether LOCKDIR exists (/var/run is on a tmpfs in Ubuntu) */ @@ -397,6 +424,12 @@ int main(int argc, char **argv)
loadoptions();
+ + if (create_pidfile() < 0) + goto cleanup; + + int pidfile_created = 1; + /* * If a facility is directly invoked from the command line, check for * a daemonization request @@ -414,9 +447,10 @@ int main(int argc, char **argv) options.logging = 1; break; case -1: /* error */ - die("Fork error, %s cannot run in background", IPTRAF_NAME); + error("Fork error, %s cannot run in background", IPTRAF_NAME); + goto cleanup; default: /* parent */ - exit(0); + goto cleanup; } }
@@ -496,10 +530,13 @@ int main(int argc, char **argv) else program_interface();
+cleanup: + if (pidfile_created) + unlink(IPTRAF_PIDFILE); erase(); update_panels(); doupdate(); endwin();
- return (0); + return 0; }
See comments in the code.
Vita
On Mon, Sep 03, 2012 at 02:06:13PM +0200, Nikola Pajkovsky wrote:
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com
src/iptraf.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/iptraf.c b/src/iptraf.c index 15789a9..9fe7138 100644 --- a/src/iptraf.c +++ b/src/iptraf.c @@ -34,6 +34,10 @@ An IP Network Statistics Utility #define WITHALL 1 #define WITHOUTALL 0
+#ifndef IPTRAF_PIDFILE +#define IPTRAF_PIDFILE "/var/run/iptraf-ng.pid" +#endif
Could you, please, make it configurable from the command line?
const char *ALLSPEC = "all";
#define CMD(name, h) { .cmd = #name, .fn = cmd_##name, .help = h } @@ -283,6 +287,29 @@ static struct options iptraf_ng_options[] = { OPT_END() };
+static int create_pidfile(void) +{
- int fd = open(IPTRAF_PIDFILE, O_WRONLY|O_CREAT, 0644);
- if (fd < 0) {
perror("can not open "IPTRAF_PIDFILE);return -1;- }
- if (lockf(fd, F_TLOCK, 0) < 0) {
perror("can not lock file "IPTRAF_PIDFILE);return -1;- }
- fcntl(fd, F_SETFD, FD_CLOEXEC);
char buf[sizeof(long) * 3 + 2];int len = sprintf(buf, "%lu\n", (long) getpid());write(fd, buf, len);ftruncate(fd, len);/* we leak opened+locked fd intentionally */return 0;
^^^^^^^^ change this to tabs!
+}
static void sanitize_dir(const char *dir) { /* Check whether LOCKDIR exists (/var/run is on a tmpfs in Ubuntu) */ @@ -397,6 +424,12 @@ int main(int argc, char **argv)
loadoptions();
The next part should go somewhere to the line 371 after this one:
if (help_opt) parse_usage_and_die(iptraf_ng_usage, iptraf_ng_options);
- if (create_pidfile() < 0)
goto cleanup;
This is before ncurses initialization. Should only remove pidfile.
- int pidfile_created = 1;
- /*
- If a facility is directly invoked from the command line, check for
- a daemonization request
@@ -414,9 +447,10 @@ int main(int argc, char **argv) options.logging = 1; break; case -1: /* error */
die("Fork error, %s cannot run in background", IPTRAF_NAME);
error("Fork error, %s cannot run in background", IPTRAF_NAME);goto cleanup;
exit(0);
goto cleanup;@@ -496,10 +530,13 @@ int main(int argc, char **argv) else program_interface();
This cleanup should go after ncurses finalization.
+cleanup:
- if (pidfile_created)
unlink(IPTRAF_PIDFILE);
Here.
- return (0);
- return 0;
}
1.7.11.5
See one more comment in the code.
Vita
On Mon, Sep 03, 2012 at 02:06:13PM +0200, Nikola Pajkovsky wrote:
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com
src/iptraf.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/iptraf.c b/src/iptraf.c index 15789a9..9fe7138 100644 --- a/src/iptraf.c +++ b/src/iptraf.c @@ -34,6 +34,10 @@ An IP Network Statistics Utility #define WITHALL 1 #define WITHOUTALL 0
+#ifndef IPTRAF_PIDFILE +#define IPTRAF_PIDFILE "/var/run/iptraf-ng.pid" +#endif
Could you, please, make it configurable from the command line?
const char *ALLSPEC = "all";
#define CMD(name, h) { .cmd = #name, .fn = cmd_##name, .help = h } @@ -283,6 +287,29 @@ static struct options iptraf_ng_options[] = { OPT_END() };
+static int create_pidfile(void) +{
- int fd = open(IPTRAF_PIDFILE, O_WRONLY|O_CREAT, 0644);
- if (fd < 0) {
perror("can not open "IPTRAF_PIDFILE);return -1;- }
- if (lockf(fd, F_TLOCK, 0) < 0) {
perror("can not lock file "IPTRAF_PIDFILE);return -1;- }
^^^^ Thease are not much descriptive errors printed to user whe ne tries to launch one more instance of iptraf-ng. Either be quiet here in create_pidfile() and print error after call to this func or print more descriptive error message here in create_pidfile().
Vita
- fcntl(fd, F_SETFD, FD_CLOEXEC);
char buf[sizeof(long) * 3 + 2];int len = sprintf(buf, "%lu\n", (long) getpid());write(fd, buf, len);ftruncate(fd, len);/* we leak opened+locked fd intentionally */return 0;^^^^^^^^ change this to tabs!
+}
static void sanitize_dir(const char *dir) { /* Check whether LOCKDIR exists (/var/run is on a tmpfs in Ubuntu) */ @@ -397,6 +424,12 @@ int main(int argc, char **argv)
loadoptions();
The next part should go somewhere to the line 371 after this one:
if (help_opt) parse_usage_and_die(iptraf_ng_usage, iptraf_ng_options);
- if (create_pidfile() < 0)
goto cleanup;This is before ncurses initialization. Should only remove pidfile.
- int pidfile_created = 1;
- /*
- If a facility is directly invoked from the command line, check for
- a daemonization request
@@ -414,9 +447,10 @@ int main(int argc, char **argv) options.logging = 1; break; case -1: /* error */
die("Fork error, %s cannot run in background", IPTRAF_NAME);
error("Fork error, %s cannot run in background", IPTRAF_NAME);goto cleanup;
exit(0);
goto cleanup;@@ -496,10 +530,13 @@ int main(int argc, char **argv) else program_interface();
This cleanup should go after ncurses finalization.
+cleanup:
- if (pidfile_created)
unlink(IPTRAF_PIDFILE);Here.
- return (0);
- return 0;
}
1.7.11.5
see comments below in the code
Vita
On Mon, Sep 03, 2012 at 02:06:11PM +0200, Nikola Pajkovsky wrote:
code was so difficult to read, that I had to rewrite it from scratch. Original code was using list structure designed only for promisc code, and that means it has to have its own adding / removing / traversing code. Not smart idea. Also I removed the code which stores and loads from disk state of promosic mode. Code does:
init path init_promisc_list(&promisc_list); save_promisc_list(promisc_list); srpromisc(1, promisc_list); destroy_promisc_list(&promisc_list);
... ...exit/error path load_promisc_list(&promisc_list); srpromisc(0, promisc_list); destroy_promisc_list(&promisc_list);
now it does
init path init_promisc_list(&promisc_list); promisc_set(promisc_list);
... ...exit/error path promisc_restore(promisc_list); destroy_promisc_list(&promisc_list);
Please, change commit message to match reality of the patch.
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com
src/detstats.c | 15 ++-- src/hostmon.c | 16 ++-- src/ifaces.c | 2 +- src/ifaces.h | 4 +- src/ifstats.c | 16 ++-- src/iptraf-ng-compat.h | 3 + src/itrafmon.c | 17 ++-- src/list.h | 6 ++ src/pktsize.c | 14 ++-- src/promisc.c | 212 +++++++++++++++---------------------------------- src/promisc.h | 30 +++---- src/serv.c | 14 ++-- 12 files changed, 121 insertions(+), 228 deletions(-)
diff --git a/src/detstats.c b/src/detstats.c index c1e415f..43b442a 100644 --- a/src/detstats.c +++ b/src/detstats.c @@ -291,7 +291,6 @@ void detstats(char *iface, time_t facilitytime) unsigned long peakpps_in = 0; unsigned long peakpps_out = 0;
struct promisc_states *promisc_list; int fd;
/*
@@ -312,11 +311,10 @@ void detstats(char *iface, time_t facilitytime) return; }
- if (first_active_facility() && options.promisc) {
init_promisc_list(&promisc_list);save_promisc_list(promisc_list);srpromisc(1, promisc_list);destroy_promisc_list(&promisc_list);
LIST_HEAD(promisc);
if (options.promisc && first_active_facility()) {
promisc_init(&promisc, iface);promisc_set_list(&promisc);}
adjust_instance_count(PROCCOUNTFILE, 1);
@@ -594,9 +592,8 @@ err: rate_destroy(&rate);
if (options.promisc && is_last_instance()) {
load_promisc_list(&promisc_list);srpromisc(0, promisc_list);destroy_promisc_list(&promisc_list);
promisc_restore_list(&promisc);promisc_destroy(&promisc);}
adjust_instance_count(PROCCOUNTFILE, -1);
diff --git a/src/hostmon.c b/src/hostmon.c index 21b1797..b807926 100644 --- a/src/hostmon.c +++ b/src/hostmon.c @@ -778,8 +778,6 @@ void hostmon(time_t facilitytime, char *ifptr)
int fd;
- struct promisc_states *promisc_list;
- if (!facility_active(LANMONIDFILE, ifptr)) mark_facility(LANMONIDFILE, "LAN monitor", ifptr); else {
@@ -796,11 +794,10 @@ void hostmon(time_t facilitytime, char *ifptr) } }
- if (first_active_facility() && options.promisc) {
init_promisc_list(&promisc_list);save_promisc_list(promisc_list);srpromisc(1, promisc_list);destroy_promisc_list(&promisc_list);
LIST_HEAD(promisc);
if (options.promisc && first_active_facility()) {
promisc_init(&promisc, ifptr);promisc_set_list(&promisc);}
adjust_instance_count(PROCCOUNTFILE, 1);
@@ -1031,9 +1028,8 @@ err_close:
err: if (options.promisc && is_last_instance()) {
load_promisc_list(&promisc_list);srpromisc(0, promisc_list);destroy_promisc_list(&promisc_list);
promisc_restore_list(&promisc);promisc_destroy(&promisc);}
adjust_instance_count(PROCCOUNTFILE, -1);
diff --git a/src/ifaces.c b/src/ifaces.c index 88151ef..437d5bb 100644 --- a/src/ifaces.c +++ b/src/ifaces.c @@ -211,7 +211,7 @@ err: /* need to preserve errno across call to close() */ return ir; }
-int dev_set_promisc(char *ifname) +int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); } diff --git a/src/ifaces.h b/src/ifaces.h index d753cac..f2eee94 100644 --- a/src/ifaces.h +++ b/src/ifaces.h @@ -17,8 +17,8 @@ int dev_get_mtu(const char *iface); int dev_get_flags(const char *iface); int dev_set_flags(const char *iface, int flags); int dev_clear_flags(const char *iface, int flags); -int dev_set_promisc(char *ifname); -int dev_clear_promisc(char *ifname); +int dev_set_promisc(const char *ifname); +int dev_clear_promisc(const char *ifname); int dev_get_ifname(int ifindex, char *ifname); int dev_bind_ifindex(const int fd, const int ifindex); int dev_bind_ifname(const int fd, const char const *ifname); diff --git a/src/ifstats.c b/src/ifstats.c index 8214838..f8943bc 100644 --- a/src/ifstats.c +++ b/src/ifstats.c @@ -451,8 +451,6 @@ void ifstats(time_t facilitytime) time_t startlog = 0; struct timeval updtime;
- struct promisc_states *promisc_list;
- if (!facility_active(GSTATIDFILE, "")) mark_facility(GSTATIDFILE, "general interface statistics", ""); else {
@@ -469,11 +467,10 @@ void ifstats(time_t facilitytime)
initiftab(&table);
- if (first_active_facility() && options.promisc) {
init_promisc_list(&promisc_list);save_promisc_list(promisc_list);srpromisc(1, promisc_list);destroy_promisc_list(&promisc_list);
LIST_HEAD(promisc);
if (options.promisc && first_active_facility()) {
promisc_init(&promisc, NULL);promisc_set_list(&promisc);}
adjust_instance_count(PROCCOUNTFILE, 1);
@@ -630,9 +627,8 @@ void ifstats(time_t facilitytime)
err: if (options.promisc && is_last_instance()) {
load_promisc_list(&promisc_list);srpromisc(0, promisc_list);destroy_promisc_list(&promisc_list);
promisc_restore_list(&promisc);promisc_destroy(&promisc);}
adjust_instance_count(PROCCOUNTFILE, -1);
diff --git a/src/iptraf-ng-compat.h b/src/iptraf-ng-compat.h index 64b3db0..1e1ef3f 100644 --- a/src/iptraf-ng-compat.h +++ b/src/iptraf-ng-compat.h @@ -86,6 +86,9 @@ } \ } while (0)
+#define likely(x) __builtin_expect(!!(x), 1) +#define unlikely(x) __builtin_expect(!!(x), 0)
extern int is_first_instance; /* in iptraf.c */ extern int daemonized; extern int exitloop; diff --git a/src/itrafmon.c b/src/itrafmon.c index b35468f..a2cbc52 100644 --- a/src/itrafmon.c +++ b/src/itrafmon.c @@ -599,8 +599,6 @@ void ipmon(time_t facilitytime, char *ifptr) int keymode = 0; char msgstring[80];
struct promisc_states *promisc_list;
int rvnfd = 0;
int instance_id;
@@ -628,12 +626,10 @@ void ipmon(time_t facilitytime, char *ifptr) } }
- if (options.promisc) {
if (first_active_facility()) {init_promisc_list(&promisc_list);save_promisc_list(promisc_list);srpromisc(1, promisc_list);}
LIST_HEAD(promisc);
if (options.promisc && first_active_facility()) {
promisc_init(&promisc, ifptr);promisc_set_list(&promisc);}
/*
@@ -1188,9 +1184,8 @@ err: close_rvn_socket(rvnfd);
if (options.promisc && is_last_instance()) {
load_promisc_list(&promisc_list);srpromisc(0, promisc_list);destroy_promisc_list(&promisc_list);
promisc_restore_list(&promisc);promisc_destroy(&promisc);}
attrset(STDATTR);
diff --git a/src/list.h b/src/list.h index daa7628..a7b495e 100644 --- a/src/list.h +++ b/src/list.h @@ -80,4 +80,10 @@ static inline int list_empty(const struct list_head *head) &pos->member != (head); \ pos = list_entry(pos->member.next, typeof(*pos), member))
+#define list_for_each_entry_safe(pos, n, head, member) \
- for (pos = list_entry((head)->next, typeof(*pos), member), \
n = list_entry(pos->member.next, typeof(*pos), member); \&pos->member != (head); \pos = n, n = list_entry(n->member.next, typeof(*n), member))#endif /* IPTRAF_NG_LIST_H */ diff --git a/src/pktsize.c b/src/pktsize.c index 44bc61f..b54d79b 100644 --- a/src/pktsize.c +++ b/src/pktsize.c @@ -161,8 +161,6 @@ void packet_size_breakdown(char *ifname, time_t facilitytime) int logging = options.logging; FILE *logfile = NULL;
struct promisc_states *promisc_list;
int fd;
if (!facility_active(PKTSIZEIDFILE, ifname))
@@ -241,11 +239,10 @@ void packet_size_breakdown(char *ifname, time_t facilitytime) updtime = tv; now = starttime = startlog = timeint = tv.tv_sec;
- LIST_HEAD(promisc); if (first_active_facility() && options.promisc) {
init_promisc_list(&promisc_list);save_promisc_list(promisc_list);srpromisc(1, promisc_list);destroy_promisc_list(&promisc_list);
promisc_init(&promisc, ifname);promisc_set_list(&promisc);}
adjust_instance_count(PROCCOUNTFILE, 1);
@@ -337,9 +334,8 @@ err: }
if (options.promisc && is_last_instance()) {
load_promisc_list(&promisc_list);srpromisc(0, promisc_list);destroy_promisc_list(&promisc_list);
promisc_restore_list(&promisc);promisc_destroy(&promisc);}
adjust_instance_count(PROCCOUNTFILE, -1);
diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags)
maybe change the name to promisc_add_dev()
{
- FILE *fd;
- char buf[IFNAMSIZ];
- struct promisc_states *ptmp;
- struct promisc_states *tail = NULL;
- *list = NULL;
- fd = open_procnetdev();
- while (get_next_iface(fd, buf, sizeof(buf))) {
if (strcmp(buf, "") != 0) {ptmp = xmalloc(sizeof(struct promisc_states));strcpy(ptmp->params.ifname, buf);if (*list == NULL) {*list = ptmp;} elsetail->next_entry = ptmp;tail = ptmp;ptmp->next_entry = NULL;/** Retrieve and save interface flags*/if ((strncmp(buf, "eth", 3) == 0)|| (strncmp(buf, "ra", 2) == 0)|| (strncmp(buf, "fddi", 4) == 0)|| (strncmp(buf, "tr", 2) == 0)|| (strncmp(buf, "ath", 3) == 0)|| (strncmp(buf, "bnep", 4) == 0)|| (strncmp(buf, "ni", 2) == 0)|| (strncmp(buf, "tap", 3) == 0)|| (strncmp(buf, "dummy", 5) == 0)|| (strncmp(buf, "br", 2) == 0)|| (strncmp(buf, "vmnet", 5) == 0)|| (strncmp(ptmp->params.ifname, "wvlan", 4) == 0)|| (strncmp(ptmp->params.ifname, "lec", 3) == 0)) {int flags = dev_get_flags(buf);if (flags < 0) {write_error("Unable to obtain interface parameters for %s",buf);ptmp->params.state_valid = 0;} else {ptmp->params.saved_state = flags;ptmp->params.state_valid = 1;}}}- }
-}
- struct promisc_list *p = xmallocz(sizeof(*p));
- strcpy(p->ifname, dev_name);
- INIT_LIST_HEAD(&p->list);
-/*
- Save interfaces and their states to a temporary file. Used only by the
- first IPTraf instance. Needed in case there are subsequent, simultaneous
- instances of IPTraf, which may still need promiscuous mode even after
- the first instance exits. These subsequent instances will need to restore
- the promiscuous state from this file.
- */
- p->flags = flags;
- list_add_tail(&p->list, promisc);
+}
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name)
This function should go into ifaces.c
{
- int fd;
- struct promisc_states *ptmp = list;
- fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
- if (fd < 0) {
write_error("Unable to save interface flags");return;
- int flags = dev_get_flags(dev_name);
- if (flags < 0) {
write_error("Unable to obtain interface parameters for %s",dev_name);return -1;
- while (ptmp != NULL) {
write(fd, &(ptmp->params), sizeof(struct promisc_params));ptmp = ptmp->next_entry;- }
- if (flags & IFF_PROMISC)
return -1;
-1 should be reserved only for error states; this function should return -1/0/1 on error (getting flags)/promisc not set/promisc set
- close(fd);
- return flags;
}
-/*
- Load promiscuous states into list
- */
-void load_promisc_list(struct promisc_states **list) +void promisc_init(struct list_head *promisc, const char *device_name) {
- int fd;
- struct promisc_states *ptmp = NULL;
- struct promisc_states *tail = NULL;
- int br;
- if (device_name) {
int flags = dev_promisc_flag(device_name);if (flags < 0)return;
- fd = open(PROMISCLISTFILE, O_RDONLY);
add_dev(promisc, device_name, flags);
if (fd < 0) {
write_error("Unable to retrieve saved interface flags");*list = NULL;return; }
do {
ptmp = xmalloc(sizeof(struct promisc_states));br = read(fd, &(ptmp->params), sizeof(struct promisc_params));
- FILE *fp = open_procnetdev();
- if (!fp)
die_errno("%s: open_procnetdev", __func__);- char dev_name[IFNAMSIZ];
- while (get_next_iface(fp, dev_name, sizeof(dev_name))) {
if (!strcmp(dev_name, ""))continue;
if (br > 0) {if (tail != NULL)tail->next_entry = ptmp;else*list = ptmp;
int flags = dev_promisc_flag(dev_name);if (flags < 0)continue;
ptmp->next_entry = NULL;tail = ptmp;} elsefree(ptmp);- } while (br > 0);
add_dev(promisc, dev_name, flags);- }
- close(fd);
- fclose(fp);
}
-/*
- Set/restore interface promiscuous mode.
- */
+static void promisc_set(const char *dev_name) +{
- int r = dev_set_promisc(dev_name);
- if (r < 0)
write_error("Failed to set promiscuous mode on %s", dev_name);+}
-void srpromisc(int mode, struct promisc_states *list) +void promisc_set_list(struct list_head *promisc) {
- struct promisc_states *ptmp;
- ptmp = list;
- while (ptmp != NULL) {
if (((strncmp(ptmp->params.ifname, "eth", 3) == 0)|| (strncmp(ptmp->params.ifname, "fddi", 4) == 0)|| (strncmp(ptmp->params.ifname, "tr", 2) == 0)|| (strncmp(ptmp->params.ifname, "ra", 2) == 0)|| (strncmp(ptmp->params.ifname, "ath", 3) == 0)|| (strncmp(ptmp->params.ifname, "wvlan", 4) == 0)|| (strncmp(ptmp->params.ifname, "lec", 3) == 0))&& (ptmp->params.state_valid)) {if (mode) {/* set promiscuous */int r = dev_set_promisc(ptmp->params.ifname);if(r < 0)write_error("Failed to set promiscuous mode on %s", ptmp->params.ifname);} else {/* restore saved state */if (ptmp->params.saved_state & IFF_PROMISC)/* was promisc, so leave it as is */continue;/* wasn't promisc, clear it */int r = dev_clear_promisc(ptmp->params.ifname);if(r < 0)write_error("Failed to clear promiscuous mode on %s", ptmp->params.ifname);}}ptmp = ptmp->next_entry;- }
- struct promisc_list *entry = NULL;
- list_for_each_entry(entry, promisc, list)
promisc_set(entry->ifname);}
-void destroy_promisc_list(struct promisc_states **list)
I would add this comment:
/* restore: since we have only devices which need to clear promisc flag * on the list, call dev_clear_promisc() on it */
+static void promisc_restore(const char *dev_name) {
- struct promisc_states *ptmp = *list;
- struct promisc_states *ctmp;
- int r = dev_clear_promisc(dev_name);
- if (r < 0)
write_error("Failed to clear promiscuous mode on %s", dev_name);+}
- if (ptmp != NULL)
ctmp = ptmp->next_entry;+void promisc_restore_list(struct list_head *promisc) +{
- struct promisc_list *entry = NULL;
- list_for_each_entry(entry, promisc, list) {
if (unlikely(entry->flags & IFF_PROMISC))continue;promisc_restore(entry->ifname);- }
+}
- while (ptmp != NULL) {
free(ptmp);ptmp = ctmp;if (ctmp != NULL)ctmp = ctmp->next_entry;+void promisc_destroy(struct list_head *promisc) +{
- struct promisc_list *entry, *tmp;
- list_for_each_entry_safe(entry, tmp, promisc, list) {
list_del(&entry->list);free(entry);} diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/*
- promisc.h - definitions for promiscuous state save/recovery
- Thanks to Holger Friese
- evildead@bs-pc5.et-inf.fho-emden.de for the base patch.
- Applied it, but then additional issues came up and I ended up doing more
- than slight modifications. struct iflist is becoming way too large for
- comfort and for something as little as this.
- */
+#include "list.h"
-struct promisc_params { +struct promisc_list {
- struct list_head list; char ifname[IFNAMSIZ];
- int saved_state;
- int state_valid;
- int flags;
I would rather see only "int promisc:1" with boolean meaning of promiscuous mode set or not set.
};
-struct promisc_states {
- struct promisc_params params;
- struct promisc_states *next_entry;
-};
-void init_promisc_list(struct promisc_states **list); -void save_promisc_list(struct promisc_states *list); -void load_promisc_list(struct promisc_states **list); -void srpromisc(int mode, struct promisc_states *promisc_list); -void destroy_promisc_list(struct promisc_states **list); +void promisc_init(struct list_head *promisc, const char *device_name); +void promisc_destroy(struct list_head *promisc);
+void promisc_set_list(struct list_head *promisc); +void promisc_restore_list(struct list_head *promisc);
#endif /* IPTRAF_NG_PROMISC_H */ diff --git a/src/serv.c b/src/serv.c index 513cc94..84e4e94 100644 --- a/src/serv.c +++ b/src/serv.c @@ -779,8 +779,6 @@ void servmon(char *ifname, time_t facilitytime)
FILE *logfile = NULL;
- struct promisc_states *promisc_list;
- WINDOW *sortwin; PANEL *sortpanel;
@@ -812,11 +810,10 @@ void servmon(char *ifname, time_t facilitytime)
loadaddports(&ports);
- LIST_HEAD(promisc); if (first_active_facility() && options.promisc) {
init_promisc_list(&promisc_list);save_promisc_list(promisc_list);srpromisc(1, promisc_list);destroy_promisc_list(&promisc_list);
promisc_init(&promisc, ifname);promisc_set_list(&promisc);}
adjust_instance_count(PROCCOUNTFILE, 1);
@@ -1091,9 +1088,8 @@ err: endservent();
if (options.promisc && is_last_instance()) {
load_promisc_list(&promisc_list);srpromisc(0, promisc_list);destroy_promisc_list(&promisc_list);
promisc_restore_list(&promisc);promisc_destroy(&promisc);}
adjust_instance_count(PROCCOUNTFILE, -1);
-- 1.7.11.5
Vitezslav Samel vitezslav@samel.cz writes:
Please, change commit message to match reality of the patch.
ech...
diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags)
maybe change the name to promisc_add_dev()
it's small static helper which adds device to promisc list. It could be named with promisc_ prefix. I don't care. I'm pretty happy even without promisc_ prefix
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name)
This function should go into ifaces.c
why not
{
- int fd;
- struct promisc_states *ptmp = list;
- fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
- if (fd < 0) {
write_error("Unable to save interface flags");return;
- int flags = dev_get_flags(dev_name);
- if (flags < 0) {
write_error("Unable to obtain interface parameters for %s",dev_name);return -1;
- while (ptmp != NULL) {
write(fd, &(ptmp->params), sizeof(struct promisc_params));ptmp = ptmp->next_entry;- }
- if (flags & IFF_PROMISC)
return -1;-1 should be reserved only for error states; this function should return -1/0/1 on error (getting flags)/promisc not set/promisc set
that func is about: 'if you can't give me flags about device, you are not worth it. If you already have set promisc flags, you are not worth it'.
I don't see why do we have to distinguish err and not set.
I would add this comment:
/* restore: since we have only devices which need to clear promisc flag
- on the list, call dev_clear_promisc() on it
*/
if we need comment here, that means something is wrong even with that simple function.
int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); }
int dev_clear_promisc(char *ifname) { return dev_clear_flags(ifname, IFF_PROMISC); }
better question is why this is not a macro?
#define dev_set_promisc(dev) dev_set_flags((dev), IFF_PROMISC) #define dev_clear_promisc(dev) dev_clear_flags((dev), IFF_PROMISC)
it would make things more simple.
diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/*
- promisc.h - definitions for promiscuous state save/recovery
- Thanks to Holger Friese
- evildead@bs-pc5.et-inf.fho-emden.de for the base patch.
- Applied it, but then additional issues came up and I ended up doing more
- than slight modifications. struct iflist is becoming way too large for
- comfort and for something as little as this.
- */
+#include "list.h"
-struct promisc_params { +struct promisc_list {
- struct list_head list; char ifname[IFNAMSIZ];
- int saved_state;
- int state_valid;
- int flags;
I would rather see only "int promisc:1" with boolean meaning of promiscuous mode set or not set.
if we guarantee, that promics_list will contains only devices with flags up, that we don't even need this. I'm not so sure it this is good idea.
On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel vitezslav@samel.cz writes:
Please, change commit message to match reality of the patch.
ech...
diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags)
maybe change the name to promisc_add_dev()
it's small static helper which adds device to promisc list. It could be named with promisc_ prefix. I don't care. I'm pretty happy even without promisc_ prefix
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name)
This function should go into ifaces.c
why not
{
- int fd;
- struct promisc_states *ptmp = list;
- fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
- if (fd < 0) {
write_error("Unable to save interface flags");return;
- int flags = dev_get_flags(dev_name);
- if (flags < 0) {
write_error("Unable to obtain interface parameters for %s",dev_name);return -1;
- while (ptmp != NULL) {
write(fd, &(ptmp->params), sizeof(struct promisc_params));ptmp = ptmp->next_entry;- }
- if (flags & IFF_PROMISC)
return -1;-1 should be reserved only for error states; this function should return -1/0/1 on error (getting flags)/promisc not set/promisc set
that func is about: 'if you can't give me flags about device, you are not worth it. If you already have set promisc flags, you are not worth it'.
I don't see why do we have to distinguish err and not set.
Then change the name of this function.
I would add this comment:
/* restore: since we have only devices which need to clear promisc flag
- on the list, call dev_clear_promisc() on it
*/
if we need comment here, that means something is wrong even with that simple function.
Because of naming of the function: restore is not the same as clear (promisc flag).
int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); }
int dev_clear_promisc(char *ifname) { return dev_clear_flags(ifname, IFF_PROMISC); }
better question is why this is not a macro?
#define dev_set_promisc(dev) dev_set_flags((dev), IFF_PROMISC) #define dev_clear_promisc(dev) dev_clear_flags((dev), IFF_PROMISC)
it would make things more simple.
I don't see where it is more simple.
diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/*
- promisc.h - definitions for promiscuous state save/recovery
- Thanks to Holger Friese
- evildead@bs-pc5.et-inf.fho-emden.de for the base patch.
- Applied it, but then additional issues came up and I ended up doing more
- than slight modifications. struct iflist is becoming way too large for
- comfort and for something as little as this.
- */
+#include "list.h"
-struct promisc_params { +struct promisc_list {
- struct list_head list; char ifname[IFNAMSIZ];
- int saved_state;
- int state_valid;
- int flags;
I would rather see only "int promisc:1" with boolean meaning of promiscuous mode set or not set.
if we guarantee, that promics_list will contains only devices with flags up, that we don't even need this. I'm not so sure it this is good idea.
I don't know what you mean by this: devices with flags up: are you testing if device is up? Functions in promisc.c should only know about promiscuity of interface and nothing else. And in promisc_restore_list() you could test only on entry->was_promiscuous and leave IFF_PROMISC testing to ifaces.c.
Vita
Vitezslav Samel vitezslav@samel.cz writes:
On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel vitezslav@samel.cz writes:
Please, change commit message to match reality of the patch.
ech...
diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags)
maybe change the name to promisc_add_dev()
it's small static helper which adds device to promisc list. It could be named with promisc_ prefix. I don't care. I'm pretty happy even without promisc_ prefix
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name)
This function should go into ifaces.c
why not
{
- int fd;
- struct promisc_states *ptmp = list;
- fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
- if (fd < 0) {
write_error("Unable to save interface flags");return;
- int flags = dev_get_flags(dev_name);
- if (flags < 0) {
write_error("Unable to obtain interface parameters for %s",dev_name);return -1;
- while (ptmp != NULL) {
write(fd, &(ptmp->params), sizeof(struct promisc_params));ptmp = ptmp->next_entry;- }
- if (flags & IFF_PROMISC)
return -1;-1 should be reserved only for error states; this function should return -1/0/1 on error (getting flags)/promisc not set/promisc set
that func is about: 'if you can't give me flags about device, you are not worth it. If you already have set promisc flags, you are not worth it'.
I don't see why do we have to distinguish err and not set.
Then change the name of this function.
I would add this comment:
/* restore: since we have only devices which need to clear promisc flag
- on the list, call dev_clear_promisc() on it
*/
if we need comment here, that means something is wrong even with that simple function.
Because of naming of the function: restore is not the same as clear (promisc flag).
int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); }
int dev_clear_promisc(char *ifname) { return dev_clear_flags(ifname, IFF_PROMISC); }
better question is why this is not a macro?
#define dev_set_promisc(dev) dev_set_flags((dev), IFF_PROMISC) #define dev_clear_promisc(dev) dev_clear_flags((dev), IFF_PROMISC)
it would make things more simple.
I don't see where it is more simple.
diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/*
- promisc.h - definitions for promiscuous state save/recovery
- Thanks to Holger Friese
- evildead@bs-pc5.et-inf.fho-emden.de for the base patch.
- Applied it, but then additional issues came up and I ended up doing more
- than slight modifications. struct iflist is becoming way too large for
- comfort and for something as little as this.
- */
+#include "list.h"
-struct promisc_params { +struct promisc_list {
- struct list_head list; char ifname[IFNAMSIZ];
- int saved_state;
- int state_valid;
- int flags;
I would rather see only "int promisc:1" with boolean meaning of promiscuous mode set or not set.
if we guarantee, that promics_list will contains only devices with flags up, that we don't even need this. I'm not so sure it this is good idea.
I don't know what you mean by this: devices with flags up: are you testing if device is up? Functions in promisc.c should only know about promiscuity of interface and nothing else. And in promisc_restore_list() you could test only on entry->was_promiscuous and leave IFF_PROMISC testing to ifaces.c.
the sentence should be: if we guarantee, that promics_list will contains only devices with promisc flag up, that we don't even need flags/promisc member.
On Tue, Sep 04, 2012 at 03:21:36PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel vitezslav@samel.cz writes:
On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel vitezslav@samel.cz writes:
Please, change commit message to match reality of the patch.
ech...
diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags)
maybe change the name to promisc_add_dev()
it's small static helper which adds device to promisc list. It could be named with promisc_ prefix. I don't care. I'm pretty happy even without promisc_ prefix
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name)
This function should go into ifaces.c
why not
{
- int fd;
- struct promisc_states *ptmp = list;
- fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
- if (fd < 0) {
write_error("Unable to save interface flags");return;
- int flags = dev_get_flags(dev_name);
- if (flags < 0) {
write_error("Unable to obtain interface parameters for %s",dev_name);return -1;
- while (ptmp != NULL) {
write(fd, &(ptmp->params), sizeof(struct promisc_params));ptmp = ptmp->next_entry;- }
- if (flags & IFF_PROMISC)
return -1;-1 should be reserved only for error states; this function should return -1/0/1 on error (getting flags)/promisc not set/promisc set
that func is about: 'if you can't give me flags about device, you are not worth it. If you already have set promisc flags, you are not worth it'.
I don't see why do we have to distinguish err and not set.
Then change the name of this function.
I would add this comment:
/* restore: since we have only devices which need to clear promisc flag
- on the list, call dev_clear_promisc() on it
*/
if we need comment here, that means something is wrong even with that simple function.
Because of naming of the function: restore is not the same as clear (promisc flag).
int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); }
int dev_clear_promisc(char *ifname) { return dev_clear_flags(ifname, IFF_PROMISC); }
better question is why this is not a macro?
#define dev_set_promisc(dev) dev_set_flags((dev), IFF_PROMISC) #define dev_clear_promisc(dev) dev_clear_flags((dev), IFF_PROMISC)
it would make things more simple.
I don't see where it is more simple.
diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/*
- promisc.h - definitions for promiscuous state save/recovery
- Thanks to Holger Friese
- evildead@bs-pc5.et-inf.fho-emden.de for the base patch.
- Applied it, but then additional issues came up and I ended up doing more
- than slight modifications. struct iflist is becoming way too large for
- comfort and for something as little as this.
- */
+#include "list.h"
-struct promisc_params { +struct promisc_list {
- struct list_head list; char ifname[IFNAMSIZ];
- int saved_state;
- int state_valid;
- int flags;
I would rather see only "int promisc:1" with boolean meaning of promiscuous mode set or not set.
if we guarantee, that promics_list will contains only devices with flags up, that we don't even need this. I'm not so sure it this is good idea.
I don't know what you mean by this: devices with flags up: are you testing if device is up? Functions in promisc.c should only know about promiscuity of interface and nothing else. And in promisc_restore_list() you could test only on entry->was_promiscuous and leave IFF_PROMISC testing to ifaces.c.
the sentence should be: if we guarantee, that promics_list will contains only devices with promisc flag up, that we don't even need flags/promisc member.
That's the best solution: only list with interface name.
Vita
Vitezslav Samel vitezslav@samel.cz writes:
On Tue, Sep 04, 2012 at 03:21:36PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel vitezslav@samel.cz writes:
On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel vitezslav@samel.cz writes:
Please, change commit message to match reality of the patch.
ech...
diff --git a/src/promisc.c b/src/promisc.c index a0f5364..cb3e1dd 100644 --- a/src/promisc.c +++ b/src/promisc.c @@ -17,178 +17,98 @@ promisc.c - handles the promiscuous mode flag for the Ethernet/FDDI/
#define PROMISC_MSG_MAX 80
-void init_promisc_list(struct promisc_states **list) +static void add_dev(struct list_head *promisc, const char *dev_name, int flags)
maybe change the name to promisc_add_dev()
it's small static helper which adds device to promisc list. It could be named with promisc_ prefix. I don't care. I'm pretty happy even without promisc_ prefix
-void save_promisc_list(struct promisc_states *list) +static int dev_promisc_flag(const char *dev_name)
This function should go into ifaces.c
why not
{
- int fd;
- struct promisc_states *ptmp = list;
- fd = open(PROMISCLISTFILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
- if (fd < 0) {
write_error("Unable to save interface flags");return;
- int flags = dev_get_flags(dev_name);
- if (flags < 0) {
write_error("Unable to obtain interface parameters for %s",dev_name);return -1;
- while (ptmp != NULL) {
write(fd, &(ptmp->params), sizeof(struct promisc_params));ptmp = ptmp->next_entry;- }
- if (flags & IFF_PROMISC)
return -1;-1 should be reserved only for error states; this function should return -1/0/1 on error (getting flags)/promisc not set/promisc set
that func is about: 'if you can't give me flags about device, you are not worth it. If you already have set promisc flags, you are not worth it'.
I don't see why do we have to distinguish err and not set.
Then change the name of this function.
I would add this comment:
/* restore: since we have only devices which need to clear promisc flag
- on the list, call dev_clear_promisc() on it
*/
if we need comment here, that means something is wrong even with that simple function.
Because of naming of the function: restore is not the same as clear (promisc flag).
int dev_set_promisc(const char *ifname) { return dev_set_flags(ifname, IFF_PROMISC); }
int dev_clear_promisc(char *ifname) { return dev_clear_flags(ifname, IFF_PROMISC); }
better question is why this is not a macro?
#define dev_set_promisc(dev) dev_set_flags((dev), IFF_PROMISC) #define dev_clear_promisc(dev) dev_clear_flags((dev), IFF_PROMISC)
it would make things more simple.
I don't see where it is more simple.
diff --git a/src/promisc.h b/src/promisc.h index f062ca1..1996982 100644 --- a/src/promisc.h +++ b/src/promisc.h @@ -1,31 +1,19 @@ #ifndef IPTRAF_NG_PROMISC_H #define IPTRAF_NG_PROMISC_H
-/*
- promisc.h - definitions for promiscuous state save/recovery
- Thanks to Holger Friese
- evildead@bs-pc5.et-inf.fho-emden.de for the base patch.
- Applied it, but then additional issues came up and I ended up doing more
- than slight modifications. struct iflist is becoming way too large for
- comfort and for something as little as this.
- */
+#include "list.h"
-struct promisc_params { +struct promisc_list {
- struct list_head list; char ifname[IFNAMSIZ];
- int saved_state;
- int state_valid;
- int flags;
I would rather see only "int promisc:1" with boolean meaning of promiscuous mode set or not set.
if we guarantee, that promics_list will contains only devices with flags up, that we don't even need this. I'm not so sure it this is good idea.
I don't know what you mean by this: devices with flags up: are you testing if device is up? Functions in promisc.c should only know about promiscuity of interface and nothing else. And in promisc_restore_list() you could test only on entry->was_promiscuous and leave IFF_PROMISC testing to ifaces.c.
the sentence should be: if we guarantee, that promics_list will contains only devices with promisc flag up, that we don't even need flags/promisc member.
That's the best solution: only list with interface name.
ok, it will be there
iptraf-ng@lists.fedorahosted.org
-
Nikola Pajkovsky -
Vitezslav Samel