Re: What's wrong?

[Juan Cespedes <cespedes@debian.org>]

On Tue, Jan 05, 1999 at 10:52:24AM +0100, Tibor Koleszar wrote:
> I'm writing an mrtg-like program. It's finished but i dont want to use the
> kernel's ip accounting, so I try to write a RAW SOCKET based traffic
> counter. I've written a small program following by the socket faq and man
> pages, but i think it's reading bad ip headers. Here is the program:

	Well, I have many things to comment about your code:

> extern int errno;
* Please don't define `errno'. Include <errno.h> instead.

> struct ip_pkt {
>  struct iphdr ip;
>  char *buffer;
> } pkt;
* That struct is just 24 bytes: 20 for a `struct iphdr' and 4 for a
  `char *'. Maybe you wanted to use `char buffer[1024]' or something
  like that.
* The size of the IP header in a packet is not always 20 bytes (that
  is, `sizeof(struct iphdr)'; in an IP packet there can be options;
  the correct size of an IP header is `4*ip->ihl' (in bytes).

>    if ((s = socket(AF_INET, SOCK_PACKET, PROTO)) == -1) {
* SOCK_PACKET is obsoleted in linux-2.1 and linux-2.2. You should use
  `socket(PF_PACKET, SOCK_RAW, PROTO)' and, if that fails, use
  your `socket(PF_INET, SOCK_PACKET, PROTO)'.
* SOCK_PACKET (and PF_PACKET) adds a 14-byte header to each packet
  received: the ethernet header.  Thus, you are interpreting an
  ethernet header as an IP packet in your code.

>      read(s, &pkt, sizeof(struct ip_pkt));
* I think you should use `recv' or `recvfrom' instead of `read'.

	In short, I would rewrite all that code this way:

----------------------------------------------------------------------
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/if_ether.h>
#include <netinet/in.h>
#include <netinet/ip.h>

#define PROTO htons(0x0800)

char buffer[65536];

int main(void)
{
	int s;
	struct in_addr in;
	struct iphdr * ip;

	s = socket(PF_PACKET, SOCK_RAW, PROTO);
	if (s == -1) {
		s = socket(PF_INET, SOCK_PACKET, PROTO);
	}
	if (s == -1) {
		perror("socket");
		exit(1);
	}

	while(1) {
		recv(s, buffer, 65536, 0);
		ip = (struct iphdr *)(buffer+sizeof(struct ethhdr));
		in.s_addr = ip->saddr;
		printf("IP Packet from: %s\n", inet_ntoa(in));
	}
}
----------------------------------------------------------------------

-- 
Juan Cespedes