Hi Luca, Hi all, I'm afraid, but I think your patch has a race condition under (at least under Linux). In Linux two syscalls are required to read a packet. First you read the packet, that you read the timestamp value with an ioctl. If one thread gets scheduled just after the packet data is read, the timestamp value will become bogus. Furthermore the pcap_read_* functions also read-modify fields in the pcap handle (p->breakloop, p->cc, etc). I don't know whether they are meaningful for pcap_next_pkt, but if they are, they are another race condition.
I think you won't be able to achieve reentrancy when reading from one pcap handle without locking! Note however, that reading from different pcap handles (e.g., one handle per thread) is already thread safe, since no global data structures are used. I general I'm not sure, whether a reentrant libpcap would have benefits. When two threads read from the same pcap handle they will receive packets in an arbitrary order. So either, the application doesn't care about inter-packet relations at all, which seems unlikely. Or the threads must be synchronized by the application anyway. However, your patch can help when developing multi-threaded applications. Being able to specify the buffer to which the packet is written has the advantage, that it might save a memcpy operation to copy the packet to a dedicated storage area. When I use libpcap I almost always do a memcpy after the pcap_next call.... So in short, I think the patch is nice idea, but it does not provide reentrancy and thus should not be labeled 'reentrant'. Just my 2ct. Happy new year, Gregor
signature.asc
Description: OpenPGP digital signature
