[prev in list] [next in list] [prev in thread] [next in thread] 

List:       tcpdump-workers
Subject:    Re: [tcpdump-workers] libpcap linux mmap patch
From:       Guy Harris <guy () alum ! mit ! edu>
Date:       2008-02-02 20:48:03
Message-ID: 47A4D703.5040801 () alum ! mit ! edu
[Download RAW message or body]

Abeni Paolo wrote:

>> *) If pcap_loop is called with cnt=0 (ngrep erroneously does that), it
>> will busy-loop forever. pcap_read_linux_mmap doesn't handle that case,
>> it returns 0, which is asymmetric to pcap_read_linux's behavior, which
>> reads one packet.
> 
> I think there is a little confusion about the 'cnt' parameter.
> According to man page a value of 0 should cause no packet to be read,
> but into pcap_loop a value of '0' is handled like negative values (i.e.
> loop forever). If the behavior described into the man page is the
> preferred one, than also your second patch looks correct to me (but it
> can break application which 'misused' the pcap API using a value of cnt
> == 0 to loop forever).

Looking at libpcap 0.4 (the last release from LBL), in pcap_dispatch():

	on systems with BPF (*BSD, Mac OS X), a "cnt" value of 0 is equivalent 
to a "cnt" value of -1 (the loop checks for "++n >= cnt && cnt > 0");

	on systems with DLPI (Solaris 2 and later, HP-UX), a "cnt" value of 0 
is equivalent to a "cnt" value of 1 (the loop checks for "++n >= cnt && 
cnt >= 0", but the check is done *after* the call to the callback 
routine, so it doesn't prevent the first call);

	on systems with NIT (SunOS 3.x) and STREAMS NIT (SunOS 4.x), "cnt" is 
handled as it is on systems with DLPI;

	on Linux systems (0.4 had a completely different libpcap implementation 
on Linux, with a number of problems, which is why it was later 
completely replaced), and on Irix, the "cnt" value is ignored (only one 
packet is delivered per call to pcap_dispatch, because Linux and Irix 
don't have a scheme to deliver, on a socket, a buffer with multiple 
packets).

pcap-win32.c, added by WinPcap, works the same way that pcap-bpf.c does, 
i.e. a "cnt" value of 0 is equivalent to a "cnt" value of -1.

pcap-dag.c, in effect, does so as well, as it checks, after calling the 
callback and counting the packet, whether the count of processed packets 
is equal to "cnt", which can't be the case if "cnt" is 0 or -1.

pcap-septel.c works the same way that DLPI does, as it checks, after 
processing a packet, whether the count is < "cnt".

pcap-bt-linux.c processes one packet at a time, and ignores the count.

pcap-usb-linux.c processes one packet at a time, and ignores the count, 
except when it's using the memory-mapped interface, in which case a 
count of 0 might well truly cause no packets to be processed (as it 
causes the count of events to be fetched to be 0).

When capturing (rather than reading from a savefile), pcap_loop() is, in 
effect:

	for (;;) {
		/* Keep reading until we get at least one packet */
		do {
			n = pcap_dispatch(p, cnt, callback, user);
		} while (n == 0);
		if (n < 0)
			return (n);	/* error */
		if (cnt > 0) {
			cnt -= n;
			if (cnt <= 0)
				return (0);	/* done */
		}
	}

which means that a "cnt" value of 0 means it'll keep calling 
pcap_dispatch(), so it will be equivalent to a "cnt" value of -1.

Given that:

	it makes no sense to have a "cnt" value of 0 mean "deliver at most 0 
packets";

	it doesn't actually mean that on any platforms;

	any application that depends on a "cnt" value of 0 *not* meaning 
"deliver a complete buffer full of packets" in pcap_dispatch() (i.e., 
being equivalent to a "cnt" value of -1) isn't going to work on *BSD;

	a "cnt" value of 0 to pcap_loop() means "loop forever", it just means 
that, on systems with NIT, STREAMS NIT, or DLPI, it'll do it 
inefficiently (repeatedly calling the "read" routine to read a single 
packet);

I'd say that the right behavior should be to have a "cnt" value of 0 be 
treated the same as a "cnt" value of -1, which means fixing some of the 
existing modules and fixing the man page.

I'll make those changes.
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.
[prev in list] [next in list] [prev in thread] [next in thread]