Re: [PATCH]: pcap and zero copy.

[Yoann Vandoorselaere <yoann () prelude-ids org>]

On Mon, 2004-01-05 at 19:11, Guy Harris wrote:
> On Sun, Jan 04, 2004 at 12:02:13PM +0000, Yoann Vandoorselaere wrote:
> > - are people interested by the inclusion of this patch ?
>
> Yes, as long as it's done on *all* platforms, and done against the
> current CVS version (and doesn't change "pcap_next()" - if
> "pcap_next_ex()" can handle this without change, that'd be better). 
> I've looked at doing it on other platforms, but haven't had time to work
> on it.

I'm already working on porting the patch to libpcap-0.8.1, and will send
the raw patch (without const change) as soon as possible.

I'll also work on adding support for pcap-win32.c (most of the
architecture are now supported in the patch - but I'll double check).

Also, is the API in it's current form okay with you ?

> Most platforms are either like BPF (a read from the kernel can return
> multiple packets) or Linux (a read from the kernel returns only a single
> packet).

Yep, the patch work on both type of implementation and was thoroughly
tested on BSD platform and Linux platform.

> Note that it would also need to work with the Linux turbopacket stuff
> once that gets added to libpcap.  The problem with the turbopacket stuff
> is that there's no buffer into which stuff is read from the kernel - the
> packets come from a buffer shared with the kernel.  That'd probably
> require that a buffer be allocated for each packet and a copy done to
> that buffer, which means that the turbopacket stuff would involve *more*
> copies (one from skbuff to shared buffer, one from shared buffer to
> mallocated buffer, rather than one from skbuff to mallocated buffer with
> a "recvfrom()").

Well, the point with this patch is that if the application doesn't use
the pcap_set_alloc_func() / pcap_set_freeing_func() function, then the
current implementation (static buffer, no malloc) will be used. 

So if the the turbopacket implementation want to use the buffer that
come from the kernel, it will be fine, as long as the alloc / freeing
function pointer are not set.

However, I agree that a policy where the application only care about
what it want, and not the way it's done would be better. It'd require
more work on the pcap side however.

> The right way to handle that might be to have a way of specifying, at
> open time, that packet buffers need to be persistent.  If that's the
> case, we probably wouldn't use the turbopacket stuff on Linux.  (It has
> been claimed that some libpcap applications depend on parent and child
> processes being able to share access to a libpcap descriptor, and that
> the turbopacket packet stuff would break that, which is why Phil Wood's
> turbopacket patch has an environment variable you have to set in order
> to use turbopacket mode; in the long term, I'd like to have a new API
> for opening live captures, that allows a number of things to be
> specified at open time:
>
>       whether to open for capturing, sending, or capturing and sending
>       (so that, on BPF, the device can be opened for reading, writing,
>       or reading and writing);
>
>       whether to open in monitor mode for 802.11 devices (to hide the
>       rather, umm, *diverse* ways of doing that on different OSes and
>       different devices on the *same* OS inside libpcap);
>
>       whether the libpcap descriptor has to be shareable between
>       parent and child processes;
>
>       a capture buffer size (BPF doesn't let you set that once you've
>       bound to a device);
>
> and possibly some other things.)

Adding kind of pcap_set_flags() function might help here, and is not in
contradiction with the current patch (the public api for the patch would
remain the same, etc)... 

So am I right saying that before the patch get included the following
need to be done :

- remove constness fix from the patch.
- remove use of zero sized array (some architecture doesn't support it).
- port to libpcap-0.8.1.
- port to yet unsuported backend.
- move pcap_next() modification to pcap_next_ex()

Will the patch finally make it's way to the pcap main repository once
this is done ?


Thanks,

-- 
Yoann Vandoorselaere <yoann () prelude-ids org>

Attachment: signature.asc
Description: This is a digitally signed message part