Re: [libpcap][patch] appending to a capture

[Mark Johnston <markjdb () gmail com>]

Hi Darren,

On Tue, May 31, 2011 at 03:53:22PM -0700, Darren Reed wrote:
> Hi Mark,
>
> I must admit that I don't see the point of this patch.
>
> A pcap data file, with packets in it, is something that
> I would create using tcpdump over a specific period
> of time. The data file is thus associated with a very
> specific set of actions. To then append data to that
> file without that data being associated with the
> original action seems wrong.

In my tree at work, the function in used in a program quite similar
to tcpflow. I agree that it doesn't make sense to cat the output of
multiple tcpdump sessions into a single capture file, but it depends on
what I want to do with the recorded packets... in my case they're used
by another program to replay captured flows, so the context in which
they were originally captured doesn't really matter.

> That said, I can also imagine people using this function
> and running into huge performance problems.

I don't see how that is. The function essentially does the following:

- Check if we're writing to stdout. If so, write a header and return.
- Open the file, read the header and make sure it matches the one passed
  into the function.
- If it does, lseek to EOF, and if the file was empty, write a header.
- Return the descriptor.

How could this cause a performance problem?

> You might be better off spending some time working
> on additions to editcap that include concatenating
> two or more pcap files.

Shouldn't a function that manipulates capture files go into libpcap? I'm
not trying to solve a problem I'm having at the moment; rather, this
function has been in our tree for a long time, and I'd like to
contribute it upstream based on some interest that I saw. I'm happy to
modify it if that's what I need to do, but I think this functionality
should be in a library, not in a program.

Thanks,
-Mark

> On 30/05/11 01:58 PM, Mark Johnston wrote:
> > Hello all,
> >
> > I submitted a patch to the sourceforge tracker a while ago and didn't
> > receive any response. It adds a dump append function which verifies that
> > we only append if the link-layer type is the same as that in the capture
> > to append to. I submitted this based on a (rather old) thread:
> >
> > http://permalink.gmane.org/gmane.network.tcpdump.devel/1469
> >
> > My original submission is here:
> >
> > http://sourceforge.net/tracker/?func=detail&aid=3086711&group_id=53067&atid=469579
> >
> > I regenerated the patch against 1.1.1 and pasted it inline. I was hoping
> > that someone would be interested in reviewing it and giving some feedback,
> > and letting me know if there's any interest in having it committed.
> >
> > Thanks!
> > -Mark
> >
> >
> > diff --git a/pcap/pcap.h b/pcap/pcap.h
> > index 05ba31f..abf5d5b 100644
> > --- a/pcap/pcap.h
> > +++ b/pcap/pcap.h
> > @@ -337,6 +337,7 @@ int     pcap_fileno(pcap_t *);
> >
> >   pcap_dumper_t *pcap_dump_open(pcap_t *, const char *);
> >   pcap_dumper_t *pcap_dump_fopen(pcap_t *, FILE *fp);
> > +pcap_dumper_t *pcap_dump_append(pcap_t *, const char *);
> >   FILE      *pcap_dump_file(pcap_dumper_t *);
> >   long      pcap_dump_ftell(pcap_dumper_t *);
> >   int       pcap_dump_flush(pcap_dumper_t *);
> > diff --git a/sf-pcap.c b/sf-pcap.c
> > index 9d55dae..a3b0757 100644
> > --- a/sf-pcap.c
> > +++ b/sf-pcap.c
> > @@ -56,6 +56,7 @@ static const char rcsid[] _U_ =
> >   #include<stdio.h>
> >   #include<stdlib.h>
> >   #include<string.h>
> > +#include<unistd.h>
> >
> >   #include "pcap-int.h"
> >
> > @@ -579,6 +580,65 @@ pcap_dump_fopen(pcap_t *p, FILE *f)
> >     return (pcap_setup_dump(p, linktype, f, "stream"));
> >   }
> >
> > +pcap_dumper_t *
> > +pcap_dump_append(pcap_t *p, const char *fname)
> > +{
> > +
> > +   FILE *f;
> > +   int linktype;
> > +   int exists = 0, amt_read;
> > +   struct pcap_file_header ph;
> > +
> > +   linktype = dlt_to_linktype(p->linktype);
> > +   if (linktype == -1) {
> > +           snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
> > +               "%s: link-layer type %d isn't supported in savefiles",
> > +               fname, linktype);
> > +           return (NULL);
> > +   }
> > +   if (fname[0] == '-'&&  fname[1] == '\0') {
> > +           sf_write_header(stdout, linktype, p->tzoff, p->snapshot);
> > +           return ((pcap_dumper_t *)stdout);
> > +   }
> > +
> > +   exists = !access(fname, R_OK);
> > +   f = fopen(fname, "r+");
> > +   if (f == NULL) {
> > +           snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "%s: %s",
> > +               fname, pcap_strerror(errno));
> > +           return (NULL);
> > +   }
> > +
> > +   /* Read the header and make sure it's of the same linktype. */
> > +   amt_read = fread(&ph, 1, sizeof (ph), f);
> > +   if (amt_read != sizeof (ph)) {
> > +           if (ferror(f)) {
> > +                   snprintf(p->errbuf, PCAP_ERRBUF_SIZE, "%s: %s",
> > +                       fname, pcap_strerror(errno));
> > +                   return (NULL);
> > +           } else if (feof(f)&&  amt_read>  0) {
> > +                   snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
> > +                       "%s: truncated pcap file header", fname);
> > +                   return (NULL);
> > +           }
> > +   }
> > +
> > +   /*
> > +    * If a header is already present and doesn't match the linktype,
> > +    * return an error.
> > +    */
> > +   if (amt_read>  0&&  linktype != ph.linktype) {
> > +           snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
> > +               "%s: invalid linktype, cannot append to file", fname);
> > +           return (NULL);
> > +   }
> > +
> > +   fseek(f, 0, SEEK_END);
> > +   if (!exists)
> > +           (void)sf_write_header(f, linktype, p->tzoff, p->snapshot);
> > +   return ((pcap_dumper_t *)f);
> > +}
> > +
> >   FILE *
> >   pcap_dump_file(pcap_dumper_t *p)
> >   {
> > -
> > This is the tcpdump-workers list.
> > Visit https://cod.sandelman.ca/ to unsubscribe.
> >    
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.