pcap_activate problem after pcap_set_buffer_size with 32k (TPACKET V3)

[mkeeler]

Note: only occurs with the TPACKET_V3 interface

The problem I am running into is that after setting the pcap buffer size to 32KB pcap_activate returns with a warning but all further operations I have tried (pcap_setfilter etc.) on the pcap_t fail with EBADF from the kernel. There are a few issues in play here.

1. In pcap-linux.c when when activate_mmap is called it is returning -1. When this happens the status to return from pcap_activate gets set to 1 (which happens to be PCAP_WARNING). However there is then a goto fail line which ends up causing pcap_cleanup_linux to be called which seems to be closing the underlying socket. I would recommend the error handling here be cleaned up. Either make sure when mmap fails but is supported to return an error indicating that the pcap_t shouldn't be used or fall back gracefully to un-mmapped packet capture. Either solution would end up with attempts to use a pcap_t which has no valid fd. From my perspective a warning that mmap failed is reasonable as performance might be degraded but I would expect the pcap_t to still be usable.

2. The second issue and the root cause of the problems seems to be that in create_ring for TPACKET v3 unconditionally uses MAXIMUM_SNAPLEN as the frame length. When the set buffer size is less than that MAXIMUM_SNAPLEN (which appears to be 256KB) the number of frames to be mapped gets set to 0. That block of code could be something like:

if ( handle->opt.buffer_size < MAXIMUM_SNAPLEN )
{
  req.tp_frame_size = handle->opt.buffer_size;
  req.tp_frame_nr = 1;
}
else
{
  req.tp_frame_size = MAXIMUM_SNAPLEN;
  req.tp_frame_nr = handle->opt.buffer_size/req.tp_frame_size;
}

Essentially req.tp_frame_nr should always be at least 1 or the mmap is going to fail. The code above ensures that it will. There may be reasons why its done this way that are not immediately obvious to me but this bit of code would fix my problems. The following code reproduces the problem reliably for me.

#include <stdio.h>
#include <pcap/pcap.h>

int main(int argc, const char * argv[])
{
   pcap_t * p = NULL;
   char errbuf[PCAP_ERRBUF_SIZE] = "";
   int e;

   if ( argc != 2 )
   {
      fprintf(stderr, "usage: pcap-test <interface name>\n");
      return 1;
   }

   p = pcap_create(argv[1], errbuf);
   if ( p == NULL )
   {
      fprintf(stderr, "Failed to create pcap\n");
      return 1;
   }

   pcap_set_snaplen(p, 1500);
   pcap_set_promisc(p, 0);
   pcap_set_timeout(p, 1);
   pcap_set_buffer_size(p, 32768);

   e = pcap_activate(p);
   if ( e != 0 )
   {
      fprintf(stderr, "pcap_activate %s - %d - %s\n", e < 0 ? "failed" : "warning", e, pcap_geterr(p));
   }
   else
   {
      printf("pcap_activate successful\n");
   }

   printf("pcap selectable fd - %d\n", pcap_get_selectable_fd(p));

   pcap_close(p);
   return 0;
}

Compile: gcc test.c -lpcap -opcap-test
Run: pcap-test

[mkeeler]

@guyharris While the commit you added would prevent this from completely blowing up it does it in a way that memory usage cannot be controlled.

The real application I am working with creates many pcap handles concurrently and with this using a minimum of MAXIMUM_SNAPLEN bytes for the buffer memory usage grows faster than I would like. Is there a reason why the tpacket v3 frame size has to be MAXIMUM_SNAPLEN. Couldn't the frame size be reduced to the requested buffer size if its less than MAXIMUM_SNAPLEN or would this cause some issue?

[guyharris]

Note that the frame size must be at least large enough to hold one packet of the specified snapshot length.

[mkeeler]

Certainly, that makes sense. So the frame size could be chosen like: min(max(handle->snapshot, handle->opt.buffer_size), MAXIMUM_SNAPLEN). That way the frame size is at least as big as the snapshot length that has been asked for, but could be bigger when the requested buffer size is larger than the snapshot length. And if all that exceeds MAXIMUM_SNAPLEN then the frame size gets capped and multiple frames can be used.

[guyharris]

Unfortunately, you also have to take into account the per-packet overhead, such as the tpacket header and the padding.

[mcr]

It seems that we should know what the tpacket header size is? @mkeeler, can you suggest a better formula? (and re-open if you do)