Hi Lennert, I'm familiar with the context and the patch, on the pmacct side of the things, looks sane. The only thing i noticed is that a default value for config.pcap_protocol is never imposed.
I'd be curious what is your strategy to move this forward, especially on the libpcap side. If it makes any convenient to you, we can indeed merge the pmacct part - since it's all wrapped around that PCAP_SET_PROTOCOL definition - and see/re-evaluate how things stand later, at the time of 1.7.0 release. Paolo On Fri, Jun 30, 2017 at 05:10:29PM +0300, Lennert Buytenhek wrote: > Hi! > > Attached is a patch against pmacctd that allows you to specify a > specific PF_PACKET protocol to capture, using the following option: > > pcap_protocol: 0xdead > > This functionality depends on an extension to libpcap which is not in > mainline libpcap (yet), so I've attached the corresponding libpcap patch > as well, for reference. > > The main use case for this is running pmacctd on Arista switches, where > packets sampled by the switch fabric are delivered to virtual interfaces > on the management CPU with skb->protocol set to the magic value 0x002d > (independently of the sampled packet's ethertype!), and we want to > capture only those sampled packets, and not the packets generated by or > destined for the switch's control plane, which also traverse those > virtual interfaces. > > This pmacctd patch depends on a libpcap patch that hasn't been merged or > even submitted upstream yet, and I would first like to get some feedback > on the pmacctd patch. If it looks okay, I can then use that feedback > when submitting the libpcap patch. > > Any feedback appreciated! > > > Cheers, > Lennert > diff --git a/configure.ac b/configure.ac > index 415f7b3..8f41b69 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -447,7 +447,7 @@ if test x"$PFRING_LIB_FOUND" = x""; then > ERROR: missing pcap library. Refer to: http://www.tcpdump.org/ > ])]) > > - AC_CHECK_LIB([pcap], [pcap_setnonblock], [ AC_DEFINE(PCAP_7, 1) ], []) > + AC_CHECK_LIB([pcap], [pcap_set_protocol], [ AC_DEFINE(PCAP_SET_PROTOCOL, > 1) ], []) > AC_CHECK_LIB([pcap], [bpf_filter], [ AC_DEFINE(PCAP_NOBPF, 1) ], []) > else > dnl Unable to test: we should check for these libs > diff --git a/src/cfg.h b/src/cfg.h > index 3206282..58c315c 100644 > --- a/src/cfg.h > +++ b/src/cfg.h > @@ -447,6 +447,7 @@ struct configuration { > #endif > int promisc; /* pcap_open_live() promisc parameter */ > char *clbuf; /* pcap filter */ > + int pcap_protocol; > char *pcap_savefile; > char *dev; > int if_wait; > diff --git a/src/cfg_handlers.c b/src/cfg_handlers.c > index d9237c0..25885ec 100644 > --- a/src/cfg_handlers.c > +++ b/src/cfg_handlers.c > @@ -462,6 +462,18 @@ int cfg_key_pcap_filter(char *filename, char *name, char > *value_ptr) > return changes; > } > > +int cfg_key_pcap_protocol(char *filename, char *name, char *value_ptr) > +{ > + struct plugins_list_entry *list = plugins_list; > + int value, changes = 0; > + > + value = strtol(value_ptr, NULL, 0); > + for (; list; list = list->next, changes++) list->cfg.pcap_protocol = value; > + if (name) Log(LOG_WARNING, "WARN: [%s] plugin name not supported for key > 'pcap_protocol'. Globalized.\n", filename); > + > + return changes; > +} > + > int cfg_key_interface(char *filename, char *name, char *value_ptr) > { > struct plugins_list_entry *list = plugins_list; > diff --git a/src/cfg_handlers.h b/src/cfg_handlers.h > index 41100ef..5fec930 100644 > --- a/src/cfg_handlers.h > +++ b/src/cfg_handlers.h > @@ -42,6 +42,7 @@ EXT int cfg_key_aggregate_primitives(char *, char *, char > *); > EXT int cfg_key_snaplen(char *, char *, char *); > EXT int cfg_key_aggregate_filter(char *, char *, char *); > EXT int cfg_key_pcap_filter(char *, char *, char *); > +EXT int cfg_key_pcap_protocol(char *, char *, char *); > EXT int cfg_key_use_ip_next_hop(char *, char *, char *); > EXT int cfg_key_thread_stack(char *, char *, char *); > EXT int cfg_key_interface(char *, char *, char *); > diff --git a/src/pmacct-data.h b/src/pmacct-data.h > index 6f5ac0a..64319f6 100644 > --- a/src/pmacct-data.h > +++ b/src/pmacct-data.h > @@ -320,6 +320,7 @@ static const struct _dictionary_line dictionary[] = { > {"aggregate_filter", cfg_key_aggregate_filter}, > {"promisc", cfg_key_promisc}, > {"pcap_filter", cfg_key_pcap_filter}, > + {"pcap_protocol", cfg_key_pcap_protocol}, > {"core_proc_name", cfg_key_proc_name}, > {"proc_priority", cfg_key_proc_priority}, > {"pmacctd_as", cfg_key_nfacctd_as_new}, > diff --git a/src/pmacctd.c b/src/pmacctd.c > index 7e13796..c7e9d34 100644 > --- a/src/pmacctd.c > +++ b/src/pmacctd.c > @@ -91,6 +91,59 @@ void usage_daemon(char *prog_name) > printf("For suggestions, critics, bugs, contact me: %s.\n", MANTAINER); > } > > +static pcap_t *do_pcap_open(const char *device, int snaplen, int promisc, > + int to_ms, int protocol, char *errbuf) > +{ > + pcap_t *p; > + int ret; > + > + p = pcap_create(device, errbuf); > + if (p == NULL) > + return NULL; > + > + ret = pcap_set_snaplen(p, snaplen); > + if (ret < 0) > + goto err; > + > + ret = pcap_set_promisc(p, promisc); > + if (ret < 0) > + goto err; > + > + ret = pcap_set_timeout(p, to_ms); > + if (ret < 0) > + goto err; > + > +#ifdef PCAP_SET_PROTOCOL > + ret = pcap_set_protocol(p, protocol); > + if (ret < 0) > + goto err; > +#else > + if (protocol) > + Log(LOG_WARNING, "WARN ( %s/core ): pcap_protocol specified but linked > against a version of libpcap that does not support pcap_set_protocol.\n", > config.name); > +#endif > + > + ret = pcap_activate(p); > + if (ret < 0) > + goto err; > + > + return p; > + > +err: > + if (ret == PCAP_ERROR) > + snprintf(errbuf, PCAP_ERRBUF_SIZE, "%s: %s", device, pcap_geterr(p)); > + else if (ret == PCAP_ERROR_NO_SUCH_DEVICE || > + ret == PCAP_ERROR_PERM_DENIED || > + ret == PCAP_ERROR_PROMISC_PERM_DENIED) > + snprintf(errbuf, PCAP_ERRBUF_SIZE, "%s: %s (%s)", device, > + pcap_statustostr(ret), pcap_geterr(p)); > + else > + snprintf(errbuf, PCAP_ERRBUF_SIZE, "%s: %s", device, > + pcap_statustostr(ret)); > + > + pcap_close(p); > + > + return NULL; > +} > > int main(int argc,char **argv, char **envp) > { > @@ -692,9 +745,9 @@ int main(int argc,char **argv, char **envp) > > throttle_startup: > if (config.dev) { > - if ((device.dev_desc = pcap_open_live(config.dev, psize, config.promisc, > 1000, errbuf)) == NULL) { > + if ((device.dev_desc = do_pcap_open(config.dev, psize, config.promisc, > 1000, config.pcap_protocol, errbuf)) == NULL) { > if (!config.if_wait) { > - Log(LOG_ERR, "ERROR ( %s/core ): pcap_open_live(): %s\n", > config.name, errbuf); > + Log(LOG_ERR, "ERROR ( %s/core ): do_pcap_open(): %s\n", config.name, > errbuf); > exit_all(1); > } > else { > @@ -901,7 +954,7 @@ int main(int argc,char **argv, char **envp) > Log(LOG_WARNING, "WARN ( %s/core ): %s has become unavailable; > throttling ...\n", config.name, config.dev); > throttle_loop: > sleep(5); /* XXX: user defined ? */ > - if ((device.dev_desc = pcap_open_live(config.dev, psize, > config.promisc, 1000, errbuf)) == NULL) > + if ((device.dev_desc = do_pcap_open(config.dev, psize, config.promisc, > 1000, config.pcap_protocol, errbuf)) == NULL) > goto throttle_loop; > pcap_setfilter(device.dev_desc, &filter); > device.active = TRUE; > diff --git a/Makefile.in b/Makefile.in > index bbe470c..35f2ea4 100644 > --- a/Makefile.in > +++ b/Makefile.in > @@ -231,6 +231,7 @@ MAN3PCAP_NOEXPAND = \ > pcap_set_datalink.3pcap \ > pcap_set_immediate_mode.3pcap \ > pcap_set_promisc.3pcap \ > + pcap_set_protocol.3pcap \ > pcap_set_rfmon.3pcap \ > pcap_set_snaplen.3pcap \ > pcap_set_timeout.3pcap \ > diff --git a/pcap-int.h b/pcap-int.h > index 66c3d06..c6d1bfa 100644 > --- a/pcap-int.h > +++ b/pcap-int.h > @@ -112,6 +112,7 @@ struct pcap_opt { > int timeout; /* timeout for buffering */ > u_int buffer_size; > int promisc; > + int protocol; > 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" */ > diff --git a/pcap-linux.c b/pcap-linux.c > index 179fd34..cf74a5e 100644 > --- a/pcap-linux.c > +++ b/pcap-linux.c > @@ -425,7 +425,7 @@ static int iface_get_id(int fd, const char > *device, char *ebuf); > static int iface_get_mtu(int fd, const char *device, char *ebuf); > static int iface_get_arptype(int fd, const char *device, char *ebuf); > #ifdef HAVE_PF_PACKET_SOCKETS > -static int iface_bind(int fd, int ifindex, char *ebuf); > +static int iface_bind(int fd, int ifindex, char *ebuf, int protocol); > #ifdef IW_MODE_MONITOR > static int has_wext(int sock_fd, const char *device, char *ebuf); > #endif /* IW_MODE_MONITOR */ > @@ -1008,6 +1008,17 @@ is_bonding_device(int fd, const char *device) > } > #endif /* IW_MODE_MONITOR */ > > +static int pcap_protocol(pcap_t *handle) > +{ > + int protocol; > + > + protocol = handle->opt.protocol; > + if (protocol == 0) > + protocol = ETH_P_ALL; > + > + return htons(protocol); > +} > + > static int > pcap_can_set_rfmon_linux(pcap_t *handle) > { > @@ -1059,7 +1070,7 @@ pcap_can_set_rfmon_linux(pcap_t *handle) > * (We assume that if we have Wireless Extensions support > * we also have PF_PACKET support.) > */ > - sock_fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); > + sock_fd = socket(PF_PACKET, SOCK_RAW, pcap_protocol(handle)); > if (sock_fd == -1) { > (void)pcap_snprintf(handle->errbuf, PCAP_ERRBUF_SIZE, > "socket: %s", pcap_strerror(errno)); > @@ -3328,6 +3339,7 @@ activate_new(pcap_t *handle) > struct pcap_linux *handlep = handle->priv; > const char *device = handle->opt.device; > int is_any_device = (strcmp(device, "any") == 0); > + int protocol = pcap_protocol(handle); > int sock_fd = -1, arptype; > #ifdef HAVE_PACKET_AUXDATA > int val; > @@ -3346,8 +3358,8 @@ activate_new(pcap_t *handle) > * try a SOCK_RAW socket for the raw interface. > */ > sock_fd = is_any_device ? > - socket(PF_PACKET, SOCK_DGRAM, htons(ETH_P_ALL)) : > - socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); > + socket(PF_PACKET, SOCK_DGRAM, protocol) : > + socket(PF_PACKET, SOCK_RAW, protocol); > > if (sock_fd == -1) { > if (errno == EINVAL || errno == EAFNOSUPPORT) { > @@ -3464,8 +3476,7 @@ activate_new(pcap_t *handle) > "close: %s", pcap_strerror(errno)); > return PCAP_ERROR; > } > - sock_fd = socket(PF_PACKET, SOCK_DGRAM, > - htons(ETH_P_ALL)); > + sock_fd = socket(PF_PACKET, SOCK_DGRAM, protocol); > if (sock_fd == -1) { > pcap_snprintf(handle->errbuf, PCAP_ERRBUF_SIZE, > "socket: %s", pcap_strerror(errno)); > @@ -3529,7 +3540,7 @@ activate_new(pcap_t *handle) > } > > if ((err = iface_bind(sock_fd, handlep->ifindex, > - handle->errbuf)) != 1) { > + handle->errbuf, handle->opt.protocol)) != 1) { > close(sock_fd); > if (err < 0) > return err; > @@ -5310,7 +5321,7 @@ iface_get_id(int fd, const char *device, char *ebuf) > * or a PCAP_ERROR_ value on a hard error. > */ > static int > -iface_bind(int fd, int ifindex, char *ebuf) > +iface_bind(int fd, int ifindex, char *ebuf, int protocol) > { > struct sockaddr_ll sll; > int err; > @@ -5319,7 +5330,7 @@ iface_bind(int fd, int ifindex, char *ebuf) > memset(&sll, 0, sizeof(sll)); > sll.sll_family = AF_PACKET; > sll.sll_ifindex = ifindex; > - sll.sll_protocol = htons(ETH_P_ALL); > + sll.sll_protocol = htons(protocol ? : ETH_P_ALL); > > if (bind(fd, (struct sockaddr *) &sll, sizeof(sll)) == -1) { > if (errno == ENETDOWN) { > diff --git a/pcap.3pcap.in b/pcap.3pcap.in > index 544e79c..9a53ee4 100644 > --- a/pcap.3pcap.in > +++ b/pcap.3pcap.in > @@ -394,6 +394,11 @@ set promiscuous mode for a not-yet-activated > .B pcap_t > for live capture > .TP > +.BR pcap_set_protocol (3PCAP) > +set capture protocol for a not-yet-activated > +.B pcap_t > +for live capture > +.TP > .BR pcap_set_rfmon (3PCAP) > set monitor mode for a not-yet-activated > .B pcap_t > diff --git a/pcap.c b/pcap.c > index bb5e648..b25b7db 100644 > --- a/pcap.c > +++ b/pcap.c > @@ -1338,6 +1338,7 @@ pcap_create_common(char *ebuf, size_t size) > p->opt.timeout = 0; /* no timeout specified */ > p->opt.buffer_size = 0; /* use the platform's default */ > p->opt.promisc = 0; > + p->opt.protocol = 0; > p->opt.rfmon = 0; > p->opt.immediate = 0; > p->opt.tstamp_type = -1; /* default to not setting time stamp > type */ > @@ -1381,6 +1382,15 @@ pcap_set_promisc(pcap_t *p, int promisc) > } > > int > +pcap_set_protocol(pcap_t *p, int protocol) > +{ > + if (pcap_check_activated(p)) > + return (PCAP_ERROR_ACTIVATED); > + p->opt.protocol = protocol; > + return (0); > +} > + > +int > pcap_set_rfmon(pcap_t *p, int rfmon) > { > if (pcap_check_activated(p)) > diff --git a/pcap/pcap.h b/pcap/pcap.h > index adff853..76ecec4 100644 > --- a/pcap/pcap.h > +++ b/pcap/pcap.h > @@ -315,6 +315,7 @@ PCAP_API int pcap_lookupnet(const char *, > bpf_u_int32 *, bpf_u_int32 *, char *); > PCAP_API pcap_t *pcap_create(const char *, char *); > PCAP_API int pcap_set_snaplen(pcap_t *, int); > PCAP_API int pcap_set_promisc(pcap_t *, int); > +PCAP_API int pcap_set_protocol(pcap_t *, int); > 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); > diff --git a/pcap_set_protocol.3pcap b/pcap_set_protocol.3pcap > new file mode 100644 > index 0000000..b9acf16 > --- /dev/null > +++ b/pcap_set_protocol.3pcap > @@ -0,0 +1,47 @@ > +.\" Copyright (c) 1994, 1996, 1997 > +.\" The Regents of the University of California. All rights reserved. > +.\" > +.\" Redistribution and use in source and binary forms, with or without > +.\" modification, are permitted provided that: (1) source code distributions > +.\" retain the above copyright notice and this paragraph in its entirety, (2) > +.\" distributions including binary code include the above copyright notice > and > +.\" this paragraph in its entirety in the documentation or other materials > +.\" provided with the distribution, and (3) all advertising materials > mentioning > +.\" features or use of this software display the following acknowledgement: > +.\" ``This product includes software developed by the University of > California, > +.\" Lawrence Berkeley Laboratory and its contributors.'' Neither the name of > +.\" the University nor the names of its contributors may be used to endorse > +.\" or promote products derived from this software without specific prior > +.\" written permission. > +.\" THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED > +.\" WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF > +.\" MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. > +.\" > +.TH PCAP_SET_PROTOCOL 3PCAP "3 January 2014" > +.SH NAME > +pcap_set_protocol \- set capture protocol for a not-yet-activated > +capture handle > +.SH SYNOPSIS > +.nf > +.ft B > +#include <pcap/pcap.h> > +.LP > +.ft B > +int pcap_set_protocol(pcap_t *p, int protocol); > +.ft > +.fi > +.SH DESCRIPTION > +.B pcap_set_protocol() > +sets the capture protocol to use on a capture handle when the handle > +is activated. > +If > +.I protocol > +is non-zero, packets of that protocol will be captured when the > +handle is activated, otherwise, all packets will be captured. > +.SH RETURN VALUE > +.B pcap_set_protocol() > +returns 0 on success or > +.B PCAP_ERROR_ACTIVATED > +if called on a capture handle that has been activated. > +.SH SEE ALSO > +pcap(3PCAP), pcap_create(3PCAP), pcap_activate(3PCAP) > _______________________________________________ > pmacct-discussion mailing list > http://www.pmacct.net/#mailinglists _______________________________________________ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
