7:06 a.m.
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(a)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(a)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);
--
1.7.11.5
7:06 a.m.
New subject: [PATCH 2/3] instances: knock out instaneces.*
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(a)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);
--
1.7.11.5
4:06 a.m.
New subject: [PATCH 2/3] instances: knock out instaneces.*
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(a)redhat.com>
Signed-off-by: Vitezslav Samel <vitezslav(a)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;
}
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);
--
1.7.11.5
7:06 a.m.
New subject: [PATCH 3/3] locking: only one instance of iptraf can be run
Signed-off-by: Nikola Pajkovsky <npajkovs(a)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;
}
--
1.7.11.5
4:59 a.m.
New subject: [PATCH 3/3] locking: only one instance of iptraf can be run
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(a)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;
default: /* parent */
- 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);
erase();
update_panels();
doupdate();
endwin();
Here.
- return (0);
+ return 0;
}
--
1.7.11.5
5:55 a.m.
New subject: [PATCH 3/3] locking: only one instance of iptraf can be run
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(a)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;
> default: /* parent */
> - 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);
> erase();
> update_panels();
> doupdate();
> endwin();
Here.
>
> - return (0);
> + return 0;
> }
> --
> 1.7.11.5
4:04 a.m.
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(a)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;
- } 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)
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;
- } 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)
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(a)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
7:44 a.m.
Vitezslav Samel <vitezslav(a)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(a)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.
--
Nikola
8:06 a.m.
On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel <vitezslav(a)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(a)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
8:21 a.m.
Vitezslav Samel <vitezslav(a)samel.cz> writes:
On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
> Vitezslav Samel <vitezslav(a)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(a)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.
--
Nikola
8:25 a.m.
On Tue, Sep 04, 2012 at 03:21:36PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel <vitezslav(a)samel.cz> writes:
> On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
>> Vitezslav Samel <vitezslav(a)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(a)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
8:33 a.m.
Vitezslav Samel <vitezslav(a)samel.cz> writes:
On Tue, Sep 04, 2012 at 03:21:36PM +0200, Nikola Pajkovsky wrote:
> Vitezslav Samel <vitezslav(a)samel.cz> writes:
>
> > On Tue, Sep 04, 2012 at 02:44:11PM +0200, Nikola Pajkovsky wrote:
> >> Vitezslav Samel <vitezslav(a)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(a)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
--
Nikola
4250
days inactive
4251
days old
iptraf-ng@lists.fedorahosted.org
11 comments
2 participants
participants (2)
-
Nikola Pajkovsky -
Vitezslav Samel