Andrei0427
Andrei0427

Reputation: 573

Code works for some time then just stops

I have made an application that monitors an interface and returns a packets per second reading, how ever when executed it runs fine for about 30 seconds till I open a YouTube page to get the counter running a little high. A couple of seconds later the application freezes and does nothing. This happens in irregular intervals so Im guessing its something with the counting, theres the code, its written in C.

#include <stdio.h>
#include <pcap.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <time.h>
#include <pthread.h>

void callThreads(u_char *useless, const struct pcap_pkthdr* pkthdr, const u_char* packet);
void *packetcalc(void *threadid);

static struct timespec time1, time2;
static int t = 0, i = 0;
static long rc;
static pthread_t threads[1];

int main(int argc, char *argv[]){

    pcap_t* descr;
    char errbuf[PCAP_ERRBUF_SIZE];


    descr = pcap_open_live("eth0", BUFSIZ, 1, -1, errbuf);

    if(descr == NULL){
        printf("Error: pcap_open_live()\n");
        return 1;
    }
    clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &time1);
    pcap_loop(descr, 0, callThreads, NULL);
    return 0;
}

void callThreads(u_char *useless, const struct pcap_pkthdr* pkthdr, const u_char* packet){
    if(i >= 2147483647){
        //In case i gets full from counting too many packets        
        i = 0;
        time1.tv_sec = 0;
    }

    ++i;
    rc = pthread_create(&threads[t], NULL, packetcalc, (void *) t); 
}

void *packetcalc(void *threadid){

    static int j;
    static int temp = 0;

    clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &time1);
    if(temp != time1.tv_sec){
        j = (i / time1.tv_sec);
        printf("Packets: %d/sec\t(%d)\t(%d)\n", j, i, (int)time1.tv_sec);
        temp = time1.tv_sec;
    }


    pthread_exit(NULL);
}

Edit: Could it also be that I'm running this code in a VM that only has 1 CPU allocated to it due to the multithreading?

Upvotes: 0

Views: 172

Answers (2)

Nikolai Fetissov
Nikolai Fetissov

Reputation: 84159

You are creating a thread per packet, which is a horrible idea. It should be enough to just print whatever counter you need right out of the callback function you give to pcap_loop(3).

Upvotes: 2

Hristo Iliev
Hristo Iliev

Reputation: 74375

There are several problems with your code. First, you create the threads using the default thread attributes, which means that they are created as joinable threads, i.e. you must call pthread_join() later or the thread control structures would remain lingering around. Hence there is a memory leak in your code. May be you should check the return value from pthread_create in order to detect when an error occurs, e.g. the system was not able to create new threads and your packet counting routine has stopped being invoked. You can also create new threads in detached state using the following code:

pthread_attr_t attr;

pthread_attribute_init(&attr);
pthread_attribute_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);

pthread_create(&threadid, &attr, packetcalc, (void *) t);

pthread_attribute_destroy(&attr);

Detached threads do not need to be joined later. They release all resources upon thread exit.

Second, you threads use some global variables as if they are private when in reality they are shared. This includes the global time1, as well as the local j and temp, which are declared static and hence are shared among the threads.

Note that creating threads is an expensive operation. While your code waits for pthread_create to finish, new packets may arrive and fill up the circular buffer, used by libpcap, hence you might lose some packets. As a matter of fact, using one thread per packet is a very bad idea. Instead use only two threads - one that runs the pcap loop and one that periodically counts the number of packets and calculates and prints the packet rate.

Upvotes: 1

Related Questions