Re: [PATCH] Re: Bug: Counting dropped packets in

[Dustin Spicuzza <dustin () virtualroadside com>]

Stephen Donnelly wrote:
> Dustin Spicuzza wrote:
>
> > > So after reading the libpcap and kernel source, I see that this is
> > > actually how its supposed to work. But it *seems* like it would be quite
> > > nice if we could grab the interface driver drop statistics as well as
> > > drops resulting from the capture buffer filling up.
> > >
> > > I'm looking for a way to query the kernel about this number -- should I
> > > bother submitting a patch if I can get it, or is the current behavior
> > > more desired?
> >
> > It appears that the only way to get this number is by looking at
> > /proc/stats/dev, so I added something that can parse it for the correct
> > interface, and get the drop stats from there. Since those are cumulative
> > over the boot time of the system, it stores a number and does
> > incremental updates of the number. It doesn't do this when not in
> > promiscuous mode however, since that doesn't make any sense.
> >
> > It works for me, let me know what you think.
>
> The current 'drop' count in libpcap is not intuitive, and frequently
> arguably undercounts since it does not include 'rx buffer overflow' and
> similar interface/OS specific packet loss. OTOH, the documentation is
> quite clear about what it does count I think.

No, its not; its actually quite vague. From the man page:

"pcap_stats() fills in the pcap_stat structure pointed to by its second
argument.  The values represent packet statistics from the start of the
run to the time of the call."

The code has a few comments as to what it counts, and *that* is quite
clear. However, I would imagine not too many people will read that
unless they're using pcap from source.

And, after looking at it, there *is* support for drops by the interface
in pcap_stat(), its just currently not used by very many platforms. I
didn't see this previously, but looking at pcap.h:

/*
 * As returned by the pcap_stats()
 */
struct pcap_stat {
        u_int ps_recv;          /* number of packets received */
        u_int ps_drop;          /* number of packets dropped */
        u_int ps_ifdrop;        /* drops by interface XXX not yet
supported */
#ifdef WIN32
        u_int bs_capt;          /* number of packets that reach the
application */
#endif /* WIN32 */
};


So... I've changed my patch to populate ps_ifdrop instead, and it should
be good to go, without screwing with current applications. I suppose the
man page should be updated to mention that ps_ifdrop is only supported
on a few platforms (I noticed that grep shows one or two instances where
it is used).

Dustin






-- 
Innovation is just a problem away

diff --git a/pcap-int.h b/pcap-int.h
index a548220..5786ea5 100644
--- a/pcap-int.h
+++ b/pcap-int.h
@@ -139,6 +139,7 @@ struct pcap_md {
        u_int   tp_version;     /* version of tpacket_hdr for mmaped ring */
        u_int   tp_hdrlen;      /* hdrlen of tpacket_hdr for mmaped ring */
        u_char  *oneshot_buffer; /* buffer for copy of packet */
+       long    proc_dropped; /* packets reported dropped by /proc/net/dev */
 #endif /* linux */
 
 #ifdef HAVE_DAG_API
diff --git a/pcap-linux.c b/pcap-linux.c
index 25208f6..c529d56 100644
--- a/pcap-linux.c
+++ b/pcap-linux.c
@@ -891,6 +891,60 @@ pcap_can_set_rfmon_linux(pcap_t *handle)
        return 0;
 }
 
+/* grabs the number of dropped packets by the interface from /proc/net/dev */
+static
+long int linux_if_drops(const char * if_name)
+{
+       char buffer[512];
+       char * bufptr;
+       FILE * file;
+       int field_to_convert = 3, if_name_sz = strlen(if_name);
+       long int dropped_pkts = 0;
+       
+       file = fopen("/proc/net/dev", "r");
+       if (!file)
+               return 0;
+
+       while (!dropped_pkts && fgets( buffer, sizeof(buffer), file ))
+       {
+               /*      search for 'bytes' -- if its in there, then
+                       that means we need to grab the fourth field. otherwise
+                       grab the third field. */
+               if (field_to_convert != 4 && strstr(buffer, "bytes"))
+               {
+                       field_to_convert = 4;
+                       continue;
+               }
+       
+               /* find iface and make sure it actually matches -- space before the name and : after it */
+               if ((bufptr = strstr(buffer, if_name)) &&
+                       (bufptr == buffer || *(bufptr-1) == ' ') &&
+                       *(bufptr + if_name_sz) == ':')
+               {
+                       bufptr = bufptr + if_name_sz + 1;
+
+                       /* grab the nth field from it */
+                       while( --field_to_convert && *bufptr != '\0')
+                       {
+                               while (*bufptr != '\0' && *(bufptr++) == ' ');
+                               while (*bufptr != '\0' && *(bufptr++) != ' ');
+                       }
+                       
+                       /* get rid of any final spaces */
+                       while (*bufptr != '\0' && *bufptr == ' ') bufptr++;
+                       
+                       if (*bufptr != '\0')
+                               dropped_pkts = strtol(bufptr, NULL, 10);
+
+                       break;
+               }
+       }
+       
+       fclose(file);
+       return dropped_pkts;
+} 
+
+
 /*
  * With older kernels promiscuous mode is kind of interesting because we
  * have to reset the interface before exiting. The problem can't really
@@ -1066,6 +1120,14 @@ pcap_activate_linux(pcap_t *handle)
                         pcap_strerror(errno) );
                return PCAP_ERROR;
        }
+       
+       /*
+        * If we're in promiscuous mode, then we probably want 
+        * to see when the interface drops packets too, so get an
+        * initial count from /proc/net/dev
+        */
+       if (handle->opt.promisc)
+               handle->md.proc_dropped = linux_if_drops(handle->md.device);
 
        /*
         * Current Linux kernels use the protocol family PF_PACKET to
@@ -1564,6 +1626,18 @@ pcap_stats_linux(pcap_t *handle, struct pcap_stat *stats)
        socklen_t len = sizeof (struct tpacket_stats);
 #endif
 
+       long if_dropped = 0;
+       
+       /* 
+        *      To fill in ps_ifdrop, we parse /proc/net/dev for the number
+        */
+       if (handle->opt.promisc)
+       {
+               if_dropped = handle->md.proc_dropped;
+               handle->md.proc_dropped = linux_if_drops(handle->md.device);
+               handle->md.stat.ps_ifdrop += (handle->md.proc_dropped - if_dropped);
+       }
+
 #ifdef HAVE_TPACKET_STATS
        /*
         * Try to get the packet counts from the kernel.
@@ -1584,6 +1658,8 @@ pcap_stats_linux(pcap_t *handle, struct pcap_stat *stats)
                 *      dropped by the interface driver.  It counts only
                 *      packets that passed the filter.
                 *
+                *      See above for ps_ifdrop. 
+                *
                 *      Both statistics include packets not yet read from
                 *      the kernel by libpcap, and thus not yet seen by
                 *      the application.
@@ -1647,16 +1723,22 @@ pcap_stats_linux(pcap_t *handle, struct pcap_stat *stats)
         *
         *      "ps_drop" is not supported.
         *
+        *      "ps_ifdrop" is supported. It will return the number
+        *      of drops the interface reports in /proc/net/dev
+        *
         *      "ps_recv" doesn't include packets not yet read from
         *      the kernel by libpcap.
         *
         * We maintain the count of packets processed by libpcap in
         * "md.packets_read", for reasons described in the comment
         * at the end of pcap_read_packet().  We have no idea how many
-        * packets were dropped.
+        * packets were dropped by the kernel buffers -- but we know 
+        * how many the interface dropped, so we can return that.
         */
+        
        stats->ps_recv = handle->md.packets_read;
        stats->ps_drop = 0;
+       stats->ps_ifdrop = handle->md.stat.ps_drop;
        return 0;
 }
 
diff --git a/pcap/pcap.h b/pcap/pcap.h
index 94fb722..5886c49 100644
--- a/pcap/pcap.h
+++ b/pcap/pcap.h
@@ -163,7 +163,7 @@ struct pcap_pkthdr {
 struct pcap_stat {
        u_int ps_recv;          /* number of packets received */
        u_int ps_drop;          /* number of packets dropped */
-       u_int ps_ifdrop;        /* drops by interface XXX not yet supported */
+       u_int ps_ifdrop;        /* drops by interface -- only supported on some platforms */
 #ifdef WIN32
        u_int bs_capt;          /* number of packets that reach the application */
 #endif /* WIN32 */

Attachment: signature.asc
Description: OpenPGP digital signature