3
\$\begingroup\$

I am a beginner in C and wrote this code to implement ARP.

3 threads are present - one to respond/receive any ARP packet targeted to me, one to refresh the ARP table on a periodic basis, and one to transmit packets present in the buffer.

An ARP table is implemented in a hash table. The sensitive areas are:

  1. A reply needs to be sent as soon as a request is received.
  2. An ARP table lookup should be very fast.

Please share your suggestions to improve the performance of the code. Also, please suggest some easier ways/tools to measure and share performance/machine cycles.

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/socket.h>
#include <linux/if_packet.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <netinet/in.h>
#include <net/ethernet.h>
#include <string.h>
#include <pthread.h>
#include <time.h>
#include <stdlib.h>
#include <stdbool.h>


#define ETH_HW_ADDR_LEN  6
#define IP_ADDR_LEN      4
#define ARP_ETHERTYPE    0x0806
#define ETHER_HW_TYPE    1
#define IP_ETHERTYPE     0x0800
#define ETH_ADDR_LEN     6

//The following opcodes are actually 0x0001 and 0x0002. It is written in Network byte order

#define ARP_REQ_OPCODE   0x0100
#define ARP_RES_OPCODE   0x0200

#define ARP_RESPONSE_WAITTIME 1       //TIME to wait for arp reponse in seconds
#define ARP_NUMBER_OF_TRIES 3         //Number of times ARP request need to be sent if reply not received
#define ARP_TIMEOUT 30                //seconds till an ARP entry is valid
#define ARP_REFRESHER_PERIODICITY 60  //Periodicity of ARP refresher in seconds

#define TX_BUFFER_SIZE 1000  //size of tx buffer
#define ARP_TABLE_SIZE 1000  //size of ARP lookup table

typedef struct ethernet_header {

  uint8_t dst_mac[ETH_ADDR_LEN];
  uint8_t src_mac[ETH_ADDR_LEN];

}ethernet_header;

typedef struct arp_header {

  uint16_t ethertype;
  uint16_t hw_type;
  uint16_t protocol_type;
  uint8_t  hw_addr_size;
  uint8_t  protocol_addr_size;
  uint16_t opcode;
  uint8_t  sender_mac[ETH_ADDR_LEN];
  uint8_t  sender_ip[IP_ADDR_LEN];
  uint8_t  target_mac[ETH_ADDR_LEN];
  uint8_t  target_ip[IP_ADDR_LEN];
  uint8_t  padding[18];

}arp_header;

typedef struct ARP_packet {

  ethernet_header eth_h;
  arp_header arp_h;

}Tx_packet;

typedef struct Tx_buff {

      Tx_packet *Tx_array[TX_BUFFER_SIZE];
      int head;
      int tail;

}Tx_buff;

typedef struct ARP_entry {

    uint8_t  ip[IP_ADDR_LEN];
    uint8_t  mac[ETH_ADDR_LEN];
    /* This variable can be incremented periodically to determine how old is an entry*/
    uint16_t howOld;
    struct ARP_entry *next;

}ARP_entry_t;

/* Functions */

/* general functions*/
void  *allocateMemory(int);
/* Functions related to ARP Table*/
bool  initializeARPTable();
int   ARPTableLookupHashFunction(uint8_t[]);
bool  ARPTableInsert(uint8_t [], uint8_t[]);
bool  ARPTableLookup( uint8_t[], uint8_t[]);
bool  deleteARPTable();
bool  showAllARPEntries();
/* Tx buffer and Transmission*/
bool  insertTxBuffer(Tx_packet*);
void  *Transmission();
/* related to ARP initiator and responder*/
void  createArpHeader(int);
void  *ARP_responder();
bool  ARP_initiator(uint8_t[]);
/* related to ARP refresher*/
ARP_entry_t *deleteOldEntries(ARP_entry_t*);
bool  refreshARPTable();
void  *ARP_refresher();


//Tx Buffer where ARP_initiator would place all the requests to be sent.
Tx_buff Tx_buffer = { .head = 0, .tail = -1 };

void **ARP_Table=NULL; //datastructure to store ARP entries.

time_t referenceTime=0; //the time at which the program starts
time_t lastRefreshedTime=0;
int noOfARPEntries = 0; //stores the number of arp entries present in the ARP table.

int socId;         //raw socket id
char *ifname=NULL; //input interface name
uint8_t initiatedIp[IP_ADDR_LEN] = {0}; // the IP for which ARP request is initiated

ethernet_header *eth; // ready packet with default values constructed using createArpHeader API.

//Variable which stores the status of the ARP responder and Transmission threads.
bool threadRunning=1;

/*****************************************************************************************
 * Function : allocateMemory()                                                        *
 * DESCRIPTION: This function is used to allocate memory using malloc. In case memory not
            available, it will throw error.                                                *
 * Args: int size : size of the memory block
 * Return Type: Void Pointer                                                            *
 * **************************************************************************************/

void* allocateMemory(int size)
{
    void *ptr = NULL;
    if((ptr=malloc(size))== NULL)
    {
            printf("Unable to allocate memory");
            exit;
    }
   return ptr;
}

bool initializeARPTable()
{
    int i;
    /* Allocate memory to hold ARP_TABLE_SIZE number of pointers*/
    ARP_Table=allocateMemory(sizeof(void *) * ARP_TABLE_SIZE);
    /* Initialize all pointer to NULL*/
    for(i=0;i<=ARP_TABLE_SIZE;i++)
        ARP_Table[i] = NULL;
    return 1;
}

int ARPTableLookupHashFunction(uint8_t ip[IP_ADDR_LEN])
{
    return ip[2];  //hash function is the last byte of the IP address
}

bool ARPTableInsert(uint8_t ip[IP_ADDR_LEN], uint8_t mac[ETH_ADDR_LEN])
{
    int ARPTableIndex = ARPTableLookupHashFunction(ip); //get the index using hash function.
    ARP_entry_t *node  = (ARP_entry_t *)ARP_Table[ARPTableIndex];

    /* check whether IP already exists*/
    while(node != NULL)
    {
        if(
                ip[0] == node->ip[0] &&
                ip[1] == node->ip[1] &&
                ip[2] == node->ip[2] &&
                ip[3] == node->ip[3]
        )break;

        node=node->next;

    }
    /*if node is not NULL, a matching IP is found*/
    if(node != NULL) {
        //memcpy(node->mac, mac, ETH_ADDR_LEN);
        node->mac[0] = mac[0];
        node->mac[1] = mac[1];
        node->mac[2] = mac[2];
        node->mac[3] = mac[3];
        node->mac[4] = mac[4];
        node->mac[5] = mac[5];
        /*update the timer to current time*/
        node->howOld = (uint16_t)(time(NULL) - referenceTime);
        return 1;
    }
    /* if no matching IP found, create a new node. */
    ARP_entry_t *newnode = allocateMemory(sizeof(ARP_entry_t));

    //insert the newnode
    newnode->next = ARP_Table[ARPTableIndex];  //newnode->next will point to the address pointed by the table.

    newnode->ip[0]=ip[0];
    newnode->ip[1]=ip[1];
    newnode->ip[2]=ip[2];
    newnode->ip[3]=ip[3];

    newnode->mac[0]=mac[0];
    newnode->mac[1]=mac[1];
    newnode->mac[2]=mac[2];
    newnode->mac[3]=mac[3];
    newnode->mac[4]=mac[4];
    newnode->mac[5]=mac[5];

    newnode->howOld = (uint16_t)(time(NULL) - referenceTime);

    ARP_Table[ARPTableIndex] = newnode;  //Table will have the address of the newnode.
    ++noOfARPEntries; //increment the number of entries present
    return 1;
}

bool ARPTableLookup( uint8_t inputIp[IP_ADDR_LEN], uint8_t resolvedMac[ETH_ADDR_LEN] )
{
    int ARPTableIndex = ARPTableLookupHashFunction(inputIp);
    int noOfTries = ARP_NUMBER_OF_TRIES; // number of times arp request to be resent in case reply not received

    ARP_entry_t *node = NULL, *nextNode = NULL;

    LOOKUP: //LOOKUP label is used to lookup the table again and again after sending ARP request again and again.
        node  = (ARP_entry_t *)ARP_Table[ARPTableIndex];

        while(node != NULL)
        {
            nextNode = node->next;
            /* If given IP matches IP of the node*/
            if(
                    inputIp[0]==node->ip[0] &&
                    inputIp[1]==node->ip[1] &&
                    inputIp[2]==node->ip[2] &&
                    inputIp[3]==node->ip[3]

              ){
                /* If the entry has timed out, break from the loop to send ARP requests*/
                if( (time(NULL) - (node->howOld + referenceTime)) > ARP_TIMEOUT )
                {
                    //free(node);
                    break;
                }
                /* if the entry has not timed out, return the output mac*/
                resolvedMac[0]=node->mac[0];
                resolvedMac[1]=node->mac[1];
                resolvedMac[2]=node->mac[2];
                resolvedMac[3]=node->mac[3];
                resolvedMac[4]=node->mac[4];
                resolvedMac[5]=node->mac[5];
                return 1;

            }
            node=nextNode;

        }
    /* when the no of retries has been reached, keep sending ARP request for configured no. of times and LOOKUP*/
    if(noOfTries--) {

        ARP_initiator(inputIp);
        sleep(ARP_RESPONSE_WAITTIME);
        goto LOOKUP;
    }

    return 0;
}

/* Function is used to delete the entire ARP hash Table*/
bool deleteARPTable()
{
    int i;
    ARP_entry_t *currentNode, *nextNode;
    for(i=0; i < ARP_TABLE_SIZE; i++)
    {
        currentNode=ARP_Table[i];
        while (currentNode != NULL)
        {
            nextNode = currentNode->next;
            free(currentNode);
            currentNode = nextNode;
        }
        ARP_Table[i] = NULL;
    }
    noOfARPEntries = 0; //set number of entries present as 0
    return 1;
}

/* Function is used to display all the ARP entries present in the ARP Table*/
bool showAllARPEntries()
{
    int i,j=1;
    ARP_entry_t *node;

    printf("s.no.\tIP ADDRESS\t\tMAC ADDRESS\t\tAge in sec\n");

    for(i=0; i < ARP_TABLE_SIZE; i++)
    {
        node=ARP_Table[i];
        while (node != NULL)
        {
            printf("%d    \t%d.%d.%d.%d\t\t%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\t    %d\n",j,node->ip[0],node->ip[1],node->ip[2],node->ip[3],node->mac[0],node->mac[1],node->mac[2],node->mac[3],node->mac[4],node->mac[5],time(NULL) - (node->howOld + referenceTime) );
            ++j;
            node = node->next;
        }
    }
    return 1;
}

/* Function used to insert an ARP request/reply into the Tx buffer*/
bool insertTxBuffer(Tx_packet *packetIntoBuffer)
{
    /*If the head is equal to tail, then there is no space in the buffer. Otherwsie append the packet in the buffer and move head to next position*/
    if( (Tx_buffer.head % TX_BUFFER_SIZE) != Tx_buffer.tail )
    {
        Tx_buffer.Tx_array[Tx_buffer.head] = packetIntoBuffer;
        Tx_buffer.head=++Tx_buffer.head % TX_BUFFER_SIZE;
        return 1;
    }
    printf("Unable to insert into Tx Buffer since the buffer is full.\n");
    return 0;

}

/* This thread is used to read the Tx buffer and send packets whenever present*/
void *Transmission()
{
    int tx_length;
    char *opcode_type=allocateMemory(sizeof(char *) * 10);

    while(1 && threadRunning)
    {
        /* If the next position of tail is equal to head, there is no packet in the buffer. Otherwise, increment tail and send the packet*/
        if(((Tx_buffer.tail+1) % TX_BUFFER_SIZE )!=Tx_buffer.head)
        {
            Tx_buffer.tail=(++Tx_buffer.tail) % TX_BUFFER_SIZE;
            tx_length=write(socId, Tx_buffer.Tx_array[Tx_buffer.tail], sizeof(Tx_packet));

            //Check whether it is request or reply to print
            if(((Tx_buffer.Tx_array[Tx_buffer.tail])->arp_h).opcode == ARP_REQ_OPCODE) { strcpy(opcode_type, "request"); } else { strcpy(opcode_type, "reply");}

            if(-1 == tx_length)
            {
                printf(" write error.Unable to write in the socket.\n");
                return NULL;
            } else {

                printf(" ARP %s sent to %d.%d.%d.%d \n", opcode_type, ( ( Tx_buffer.Tx_array[Tx_buffer.tail] )->arp_h ).target_ip[0],
                                                         ( ( Tx_buffer.Tx_array[Tx_buffer.tail] )->arp_h ).target_ip[1],
                                                         ( ( Tx_buffer.Tx_array[Tx_buffer.tail] )->arp_h ).target_ip[2],
                                                         ( ( Tx_buffer.Tx_array[Tx_buffer.tail] )->arp_h ).target_ip[3]);
            }


        }
        //wait for 1 ms.
        usleep(1000);
    }
    //free memory
    if(opcode_type != NULL) free(opcode_type);
    return NULL;
}

/*****************************************************************************************
 * Function : createArpHeader()                                                        *
 * DESCRIPTION: This function is used to create ARP header which will be used by
            ARP_responder() and ARP_initiator.

 * Args: opcode : ARP_RES_OPCODE or ARP_REQ_OPCODE
 * **************************************************************************************/

void createArpHeader(int opcode)
{
    eth = (ethernet_header *)allocateMemory(sizeof(ethernet_header)+sizeof(arp_header));
    arp_header *arp = (arp_header *)(eth+1);

    struct sockaddr_ll sll;
        struct ifreq ifr;
    int i;

    bzero(&sll, sizeof(sll));
    bzero(&ifr, sizeof(ifr));
    bzero(eth, sizeof(ethernet_header)+sizeof(arp_header));


// GET INTERFACE INDEX
        strncpy((char *)ifr.ifr_name, ifname, IFNAMSIZ);
        if((ioctl(socId, SIOCGIFINDEX, &ifr)) == -1)
        {
                printf("Error in getting Interface index !\nPlease configure correct interface\n");
                return;
        }
// GET MAC ADDRESS
        if((ioctl(socId, SIOCGIFHWADDR, &ifr)) == -1)
        {
                printf("Error in getting Interface MAC !\nPlease configure correct interface\n");
                return;
        }

    memcpy(&(eth->src_mac), ifr.ifr_hwaddr.sa_data, 6 * sizeof (uint8_t));
    memset(&(eth->dst_mac), 0xff, 6 * sizeof (uint8_t));

//GET IP ADDRESS
        if((ioctl(socId, SIOCGIFADDR, &ifr)) == -1)
        {
                printf("Error in getting Interface MAC !\nPlease configure correct interface\n");
                return;
        }
//FORM ARP HEADER
    arp->ethertype          = htons(ETH_P_ARP);
    arp->hw_type            = htons(ETHER_HW_TYPE);
    arp->protocol_type      = htons(ETH_P_IP);
    arp->hw_addr_size       = ETH_HW_ADDR_LEN;
    arp->protocol_addr_size = IP_ADDR_LEN;
    arp->opcode             = opcode;

    memcpy (&(arp->sender_mac), &(eth->src_mac), 6 * sizeof (uint8_t));
    memcpy (&(arp->sender_ip),  &((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr, 4 * sizeof (uint8_t));
    memset (&(arp->target_mac), 0x00, 6 * sizeof (uint8_t));
    memset (&(arp->target_ip),  0x00, 4 * sizeof (uint8_t));
    bzero(&(arp->padding), 18);

//BIND RAW SOCKET to the INTERFACE

        sll.sll_family   = AF_PACKET;
        sll.sll_ifindex  = ifr.ifr_ifindex;
        sll.sll_protocol = htons(ETH_P_ARP);

        if((bind(socId, (struct sockaddr *)&sll, sizeof(sll)))== -1)
        {
                printf("Error: Could not bind raw socket to interface\n");
                return;
        }

}
/*****************************************************************************************
 * Function : ARP_responder()                                                        *
 * DESCRIPTION: This function listens all ARP packets in the socket and send ARP reply
            if the request is destined to the specified interface                  *
 * Args: none.
 * Return Type: Void Pointer                                                            *
 * **************************************************************************************/

void* ARP_responder()
{

//rx_eth_header is to store the received ARP request and tx_eth_header is to store the ARP reply to be transmitted.

    ethernet_header *rx_eth_header = allocateMemory((sizeof(ethernet_header)+sizeof(arp_header)));
    arp_header *rx_arp=(arp_header *)(rx_eth_header+1);

    int rx_length, tx_length, i;

//call createArpHeader to create ARP response
    createArpHeader(ARP_RES_OPCODE);

    ethernet_header *tx_eth_header=allocateMemory(sizeof(ethernet_header)+sizeof(arp_header));
    arp_header *tx_arp=(arp_header *)(tx_eth_header+1);
    memcpy(tx_eth_header,eth, sizeof(ethernet_header)+sizeof(arp_header));

    //Until we receive packets and thread status is ON
    while(((rx_length=recvfrom(socId, rx_eth_header, sizeof(arp_header)+sizeof(ethernet_header), 0, NULL, NULL))>0) && threadRunning ) {

            if(
                //Check whether TARGET IP is my IP.

                rx_arp->target_ip[3]==tx_arp->sender_ip[3] &&
                rx_arp->target_ip[2]==tx_arp->sender_ip[2] &&
                rx_arp->target_ip[1]==tx_arp->sender_ip[1] &&
            rx_arp->target_ip[0]==tx_arp->sender_ip[0]

        )
        {
            // Check whether its a ARP Request. If so, send ARP Reply.
            if(rx_arp->opcode==ARP_REQ_OPCODE)
                {
                    // Copy his MAC and IP to my ARP reply
                            tx_arp->target_mac[0]=rx_arp->sender_mac[0];
                tx_arp->target_mac[1]=rx_arp->sender_mac[1];
                tx_arp->target_mac[2]=rx_arp->sender_mac[2];
                tx_arp->target_mac[3]=rx_arp->sender_mac[3];
                tx_arp->target_mac[4]=rx_arp->sender_mac[4];
                tx_arp->target_mac[5]=rx_arp->sender_mac[5];

                tx_eth_header->dst_mac[0]=tx_arp->target_mac[0];
                tx_eth_header->dst_mac[1]=tx_arp->target_mac[1];
                tx_eth_header->dst_mac[2]=tx_arp->target_mac[2];
                tx_eth_header->dst_mac[3]=tx_arp->target_mac[3];
                tx_eth_header->dst_mac[4]=tx_arp->target_mac[4];
                tx_eth_header->dst_mac[5]=tx_arp->target_mac[5];

                    tx_arp->target_ip[0]=rx_arp->sender_ip[0];
                tx_arp->target_ip[1]=rx_arp->sender_ip[1];
                tx_arp->target_ip[2]=rx_arp->sender_ip[2];
                tx_arp->target_ip[3]=rx_arp->sender_ip[3];

                //Append ARP reply packet to the Tx_buffer
                insertTxBuffer((Tx_packet *)tx_eth_header);
                //Add his MAC details into ARP table
                ARPTableInsert(rx_arp->sender_ip, rx_arp->sender_mac);

                printf("Received ARP request from %d.%d.%d.%d\n",tx_arp->target_ip[0], tx_arp->target_ip[1], tx_arp->target_ip[2], tx_arp->target_ip[3]);

            }
            //If its an ARP Reply && check whether 'target_ip' matches the IP we initiated.
            else if(
                rx_arp->opcode==ARP_RES_OPCODE &&
                rx_arp->sender_ip[3]==initiatedIp[3] &&
                        rx_arp->sender_ip[2]==initiatedIp[2] &&
                rx_arp->sender_ip[1]==initiatedIp[1] &&
                rx_arp->sender_ip[0]==initiatedIp[0]

            )
            {  //Insert into the ARP_Table
               ARPTableInsert(initiatedIp,rx_arp->sender_mac);
               printf("%d.%d.%d.%d is at %.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n", initiatedIp[0],initiatedIp[1],initiatedIp[2],initiatedIp[3],rx_arp->sender_mac[0],rx_arp->sender_mac[1],rx_arp->sender_mac[2],rx_arp->sender_mac[3],rx_arp->sender_mac[4],rx_arp->sender_mac[5]);
            }
         } else continue;

    }
    if(threadRunning) {
            printf("Unable to receive from socket\n");
    }
    close(socId);

//FREE all POINTERS

    free(rx_eth_header);
    free(tx_eth_header);
    rx_eth_header=NULL;
    tx_eth_header=NULL;
    if(eth!=NULL){
            free(eth);
            eth=NULL;
    }
    return NULL;
}

/*****************************************************************************************
 * Function : ARP_initiator()                                                        *
 * DESCRIPTION: This function is used to send ARP request
 * Args: none.
 * **************************************************************************************/

bool ARP_initiator(uint8_t target_ip[IP_ADDR_LEN])
{
    int tx_length;
    int i;

//The pointer 'eth' already has the ARP request format. Only Target IP and Opcode need to be updated.

    memcpy( &((arp_header *)(eth+1))->target_ip, target_ip, IP_ADDR_LEN);
    ((arp_header *)(eth+1))->opcode=ARP_REQ_OPCODE ;

    // Insert into Tx buffer
    insertTxBuffer((Tx_packet *)eth);
    return 1;
}

/*This function will recursively called to delete old/timeout entries in a list of entries*/
ARP_entry_t *deleteOldEntries(ARP_entry_t *currentNode)
{

    ARP_entry_t *nextNode;
    nextNode = currentNode->next;

    if ( nextNode->next != NULL )
    {
        deleteOldEntries(nextNode); //recursive call until node before last node is reached.

    }
    /* In case of timeout, delete the node*/
    if(((time(NULL) - (nextNode->howOld + referenceTime)) > ARP_TIMEOUT))
    {
        printf("ARP entry for %d.%d.%d.%d has expired\n", nextNode->ip[0],  nextNode->ip[1], nextNode->ip[2], nextNode->ip[3]);
        currentNode->next = nextNode->next;
        free(nextNode); nextNode = NULL;
        --noOfARPEntries; //decrement the no. of arp entries

    }
    //printf("return pointer of function is %p\n", currentNode);
    return currentNode;
}

/*This fiunction will refresh the ARP table*/
bool refreshARPTable()
{
    int i;
    ARP_entry_t *dummyNode = allocateMemory(sizeof(ARP_entry_t));

    printf("Refreshing ARP entries...\n");
    for(i=0;i<ARP_TABLE_SIZE;i++){
        if(ARP_Table[i] != NULL)
        {
            dummyNode->next = ARP_Table[i]; // a dummy node is created such that its next node is the first node of the list.
            ARP_Table[i] = (deleteOldEntries(dummyNode))->next; // dummy node will be returned by the function. Its next pointer would have the actual first node.
        }
    }
    lastRefreshedTime = time(NULL);
    if(dummyNode != NULL)
        free(dummyNode);
    return 1;
}

/* This thread will periodically trigger refreshARPTable function for every 'ARP_REFRESHER_PERIODICITY' seconds */
void* ARP_refresher()
{
    while(threadRunning)
    {
        //sleep to maintain periodicity
        sleep(ARP_REFRESHER_PERIODICITY);
        /* start refreshing if the current time is higher than the last refreshed time and no. of ARP entries is atleast one*/
        if(noOfARPEntries && ((time(NULL) - lastRefreshedTime) > ARP_REFRESHER_PERIODICITY))
            refreshARPTable();
    }
    return NULL;
}

int main(int argc, char *argv[])
{
    ifname = allocateMemory(10 * sizeof(char));
    strcpy(ifname,argv[2]);

    uint8_t *target_mac=(uint8_t *)allocateMemory(sizeof(uint8_t) * ETH_ADDR_LEN);

    initializeARPTable();
    referenceTime=time(NULL);     //time of start of the program acts as a reference for every ARP entries
    lastRefreshedTime = time(NULL); // time at which ARP table was refreshed
//Create Raw socket
    socId = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ARP));
    if (socId == -1)
    {
            printf("Error in creating raw socket");
            return 0;
    }

//Create ARP responder thread
    pthread_t thread1,thread2,thread3;
    pthread_create(&thread1,NULL,Transmission,NULL);
    pthread_create(&thread2,NULL,ARP_responder,NULL);
    usleep(10000);
    pthread_create(&thread3,NULL,ARP_refresher,NULL);

    //char *input=malloc(30*sizeof(char));
    char *input = NULL;
        input = allocateMemory (30 * sizeof(char));
    printf(">");
    while(fgets(input,30*sizeof(char),stdin) != NULL)
    {
            switch (input[0])
            {
                    case 's':
                            //Convert IP address to numeric format
                inet_aton(&input[9],initiatedIp);
                            ARP_initiator(initiatedIp);
                            break;
                    case '?':
                            printf("\nsend_arp <Target_IP>");
                printf("\nlookup <target_ip>");
                printf("\nview arp table");
                    printf("\ndelete arp table");
                    printf("\nrefresh arp table");
                printf("\nexit\n");
                            printf("\n>");
                            break;
            case 'l':
                inet_aton(&input[7],initiatedIp);
                if(ARPTableLookup( initiatedIp,target_mac ))
                     printf("%d.%d.%d.%d is at %.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n", initiatedIp[0],initiatedIp[1],initiatedIp[2],initiatedIp[3],target_mac[0],target_mac[1],target_mac[2],target_mac[3],target_mac[4],target_mac[5]);
                else
                    printf("Lookup failed! No such ARP entry.\n");
                printf(">");
                break;
            case 'd':
                deleteARPTable();
            case 'v':
                showAllARPEntries();
                printf(">");
                break;
            case 'r':
                refreshARPTable();
                printf(">");
                break;
            case 'e':
                            threadRunning=0; //CLOSE RESPONDER THREAD;
                usleep(500000);//wait for child thread to get teminated.
                deleteARPTable();//delete all the ARP entries
                            //usleep(500000);
                free(ARP_Table);   ARP_Table = NULL;
                free(input);       input = NULL;
                free(target_mac);  target_mac = NULL;
                free(ifname);      ifname = NULL;
                            return 0;
                    default:
                            printf(">");

            }
    }

        return 0;
}

Output:

-bash-3.2# ./arp_karthik.o -i eth0
>
>?
send_arp <Target_IP>
lookup <target_ip>
view arp table
delete arp table
refresh arp table
exit

>send_arp 192.168.10.1
 ARP request sent to 192.168.10.1
 192.168.10.1 is at 00:ff:f2:c8:03:88

>send_arp 192.168.10.13
  ARP request sent to 192.168.10.13
  192.168.10.13 is at 00:30:48:9a:2e:14

>view arp table
  s.no.   IP ADDRESS              MAC ADDRESS             Age in sec
  1       192.168.10.13           00:30:48:9a:2e:14           8
  2       192.168.10.1             00:ff:f2:c8:03:88             18
>
>delete arp table
  s.no.   IP ADDRESS              MAC ADDRESS             Age in sec

>send_arp 192.168.12.217
   ARP request sent to 192.168.12.217
   192.168.12.217 is at 00:ff:f2:c8:03:88

>Refreshing ARP entries...
   ARP entry for 192.168.12.217 has expired

>view arp table
s.no.   IP ADDRESS              MAC ADDRESS             Age in sec
>
>
\$\endgroup\$
0
5
\$\begingroup\$
  • UB

    Constructs like

        Tx_buffer.head=++Tx_buffer.head % TX_BUFFER_SIZE
    

    invoke undefined behaviour. Technically this makes the code broken.

  • Thread safety

    is not there. Shared structures ARP_Table and TX_buffer are accessed by multiple threads with no synchronization whatsoever. A disaster waiting to happen.

  • Buggy responder

    ARP_responder creates only one instance of tx_eth_header, but inserts it into the TX_buffer in the loop. Meaning that the buffer may contain multiple pointers to the same instance. Meaning that all of them point to the same dst_mac. Meaning that the responder may respond with the wrong data.

  • Too many allocations

    For objects as small as APR_entry_t, opcode_type, ethernet_header, etc. an allocate/free within same scope only leads to heap fragmentation. It is perfectly safe to keep them as local variables.

  • Endianness

    //The following opcodes are actually 0x0001 and 0x0002. It is written in Network byte order

    Not so. It is only in a network byte order on a little endian machine. On a big endian machine code will break. Define them naturally 0x0001, 0x0002 and use htons as for any other value.

  • Refresh

    Old entry deletion is weird. I don't see the need for dummyEntry and/or recursion.

  • More

    There are many more objections which are irrelevant until major problems are fixed.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks a lot @vnp. Will update soon after fixing these major issues. \$\endgroup\$ Nov 26 '15 at 8:14
  • \$\begingroup\$ @Karthik Please roll back the fixes - this invalidates the review. You are welcome to post the fixed code in the follow-up question. \$\endgroup\$
    – vnp
    Dec 8 '15 at 20:44
  • \$\begingroup\$ sure, could you please say how to roll back and what is a follow-up question \$\endgroup\$ Dec 8 '15 at 20:55
  • \$\begingroup\$ @Karthik I rolled it back. A follow-up question is just a regular question, preferably with a link to the initial question. For example, "I posted a such-and-such question (make it a hyperlink), here is an updated version of the code". \$\endgroup\$
    – vnp
    Dec 8 '15 at 21:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.