On Mon, Jul 03, 2017 at 01:54:33AM +0000, Paolo Lucente wrote: > Hi Lennert,
Hello, > 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. Thanks for the feedback! As far as I understand, the default value for config.pcap_protocol would be zero, and that would cause pcap_set_protocol() to not be used. > 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. Your positive feedback is enough for now, I think -- I'll use that to try to get the patch merged into libpcap, and if that's successful, I'll report back here. Cheers, Lennert > 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 _______________________________________________ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
