Re: [tcpdump-workers] Freeing memory in libpcap
Guy Harris <[EMAIL PROTECTED]> writes: > On Tue, Jul 24, 2001 at 09:05:36AM +0200, Yoann Vandoorselaere wrote: > > Is there any way to capture the packet one by read on any OS ? > > Perhaps, but I'm not certain that there is (and suspect that it's not > possible on all of the OSes I listed) - and that has other problems, > e.g. the reason why single reads return multiple packets is to reduce > the number of system calls made if there's a lot of captured traffic. Now, it would be interesting to see the cost of read() versus the copy alternative... -- Yoann Vandoorselaere | Unix IS user friendly. It's just selective about who its MandrakeSoft | friends are. - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
Re: [tcpdump-workers] Freeing memory in libpcap
On Tue, Jul 24, 2001 at 09:05:36AM +0200, Yoann Vandoorselaere wrote: > Is there any way to capture the packet one by read on any OS ? Perhaps, but I'm not certain that there is (and suspect that it's not possible on all of the OSes I listed) - and that has other problems, e.g. the reason why single reads return multiple packets is to reduce the number of system calls made if there's a lot of captured traffic. - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
Re: [tcpdump-workers] Freeing memory in libpcap
Guy Harris <[EMAIL PROTECTED]> writes:
> On Tue, Jul 24, 2001 at 08:26:57AM +0200, Yoann Vandoorselaere wrote:
> > Sure, it will behave differently, but should work without problem on
> > all architectures...
>
> With your patch, attempts to call "pcap_set_alloc_func()" on OSes other
> than Linux will fail, because that function doesn't exist. I consider
> that a problem for applications that want the packet data supplied to
> the callback to persist after the callback routine returns.
>
> So "pcap_set_alloc_func()" has to be written for the other OSes as well.
You didn't looked at the new version of this patch.
(which I attach here). This one should work for all OS. (it modify
pcap.c, not pcap-linux.c).
> The question then is "if the application has called
> 'pcap_set_alloc_func()', what should libpcap do on platforms where more
> than one packet is read into the buffer in a system call?"
>
> Should it just allocate a buffer for each packet, and copy from the
> buffer into the packet? That's the easiest solution, but it does mean
> you have to copy the packet.
>
> Or should it allocate a buffer for each read? That'd avoid the copy,
> but it'd mean that libpcap would have to keep track of the buffers, and
> provide a routine to free them, as the pointers handed to the callback
> routine will be pointers into the middle of that buffer, not pointers to
> independently allocated buffers.
>
> A further question is raised by the memory-mapped capture mechanism in
> Linux - if we add support for that, then, if the application has called
> 'pcap_set_alloc_func()', should libpcap allocate a buffer and copy from
> the memory-mapped buffer into the allocated buffer?
>
> I suspect "allocate a buffer for the packet and copy into it" is the
> right answer, but I'd like to see whether anybody else has any
> suggestions first.
Is there any way to capture the packet one by read on any OS ?
That would solve the problem...
New, generic, version of the patch :
Common subdirectories: libpcap.orig/CVS and libpcap/CVS
Common subdirectories: libpcap.orig/SUNOS4 and libpcap/SUNOS4
Common subdirectories: libpcap.orig/bpf and libpcap/bpf
Common subdirectories: libpcap.orig/lbl and libpcap/lbl
diff -up libpcap.orig/pcap-int.h libpcap/pcap-int.h
--- libpcap.orig/pcap-int.h Thu Dec 21 11:29:23 2000
+++ libpcap/pcap-int.h Mon May 28 10:32:24 2001
@@ -105,6 +105,8 @@ struct pcap {
struct bpf_program fcode;
char errbuf[PCAP_ERRBUF_SIZE];
+
+void *(*alloc)(size_t size);
};
/*
diff -up libpcap.orig/pcap.c libpcap/pcap.c
--- libpcap.orig/pcap.c Sat Dec 16 11:43:31 2000
+++ libpcap/pcap.c Mon May 28 10:36:07 2001
@@ -46,6 +46,7 @@ static const char rcsid[] =
#include
#include
#include
+#include
#ifdef HAVE_OS_PROTO_H
#include "os-proto.h"
@@ -56,10 +57,22 @@ static const char rcsid[] =
int
pcap_dispatch(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
{
-
+int ret;
+
+if ( p->alloc && ! p->buffer && ! (p->buffer = p->alloc(p->bufsize)) ) {
+snprintf(p->errbuf, sizeof(p->errbuf), "malloc: %s", pcap_strerror(errno));
+return -1;
+}
+
if (p->sf.rfile != NULL)
- return (pcap_offline_read(p, cnt, callback, user));
- return (pcap_read(p, cnt, callback, user));
+ ret = (pcap_offline_read(p, cnt, callback, user));
+ else
+ret = (pcap_read(p, cnt, callback, user));
+
+if ( ret > 0 )
+p->buffer = NULL;
+
+return ret;
}
int
@@ -67,6 +80,11 @@ pcap_loop(pcap_t *p, int cnt, pcap_handl
{
register int n;
+if ( p->alloc && ! (p->buffer = p->alloc(p->bufsize)) ) {
+snprintf(p->errbuf, sizeof(p->errbuf), "malloc: %s", pcap_strerror(errno));
+return -1;
+}
+
for (;;) {
if (p->sf.rfile != NULL)
n = pcap_offline_read(p, cnt, callback, user);
@@ -203,6 +221,14 @@ pcap_open_dead(int linktype, int snaplen
return p;
}
+
+void pcap_set_alloc_func(pcap_t *p, void *(*alloc)(size_t size))
+{
+free(p->buffer);
+p->buffer = NULL;
+p->alloc = alloc;
+}
+
void
pcap_close(pcap_t *p)
{
@@ -218,9 +244,14 @@ pcap_close(pcap_t *p)
(void)fclose(p->sf.rfile);
if (p->sf.base != NULL)
free(p->sf.base);
- } else if (p->buffer != NULL)
+ } else if (p->buffer != NULL && ! p->alloc )
free(p->buffer);
pcap_freecode(&p->fcode);
free(p);
}
+
+
+
+
+
diff -up libpcap.orig/pcap.h libpcap/pcap.h
--- libpcap.orig/pcap.h Sat Oct 28 02:01:31 2000
+++ libpcap/pcap.h Mon May 28 10:32:24 2001
@@ -157,6 +157,12 @@ int pcap_is_swapped(pcap_t *);
int pcap_major_version(pcap_t *);
int pcap_minor_version(pcap_t *);
+/*
+ * Set allocation function,
+ * this allow application to have their own buffer managment.
+ */
+void pcap_set_alloc_func(pcap_t *p, void *(*alloc)(size_t size));
+
/* XXX */
FILE *pcap_file(pcap_t *);
int pcap_fileno(pcap_t *);
--
Yoann Vandoorselaer
Re: [tcpdump-workers] Freeing memory in libpcap
On Tue, Jul 24, 2001 at 08:26:57AM +0200, Yoann Vandoorselaere wrote: > Sure, it will behave differently, but should work without problem on > all architectures... With your patch, attempts to call "pcap_set_alloc_func()" on OSes other than Linux will fail, because that function doesn't exist. I consider that a problem for applications that want the packet data supplied to the callback to persist after the callback routine returns. So "pcap_set_alloc_func()" has to be written for the other OSes as well. The question then is "if the application has called 'pcap_set_alloc_func()', what should libpcap do on platforms where more than one packet is read into the buffer in a system call?" Should it just allocate a buffer for each packet, and copy from the buffer into the packet? That's the easiest solution, but it does mean you have to copy the packet. Or should it allocate a buffer for each read? That'd avoid the copy, but it'd mean that libpcap would have to keep track of the buffers, and provide a routine to free them, as the pointers handed to the callback routine will be pointers into the middle of that buffer, not pointers to independently allocated buffers. A further question is raised by the memory-mapped capture mechanism in Linux - if we add support for that, then, if the application has called 'pcap_set_alloc_func()', should libpcap allocate a buffer and copy from the memory-mapped buffer into the allocated buffer? I suspect "allocate a buffer for the packet and copy into it" is the right answer, but I'd like to see whether anybody else has any suggestions first. - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
Re: [tcpdump-workers] Freeing memory in libpcap
Guy Harris <[EMAIL PROTECTED]> writes: > On Mon, Jul 23, 2001 at 07:10:14PM +0200, Yoann Vandoorselaere wrote: > > But it would be a good thing to be able to specify it's own buffer allocation > > policy. I sent a patch a while ago, but nobody answered (note it had a bug). > > ...and that it only adds that functionality on one platform. > > What should it do on, for example, platforms where a single kernel call > returns multiple packets (BSD, SunOS 3.x/4.x/5.x, Digital UNIX), or on > platforms that support a memory-mapped buffer (Linux 2.4[.x] kernel, > although we haven't yet put in any case for that, and perhaps the BSDs > in the future)? Should it fail? Or should it, on the first type of > platform, allocate buffers on each read (rather than on each packet) > and, on the second type of platform, allocate a buffer for each packet > and copy into it? Sure, it will behave differently, but should work without problem on all architectures... Or am I missing something ? -- Yoann Vandoorselaere | Tiniest "mesures unities?" MandrakeSoft | - lenght : millimeter | - volume : milliliter | - intelligence : military man - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
Re: [tcpdump-workers] Freeing memory in libpcap
On Mon, Jul 23, 2001 at 07:10:14PM +0200, Yoann Vandoorselaere wrote: > But it would be a good thing to be able to specify it's own buffer allocation > policy. I sent a patch a while ago, but nobody answered (note it had a bug). ...and that it only adds that functionality on one platform. What should it do on, for example, platforms where a single kernel call returns multiple packets (BSD, SunOS 3.x/4.x/5.x, Digital UNIX), or on platforms that support a memory-mapped buffer (Linux 2.4[.x] kernel, although we haven't yet put in any case for that, and perhaps the BSDs in the future)? Should it fail? Or should it, on the first type of platform, allocate buffers on each read (rather than on each packet) and, on the second type of platform, allocate a buffer for each packet and copy into it? - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
Re: [tcpdump-workers] Freeing memory in libpcap
Guy Harris <[EMAIL PROTECTED]> writes:
[Sorry for the duplicate]
> > For example, each time pcap_dispatch() or pcap_loop() calls the packet
> > handler function, a (u_char *) pointer to the new packet data is passed
> > to the packet handler; once I'm done with the packet, should I free()
> > this memory?
>
> No. That memory is managed by libpcap; note that if you want to
> preserve the contents of the packet data, you must make a copy of it, as
> libpcap makes no guarantee that the data in the packet will remain at
> that location once the packet handler function returns (in fact, it
> probably *won't* remain there, other stuff will be read into the buffer
> if "pcap_dispatch()" or "pcap_loop()" reads another bufferful of packets
> from the kernel).
But it would be a good thing to be able to specify it's own buffer allocation
policy. I sent a patch a while ago, but nobody answered (note it had a bug).
I attach a new version here.
Common subdirectories: libpcap.orig/CVS and libpcap/CVS
Common subdirectories: libpcap.orig/SUNOS4 and libpcap/SUNOS4
Common subdirectories: libpcap.orig/bpf and libpcap/bpf
Common subdirectories: libpcap.orig/lbl and libpcap/lbl
diff -up libpcap.orig/pcap-int.h libpcap/pcap-int.h
--- libpcap.orig/pcap-int.h Thu Dec 21 11:29:23 2000
+++ libpcap/pcap-int.h Mon May 28 10:32:24 2001
@@ -105,6 +105,8 @@ struct pcap {
struct bpf_program fcode;
char errbuf[PCAP_ERRBUF_SIZE];
+
+void *(*alloc)(size_t size);
};
/*
diff -up libpcap.orig/pcap.c libpcap/pcap.c
--- libpcap.orig/pcap.c Sat Dec 16 11:43:31 2000
+++ libpcap/pcap.c Mon May 28 10:36:07 2001
@@ -46,6 +46,7 @@ static const char rcsid[] =
#include
#include
#include
+#include
#ifdef HAVE_OS_PROTO_H
#include "os-proto.h"
@@ -56,10 +57,22 @@ static const char rcsid[] =
int
pcap_dispatch(pcap_t *p, int cnt, pcap_handler callback, u_char *user)
{
-
+int ret;
+
+if ( p->alloc && ! p->buffer && ! (p->buffer = p->alloc(p->bufsize)) ) {
+snprintf(p->errbuf, sizeof(p->errbuf), "malloc: %s", pcap_strerror(errno));
+return -1;
+}
+
if (p->sf.rfile != NULL)
- return (pcap_offline_read(p, cnt, callback, user));
- return (pcap_read(p, cnt, callback, user));
+ ret = (pcap_offline_read(p, cnt, callback, user));
+ else
+ret = (pcap_read(p, cnt, callback, user));
+
+if ( ret > 0 )
+p->buffer = NULL;
+
+return ret;
}
int
@@ -67,6 +80,11 @@ pcap_loop(pcap_t *p, int cnt, pcap_handl
{
register int n;
+if ( p->alloc && ! (p->buffer = p->alloc(p->bufsize)) ) {
+snprintf(p->errbuf, sizeof(p->errbuf), "malloc: %s", pcap_strerror(errno));
+return -1;
+}
+
for (;;) {
if (p->sf.rfile != NULL)
n = pcap_offline_read(p, cnt, callback, user);
@@ -203,6 +221,14 @@ pcap_open_dead(int linktype, int snaplen
return p;
}
+
+void pcap_set_alloc_func(pcap_t *p, void *(*alloc)(size_t size))
+{
+free(p->buffer);
+p->buffer = NULL;
+p->alloc = alloc;
+}
+
void
pcap_close(pcap_t *p)
{
@@ -218,9 +244,14 @@ pcap_close(pcap_t *p)
(void)fclose(p->sf.rfile);
if (p->sf.base != NULL)
free(p->sf.base);
- } else if (p->buffer != NULL)
+ } else if (p->buffer != NULL && ! p->alloc )
free(p->buffer);
pcap_freecode(&p->fcode);
free(p);
}
+
+
+
+
+
diff -up libpcap.orig/pcap.h libpcap/pcap.h
--- libpcap.orig/pcap.h Sat Oct 28 02:01:31 2000
+++ libpcap/pcap.h Mon May 28 10:32:24 2001
@@ -157,6 +157,12 @@ int pcap_is_swapped(pcap_t *);
int pcap_major_version(pcap_t *);
int pcap_minor_version(pcap_t *);
+/*
+ * Set allocation function,
+ * this allow application to have their own buffer managment.
+ */
+void pcap_set_alloc_func(pcap_t *p, void *(*alloc)(size_t size));
+
/* XXX */
FILE *pcap_file(pcap_t *);
int pcap_fileno(pcap_t *);
--
Yoann Vandoorselaere | In a world without walls or fences,
MandrakeSoft | what use do we have for windows or gates?
Re: [tcpdump-workers] Freeing memory in libpcap
Guy Harris <[EMAIL PROTECTED]> writes: > > For example, each time pcap_dispatch() or pcap_loop() calls the packet > > handler function, a (u_char *) pointer to the new packet data is passed > > to the packet handler; once I'm done with the packet, should I free() > > this memory? > > No. That memory is managed by libpcap; note that if you want to > preserve the contents of the packet data, you must make a copy of it, as > libpcap makes no guarantee that the data in the packet will remain at > that location once the packet handler function returns (in fact, it > probably *won't* remain there, other stuff will be read into the buffer > if "pcap_dispatch()" or "pcap_loop()" reads another bufferful of packets > from the kernel). But it would be a good thing to be able to specify it's own buffer allocation policy. I sent a patch a while ago, but nobody answered (note it had a bug). I attach a new version here. > > Should I do the same for the (struct pcap_pkthdr *) pointer > > that is also passed to the packet handler function? > > No. That memory is also managed by libpcap (and you have to save that > data yourself as well, if you want it to persist after the packet > handler returns). > > > Or, do pcap_dispatch() and pcap_loop() automatically free the memory > > once the packet handler function returns? > > No, it's more complicated than that - the packet buffer is allocated by > libpcap on an open, *reused* for each new bufferful of packet data read > from the kernel, and freed on a close. > > > If so, what about pcap_next()? > > "pcap_next()" calls "pcap_dispatch()", and the packet buffer to which it > returns a pointer is the one to which "pcap_dispatch()" returned a > pointer, so the same rules apply. > - > This is the TCPDUMP workers list. It is archived at > http://www.tcpdump.org/lists/workers/index.html > To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe > > -- Yoann Vandoorselaere | In a world without walls or fences, MandrakeSoft | what use do we have for windows or gates? - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
Re: [tcpdump-workers] Freeing memory in libpcap
> For example, each time pcap_dispatch() or pcap_loop() calls the packet > handler function, a (u_char *) pointer to the new packet data is passed > to the packet handler; once I'm done with the packet, should I free() > this memory? No. That memory is managed by libpcap; note that if you want to preserve the contents of the packet data, you must make a copy of it, as libpcap makes no guarantee that the data in the packet will remain at that location once the packet handler function returns (in fact, it probably *won't* remain there, other stuff will be read into the buffer if "pcap_dispatch()" or "pcap_loop()" reads another bufferful of packets from the kernel). > Should I do the same for the (struct pcap_pkthdr *) pointer > that is also passed to the packet handler function? No. That memory is also managed by libpcap (and you have to save that data yourself as well, if you want it to persist after the packet handler returns). > Or, do pcap_dispatch() and pcap_loop() automatically free the memory > once the packet handler function returns? No, it's more complicated than that - the packet buffer is allocated by libpcap on an open, *reused* for each new bufferful of packet data read from the kernel, and freed on a close. > If so, what about pcap_next()? "pcap_next()" calls "pcap_dispatch()", and the packet buffer to which it returns a pointer is the one to which "pcap_dispatch()" returned a pointer, so the same rules apply. - This is the TCPDUMP workers list. It is archived at http://www.tcpdump.org/lists/workers/index.html To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
