7:26 a.m.
Right now on every received packet we get the interface name
on which it arrived. This is not needed and is slow.
Make use of ifindex and retrieve interface name only when needed
(when capturing on "All interfaces").
Vitezslav Samel (3):
get the interface name only when needed
remove unused params from getpacket() and processpacket()
ifstats.c: detstats(): warning cleanup
src/hostmon.c | 19 ++++++++++++--
src/ifaces.c | 38 ++++++++++++++++++++++++++++++
src/ifaces.h | 2 +
src/ifstats.c | 70 ++++++++++++++++++++++++++++---------------------------
src/ifstats.h | 1 +
src/itrafmon.c | 19 ++++++++++++--
src/packet.c | 16 +-----------
src/packet.h | 4 +-
src/pktsize.c | 6 +---
src/serv.c | 5 +--
10 files changed, 117 insertions(+), 63 deletions(-)
--
1.7.8.4
7:26 a.m.
New subject: [PATCH 1/3] get the interface name only when needed
Right now on every received packet we get the interface name
on which it arrived. This is not needed and is slow.
Make use of ifindex and retrieve interface name only when needed
(when capturing on "All interfaces").
Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
---
src/hostmon.c | 19 +++++++++++++--
src/ifaces.c | 38 ++++++++++++++++++++++++++++++
src/ifaces.h | 2 +
src/ifstats.c | 69 +++++++++++++++++++++++++++++--------------------------
src/ifstats.h | 1 +
src/itrafmon.c | 19 +++++++++++++--
src/packet.c | 8 ++++--
src/pktsize.c | 5 +--
src/serv.c | 5 +--
9 files changed, 118 insertions(+), 48 deletions(-)
diff --git a/src/hostmon.c b/src/hostmon.c
index 59be4e9..d8854e8 100644
--- a/src/hostmon.c
+++ b/src/hostmon.c
@@ -678,7 +678,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
int is_ip;
int ch;
- char ifname[IFNAMSIZ];
+ char *ifname = ifptr;
struct timeval tv;
unsigned long starttime;
@@ -814,7 +814,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.tabwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.tabwin);
if (ch != ERR) {
if (keymode == 0) {
@@ -864,16 +864,29 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
}
}
if (br > 0) {
+ char ifnamebuf[IFNAMSIZ];
+
pkt_result =
processpacket(buf, &ipacket, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, ifname,
+ MATCH_OPPOSITE_USECONFIG, NULL,
0);
if (pkt_result != PACKET_OK)
continue;
+ if (!ifptr) {
+ /* we're capturing on "All interfaces", */
+ /* so get the name of the interface */
+ /* of this packet */
+ if (iface_get_ifname(fromaddr.sll_ifindex, ifname, sizeof(ifname)) != 0) {
+ write_error("Unable to get interface name");
+ break; /* can't get interface name, get out! */
+ }
+ ifname = ifnamebuf;
+ }
+
if ((fromaddr.sll_hatype == ARPHRD_ETHER)
|| (fromaddr.sll_hatype == ARPHRD_FDDI)
/* fix me
diff --git a/src/ifaces.c b/src/ifaces.c
index c1e0be5..8b47769 100644
--- a/src/ifaces.c
+++ b/src/ifaces.c
@@ -100,6 +100,44 @@ void err_iface_down(void)
write_error("Specified interface not active");
}
+int iface_get_ifindex(const char const *iface)
+{
+ int fd;
+ int ir;
+ struct ifreq ifr;
+
+ fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
+
+ strcpy(ifr.ifr_name, iface);
+ ir = ioctl(fd, SIOCGIFINDEX, &ifr);
+
+ close(fd);
+
+ if (ir != 0)
+ return(ir);
+
+ return(ifr.ifr_ifindex);
+}
+
+int iface_get_ifname(const int ifindex, char *ifname, int ifname_space)
+{
+ int fd;
+ int ir;
+ struct ifreq ifr;
+
+ fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
+
+ ifr.ifr_ifindex = ifindex;
+ ir = ioctl(fd, SIOCGIFNAME, &ifr);
+
+ close(fd);
+
+ if (ir == 0)
+ strncpy(ifname, ifr.ifr_name, ifname_space);
+
+ return(ir);
+}
+
void isdn_iface_check(int *fd, char *ifname)
{
if (*fd == -1) {
diff --git a/src/ifaces.h b/src/ifaces.h
index 5bf313d..d1b01e8 100644
--- a/src/ifaces.h
+++ b/src/ifaces.h
@@ -11,6 +11,8 @@ ifaces.h - prototype declaration for interface support determination
FILE *open_procnetdev(void);
int get_next_iface(FILE * fd, char *ifname, int n);
int iface_up(char *iface);
+extern int iface_get_ifindex(const char const *iface);
+extern int iface_get_ifname(const int ifindex, char *ifname, int ifname_space);
void err_iface_down(void);
void isdn_iface_check(int *fd, char *ifname);
char *gen_iface_msg(char *ifptr);
diff --git a/src/ifstats.c b/src/ifstats.c
index 9997a34..7a0de75 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -116,6 +116,8 @@ void initiflist(struct iflist **list)
}
while (get_next_iface(fd, ifname, sizeof(ifname))) {
+ int ifindex;
+
if (!*ifname)
continue;
@@ -131,6 +133,9 @@ void initiflist(struct iflist **list)
if (!iface_up(ifname))
continue;
+ ifindex = iface_get_ifindex(ifname);
+ if(ifindex < 0)
+ continue;
/*
* At this point, the interface is now sure to be up and running.
*/
@@ -138,6 +143,7 @@ void initiflist(struct iflist **list)
itmp = xmalloc(sizeof(struct iflist));
memset(itmp, 0, sizeof(struct iflist));
strcpy(itmp->ifname, ifname);
+ itmp->ifindex = ifindex;
index++;
itmp->index = index;
@@ -156,36 +162,32 @@ void initiflist(struct iflist **list)
fclose(fd);
}
-void positionptr(struct iftab *table, struct iflist **ptmp, char *ifname)
+struct iflist *positionptr(struct iflist *iflist, const int ifindex)
{
- struct iflist *plast = NULL;
- int ok = 0;
-
- *ptmp = table->head;
+ struct iflist *ptmp = iflist;
+ struct iflist *last = ptmp;
- while ((*ptmp != NULL) && (!ok)) {
- ok = (strcmp((*ptmp)->ifname, ifname) == 0);
-
- if (!ok) {
- if ((*ptmp)->next_entry == NULL)
- plast = *ptmp;
-
- *ptmp = (*ptmp)->next_entry;
- }
+ while ((ptmp != NULL) && (ptmp->ifindex != ifindex)) {
+ last = ptmp;
+ ptmp = ptmp->next_entry;
}
-
- if (*ptmp == NULL) {
- *ptmp = xmalloc(sizeof(struct iflist));
- memset(*ptmp, 0, sizeof(struct iflist));
- (*ptmp)->index = plast->index + 1;
- plast->next_entry = *ptmp;
- (*ptmp)->prev_entry = plast;
- (*ptmp)->next_entry = NULL;
- strcpy((*ptmp)->ifname, ifname);
-
- if ((*ptmp)->index <= LINES - 4)
- table->lastvisible = *ptmp;
+ /* no interface was found, try to create new one */
+ if(ptmp == NULL) {
+ struct iflist *itmp;
+
+ itmp = xmallocz(sizeof(struct iflist));
+ itmp->ifindex = ifindex;
+ itmp->index = last->index + 1;
+ if(!iface_get_ifname(ifindex, itmp->ifname, sizeof(itmp->ifname)))
+ return(NULL);
+
+ /* last can't be NULL otherwise we will have empty iflist */
+ last->next_entry = itmp;
+ itmp->prev_entry = last;
+ itmp->next_entry = NULL;
+ ptmp = itmp;
}
+ return(ptmp);
}
void destroyiflist(struct iflist *list)
@@ -407,7 +409,6 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
FILE *logfile = NULL;
int br;
- char ifname[IFNAMSIZ];
int ch;
@@ -521,7 +522,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.statwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.statwin);
switch (ch) {
case ERR:
@@ -562,13 +563,15 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
NULL, NULL, NULL, &fromaddr,
ofilter,
MATCH_OPPOSITE_USECONFIG,
- ifname, options->v6inv4asv6);
+ NULL, options->v6inv4asv6);
if (pkt_result != PACKET_OK
&& pkt_result != MORE_FRAGMENTS)
continue;
- positionptr(&table, &ptmp, ifname);
+ ptmp = positionptr(table.head, fromaddr.sll_ifindex);
+ if(!ptmp)
+ continue;
ptmp->total++;
@@ -745,7 +748,6 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
struct ip6_hdr *ip6packet = NULL;
unsigned int iphlen;
- char ifname[IFNAMSIZ];
struct sockaddr_ll fromaddr;
int br;
@@ -983,7 +985,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, ifname, statwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, statwin);
switch (ch) {
case ERR:
@@ -1013,7 +1015,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
processpacket(buf, &packet, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, ifname,
+ MATCH_OPPOSITE_USECONFIG, NULL,
options->v6inv4asv6);
if (pkt_result != PACKET_OK
@@ -1126,6 +1128,7 @@ void selectiface(char *ifname, int withall, int *aborted)
if ((withall) && (list != NULL)) {
ptmp = xmalloc(sizeof(struct iflist));
strncpy(ptmp->ifname, "All interfaces", sizeof(ptmp->ifname));
+ ptmp->ifindex = 0;
ptmp->prev_entry = NULL;
list->prev_entry = ptmp;
diff --git a/src/ifstats.h b/src/ifstats.h
index 781c8df..1dfb517 100644
--- a/src/ifstats.h
+++ b/src/ifstats.h
@@ -9,6 +9,7 @@ ifstats.h - structure definitions for interface counts
struct iflist {
char ifname[IFNAMSIZ];
+ int ifindex;
unsigned int encap;
unsigned long long iptotal;
unsigned long long ip6total;
diff --git a/src/itrafmon.c b/src/itrafmon.c
index dea71b2..8815a92 100644
--- a/src/itrafmon.c
+++ b/src/itrafmon.c
@@ -602,7 +602,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
int curwin = 0;
int readlen;
- char ifname[IFNAMSIZ];
+ char *ifname = ifptr;
unsigned long long total_pkts = 0;
@@ -758,6 +758,8 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
starttime = timeint = closedint = tv.tv_sec;
while (!exitloop) {
+ char ifnamebuf[IFNAMSIZ];
+
gettimeofday(&tv, NULL);
now = tv.tv_sec;
unow = tv.tv_sec * 1e+06 + tv.tv_usec;
@@ -833,7 +835,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
rotate_flag = 0;
}
- getpacket(fd, tpacket, &fromaddr, &ch, &readlen, ifname,
+ getpacket(fd, tpacket, &fromaddr, &ch, &readlen, NULL,
table.tcpscreen);
if (ch == ERR)
@@ -1042,11 +1044,22 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
(unsigned int *) &readlen, &br,
&sport, &dport, &fromaddr,
ofilter, MATCH_OPPOSITE_ALWAYS,
- ifname, options->v6inv4asv6);
+ NULL, options->v6inv4asv6);
if (pkt_result != PACKET_OK)
continue;
+ if (!ifptr) {
+ /* we're capturing on "All interfaces", */
+ /* so get the name of the interface */
+ /* of this packet */
+ if (iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf, sizeof(ifnamebuf)) != 0) {
+ write_error("Unable to get interface name");
+ break; /* error getting interface name, get out! */
+ }
+ ifname = ifnamebuf;
+ }
+
switch(fromaddr.sll_protocol) {
case ETH_P_IP:
ippacket = (struct iphdr *) packet;
diff --git a/src/packet.c b/src/packet.c
index f99d959..d8211a9 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -143,9 +143,11 @@ void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int
*ch,
fromlen = sizeof(struct sockaddr_ll);
*br = recvfrom(fd, buf, MAX_PACKET_SIZE, 0,
(struct sockaddr *) fromaddr, &fromlen);
- ifr.ifr_ifindex = fromaddr->sll_ifindex;
- ioctl(fd, SIOCGIFNAME, &ifr);
- strcpy(ifname, ifr.ifr_name);
+ if(ifname) {
+ ifr.ifr_ifindex = fromaddr->sll_ifindex;
+ ioctl(fd, SIOCGIFNAME, &ifr);
+ strcpy(ifname, ifr.ifr_name);
+ }
}
if (!daemonized && FD_ISSET(0, &set))
*ch = wgetch(win);
diff --git a/src/pktsize.c b/src/pktsize.c
index 5bfbeaf..ce0fd78 100644
--- a/src/pktsize.c
+++ b/src/pktsize.c
@@ -155,7 +155,6 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
char buf[MAX_PACKET_SIZE];
int br;
char *ipacket;
- char iface[IFNAMSIZ];
unsigned int mtu;
struct sockaddr_ll fromaddr;
@@ -294,7 +293,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
&& (((now - starttime) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, iface, win);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, win);
if (ch != ERR) {
switch (ch) {
@@ -317,7 +316,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
processpacket(buf, &ipacket, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, iface,
+ MATCH_OPPOSITE_USECONFIG, NULL,
0);
if (pkt_result != PACKET_OK)
diff --git a/src/serv.c b/src/serv.c
index 912b8e4..9566ecc 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -700,7 +700,6 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
struct sockaddr_ll fromaddr;
int br;
- char iface[IFNAMSIZ];
unsigned int idx = 1;
unsigned int sport = 0;
@@ -864,7 +863,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
&& (((now - starttime) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, iface, list.win);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, list.win);
if (ch == ERR)
goto no_key_ready;
@@ -999,7 +998,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
processpacket(buf, &ipacket, (unsigned int *) &br,
&tot_br, &sport, &dport, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, iface,
+ MATCH_OPPOSITE_USECONFIG, NULL,
options->v6inv4asv6);
if (pkt_result != PACKET_OK)
--
1.7.8.4
1:43 p.m.
New subject: [PATCH 1/3] get the interface name only when needed
Vitezslav Samel <vitezslav(a)samel.cz> writes:
Right now on every received packet we get the interface name
on which it arrived. This is not needed and is slow.
Make use of ifindex and retrieve interface name only when needed
(when capturing on "All interfaces").
Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
---
src/hostmon.c | 19 +++++++++++++--
src/ifaces.c | 38 ++++++++++++++++++++++++++++++
src/ifaces.h | 2 +
src/ifstats.c | 69 +++++++++++++++++++++++++++++--------------------------
src/ifstats.h | 1 +
src/itrafmon.c | 19 +++++++++++++--
src/packet.c | 8 ++++--
src/pktsize.c | 5 +--
src/serv.c | 5 +--
9 files changed, 118 insertions(+), 48 deletions(-)
this patch looks to me suspicious
diff --git a/src/hostmon.c b/src/hostmon.c
index 59be4e9..d8854e8 100644
--- a/src/hostmon.c
+++ b/src/hostmon.c
@@ -678,7 +678,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
int is_ip;
int ch;
- char ifname[IFNAMSIZ];
+ char *ifname = ifptr;
struct timeval tv;
unsigned long starttime;
@@ -814,7 +814,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.tabwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.tabwin);
if (ch != ERR) {
if (keymode == 0) {
@@ -864,16 +864,29 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
}
}
if (br > 0) {
+ char ifnamebuf[IFNAMSIZ];
+
pkt_result =
processpacket(buf, &ipacket, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, ifname,
+ MATCH_OPPOSITE_USECONFIG, NULL,
0);
if (pkt_result != PACKET_OK)
continue;
+ if (!ifptr) {
+ /* we're capturing on "All interfaces", */
+ /* so get the name of the interface */
+ /* of this packet */
+ if (iface_get_ifname(fromaddr.sll_ifindex, ifname, sizeof(ifname)) != 0) {
iface_get_ifname() has too much parameters. sizeof(ifname) is alway
IFNAMSIZ. get rid of it.
+ write_error("Unable to get interface name");
+ break; /* can't get interface name, get out! */
+ }
+ ifname = ifnamebuf;
bug. you fill ifname in iface_get_ifname() by resolving name given by
sll_ifindex and here [ifname = ifnamebuf] you dicard resovled name.
I susspect you that you wanted to use it like that:
if (iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf, sizeof(ifname)) != 0) {
^^^^^^^^^ not ifname
I also don't like to use if (func()).
it's way better to use
int r = func(params);
if (r)
do_something();
+ }
+
if ((fromaddr.sll_hatype == ARPHRD_ETHER)
|| (fromaddr.sll_hatype == ARPHRD_FDDI)
/* fix me
diff --git a/src/ifaces.c b/src/ifaces.c
index c1e0be5..8b47769 100644
--- a/src/ifaces.c
+++ b/src/ifaces.c
@@ -100,6 +100,44 @@ void err_iface_down(void)
write_error("Specified interface not active");
}
+int iface_get_ifindex(const char const *iface)
if you want to have a constant pointer to constant data than it is
_const char * const iface_
const char *iface is enough
+{
+ int fd;
+ int ir;
+ struct ifreq ifr;
+
+ fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
bug. what it socket fails and fd = -1? According to man pages:
int ioctl(int d, int request, ...);
"The argument d must be an open file descriptor."
+ strcpy(ifr.ifr_name, iface);
+ ir = ioctl(fd, SIOCGIFINDEX, &ifr);
+
+ close(fd);
+
+ if (ir != 0)
+ return(ir);
+
+ return(ifr.ifr_ifindex);
+}
we must check socket(), ioctl() and inform user and return appropriate
err code. correct your coding style _return(foo)_ to _retun foo_
just curiosity. why are you using
int fd;
fd = socket();
insted of
int fd = socket(); ?
+int iface_get_ifname(const int ifindex, char *ifname, int
ifname_space)
^^^^^ pointless const casting
it already is constant, because the ifindex value is copied from origin.
+{
+ int fd;
+ int ir;
+ struct ifreq ifr;
+
+ fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +
> + ifr.ifr_ifindex = ifindex;
simply init struct
struct ifreq ifr = {
.ifr_ifindex = ifindex;
};
and you will kill two birds with one stone.
+ ir = ioctl(fd, SIOCGIFNAME, &ifr);
+
+ close(fd);
+
+ if (ir == 0)
+ strncpy(ifname, ifr.ifr_name, ifname_space);
+
+ return(ir);
+}
+
ditto w/ checking socket() and ioctl().
int iface_get_ifname(int ifindex, char *ifname)
{
int fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (fd < 0)
return fd;
struct ifreq ifr = {
.ifr_ifindex = ifindex;
};
int ir = ioctl(fd, SIOCGIFNAME, &ifr);
if (ir < 0)
return ir;
return strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
}
whenever iface_get_ifname() returns < 0, print errno
void isdn_iface_check(int *fd, char *ifname)
{
if (*fd == -1) {
diff --git a/src/ifaces.h b/src/ifaces.h
index 5bf313d..d1b01e8 100644
--- a/src/ifaces.h
+++ b/src/ifaces.h
@@ -11,6 +11,8 @@ ifaces.h - prototype declaration for interface support determination
FILE *open_procnetdev(void);
int get_next_iface(FILE * fd, char *ifname, int n);
int iface_up(char *iface);
+extern int iface_get_ifindex(const char const *iface);
+extern int iface_get_ifname(const int ifindex, char *ifname, int ifname_space);
void err_iface_down(void);
void isdn_iface_check(int *fd, char *ifname);
char *gen_iface_msg(char *ifptr);
diff --git a/src/ifstats.c b/src/ifstats.c
index 9997a34..7a0de75 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -116,6 +116,8 @@ void initiflist(struct iflist **list)
}
while (get_next_iface(fd, ifname, sizeof(ifname))) {
+ int ifindex;
+
if (!*ifname)
continue;
@@ -131,6 +133,9 @@ void initiflist(struct iflist **list)
if (!iface_up(ifname))
continue;
+ ifindex = iface_get_ifindex(ifname);
+ if(ifindex < 0)
+ continue;
/*
* At this point, the interface is now sure to be up and running.
*/
@@ -138,6 +143,7 @@ void initiflist(struct iflist **list)
itmp = xmalloc(sizeof(struct iflist));
memset(itmp, 0, sizeof(struct iflist));
strcpy(itmp->ifname, ifname);
+ itmp->ifindex = ifindex;
index++;
itmp->index = index;
@@ -156,36 +162,32 @@ void initiflist(struct iflist **list)
fclose(fd);
}
-void positionptr(struct iftab *table, struct iflist **ptmp, char *ifname)
+struct iflist *positionptr(struct iflist *iflist, const int ifindex)
{
- struct iflist *plast = NULL;
- int ok = 0;
-
- *ptmp = table->head;
+ struct iflist *ptmp = iflist;
+ struct iflist *last = ptmp;
- while ((*ptmp != NULL) && (!ok)) {
- ok = (strcmp((*ptmp)->ifname, ifname) == 0);
-
- if (!ok) {
- if ((*ptmp)->next_entry == NULL)
- plast = *ptmp;
-
- *ptmp = (*ptmp)->next_entry;
- }
+ while ((ptmp != NULL) && (ptmp->ifindex != ifindex)) {
+ last = ptmp;
+ ptmp = ptmp->next_entry;
}
-
- if (*ptmp == NULL) {
- *ptmp = xmalloc(sizeof(struct iflist));
- memset(*ptmp, 0, sizeof(struct iflist));
- (*ptmp)->index = plast->index + 1;
- plast->next_entry = *ptmp;
- (*ptmp)->prev_entry = plast;
- (*ptmp)->next_entry = NULL;
- strcpy((*ptmp)->ifname, ifname);
-
- if ((*ptmp)->index <= LINES - 4)
- table->lastvisible = *ptmp;
+ /* no interface was found, try to create new one */
+ if(ptmp == NULL) {
+ struct iflist *itmp;
+
+ itmp = xmallocz(sizeof(struct iflist));
+ itmp->ifindex = ifindex;
+ itmp->index = last->index + 1;
+ if(!iface_get_ifname(ifindex, itmp->ifname, sizeof(itmp->ifname)))
+ return(NULL);
+
+ /* last can't be NULL otherwise we will have empty iflist */
+ last->next_entry = itmp;
+ itmp->prev_entry = last;
+ itmp->next_entry = NULL;
+ ptmp = itmp;
}
+ return(ptmp);
}
void destroyiflist(struct iflist *list)
@@ -407,7 +409,6 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
FILE *logfile = NULL;
int br;
- char ifname[IFNAMSIZ];
int ch;
@@ -521,7 +522,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.statwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.statwin);
switch (ch) {
case ERR:
@@ -562,13 +563,15 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
NULL, NULL, NULL, &fromaddr,
ofilter,
MATCH_OPPOSITE_USECONFIG,
- ifname, options->v6inv4asv6);
+ NULL, options->v6inv4asv6);
if (pkt_result != PACKET_OK
&& pkt_result != MORE_FRAGMENTS)
continue;
- positionptr(&table, &ptmp, ifname);
+ ptmp = positionptr(table.head, fromaddr.sll_ifindex);
+ if(!ptmp)
+ continue;
ptmp->total++;
@@ -745,7 +748,6 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
struct ip6_hdr *ip6packet = NULL;
unsigned int iphlen;
- char ifname[IFNAMSIZ];
struct sockaddr_ll fromaddr;
int br;
@@ -983,7 +985,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, ifname, statwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, statwin);
switch (ch) {
case ERR:
@@ -1013,7 +1015,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
processpacket(buf, &packet, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, ifname,
+ MATCH_OPPOSITE_USECONFIG, NULL,
options->v6inv4asv6);
if (pkt_result != PACKET_OK
@@ -1126,6 +1128,7 @@ void selectiface(char *ifname, int withall, int *aborted)
if ((withall) && (list != NULL)) {
ptmp = xmalloc(sizeof(struct iflist));
strncpy(ptmp->ifname, "All interfaces", sizeof(ptmp->ifname));
+ ptmp->ifindex = 0;
ptmp->prev_entry = NULL;
list->prev_entry = ptmp;
diff --git a/src/ifstats.h b/src/ifstats.h
index 781c8df..1dfb517 100644
--- a/src/ifstats.h
+++ b/src/ifstats.h
@@ -9,6 +9,7 @@ ifstats.h - structure definitions for interface counts
struct iflist {
char ifname[IFNAMSIZ];
+ int ifindex;
unsigned int encap;
unsigned long long iptotal;
unsigned long long ip6total;
diff --git a/src/itrafmon.c b/src/itrafmon.c
index dea71b2..8815a92 100644
--- a/src/itrafmon.c
+++ b/src/itrafmon.c
@@ -602,7 +602,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
int curwin = 0;
int readlen;
- char ifname[IFNAMSIZ];
+ char *ifname = ifptr;
unsigned long long total_pkts = 0;
@@ -758,6 +758,8 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
starttime = timeint = closedint = tv.tv_sec;
while (!exitloop) {
+ char ifnamebuf[IFNAMSIZ];
+
gettimeofday(&tv, NULL);
now = tv.tv_sec;
unow = tv.tv_sec * 1e+06 + tv.tv_usec;
@@ -833,7 +835,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
rotate_flag = 0;
}
- getpacket(fd, tpacket, &fromaddr, &ch, &readlen, ifname,
+ getpacket(fd, tpacket, &fromaddr, &ch, &readlen, NULL,
table.tcpscreen);
if (ch == ERR)
@@ -1042,11 +1044,22 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
(unsigned int *) &readlen, &br,
&sport, &dport, &fromaddr,
ofilter, MATCH_OPPOSITE_ALWAYS,
- ifname, options->v6inv4asv6);
+ NULL, options->v6inv4asv6);
if (pkt_result != PACKET_OK)
continue;
+ if (!ifptr) {
+ /* we're capturing on "All interfaces", */
+ /* so get the name of the interface */
+ /* of this packet */
+ if (iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf, sizeof(ifnamebuf)) != 0) {
+ write_error("Unable to get interface name");
+ break; /* error getting interface name, get out! */
+ }
+ ifname = ifnamebuf;
+ }
+
ha! this is correct ;)
switch(fromaddr.sll_protocol) {
case ETH_P_IP:
ippacket = (struct iphdr *) packet;
diff --git a/src/packet.c b/src/packet.c
index f99d959..d8211a9 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -143,9 +143,11 @@ void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int
*ch,
fromlen = sizeof(struct sockaddr_ll);
*br = recvfrom(fd, buf, MAX_PACKET_SIZE, 0,
(struct sockaddr *) fromaddr, &fromlen);
- ifr.ifr_ifindex = fromaddr->sll_ifindex;
- ioctl(fd, SIOCGIFNAME, &ifr);
- strcpy(ifname, ifr.ifr_name);
+ if(ifname) {
^^ if *space* (
+ ifr.ifr_ifindex = fromaddr->sll_ifindex;
+ ioctl(fd, SIOCGIFNAME, &ifr);
+ strcpy(ifname, ifr.ifr_name);
+ }
}
if (!daemonized && FD_ISSET(0, &set))
*ch = wgetch(win);
diff --git a/src/pktsize.c b/src/pktsize.c
index 5bfbeaf..ce0fd78 100644
--- a/src/pktsize.c
+++ b/src/pktsize.c
@@ -155,7 +155,6 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
char buf[MAX_PACKET_SIZE];
int br;
char *ipacket;
- char iface[IFNAMSIZ];
unsigned int mtu;
struct sockaddr_ll fromaddr;
@@ -294,7 +293,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
&& (((now - starttime) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, iface, win);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, win);
if (ch != ERR) {
switch (ch) {
@@ -317,7 +316,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
processpacket(buf, &ipacket, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, iface,
+ MATCH_OPPOSITE_USECONFIG, NULL,
0);
if (pkt_result != PACKET_OK)
diff --git a/src/serv.c b/src/serv.c
index 912b8e4..9566ecc 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -700,7 +700,6 @@ void servmon(char *ifname, struct porttab *ports, const struct
OPTIONS *options,
struct sockaddr_ll fromaddr;
int br;
- char iface[IFNAMSIZ];
unsigned int idx = 1;
unsigned int sport = 0;
@@ -864,7 +863,7 @@ void servmon(char *ifname, struct porttab *ports, const struct
OPTIONS *options,
&& (((now - starttime) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, iface, list.win);
+ getpacket(fd, buf, &fromaddr, &ch, &br, NULL, list.win);
if (ch == ERR)
goto no_key_ready;
@@ -999,7 +998,7 @@ void servmon(char *ifname, struct porttab *ports, const struct
OPTIONS *options,
processpacket(buf, &ipacket, (unsigned int *) &br,
&tot_br, &sport, &dport, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, iface,
+ MATCH_OPPOSITE_USECONFIG, NULL,
options->v6inv4asv6);
if (pkt_result != PACKET_OK)
--
Nikola
2:19 p.m.
New subject: [PATCH 1/3] get the interface name only when needed
On Sun, Apr 08, 2012 at 08:43:56PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel <vitezslav(a)samel.cz> writes:
> Right now on every received packet we get the interface name
> on which it arrived. This is not needed and is slow.
> Make use of ifindex and retrieve interface name only when needed
> (when capturing on "All interfaces").
>
> Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
> ---
> src/hostmon.c | 19 +++++++++++++--
> src/ifaces.c | 38 ++++++++++++++++++++++++++++++
> src/ifaces.h | 2 +
> src/ifstats.c | 69 +++++++++++++++++++++++++++++--------------------------
> src/ifstats.h | 1 +
> src/itrafmon.c | 19 +++++++++++++--
> src/packet.c | 8 ++++--
> src/pktsize.c | 5 +--
> src/serv.c | 5 +--
> 9 files changed, 118 insertions(+), 48 deletions(-)
this patch looks to me suspicious
> diff --git a/src/hostmon.c b/src/hostmon.c
> index 59be4e9..d8854e8 100644
> --- a/src/hostmon.c
> +++ b/src/hostmon.c
> @@ -678,7 +678,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime,
char *ifptr,
> int is_ip;
> int ch;
>
> - char ifname[IFNAMSIZ];
> + char *ifname = ifptr;
>
> struct timeval tv;
> unsigned long starttime;
> @@ -814,7 +814,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime,
char *ifptr,
> && (((now - statbegin) / 60) >= facilitytime))
> exitloop = 1;
>
> - getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.tabwin);
> + getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.tabwin);
>
> if (ch != ERR) {
> if (keymode == 0) {
> @@ -864,16 +864,29 @@ void hostmon(const struct OPTIONS *options, int facilitytime,
char *ifptr,
> }
> }
> if (br > 0) {
> + char ifnamebuf[IFNAMSIZ];
> +
> pkt_result =
> processpacket(buf, &ipacket, (unsigned int *) &br,
> NULL, NULL, NULL, &fromaddr,
> ofilter,
> - MATCH_OPPOSITE_USECONFIG, ifname,
> + MATCH_OPPOSITE_USECONFIG, NULL,
> 0);
>
> if (pkt_result != PACKET_OK)
> continue;
>
> + if (!ifptr) {
> + /* we're capturing on "All interfaces", */
> + /* so get the name of the interface */
> + /* of this packet */
> + if (iface_get_ifname(fromaddr.sll_ifindex, ifname, sizeof(ifname)) != 0) {
iface_get_ifname() has too much parameters. sizeof(ifname) is alway
IFNAMSIZ. get rid of it.
OK.
> + write_error("Unable to get interface name");
> + break; /* can't get interface name, get out! */
> + }
> + ifname = ifnamebuf;
bug. you fill ifname in iface_get_ifname() by resolving name given by
sll_ifindex and here [ifname = ifnamebuf] you dicard resovled name.
I susspect you that you wanted to use it like that:
if (iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf, sizeof(ifname)) != 0) {
^^^^^^^^^ not ifname
My fault, I will fix it.
I also don't like to use if (func()).
it's way better to use
int r = func(params);
if (r)
do_something();
OK.
> + }
> +
> if ((fromaddr.sll_hatype == ARPHRD_ETHER)
> || (fromaddr.sll_hatype == ARPHRD_FDDI)
> /* fix me
> diff --git a/src/ifaces.c b/src/ifaces.c
> index c1e0be5..8b47769 100644
> --- a/src/ifaces.c
> +++ b/src/ifaces.c
> @@ -100,6 +100,44 @@ void err_iface_down(void)
> write_error("Specified interface not active");
> }
>
> +int iface_get_ifindex(const char const *iface)
if you want to have a constant pointer to constant data than it is
_const char * const iface_
const char *iface is enough
OK.
> +{
> + int fd;
> + int ir;
> + struct ifreq ifr;
> +
> + fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
bug. what it socket fails and fd = -1? According to man pages:
int ioctl(int d, int request, ...);
"The argument d must be an open file descriptor."
Sure, I''ll fix it.
> + strcpy(ifr.ifr_name, iface);
> + ir = ioctl(fd, SIOCGIFINDEX, &ifr);
> +
> + close(fd);
> +
> + if (ir != 0)
> + return(ir);
> +
> + return(ifr.ifr_ifindex);
> +}
we must check socket(), ioctl() and inform user and return appropriate
err code. correct your coding style _return(foo)_ to _retun foo_
just curiosity. why are you using
int fd;
fd = socket();
insted of
int fd = socket(); ?
Because I don't like c99 initializers mixed in the code (old school
practices).
> +int iface_get_ifname(const int ifindex, char *ifname, int
ifname_space)
^^^^^ pointless const casting
it already is constant, because the ifindex value is copied from origin.
> +{
> + int fd;
> + int ir;
> + struct ifreq ifr;
> +
> + fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +
> + ifr.ifr_ifindex = ifindex;
simply init struct
struct ifreq ifr = {
.ifr_ifindex = ifindex;
};
and you will kill two birds with one stone.
OK.
> + ir = ioctl(fd, SIOCGIFNAME, &ifr);
> +
> + close(fd);
> +
> + if (ir == 0)
> + strncpy(ifname, ifr.ifr_name, ifname_space);
> +
> + return(ir);
> +}
> +
ditto w/ checking socket() and ioctl().
int iface_get_ifname(int ifindex, char *ifname)
{
int fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (fd < 0)
return fd;
struct ifreq ifr = {
.ifr_ifindex = ifindex;
};
int ir = ioctl(fd, SIOCGIFNAME, &ifr);
if (ir < 0)
return ir;
return strncpy(ifname, ifr.ifr_name, IFNAMSIZ);
}
whenever iface_get_ifname() returns < 0, print errno
> void isdn_iface_check(int *fd, char *ifname)
> {
> if (*fd == -1) {
> diff --git a/src/ifaces.h b/src/ifaces.h
> index 5bf313d..d1b01e8 100644
> --- a/src/ifaces.h
> +++ b/src/ifaces.h
> @@ -11,6 +11,8 @@ ifaces.h - prototype declaration for interface support
determination
> FILE *open_procnetdev(void);
> int get_next_iface(FILE * fd, char *ifname, int n);
> int iface_up(char *iface);
> +extern int iface_get_ifindex(const char const *iface);
> +extern int iface_get_ifname(const int ifindex, char *ifname, int ifname_space);
> void err_iface_down(void);
> void isdn_iface_check(int *fd, char *ifname);
> char *gen_iface_msg(char *ifptr);
> diff --git a/src/ifstats.c b/src/ifstats.c
> index 9997a34..7a0de75 100644
> --- a/src/ifstats.c
> +++ b/src/ifstats.c
> @@ -116,6 +116,8 @@ void initiflist(struct iflist **list)
> }
>
> while (get_next_iface(fd, ifname, sizeof(ifname))) {
> + int ifindex;
> +
> if (!*ifname)
> continue;
>
> @@ -131,6 +133,9 @@ void initiflist(struct iflist **list)
> if (!iface_up(ifname))
> continue;
>
> + ifindex = iface_get_ifindex(ifname);
> + if(ifindex < 0)
> + continue;
> /*
> * At this point, the interface is now sure to be up and running.
> */
> @@ -138,6 +143,7 @@ void initiflist(struct iflist **list)
> itmp = xmalloc(sizeof(struct iflist));
> memset(itmp, 0, sizeof(struct iflist));
> strcpy(itmp->ifname, ifname);
> + itmp->ifindex = ifindex;
> index++;
> itmp->index = index;
>
> @@ -156,36 +162,32 @@ void initiflist(struct iflist **list)
> fclose(fd);
> }
>
> -void positionptr(struct iftab *table, struct iflist **ptmp, char *ifname)
> +struct iflist *positionptr(struct iflist *iflist, const int ifindex)
> {
> - struct iflist *plast = NULL;
> - int ok = 0;
> -
> - *ptmp = table->head;
> + struct iflist *ptmp = iflist;
> + struct iflist *last = ptmp;
>
> - while ((*ptmp != NULL) && (!ok)) {
> - ok = (strcmp((*ptmp)->ifname, ifname) == 0);
> -
> - if (!ok) {
> - if ((*ptmp)->next_entry == NULL)
> - plast = *ptmp;
> -
> - *ptmp = (*ptmp)->next_entry;
> - }
> + while ((ptmp != NULL) && (ptmp->ifindex != ifindex)) {
> + last = ptmp;
> + ptmp = ptmp->next_entry;
> }
> -
> - if (*ptmp == NULL) {
> - *ptmp = xmalloc(sizeof(struct iflist));
> - memset(*ptmp, 0, sizeof(struct iflist));
> - (*ptmp)->index = plast->index + 1;
> - plast->next_entry = *ptmp;
> - (*ptmp)->prev_entry = plast;
> - (*ptmp)->next_entry = NULL;
> - strcpy((*ptmp)->ifname, ifname);
> -
> - if ((*ptmp)->index <= LINES - 4)
> - table->lastvisible = *ptmp;
> + /* no interface was found, try to create new one */
> + if(ptmp == NULL) {
> + struct iflist *itmp;
> +
> + itmp = xmallocz(sizeof(struct iflist));
> + itmp->ifindex = ifindex;
> + itmp->index = last->index + 1;
> + if(!iface_get_ifname(ifindex, itmp->ifname, sizeof(itmp->ifname)))
> + return(NULL);
> +
> + /* last can't be NULL otherwise we will have empty iflist */
> + last->next_entry = itmp;
> + itmp->prev_entry = last;
> + itmp->next_entry = NULL;
> + ptmp = itmp;
> }
> + return(ptmp);
> }
>
> void destroyiflist(struct iflist *list)
> @@ -407,7 +409,6 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
> FILE *logfile = NULL;
>
> int br;
> - char ifname[IFNAMSIZ];
>
> int ch;
>
> @@ -521,7 +522,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
> && (((now - statbegin) / 60) >= facilitytime))
> exitloop = 1;
>
> - getpacket(fd, buf, &fromaddr, &ch, &br, ifname, table.statwin);
> + getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.statwin);
>
> switch (ch) {
> case ERR:
> @@ -562,13 +563,15 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
> NULL, NULL, NULL, &fromaddr,
> ofilter,
> MATCH_OPPOSITE_USECONFIG,
> - ifname, options->v6inv4asv6);
> + NULL, options->v6inv4asv6);
>
> if (pkt_result != PACKET_OK
> && pkt_result != MORE_FRAGMENTS)
> continue;
>
> - positionptr(&table, &ptmp, ifname);
> + ptmp = positionptr(table.head, fromaddr.sll_ifindex);
> + if(!ptmp)
> + continue;
>
> ptmp->total++;
>
> @@ -745,7 +748,6 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
> struct ip6_hdr *ip6packet = NULL;
> unsigned int iphlen;
>
> - char ifname[IFNAMSIZ];
> struct sockaddr_ll fromaddr;
>
> int br;
> @@ -983,7 +985,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
> && (((now - statbegin) / 60) >= facilitytime))
> exitloop = 1;
>
> - getpacket(fd, buf, &fromaddr, &ch, &br, ifname, statwin);
> + getpacket(fd, buf, &fromaddr, &ch, &br, NULL, statwin);
>
> switch (ch) {
> case ERR:
> @@ -1013,7 +1015,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
> processpacket(buf, &packet, (unsigned int *) &br,
> NULL, NULL, NULL, &fromaddr,
> ofilter,
> - MATCH_OPPOSITE_USECONFIG, ifname,
> + MATCH_OPPOSITE_USECONFIG, NULL,
> options->v6inv4asv6);
>
> if (pkt_result != PACKET_OK
> @@ -1126,6 +1128,7 @@ void selectiface(char *ifname, int withall, int *aborted)
> if ((withall) && (list != NULL)) {
> ptmp = xmalloc(sizeof(struct iflist));
> strncpy(ptmp->ifname, "All interfaces", sizeof(ptmp->ifname));
> + ptmp->ifindex = 0;
>
> ptmp->prev_entry = NULL;
> list->prev_entry = ptmp;
> diff --git a/src/ifstats.h b/src/ifstats.h
> index 781c8df..1dfb517 100644
> --- a/src/ifstats.h
> +++ b/src/ifstats.h
> @@ -9,6 +9,7 @@ ifstats.h - structure definitions for interface counts
>
> struct iflist {
> char ifname[IFNAMSIZ];
> + int ifindex;
> unsigned int encap;
> unsigned long long iptotal;
> unsigned long long ip6total;
> diff --git a/src/itrafmon.c b/src/itrafmon.c
> index dea71b2..8815a92 100644
> --- a/src/itrafmon.c
> +++ b/src/itrafmon.c
> @@ -602,7 +602,7 @@ void ipmon(struct OPTIONS *options, struct filterstate
*ofilter,
> int curwin = 0;
>
> int readlen;
> - char ifname[IFNAMSIZ];
> + char *ifname = ifptr;
>
> unsigned long long total_pkts = 0;
>
> @@ -758,6 +758,8 @@ void ipmon(struct OPTIONS *options, struct filterstate
*ofilter,
> starttime = timeint = closedint = tv.tv_sec;
>
> while (!exitloop) {
> + char ifnamebuf[IFNAMSIZ];
> +
> gettimeofday(&tv, NULL);
> now = tv.tv_sec;
> unow = tv.tv_sec * 1e+06 + tv.tv_usec;
> @@ -833,7 +835,7 @@ void ipmon(struct OPTIONS *options, struct filterstate
*ofilter,
> rotate_flag = 0;
> }
>
> - getpacket(fd, tpacket, &fromaddr, &ch, &readlen, ifname,
> + getpacket(fd, tpacket, &fromaddr, &ch, &readlen, NULL,
> table.tcpscreen);
>
> if (ch == ERR)
> @@ -1042,11 +1044,22 @@ void ipmon(struct OPTIONS *options, struct filterstate
*ofilter,
> (unsigned int *) &readlen, &br,
> &sport, &dport, &fromaddr,
> ofilter, MATCH_OPPOSITE_ALWAYS,
> - ifname, options->v6inv4asv6);
> + NULL, options->v6inv4asv6);
>
> if (pkt_result != PACKET_OK)
> continue;
>
> + if (!ifptr) {
> + /* we're capturing on "All interfaces", */
> + /* so get the name of the interface */
> + /* of this packet */
> + if (iface_get_ifname(fromaddr.sll_ifindex, ifnamebuf, sizeof(ifnamebuf)) != 0)
{
> + write_error("Unable to get interface name");
> + break; /* error getting interface name, get out! */
> + }
> + ifname = ifnamebuf;
> + }
> +
ha! this is correct ;)
> switch(fromaddr.sll_protocol) {
> case ETH_P_IP:
> ippacket = (struct iphdr *) packet;
> diff --git a/src/packet.c b/src/packet.c
> index f99d959..d8211a9 100644
> --- a/src/packet.c
> +++ b/src/packet.c
> @@ -143,9 +143,11 @@ void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr,
int *ch,
> fromlen = sizeof(struct sockaddr_ll);
> *br = recvfrom(fd, buf, MAX_PACKET_SIZE, 0,
> (struct sockaddr *) fromaddr, &fromlen);
> - ifr.ifr_ifindex = fromaddr->sll_ifindex;
> - ioctl(fd, SIOCGIFNAME, &ifr);
> - strcpy(ifname, ifr.ifr_name);
> + if(ifname) {
^^ if *space* (
OK.
> + ifr.ifr_ifindex = fromaddr->sll_ifindex;
> + ioctl(fd, SIOCGIFNAME, &ifr);
> + strcpy(ifname, ifr.ifr_name);
> + }
> }
> if (!daemonized && FD_ISSET(0, &set))
> *ch = wgetch(win);
> diff --git a/src/pktsize.c b/src/pktsize.c
> index 5bfbeaf..ce0fd78 100644
> --- a/src/pktsize.c
> +++ b/src/pktsize.c
> @@ -155,7 +155,6 @@ void packet_size_breakdown(struct OPTIONS *options, char
*ifname,
> char buf[MAX_PACKET_SIZE];
> int br;
> char *ipacket;
> - char iface[IFNAMSIZ];
> unsigned int mtu;
>
> struct sockaddr_ll fromaddr;
> @@ -294,7 +293,7 @@ void packet_size_breakdown(struct OPTIONS *options, char
*ifname,
> && (((now - starttime) / 60) >= facilitytime))
> exitloop = 1;
>
> - getpacket(fd, buf, &fromaddr, &ch, &br, iface, win);
> + getpacket(fd, buf, &fromaddr, &ch, &br, NULL, win);
>
> if (ch != ERR) {
> switch (ch) {
> @@ -317,7 +316,7 @@ void packet_size_breakdown(struct OPTIONS *options, char
*ifname,
> processpacket(buf, &ipacket, (unsigned int *) &br,
> NULL, NULL, NULL, &fromaddr,
> ofilter,
> - MATCH_OPPOSITE_USECONFIG, iface,
> + MATCH_OPPOSITE_USECONFIG, NULL,
> 0);
>
> if (pkt_result != PACKET_OK)
> diff --git a/src/serv.c b/src/serv.c
> index 912b8e4..9566ecc 100644
> --- a/src/serv.c
> +++ b/src/serv.c
> @@ -700,7 +700,6 @@ void servmon(char *ifname, struct porttab *ports, const struct
OPTIONS *options,
> struct sockaddr_ll fromaddr;
> int br;
>
> - char iface[IFNAMSIZ];
> unsigned int idx = 1;
>
> unsigned int sport = 0;
> @@ -864,7 +863,7 @@ void servmon(char *ifname, struct porttab *ports, const struct
OPTIONS *options,
> && (((now - starttime) / 60) >= facilitytime))
> exitloop = 1;
>
> - getpacket(fd, buf, &fromaddr, &ch, &br, iface, list.win);
> + getpacket(fd, buf, &fromaddr, &ch, &br, NULL, list.win);
>
> if (ch == ERR)
> goto no_key_ready;
> @@ -999,7 +998,7 @@ void servmon(char *ifname, struct porttab *ports, const struct
OPTIONS *options,
> processpacket(buf, &ipacket, (unsigned int *) &br,
> &tot_br, &sport, &dport, &fromaddr,
> ofilter,
> - MATCH_OPPOSITE_USECONFIG, iface,
> + MATCH_OPPOSITE_USECONFIG, NULL,
> options->v6inv4asv6);
>
> if (pkt_result != PACKET_OK)
Thanks to review,
Vita
1:26 a.m.
New subject: [PATCH 1/3] get the interface name only when needed
Hi!
[...]
On Sun, Apr 08, 2012 at 08:43:56PM +0200, Nikola Pajkovsky wrote:
whenever iface_get_ifname() returns < 0, print errno
I tried that, but write_error("msg\n(%m)") looks very ugly; when
I tried with write_error("msg (%m)") with long msg and with long error
string, the resulting string overflowed to the next line and overwitten
"Press any key to continue" message. And I don't know how to print
multiline message with write_error().
Cheers,
Vita
3:32 a.m.
New subject: [PATCH 1/3] get the interface name only when needed
Vitezslav Samel <vitezslav(a)samel.cz> writes:
Hi!
[...]
On Sun, Apr 08, 2012 at 08:43:56PM +0200, Nikola Pajkovsky wrote:
> whenever iface_get_ifname() returns < 0, print errno
I tried that, but write_error("msg\n(%m)") looks very ugly; when
I tried with write_error("msg (%m)") with long msg and with long error
string, the resulting string overflowed to the next line and overwitten
"Press any key to continue" message. And I don't know how to print
multiline message with write_error().
Hmm, I thought so. write_error() is pretty stupid, nasty func and need
to be rewritten.
I also found sigsegv yesterdy in logging mechanism :-/
--
Nikola
7:26 a.m.
New subject: [PATCH 2/3] remove unused params from getpacket() and processpacket()
Since all the callers have ifname NULL we can remove the params
and associated code.
In processpacket() I deleted also ifdefed code hich checks for ISDN
interface (this needs to be done in other way).
Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
---
src/hostmon.c | 4 ++--
src/ifstats.c | 8 ++++----
src/itrafmon.c | 4 ++--
src/packet.c | 18 ++----------------
src/packet.h | 4 ++--
src/pktsize.c | 5 ++---
src/serv.c | 4 ++--
7 files changed, 16 insertions(+), 31 deletions(-)
diff --git a/src/hostmon.c b/src/hostmon.c
index d8854e8..f6989b2 100644
--- a/src/hostmon.c
+++ b/src/hostmon.c
@@ -814,7 +814,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.tabwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, table.tabwin);
if (ch != ERR) {
if (keymode == 0) {
@@ -870,7 +870,7 @@ void hostmon(const struct OPTIONS *options, int facilitytime, char
*ifptr,
processpacket(buf, &ipacket, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, NULL,
+ MATCH_OPPOSITE_USECONFIG,
0);
if (pkt_result != PACKET_OK)
diff --git a/src/ifstats.c b/src/ifstats.c
index 7a0de75..983ad55 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -522,7 +522,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, NULL, table.statwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, table.statwin);
switch (ch) {
case ERR:
@@ -563,7 +563,7 @@ void ifstats(const struct OPTIONS *options, struct filterstate
*ofilter,
NULL, NULL, NULL, &fromaddr,
ofilter,
MATCH_OPPOSITE_USECONFIG,
- NULL, options->v6inv4asv6);
+ options->v6inv4asv6);
if (pkt_result != PACKET_OK
&& pkt_result != MORE_FRAGMENTS)
@@ -985,7 +985,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
&& (((now - statbegin) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, NULL, statwin);
+ getpacket(fd, buf, &fromaddr, &ch, &br, statwin);
switch (ch) {
case ERR:
@@ -1015,7 +1015,7 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
processpacket(buf, &packet, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, NULL,
+ MATCH_OPPOSITE_USECONFIG,
options->v6inv4asv6);
if (pkt_result != PACKET_OK
diff --git a/src/itrafmon.c b/src/itrafmon.c
index 8815a92..810f5b4 100644
--- a/src/itrafmon.c
+++ b/src/itrafmon.c
@@ -835,7 +835,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
rotate_flag = 0;
}
- getpacket(fd, tpacket, &fromaddr, &ch, &readlen, NULL,
+ getpacket(fd, tpacket, &fromaddr, &ch, &readlen,
table.tcpscreen);
if (ch == ERR)
@@ -1044,7 +1044,7 @@ void ipmon(struct OPTIONS *options, struct filterstate *ofilter,
(unsigned int *) &readlen, &br,
&sport, &dport, &fromaddr,
ofilter, MATCH_OPPOSITE_ALWAYS,
- NULL, options->v6inv4asv6);
+ options->v6inv4asv6);
if (pkt_result != PACKET_OK)
continue;
diff --git a/src/packet.c b/src/packet.c
index d8211a9..15afc6d 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -106,13 +106,12 @@ static void adjustpacket(char *tpacket, struct sockaddr_ll
*fromaddr,
*/
void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int *ch,
- int *br, char *ifname, WINDOW * win)
+ int *br, WINDOW * win)
{
socklen_t fromlen;
fd_set set;
struct timeval tv;
int ss;
- struct ifreq ifr;
FD_ZERO(&set);
@@ -143,11 +142,6 @@ void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int
*ch,
fromlen = sizeof(struct sockaddr_ll);
*br = recvfrom(fd, buf, MAX_PACKET_SIZE, 0,
(struct sockaddr *) fromaddr, &fromlen);
- if(ifname) {
- ifr.ifr_ifindex = fromaddr->sll_ifindex;
- ioctl(fd, SIOCGIFNAME, &ifr);
- strcpy(ifname, ifr.ifr_name);
- }
}
if (!daemonized && FD_ISSET(0, &set))
*ch = wgetch(win);
@@ -157,16 +151,8 @@ int processpacket(char *tpacket, char **packet, unsigned int *br,
unsigned int *total_br, unsigned int *sport,
unsigned int *dport, struct sockaddr_ll *fromaddr,
struct filterstate *filter,
- int match_opposite, char *ifname, int v6inv4asv6)
+ int match_opposite, int v6inv4asv6)
{
-#if 0 /* reenable isdn */
- /*
- * Prepare ISDN reference descriptor and table.
- */
-
- memset(&isdntable, 0, sizeof(struct isdntab));
- isdn_iface_check(&isdnfd, ifname);
-#endif
/*
* Get IPTraf link type based on returned information and move past
* data link header.
diff --git a/src/packet.h b/src/packet.h
index 22f9a67..d9a0915 100644
--- a/src/packet.h
+++ b/src/packet.h
@@ -27,12 +27,12 @@ extern int isdnfd;
void open_socket(int *fd);
void getpacket(int fd, char *buf, struct sockaddr_ll *fromaddr, int *ch,
- int *br, char *ifname, WINDOW * win);
+ int *br, WINDOW * win);
int processpacket(char *tpacket, char **packet, unsigned int *br,
unsigned int *total_br, unsigned int *sport,
unsigned int *dport, struct sockaddr_ll *fromaddr,
struct filterstate *ofilter,
- int match_opposite, char *ifname, int v6inv4asv6);
+ int match_opposite, int v6inv4asv6);
void pkt_cleanup(void);
#endif /* IPTRAF_NG_PACKET_H */
diff --git a/src/pktsize.c b/src/pktsize.c
index ce0fd78..0612fbf 100644
--- a/src/pktsize.c
+++ b/src/pktsize.c
@@ -293,7 +293,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
&& (((now - starttime) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, NULL, win);
+ getpacket(fd, buf, &fromaddr, &ch, &br, win);
if (ch != ERR) {
switch (ch) {
@@ -316,8 +316,7 @@ void packet_size_breakdown(struct OPTIONS *options, char *ifname,
processpacket(buf, &ipacket, (unsigned int *) &br,
NULL, NULL, NULL, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, NULL,
- 0);
+ MATCH_OPPOSITE_USECONFIG, 0);
if (pkt_result != PACKET_OK)
continue;
diff --git a/src/serv.c b/src/serv.c
index 9566ecc..96d3a3c 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -863,7 +863,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
&& (((now - starttime) / 60) >= facilitytime))
exitloop = 1;
- getpacket(fd, buf, &fromaddr, &ch, &br, NULL, list.win);
+ getpacket(fd, buf, &fromaddr, &ch, &br, list.win);
if (ch == ERR)
goto no_key_ready;
@@ -998,7 +998,7 @@ void servmon(char *ifname, struct porttab *ports, const struct OPTIONS
*options,
processpacket(buf, &ipacket, (unsigned int *) &br,
&tot_br, &sport, &dport, &fromaddr,
ofilter,
- MATCH_OPPOSITE_USECONFIG, NULL,
+ MATCH_OPPOSITE_USECONFIG,
options->v6inv4asv6);
if (pkt_result != PACKET_OK)
--
1.7.8.4
1:46 p.m.
New subject: [PATCH 2/3] remove unused params from getpacket() and processpacket()
Vitezslav Samel <vitezslav(a)samel.cz> writes:
Since all the callers have ifname NULL we can remove the params
and associated code.
In processpacket() I deleted also ifdefed code hich checks for ISDN
^^^^^^ which
:)
interface (this needs to be done in other way).
Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
---
src/hostmon.c | 4 ++--
src/ifstats.c | 8 ++++----
src/itrafmon.c | 4 ++--
src/packet.c | 18 ++----------------
src/packet.h | 4 ++--
src/pktsize.c | 5 ++---
src/serv.c | 4 ++--
7 files changed, 16 insertions(+), 31 deletions(-)
patch is ok
--
Nikola
7:27 a.m.
New subject: [PATCH 3/3] ifstats.c: detstats(): warning cleanup
Remove the iphlen definition to kill the warning:
"warning: unused variable 'iphlen'"
Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
---
src/ifstats.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/ifstats.c b/src/ifstats.c
index 983ad55..f18c821 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -746,7 +746,6 @@ void detstats(char *iface, const struct OPTIONS *options, int
facilitytime,
char *packet;
struct iphdr *ipacket = NULL;
struct ip6_hdr *ip6packet = NULL;
- unsigned int iphlen;
struct sockaddr_ll fromaddr;
--
1.7.8.4
1:49 p.m.
New subject: [PATCH 3/3] ifstats.c: detstats(): warning cleanup
Vitezslav Samel <vitezslav(a)samel.cz> writes:
Remove the iphlen definition to kill the warning:
"warning: unused variable 'iphlen'"
Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz>
---
src/ifstats.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
this one hit the git repo, I was able to applied patch w/o previous two patches
--
Nikola
4399
days inactive
4405
days old
iptraf-ng@lists.fedorahosted.org
9 comments
2 participants
participants (2)
-
Nikola Pajkovsky -
Vitezslav Samel