Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Lennert Buytenhek]

D'oh.  Sorry, I'm old-fashioned, and I'm used to submitting patches for
inclusion by mailing them out to mailing lists. :)  I created a github
PR just now.  Thanks!


On Wed, Aug 30, 2017 at 02:31:13PM +, Paolo Lucente wrote:

> Lennert, nope, not missing anything :) We are on the same page and i'm
> just awaiting for your PR on GitHub. Thanks Vincent as always for your
> input. 
> 
> Paolo
>  
> On Wed, Aug 30, 2017 at 05:15:12PM +0300, Lennert Buytenhek wrote:
> > Hi Paolo,
> > 
> > Well, that's how it's in the patch.  Am I missing something? :)
> > 
> > 
> > Cheers,
> > Lennert
> > 
> > 
> > On Wed, Aug 30, 2017 at 02:14:48PM +, Paolo Lucente wrote:
> > 
> > > Hi Lennert,
> > > 
> > > That would indeed work too, yes. Go for it :) 
> > > 
> > > Paolo
> > > 
> > > On Wed, Aug 30, 2017 at 05:03:10PM +0300, Lennert Buytenhek wrote:
> > > > Hi Paolo,
> > > > 
> > > > Wouldn't the autoconf check for the symbol (pcap_set_protocol) be
> > > > sufficient here?
> > > > 
> > > > +  AC_CHECK_LIB([pcap], [pcap_set_protocol], [ 
> > > > AC_DEFINE(PCAP_SET_PROTOCOL, 1) ], [])
> > > > 
> > > > Just checking for the version number wouldn't necessarily mean that
> > > > the symbol is available, because the function will only be provided by
> > > > libpcap.so on Linux (as it has no effect on other platforms).
> > > > 
> > > > 
> > > > Cheers,
> > > > Lennert
> > > > 
> > > > 
> > > > On Wed, Aug 30, 2017 at 01:58:48PM +, Paolo Lucente wrote:
> > > > 
> > > > > Hi Lennert,
> > > > > 
> > > > > Fantastic, please proceed. I guess the patch should be wrapped around
> > > > > some version checking of libpcap a-la if greater or equal than current
> > > > > release in master then compile the code. I did jus review configure.ac
> > > > > and libpcap version is not captured plus i seem to read libpcap folks
> > > > > are anti pkg-config, that would have made things extremely simple. We
> > > > > may have to resort to something a-la MySQL (since they also do have a
> > > > > mysql-config thing and only in 5.7 added pkg-config support):
> > > > > 
> > > > > https://github.com/pmacct/pmacct/blob/master/m4/ax_lib_mysql.m4
> > > > > 
> > > > > Paolo
> > > > > 
> > > > > On Wed, Aug 30, 2017 at 03:59:34PM +0300, Lennert Buytenhek wrote:
> > > > > > Hi again!
> > > > > > 
> > > > > > 
> > > > > > On Mon, Jul 03, 2017 at 05:15:33PM +0300, Lennert Buytenhek wrote:
> > > > > > 
> > > > > > > > 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.
> > > > > > 
> > > > > > The patch was merged into libpcap:
> > > > > > 
> > > > > > 
> > > > > > https://github.com/the-tcpdump-group/libpcap/commit/55c690f6f834b4762697d7a134de439c9096c921
> > > > > > 
> > > > > > I re-tested with latest libpcap git HEAD with pmacct with the patch 
> > > > > > I
> > > > > > sent earlier (and which is down below), and things still seem to 
> > > > > > work
> > > > > > as expected, i.e. I can restrict capturing to sFlow-sampled packets
> > > > > > on Arista swithes.  I guess that means we can merge the pmacct 
> > > > > > patch.
> > > > > > 
> > > > > > Thanks!
> > > > > > 
> > > > > > 
> > > > > > Cheers,
> > > > > > Lennert
> > > > > > 
> > > > > > 
> > > > > > > > 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 

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Paolo Lucente]

Lennert, nope, not missing anything :) We are on the same page and i'm
just awaiting for your PR on GitHub. Thanks Vincent as always for your
input. 

Paolo
 
On Wed, Aug 30, 2017 at 05:15:12PM +0300, Lennert Buytenhek wrote:
> Hi Paolo,
> 
> Well, that's how it's in the patch.  Am I missing something? :)
> 
> 
> Cheers,
> Lennert
> 
> 
> On Wed, Aug 30, 2017 at 02:14:48PM +, Paolo Lucente wrote:
> 
> > Hi Lennert,
> > 
> > That would indeed work too, yes. Go for it :) 
> > 
> > Paolo
> > 
> > On Wed, Aug 30, 2017 at 05:03:10PM +0300, Lennert Buytenhek wrote:
> > > Hi Paolo,
> > > 
> > > Wouldn't the autoconf check for the symbol (pcap_set_protocol) be
> > > sufficient here?
> > > 
> > > +  AC_CHECK_LIB([pcap], [pcap_set_protocol], [ 
> > > AC_DEFINE(PCAP_SET_PROTOCOL, 1) ], [])
> > > 
> > > Just checking for the version number wouldn't necessarily mean that
> > > the symbol is available, because the function will only be provided by
> > > libpcap.so on Linux (as it has no effect on other platforms).
> > > 
> > > 
> > > Cheers,
> > > Lennert
> > > 
> > > 
> > > On Wed, Aug 30, 2017 at 01:58:48PM +, Paolo Lucente wrote:
> > > 
> > > > Hi Lennert,
> > > > 
> > > > Fantastic, please proceed. I guess the patch should be wrapped around
> > > > some version checking of libpcap a-la if greater or equal than current
> > > > release in master then compile the code. I did jus review configure.ac
> > > > and libpcap version is not captured plus i seem to read libpcap folks
> > > > are anti pkg-config, that would have made things extremely simple. We
> > > > may have to resort to something a-la MySQL (since they also do have a
> > > > mysql-config thing and only in 5.7 added pkg-config support):
> > > > 
> > > > https://github.com/pmacct/pmacct/blob/master/m4/ax_lib_mysql.m4
> > > > 
> > > > Paolo
> > > > 
> > > > On Wed, Aug 30, 2017 at 03:59:34PM +0300, Lennert Buytenhek wrote:
> > > > > Hi again!
> > > > > 
> > > > > 
> > > > > On Mon, Jul 03, 2017 at 05:15:33PM +0300, Lennert Buytenhek wrote:
> > > > > 
> > > > > > > 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.
> > > > > 
> > > > > The patch was merged into libpcap:
> > > > > 
> > > > >   
> > > > > https://github.com/the-tcpdump-group/libpcap/commit/55c690f6f834b4762697d7a134de439c9096c921
> > > > > 
> > > > > I re-tested with latest libpcap git HEAD with pmacct with the patch I
> > > > > sent earlier (and which is down below), and things still seem to work
> > > > > as expected, i.e. I can restrict capturing to sFlow-sampled packets
> > > > > on Arista swithes.  I guess that means we can merge the pmacct patch.
> > > > > 
> > > > > Thanks!
> > > > > 
> > > > > 
> > > > > Cheers,
> > > > > Lennert
> > > > > 
> > > > > 
> > > > > > > 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 

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Vincent Bernat]

 ❦ 30 août 2017 13:58 GMT, Paolo Lucente  :

> Fantastic, please proceed. I guess the patch should be wrapped around
> some version checking of libpcap a-la if greater or equal than current
> release in master then compile the code. I did jus review configure.ac
> and libpcap version is not captured plus i seem to read libpcap folks
> are anti pkg-config, that would have made things extremely simple. We
> may have to resort to something a-la MySQL (since they also do have a
> mysql-config thing and only in 5.7 added pkg-config support):
>
> https://github.com/pmacct/pmacct/blob/master/m4/ax_lib_mysql.m4

Another quick solution is to check PCAP_VERSION_(MAJOR|MINOR).

If you use autoconf, you can directly check for the existence of the
function, like this is already done with AC_CHECK_LIB (or you could use
AC_CHECK_FUNCS([pcap_set_protocol])).

In fact, after checking Lennert's patch, it's already done this way.
-- 
Make sure special cases are truly special.
- The Elements of Programming Style (Kernighan & Plauger)

___
pmacct-discussion mailing list
http://www.pmacct.net/#mailinglists

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Paolo Lucente]

Hi Lennert,

That would indeed work too, yes. Go for it :) 

Paolo

On Wed, Aug 30, 2017 at 05:03:10PM +0300, Lennert Buytenhek wrote:
> Hi Paolo,
> 
> Wouldn't the autoconf check for the symbol (pcap_set_protocol) be
> sufficient here?
> 
> +  AC_CHECK_LIB([pcap], [pcap_set_protocol], [ AC_DEFINE(PCAP_SET_PROTOCOL, 
> 1) ], [])
> 
> Just checking for the version number wouldn't necessarily mean that
> the symbol is available, because the function will only be provided by
> libpcap.so on Linux (as it has no effect on other platforms).
> 
> 
> Cheers,
> Lennert
> 
> 
> On Wed, Aug 30, 2017 at 01:58:48PM +, Paolo Lucente wrote:
> 
> > Hi Lennert,
> > 
> > Fantastic, please proceed. I guess the patch should be wrapped around
> > some version checking of libpcap a-la if greater or equal than current
> > release in master then compile the code. I did jus review configure.ac
> > and libpcap version is not captured plus i seem to read libpcap folks
> > are anti pkg-config, that would have made things extremely simple. We
> > may have to resort to something a-la MySQL (since they also do have a
> > mysql-config thing and only in 5.7 added pkg-config support):
> > 
> > https://github.com/pmacct/pmacct/blob/master/m4/ax_lib_mysql.m4
> > 
> > Paolo
> > 
> > On Wed, Aug 30, 2017 at 03:59:34PM +0300, Lennert Buytenhek wrote:
> > > Hi again!
> > > 
> > > 
> > > On Mon, Jul 03, 2017 at 05:15:33PM +0300, Lennert Buytenhek wrote:
> > > 
> > > > > 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.
> > > 
> > > The patch was merged into libpcap:
> > > 
> > >   
> > > https://github.com/the-tcpdump-group/libpcap/commit/55c690f6f834b4762697d7a134de439c9096c921
> > > 
> > > I re-tested with latest libpcap git HEAD with pmacct with the patch I
> > > sent earlier (and which is down below), and things still seem to work
> > > as expected, i.e. I can restrict capturing to sFlow-sampled packets
> > > on Arista swithes.  I guess that means we can merge the pmacct patch.
> > > 
> > > Thanks!
> > > 
> > > 
> > > Cheers,
> > > Lennert
> > > 
> > > 
> > > > > 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
> > > > > > 

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Lennert Buytenhek]

Hi Paolo,

Wouldn't the autoconf check for the symbol (pcap_set_protocol) be
sufficient here?

+  AC_CHECK_LIB([pcap], [pcap_set_protocol], [ AC_DEFINE(PCAP_SET_PROTOCOL, 1) 
], [])

Just checking for the version number wouldn't necessarily mean that
the symbol is available, because the function will only be provided by
libpcap.so on Linux (as it has no effect on other platforms).


Cheers,
Lennert


On Wed, Aug 30, 2017 at 01:58:48PM +, Paolo Lucente wrote:

> Hi Lennert,
> 
> Fantastic, please proceed. I guess the patch should be wrapped around
> some version checking of libpcap a-la if greater or equal than current
> release in master then compile the code. I did jus review configure.ac
> and libpcap version is not captured plus i seem to read libpcap folks
> are anti pkg-config, that would have made things extremely simple. We
> may have to resort to something a-la MySQL (since they also do have a
> mysql-config thing and only in 5.7 added pkg-config support):
> 
> https://github.com/pmacct/pmacct/blob/master/m4/ax_lib_mysql.m4
> 
> Paolo
> 
> On Wed, Aug 30, 2017 at 03:59:34PM +0300, Lennert Buytenhek wrote:
> > Hi again!
> > 
> > 
> > On Mon, Jul 03, 2017 at 05:15:33PM +0300, Lennert Buytenhek wrote:
> > 
> > > > 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.
> > 
> > The patch was merged into libpcap:
> > 
> > 
> > https://github.com/the-tcpdump-group/libpcap/commit/55c690f6f834b4762697d7a134de439c9096c921
> > 
> > I re-tested with latest libpcap git HEAD with pmacct with the patch I
> > sent earlier (and which is down below), and things still seem to work
> > as expected, i.e. I can restrict capturing to sFlow-sampled packets
> > on Arista swithes.  I guess that means we can merge the pmacct patch.
> > 
> > Thanks!
> > 
> > 
> > Cheers,
> > Lennert
> > 
> > 
> > > > 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
> 

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Paolo Lucente]

Hi Lennert,

Fantastic, please proceed. I guess the patch should be wrapped around
some version checking of libpcap a-la if greater or equal than current
release in master then compile the code. I did jus review configure.ac
and libpcap version is not captured plus i seem to read libpcap folks
are anti pkg-config, that would have made things extremely simple. We
may have to resort to something a-la MySQL (since they also do have a
mysql-config thing and only in 5.7 added pkg-config support):

https://github.com/pmacct/pmacct/blob/master/m4/ax_lib_mysql.m4

Paolo

On Wed, Aug 30, 2017 at 03:59:34PM +0300, Lennert Buytenhek wrote:
> Hi again!
> 
> 
> On Mon, Jul 03, 2017 at 05:15:33PM +0300, Lennert Buytenhek wrote:
> 
> > > 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.
> 
> The patch was merged into libpcap:
> 
>   
> https://github.com/the-tcpdump-group/libpcap/commit/55c690f6f834b4762697d7a134de439c9096c921
> 
> I re-tested with latest libpcap git HEAD with pmacct with the patch I
> sent earlier (and which is down below), and things still seem to work
> as expected, i.e. I can restrict capturing to sFlow-sampled packets
> on Arista swithes.  I guess that means we can merge the pmacct patch.
> 
> Thanks!
> 
> 
> Cheers,
> Lennert
> 
> 
> > > 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);
> > > > +
> > > > +  

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Lennert Buytenhek]

Hi again!


On Mon, Jul 03, 2017 at 05:15:33PM +0300, Lennert Buytenhek wrote:

> > 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.

The patch was merged into libpcap:


https://github.com/the-tcpdump-group/libpcap/commit/55c690f6f834b4762697d7a134de439c9096c921

I re-tested with latest libpcap git HEAD with pmacct with the patch I
sent earlier (and which is down below), and things still seem to work
as expected, i.e. I can restrict capturing to sFlow-sampled packets
on Arista swithes.  I guess that means we can merge the pmacct patch.

Thanks!


Cheers,
Lennert


> > 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 

Re: [pmacct-discussion] [RFC] Patches to allow capturing specific PF_PACKET protocols

[Paolo Lucente]

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