[tcpdump-workers] [RFC PATCH] Add new `pcap_set_buffer_size1` API.
From: Mario Rugiero
The current `pcap_set_buffer_size` sets a limit of 2GiB buffer size.
This changeset implements a backwards compatible mechanism to set
bigger buffers.
A new `pcap_set_buffer_size1` call is created, taking a `size_t`
instead of an `int`, allowing for buffers as big as the platform
allows.
The `buffer_size` field of `struct pcap_opt` was promoted to `size_t`.
Due to some contexts requiring smaller maximum buffers, a new field
named `max_buffer_size` of type `size_t` was added to the same structure
to account for that.
This field is initialized by default to `INT_MAX` to preserve the
behaviour of the older API.
Then, each driver is expected, but not mandated, to fix it to a more
appropriate value for the platform.
In this RFC, Linux and DPDK are used as examples.
diff --git a/pcap-dpdk.c b/pcap-dpdk.c
index 6c2f21fc..81df91e3 100644
--- a/pcap-dpdk.c
+++ b/pcap-dpdk.c
@@ -962,10 +962,12 @@ pcap_t * pcap_dpdk_create(const char *device, char *ebuf,
int *is_ours)
p = pcap_create_common(ebuf, sizeof(struct pcap_dpdk));
if (p == NULL)
return NULL;
p->activate_op = pcap_dpdk_activate;
+ p->opt.max_buffer_size = SIZE_MAX;
+
return p;
}
int pcap_dpdk_findalldevs(pcap_if_list_t *devlistp, char *ebuf)
{
diff --git a/pcap-int.h b/pcap-int.h
index b614efbe..64604bc4 100644
--- a/pcap-int.h
+++ b/pcap-int.h
@@ -110,11 +110,12 @@ extern "C" {
((c) >= 'a' && (c) <= 'f'))
struct pcap_opt {
char*device;
int timeout;/* timeout for buffering */
- u_int buffer_size;
+ size_t max_buffer_size;/* platform's max buffer size - backends
*should* override it */
+ size_t buffer_size;
int promisc;
int rfmon; /* monitor mode */
int immediate; /* immediate mode - deliver packets as soon as
they arrive */
int nonblock; /* non-blocking mode - don't wait for packets
to be delivered, return "no packets available" */
int tstamp_type;
diff --git a/pcap.c b/pcap.c
index ebb992bf..a9e8bbeb 100644
--- a/pcap.c
+++ b/pcap.c
@@ -2352,10 +2352,11 @@ pcap_create_common(char *ebuf, size_t size)
initialize_ops(p);
/* put in some defaults*/
p->snapshot = 0;/* max packet size unspecified */
p->opt.timeout = 0; /* no timeout specified */
+ p->opt.max_buffer_size = INT_MAX; /* default to old API's max for
compatibility */
p->opt.buffer_size = 0; /* use the platform's default */
p->opt.promisc = 0;
p->opt.rfmon = 0;
p->opt.immediate = 0;
p->opt.tstamp_type = -1;/* default to not setting time stamp
type */
@@ -2492,10 +2493,21 @@ pcap_set_buffer_size(pcap_t *p, int buffer_size)
}
p->opt.buffer_size = buffer_size;
return (0);
}
+int
+pcap_set_buffer_size1(pcap_t *p, size_t buffer_size)
+{
+ if (pcap_check_activated(p))
+ return (PCAP_ERROR_ACTIVATED);
+ if (buffer_size > p->opt.max_buffer_size)
+ return (PCAP_ERROR_BUFFER_TOO_BIG);
+ p->opt.buffer_size = buffer_size;
+ return (0);
+}
+
int
pcap_set_tstamp_precision(pcap_t *p, int tstamp_precision)
{
int i;
diff --git a/pcap/pcap.h b/pcap/pcap.h
index 21ea980a..f78ad351 100644
--- a/pcap/pcap.h
+++ b/pcap/pcap.h
@@ -303,10 +303,11 @@ typedef void (*pcap_handler)(u_char *, const struct
pcap_pkthdr *,
#define PCAP_ERROR_PERM_DENIED -8 /* no permission to open the
device */
#define PCAP_ERROR_IFACE_NOT_UP-9 /* interface isn't up */
#define PCAP_ERROR_CANTSET_TSTAMP_TYPE -10 /* this device doesn't support
setting the time stamp type */
#define PCAP_ERROR_PROMISC_PERM_DENIED -11 /* you don't have permission to
capture in promiscuous mode */
#define PCAP_ERROR_TSTAMP_PRECISION_NOTSUP -12 /* the requested time stamp
precision is not supported */
+#define PCAP_ERROR_BUFFER_TOO_BIG -13 /* passed an invalid argument
*/
/*
* Warning codes for the pcap API.
* These will all be positive and non-zero, so they won't look like
* errors.
@@ -338,10 +339,11 @@ PCAP_API int pcap_can_set_rfmon(pcap_t *);
PCAP_API int pcap_set_rfmon(pcap_t *, int);
PCAP_API int pcap_set_timeout(pcap_t *, int);
PCAP_API int pcap_set_tstamp_type(pcap_t *, int);
PCAP_API int pcap_set_immediate_mode(pcap_t *, int);
PCAP_API int pcap_set_buffer_size(pcap_t *, int);
+PCAP_API int pcap_set_buffer_size1(pcap_t *, size_t);
PCAP_API int pcap_set_tstamp_precision(pcap_t *, int);
PCAP_API int pcap_get_tstamp_precision(pcap_t *);
PCAP_API int pcap_activate(pcap_t *);
PCAP_API int pcap_list_tstamp_types(pcap_t *, int **);
--
2.20.1
___
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] [RFC PATCH] Add new `pcap_set_buffer_size1` API.
On Oct 2, 2019, at 2:16 PM, Mario Rugiero wrote: > A new `pcap_set_buffer_size1` call is created, taking a `size_t` > instead of an `int`, allowing for buffers as big as the platform > allows. Perhaps pcap_set_buffer_size_ext (Windows-style) would be better - a 1 at the end 1) is a bit unclear about what it means and 2) may look too much like an l (I first thought it *was* an "l", for "long", but maybe that's just the particular fixed-width font that's the default in macOS). (Or pcap_set_buffer_size_size_t, but that may be a bit awkward.) > Due to some contexts requiring smaller maximum buffers, a new field > named `max_buffer_size` of type `size_t` was added to the same structure > to account for that. There should probably be an API to get the maximum buffer size as well, for the benefit of 1) programs that want "the biggest buffer they can get" and 2) GUI programs that might have a "buffer size" field implemented as a spinbox. Should pcap_set_buffer_size also check against the maximum size, and set it to the maximum size if it's above the maximum? > This field is initialized by default to `INT_MAX` to preserve the > behaviour of the older API. > Then, each driver is expected, but not mandated, to fix it to a more > appropriate value for the platform. > In this RFC, Linux and DPDK are used as examples. Is there a maximum buffer size > INT_MAX for Linux? At least in macOS, and possibly in other BSD-flavored OSes, the sysctl variable debug.bpf_maxbufsize will indicate the maximum size. ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] [RFC PATCH] Add new `pcap_set_buffer_size1` API.
El mié., 2 oct. 2019 a las 18:46, Guy Harris () escribió: > > On Oct 2, 2019, at 2:16 PM, Mario Rugiero wrote: > > > A new `pcap_set_buffer_size1` call is created, taking a `size_t` > > instead of an `int`, allowing for buffers as big as the platform > > allows. > > Perhaps pcap_set_buffer_size_ext (Windows-style) would be better - a 1 at the > end 1) is a bit unclear about what it means and 2) may look too much like an > l (I first thought it *was* an "l", for "long", but maybe that's just the > particular fixed-width font that's the default in macOS). > > (Or pcap_set_buffer_size_size_t, but that may be a bit awkward.) > I used '1' because that's what Linux does when advertising newer versions of syscalls. '_ext' does look better, I think I'd go with that. > > Due to some contexts requiring smaller maximum buffers, a new field > > named `max_buffer_size` of type `size_t` was added to the same structure > > to account for that. > > There should probably be an API to get the maximum buffer size as well, for > the benefit of 1) programs that want "the biggest buffer they can get" and 2) > GUI programs that might have a "buffer size" field implemented as a spinbox. > I thought about that after sending the RFC, and I think it's a good idea. > Should pcap_set_buffer_size also check against the maximum size, and set it > to the maximum size if it's above the maximum? > I'd like that, but I thought it'd be better to leave it as is to avoid breaking existing programs that might rely on this check missing. For the same reason, I avoided changing the check for positivity to return an error, as I think libraries should provide mechanisms, not policies, and silently deciding on a different behaviour is the latter. > > This field is initialized by default to `INT_MAX` to preserve the > > behaviour of the older API. > > Then, each driver is expected, but not mandated, to fix it to a more > > appropriate value for the platform. > > In this RFC, Linux and DPDK are used as examples. > > Is there a maximum buffer size > INT_MAX for Linux? > It seems I forgot to commit the changes to pcap-linux.c. It did set the maximum buffer size to SIZE_MAX as well. > At least in macOS, and possibly in other BSD-flavored OSes, the sysctl > variable debug.bpf_maxbufsize will indicate the maximum size. I'm not sure how to handle this. Buffer size can only be set before activation, but filters can be set at any point. From the user POV, I wouldn't like my buffers to be limited by the maximum size of filters when I never use them. However, another user may attempt to set it after using a buffer exceeding this, and it would fail in mysterious ways. The only solution I came up with is to always use software filters when the buffer is too big. ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] [RFC PATCH] Add new `pcap_set_buffer_size1` API.
El mié., 2 oct. 2019 a las 19:48, Mario Rugiero () escribió: > I used '1' because that's what Linux does when advertising newer > versions of syscalls. > '_ext' does look better, I think I'd go with that. On the other hand, numeric versioning is more future-proof. 'pcap_set_buffer_size_ext_ext_ext wouldn't look good. ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
