From: Ani Sinha <hidden> Date: 2012-10-31 21:50:28
cc'ing netdev.
On Wed, Oct 31, 2012 at 2:01 PM, Michael Richardson [off-list ref] wrote:
Thanks for this email.
quoted
quoted
quoted
quoted
quoted
"Ani" == Ani Sinha [off-list ref] writes:
Ani> remove "inline" from vlan_core.c functions
Ani> Signed-off-by: David S. Miller [off-list ref]
Ani> As a result of this patch, with or without HW acceleration support in
Ani> the kernel driver, the vlan tag information is pulled out from the
Ani> packet and is inserted in the skb metadata. Thereafter, the vlan tag
Ani> information for a 802.1q packet can be obtained my applications
Ani> running in the user space by using the auxdata and cmsg
Ani> structure.
Do you think that the existance of this behaviour could be exposed in
sysctl, /proc/net or /sys equivalent (we still don't have /sys/net...)?
As a read only file that had a 0/1 in it?
yes, we definitely need a run time check. Whether this could be in the
form of a socket option or a /proc entry I don't know.
Should be one stanza to net/dev/sysctl_net_core, I think.
If we are fast, maybe no kernel will ship without it.
(An aside, is this auxdata/cmsg info available on UDP sockets too?)
Ani> More recently, two more patches were committed to net-next that enable
Ani> the kernel BPF filter to filter packets using their vlan tags :
Ani> http://www.spinics.net/lists/netdev/msg214758.html
Ani> http://www.spinics.net/lists/netdev/msg214759.html
okay...
Ani> Now to the real problem. The filter that is currently generated by
Ani> libpcap (gencode.c) uses offsets into the packet to obtain the vlan
Ani> tag for a packet and then compare it to the one provided by the user
Ani> in the filter expression (gen_vlan()). This will no longer work if the
Ani> tag has been pulled off from the packet itself. One has to use the
Ani> negative offsets (SKF_AD_VLAN_TAG) as used by the kernel BPF filter to
Ani> pass down the attribute that we need to compare against. Now here's a
Ani> bigger problem. How do we avoid breakage in case we run libpcap on top
Ani> of an older kernel that does not extract the tags from the packet? How
Ani> do we handle cases of parsing and filtering against captured pcap
Ani> files that already have the vlan tags reinserted into the packet data?
Ani> There might be other corner cases that I am missing and not
Ani> understanding here.
AFAIK, We will have to modify pcap-linux to produce different filters.
Ani> Are you guys aware of the problem? Is there any thoughts as to how we
Ani> may be able to address the problem or not at all?
Nobody gave us a heads up, no.
Glad that I brought this up. Actually Bill Fenner who also works for
us helped me a lot in understanding the problem and potentially how it
can be addressed.
It sounds like it's impossible to capture all traffic now, and do vlan
filtering later on?
pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work. This is slow.
ani
From: Ani Sinha <hidden> Date: 2012-10-31 22:35:27
On Wed, Oct 31, 2012 at 3:20 PM, Guy Harris [off-list ref] wrote:
On Oct 31, 2012, at 2:50 PM, Ani Sinha [off-list ref] wrote:
quoted
pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work.
*Somebody* has to do that, at least to packets that pass the filter,
yes but if the packet is passed to the filter within libpcap (when we
are not using the kernel filter) before the reinsertion, then the
filter has to be taught to look into the metadata for tag information
and not in the packet itself.
before they're handed to a libpcap-based application, for programs
that expect to see packets as they arrived from/were transmitted to
the wire to work.
I.e., the tags *should* be reinserted by libpcap, and, as I understand it, that's what the
#if defined(HAVE_PACKET_AUXDATA) && defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
...
#endif
blocks of code in pcap-linux.c in libpcap are doing.
From: Michael Richardson <hidden> Date: 2012-10-31 22:50:25
quoted
quoted
quoted
quoted
"Ani" == Ani Sinha [off-list ref] writes:
Ani> cc'ing netdev.
>> Do you think that the existance of this behaviour could be exposed in
>> sysctl, /proc/net or /sys equivalent (we still don't have /sys/net...)?
>> As a read only file that had a 0/1 in it?
Ani> yes, we definitely need a run time check. Whether this could be in the
Ani> form of a socket option or a /proc entry I don't know.
unsigned int netdev_8021q_inskb = 1;
...
{
.ctl_name = NET_CORE_8021q_INSKB,
.procname = "netdev_8021q_inskb",
.data = &netdev_8021q_inskb,
.maxlen = sizeof(int),
.mode = 0444,
.proc_handler = proc_dointvec
},
would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.
>> It sounds like it's impossible to capture all traffic now, and do vlan
>> filtering later on?
Ani> pcap files that already have the tags reinsrted should work with
Ani> current filter code. However for live traffic, one has to get the tags
Ani> from CMSG() and then reinsert it back to the packet for the current
Ani> filter to work. This is slow.
I think that this better vlan handling will be much faster, and make
things much more consistent between systems with and without hardware
offload, so it's great news.
However, a number of people use Linux machines to do traffic captures of various
kinds using interfaces dedicated to that purpose.
It would be nice if there was a way to get the packet "whole", such as
by having a second PACKET_CAPTURE point, which was before the adjustment
of the vlan. (obviously, if you had hardware, you'd want to turn that
off)
Many would like this capture point to actually eat all packets for the
interfaces it was defined for, as people doing captures never want those
packets going up the stack.
--
] He who is tired of Weird Al is tired of life! | firewalls [
] Michael Richardson, Sandelman Software Works, Ottawa, ON |net architect[
] mcr@sandelman.ottawa.on.ca http://www.sandelman.ottawa.on.ca/ |device driver[
Kyoto Plus: watch the video <http://www.youtube.com/watch?v=kzx1ycLXQSE>
then sign the petition.
From: Guy Harris <hidden> Date: 2012-10-31 23:11:23
On Oct 31, 2012, at 2:50 PM, Ani Sinha [off-list ref] wrote:
pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work.
*Somebody* has to do that, at least to packets that pass the filter, before they're handed to a libpcap-based application, for programs that expect to see packets as they arrived from/were transmitted to the wire to work.
I.e., the tags *should* be reinserted by libpcap, and, as I understand it, that's what the
#if defined(HAVE_PACKET_AUXDATA) && defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
...
#endif
blocks of code in pcap-linux.c in libpcap are doing.
Now, if filtering is being done in the *kernel*, and the tags aren't being reinserted by the kernel, then filter code stuffed into the kernel would need to differ from filter code run in userland. There's already precedent for that on Linux, with the "cooked mode" headers; those are synthesized by libpcap from the metadata returned for PF_PACKET sockets, and the code that attempts to hand the kernel a filter goes through the filter code, which was generated under the assumption that the packet begins with a "cooked mode" header, and modifies (a copy of) the code to, instead, use the special Linux-BPF-interpreter offsets to access the metadata.
The right thing to do here would be to, if possible, do the same, so that the kernel doesn't have to reinsert VLAN tags for packets that aren't going to be handed to userland.
And, yes, if that should be done for some interfaces with some kernel versions but not all interfaces for all kernel versions, there would need to be a way for libpcap to ask whether it's necessary. Is it necessary on any interfaces *before* the kernel change in question?
From: Guy Harris <hidden> Date: 2012-11-01 01:02:44
On Oct 31, 2012, at 3:35 PM, Ani Sinha [off-list ref] wrote:
yes but if the packet is passed to the filter within libpcap (when we
are not using the kernel filter) before the reinsertion,
...that would be a bug.
Currently, that bug doesn't exist in the recvfrom() code path, but *does* appear to exist in the tpacket code path - and that code path also runs the filter before the SLL header is constructed. That should be fixed.
Yes, it means constructing the SLL header and inserting the tags blah blah blah on packets that might not pass the filter, but in most cases the filtering should be done in the kernel, not userland, when doing a live capture, and in those cases where the filtering can't done in the kernel, you're already somewhat hosed by the fact that the packets are being copied to the buffer and wakeups are being delivered before the packets are being filtered.
From: Ani Sinha <hidden> Date: 2012-11-01 01:22:55
On Wed, Oct 31, 2012 at 5:50 PM, Guy Harris [off-list ref] wrote:
On Oct 31, 2012, at 3:35 PM, Ani Sinha [off-list ref] wrote:
quoted
yes but if the packet is passed to the filter within libpcap (when we
are not using the kernel filter) before the reinsertion,
...that would be a bug.
Currently, that bug doesn't exist in the recvfrom() code path, but *does* appear to exist in the tpacket code path - and that code path also runs the filter before the SLL header is constructed. That should be fixed.
From: Bill Fenner <hidden> Date: 2012-11-02 16:13:19
On Wed, Oct 31, 2012 at 6:20 PM, Guy Harris [off-list ref] wrote:
On Oct 31, 2012, at 2:50 PM, Ani Sinha [off-list ref] wrote:
quoted
pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work.
*Somebody* has to do that, at least to packets that pass the filter, before they're handed to a libpcap-based application, for programs that expect to see packets as they arrived from/were transmitted to the wire to work.
I.e., the tags *should* be reinserted by libpcap, and, as I understand it, that's what the
#if defined(HAVE_PACKET_AUXDATA) && defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
...
#endif
blocks of code in pcap-linux.c in libpcap are doing.
Now, if filtering is being done in the *kernel*, and the tags aren't being reinserted by the kernel, then filter code stuffed into the kernel would need to differ from filter code run in userland. There's already precedent for that on Linux, with the "cooked mode" headers; those are synthesized by libpcap from the metadata returned for PF_PACKET sockets, and the code that attempts to hand the kernel a filter goes through the filter code, which was generated under the assumption that the packet begins with a "cooked mode" header, and modifies (a copy of) the code to, instead, use the special Linux-BPF-interpreter offsets to access the metadata.
The right thing to do here would be to, if possible, do the same, so that the kernel doesn't have to reinsert VLAN tags for packets that aren't going to be handed to userland.
In this case, it would be incredibly complicated to do this just
postprocessing a set of bpf instructions. The problem is that when
running the filter in the kernel, the IP header, etc. are not offset,
so "off_macpl" and "off_linktype" would be zero, not 4, while
generating the rest of the expression. We would also have to insert
code when comparing the ethertype to 0x8100 to instead load the
vlan-tagged metadata, so all jumps crossing that point would have to
be adjusted, and if the "if-false" instruction was also testing the
ethertype, then the ethertype would have to be reloaded (again
inserting another instruction).
Basically, take a look at the output of "tcpdump -d tcp port 22 or
(vlan and tcp port 22)". Are the IPv4 tcp ports at x+14/x+16, or at
x+18/x+20? If we're filtering in the kernel, they're at x+14/x+16
whether the packet is vlan tagged or not. If we're filtering on the
actual packet contents (from a savefile, for example), they're at
x+18/x+20 if the packet is vlan tagged.
Also, an expression such as 'tcp port 22' would have to have some
instructions added at the beginning, for "vlan-tagged == false", or it
would match both tagged and untagged packets.
This would be much more straightforward to deal with in the code
generation phase, except until now the code generation phase hasn't
known whether the filter is headed for the kernel or not.
Bill
From: Ani Sinha <hidden> Date: 2012-11-13 22:41:21
Folks,
I'm wondering if there has been any progress on this. Are there any
thoughts on what Bill wrote in his email?
Thanks
ani
On Fri, Nov 2, 2012 at 9:13 AM, Bill Fenner [off-list ref] wrote:
On Wed, Oct 31, 2012 at 6:20 PM, Guy Harris [off-list ref] wrote:
quoted
On Oct 31, 2012, at 2:50 PM, Ani Sinha [off-list ref] wrote:
quoted
pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work.
*Somebody* has to do that, at least to packets that pass the filter,
before they're handed to a libpcap-based application, for programs that
expect to see packets as they arrived from/were transmitted to the wire to
work.
quoted
I.e., the tags *should* be reinserted by libpcap, and, as I understand
it, that's what the
quoted
#if defined(HAVE_PACKET_AUXDATA) &&
defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
quoted
...
#endif
blocks of code in pcap-linux.c in libpcap are doing.
Now, if filtering is being done in the *kernel*, and the tags aren't
being reinserted by the kernel, then filter code stuffed into the kernel
would need to differ from filter code run in userland. There's already
precedent for that on Linux, with the "cooked mode" headers; those are
synthesized by libpcap from the metadata returned for PF_PACKET sockets,
and the code that attempts to hand the kernel a filter goes through the
filter code, which was generated under the assumption that the packet
begins with a "cooked mode" header, and modifies (a copy of) the code to,
instead, use the special Linux-BPF-interpreter offsets to access the
metadata.
quoted
The right thing to do here would be to, if possible, do the same, so
that the kernel doesn't have to reinsert VLAN tags for packets that aren't
going to be handed to userland.
In this case, it would be incredibly complicated to do this just
postprocessing a set of bpf instructions. The problem is that when
running the filter in the kernel, the IP header, etc. are not offset,
so "off_macpl" and "off_linktype" would be zero, not 4, while
generating the rest of the expression. We would also have to insert
code when comparing the ethertype to 0x8100 to instead load the
vlan-tagged metadata, so all jumps crossing that point would have to
be adjusted, and if the "if-false" instruction was also testing the
ethertype, then the ethertype would have to be reloaded (again
inserting another instruction).
Basically, take a look at the output of "tcpdump -d tcp port 22 or
(vlan and tcp port 22)". Are the IPv4 tcp ports at x+14/x+16, or at
x+18/x+20? If we're filtering in the kernel, they're at x+14/x+16
whether the packet is vlan tagged or not. If we're filtering on the
actual packet contents (from a savefile, for example), they're at
x+18/x+20 if the packet is vlan tagged.
Also, an expression such as 'tcp port 22' would have to have some
instructions added at the beginning, for "vlan-tagged == false", or it
would match both tagged and untagged packets.
This would be much more straightforward to deal with in the code
generation phase, except until now the code generation phase hasn't
known whether the filter is headed for the kernel or not.
Bill
From: Ani Sinha <hidden> Date: 2012-11-13 22:42:48
Folks,
I'm wondering if there has been any progress on this. Are there any
thoughts on what Bill wrote in his email?
Thanks
On Tue, Nov 13, 2012 at 2:41 PM, Ani Sinha [off-list ref] wrote:
Folks,
I'm wondering if there has been any progress on this. Are there any thoughts
on what Bill wrote in his email?
Thanks
ani
On Fri, Nov 2, 2012 at 9:13 AM, Bill Fenner [off-list ref] wrote:
quoted
On Wed, Oct 31, 2012 at 6:20 PM, Guy Harris [off-list ref] wrote:
quoted
On Oct 31, 2012, at 2:50 PM, Ani Sinha [off-list ref] wrote:
quoted
pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work.
*Somebody* has to do that, at least to packets that pass the filter,
before they're handed to a libpcap-based application, for programs that
expect to see packets as they arrived from/were transmitted to the wire to
work.
I.e., the tags *should* be reinserted by libpcap, and, as I understand
it, that's what the
#if defined(HAVE_PACKET_AUXDATA) &&
defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
...
#endif
blocks of code in pcap-linux.c in libpcap are doing.
Now, if filtering is being done in the *kernel*, and the tags aren't
being reinserted by the kernel, then filter code stuffed into the kernel
would need to differ from filter code run in userland. There's already
precedent for that on Linux, with the "cooked mode" headers; those are
synthesized by libpcap from the metadata returned for PF_PACKET sockets, and
the code that attempts to hand the kernel a filter goes through the filter
code, which was generated under the assumption that the packet begins with a
"cooked mode" header, and modifies (a copy of) the code to, instead, use the
special Linux-BPF-interpreter offsets to access the metadata.
The right thing to do here would be to, if possible, do the same, so
that the kernel doesn't have to reinsert VLAN tags for packets that aren't
going to be handed to userland.
In this case, it would be incredibly complicated to do this just
postprocessing a set of bpf instructions. The problem is that when
running the filter in the kernel, the IP header, etc. are not offset,
so "off_macpl" and "off_linktype" would be zero, not 4, while
generating the rest of the expression. We would also have to insert
code when comparing the ethertype to 0x8100 to instead load the
vlan-tagged metadata, so all jumps crossing that point would have to
be adjusted, and if the "if-false" instruction was also testing the
ethertype, then the ethertype would have to be reloaded (again
inserting another instruction).
Basically, take a look at the output of "tcpdump -d tcp port 22 or
(vlan and tcp port 22)". Are the IPv4 tcp ports at x+14/x+16, or at
x+18/x+20? If we're filtering in the kernel, they're at x+14/x+16
whether the packet is vlan tagged or not. If we're filtering on the
actual packet contents (from a savefile, for example), they're at
x+18/x+20 if the packet is vlan tagged.
Also, an expression such as 'tcp port 22' would have to have some
instructions added at the beginning, for "vlan-tagged == false", or it
would match both tagged and untagged packets.
This would be much more straightforward to deal with in the code
generation phase, except until now the code generation phase hasn't
known whether the filter is headed for the kernel or not.
Bill
From: Michael Richardson <hidden> Date: 2012-11-14 18:58:31
quoted
quoted
quoted
quoted
"Ani" == Ani Sinha [off-list ref] writes:
Ani> I'm wondering if there has been any progress on this. Are there
Ani> any thoughts on what Bill wrote in his email?
I don't think so.
Do you have time?
_______________________________________________
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
From: Eric W. Biederman <hidden> Date: 2012-11-16 06:51:49
Ani Sinha [off-list ref] writes:
cc'ing netdev.
On Wed, Oct 31, 2012 at 2:01 PM, Michael Richardson [off-list ref] wrote:
quoted
Thanks for this email.
quoted
quoted
quoted
quoted
quoted
"Ani" == Ani Sinha [off-list ref] writes:
Ani> remove "inline" from vlan_core.c functions
Ani> Signed-off-by: David S. Miller [off-list ref]
Ani> As a result of this patch, with or without HW acceleration support in
Ani> the kernel driver, the vlan tag information is pulled out from the
Ani> packet and is inserted in the skb metadata. Thereafter, the vlan tag
Ani> information for a 802.1q packet can be obtained my applications
Ani> running in the user space by using the auxdata and cmsg
Ani> structure.
Do you think that the existance of this behaviour could be exposed in
sysctl, /proc/net or /sys equivalent (we still don't have /sys/net...)?
As a read only file that had a 0/1 in it?
yes, we definitely need a run time check. Whether this could be in the
form of a socket option or a /proc entry I don't know.
I don't see any need to add any kernel code to allow checking if vlan
tags are stripped. Vlan headers are stripped on all kernel interfaces
today. Vlan headers have been stripped on all but a handful of software
interfaces for 6+ years. For all kernels if the vlan header is stripped
it is reported in the auxdata, upon packet reception. Careful code
should also look for TP_STATUS_VLAN_VALID which allows for
distinguishing a striped vlan header of 0 from no vlan header.
The safe assumption then is that testing for vlan headers and vlan
values in bpf filters is not possible without the new bpf extentions.
It is possible to test for the presence of support of the new vlan bpf
extensions by attempting to load a filter that uses them. As only valid
filters can be loaded, old kernels that do not support filtering of vlan
tags will fail to load the a test filter with uses them.
For old kernels that do not support the new extensions it is possible to
generate code that looks at the ethernet header and sees if the
ethertype is 0x8100 and then does things with it, but that will only
work on a small handful of software only interfaces.
Eric
From: Michael Richardson <hidden> Date: 2012-11-17 22:15:18
Thank you for this reply.
quoted
quoted
quoted
quoted
"Eric" == Eric W Biederman [off-list ref] writes:
Eric> I don't see any need to add any kernel code to allow checking
Eric> if vlan tags are stripped. Vlan headers are stripped on all
Eric> kernel interfaces today. Vlan headers have been stripped on
Eric> all but a handful of software interfaces for 6+ years. For
Eric> all kernels if the vlan header is stripped it is reported in
Eric> the auxdata, upon packet reception. Careful code should also
Eric> look for TP_STATUS_VLAN_VALID which allows for distinguishing
Eric> a striped vlan header of 0 from no vlan header.
I can regularly see vlan tags on raw dumps from the untagged ("eth0")
today, running 3.2 (debian stable):
obiwan-[~] mcr 4848 %sudo tcpdump -i eth0 -n -p -e | grep -i vlan
listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
17:05:15.404909 38:60:77:38:e6:47 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 3800, p 0, ethertype ARP, Request who-has 172.30.42.1 tell 172.30.42.11, length 28
So, I'm curious about the statement that vlan tags have been stripped
for some time, because I don't see them stripped today. My desktop has
an Intel 82579V NIC in it...
Eric> For old kernels that do not support the new extensions it is
Eric> possible to generate code that looks at the ethernet header
Eric> and sees if the ethertype is 0x8100 and then does things with
Eric> it, but that will only work on a small handful of software
Eric> only interfaces.
at tcpdump.org, our concern is to release code that works on both new,
and what for kernel.org folks would be considered "ancient" systems,
such as Centos5/RHEL5 machines which are regularly still in production
in the field (sadly...), but often need the latest diagnostics.
What I hear you saying is that our existing code will work on older
kernels, and that once we have new code to use the VLAN tag extensions,
we should simply attempt to load it, and either it loads, or we get an
error, and we go back to the code we had before. That's great news.
The major concern is that if the 802.1q header is gone, although we can
retrieve it somehow (I'm not sure how the AUXDATA is presented on the
MMAP PF_PACKET interface...) we will have to reconstruct it in order to
save it properly to a savefile. Many entities, including most major
Network Telescopes (http://en.wikipedia.org/wiki/Network_telescope) use
libpcap (and often tcpdump itself) to capture packets on probe points,
and having good performance matters here...
--
] He who is tired of Weird Al is tired of life! | firewalls [
] Michael Richardson, Sandelman Software Works, Ottawa, ON |net architect[
] mcr@sandelman.ottawa.on.ca http://www.sandelman.ottawa.on.ca/ |device driver[
Kyoto Plus: watch the video <http://www.youtube.com/watch?v=kzx1ycLXQSE>
then sign the petition.
From: Daniel Borkmann <hidden> Date: 2012-11-17 23:16:57
On Sat, Nov 17, 2012 at 11:14 PM, Michael Richardson [off-list ref] wrote:
Thank you for this reply.
quoted
quoted
quoted
quoted
quoted
"Eric" == Eric W Biederman [off-list ref] writes:
Eric> I don't see any need to add any kernel code to allow checking
Eric> if vlan tags are stripped. Vlan headers are stripped on all
Eric> kernel interfaces today. Vlan headers have been stripped on
Eric> all but a handful of software interfaces for 6+ years. For
Eric> all kernels if the vlan header is stripped it is reported in
Eric> the auxdata, upon packet reception. Careful code should also
Eric> look for TP_STATUS_VLAN_VALID which allows for distinguishing
Eric> a striped vlan header of 0 from no vlan header.
I can regularly see vlan tags on raw dumps from the untagged ("eth0")
today, running 3.2 (debian stable):
obiwan-[~] mcr 4848 %sudo tcpdump -i eth0 -n -p -e | grep -i vlan
listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
17:05:15.404909 38:60:77:38:e6:47 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 3800, p 0, ethertype ARP, Request who-has 172.30.42.1 tell 172.30.42.11, length 28
So, I'm curious about the statement that vlan tags have been stripped
for some time, because I don't see them stripped today. My desktop has
an Intel 82579V NIC in it...
Speaking of netsniff-ng where we don't reconstruct VLAN headers, users
have reported that depending on the NIC/driver resp. ethtool setting,
they can come in stripped or not (in the pf_packet's rx_ring buffer).
However, I assume VLAN AUXDATA is always consistent (and so the
BPF/BPF JIT filtering).
Eric> For old kernels that do not support the new extensions it is
Eric> possible to generate code that looks at the ethernet header
Eric> and sees if the ethertype is 0x8100 and then does things with
Eric> it, but that will only work on a small handful of software
Eric> only interfaces.
at tcpdump.org, our concern is to release code that works on both new,
and what for kernel.org folks would be considered "ancient" systems,
such as Centos5/RHEL5 machines which are regularly still in production
in the field (sadly...), but often need the latest diagnostics.
What I hear you saying is that our existing code will work on older
kernels, and that once we have new code to use the VLAN tag extensions,
we should simply attempt to load it, and either it loads, or we get an
error, and we go back to the code we had before. That's great news.
From: Eric W. Biederman <hidden> Date: 2012-11-17 23:33:46
Michael Richardson [off-list ref] writes:
Thank you for this reply.
quoted
quoted
quoted
quoted
quoted
"Eric" == Eric W Biederman [off-list ref] writes:
Eric> I don't see any need to add any kernel code to allow checking
Eric> if vlan tags are stripped. Vlan headers are stripped on all
Eric> kernel interfaces today. Vlan headers have been stripped on
Eric> all but a handful of software interfaces for 6+ years. For
Eric> all kernels if the vlan header is stripped it is reported in
Eric> the auxdata, upon packet reception. Careful code should also
Eric> look for TP_STATUS_VLAN_VALID which allows for distinguishing
Eric> a striped vlan header of 0 from no vlan header.
I can regularly see vlan tags on raw dumps from the untagged ("eth0")
today, running 3.2 (debian stable):
obiwan-[~] mcr 4848 %sudo tcpdump -i eth0 -n -p -e | grep -i vlan
listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
17:05:15.404909 38:60:77:38:e6:47 > ff:ff:ff:ff:ff:ff, ethertype 802.1Q (0x8100), length 46: vlan 3800, p 0, ethertype ARP, Request who-has 172.30.42.1 tell 172.30.42.11, length 28
So, I'm curious about the statement that vlan tags have been stripped
for some time, because I don't see them stripped today. My desktop has
an Intel 82579V NIC in it...
So I just took a quick look at libpcap 1.1.1.1.
In pcap_read_packet in pcap-linux.c the code to readd the vlan header is
protected by HAVE_PACKET_AUXDATA.
In pcap_read_linux_mmap in pcap-linux.c the code to readd the vlan
header is protected by HAVE_TPACKET2.
That code is shifting the the ethernet header up 4 bytes and inserting
the vlan header in packets as we receive them.
The code is correct except for the case of packets in vlan 0. Currently
the packet reconstruction is ambiguous. The most recent kernels have
a TP_STATUS_VLAN_VALID flag that can be checked to see if the packet was
in vlan 0 or if there was no vlan at all. libpcap probably should be
taught how to handle TP_STATUS_VLAN_VALID so that it can get the vlan 0
handling correct.
Eric> For old kernels that do not support the new extensions it is
Eric> possible to generate code that looks at the ethernet header
Eric> and sees if the ethertype is 0x8100 and then does things with
Eric> it, but that will only work on a small handful of software
Eric> only interfaces.
at tcpdump.org, our concern is to release code that works on both new,
and what for kernel.org folks would be considered "ancient" systems,
such as Centos5/RHEL5 machines which are regularly still in production
in the field (sadly...), but often need the latest diagnostics.
What I hear you saying is that our existing code will work on older
kernels, and that once we have new code to use the VLAN tag extensions,
we should simply attempt to load it, and either it loads, or we get an
error, and we go back to the code we had before. That's great news.
The existing code will work as well as anything if you don't have the
VLAN tag extensions. If you are not using the VLAN tag extensions there
is no reliable way to filter for vlan tagged packets in the kernel as on
most network devices (all for the last year) the vlan header has been
stripped at the time the bpf filter is run.
So yes. Just falling back to the existing code seems as good as it is
going to get. Which isn't very good but it took 5+ years before people
cared enough to get this fixed. :(
The major concern is that if the 802.1q header is gone, although we can
retrieve it somehow (I'm not sure how the AUXDATA is presented on the
MMAP PF_PACKET interface...) we will have to reconstruct it in order to
save it properly to a savefile. Many entities, including most major
Network Telescopes (http://en.wikipedia.org/wiki/Network_telescope) use
libpcap (and often tcpdump itself) to capture packets on probe points,
and having good performance matters here...
Yes. I wasn't looking at the mmap path. The vlan information is
present in the tpacket2_hdr and tpacket3_hdr structures that accompany
packets read with the mmap interface. The old tpacket1_hdr doesn't
support the vlan_tci information so that won't work.
Not having the vlan header on the packet when the bpf filter is run is
justified by performance, and likewise reading vlan tagged packets to
userspace with the vlan header stripped is justified by performance.
Eric
From: Eric W. Biederman <hidden> Date: 2012-11-17 23:37:55
Daniel Borkmann [off-list ref] writes:
Speaking of netsniff-ng where we don't reconstruct VLAN headers, users
have reported that depending on the NIC/driver resp. ethtool setting,
they can come in stripped or not (in the pf_packet's rx_ring buffer).
However, I assume VLAN AUXDATA is always consistent (and so the
BPF/BPF JIT filtering).
Yes it was a mess before we added software stripping of the vlan headers
a year ago.
Eric
From: Ani Sinha <hidden> Date: 2012-12-06 21:20:12
On Wed, Oct 31, 2012 at 5:50 PM, Guy Harris [off-list ref] wrote:
On Oct 31, 2012, at 3:35 PM, Ani Sinha [off-list ref] wrote:
quoted
yes but if the packet is passed to the filter within libpcap (when we
are not using the kernel filter) before the reinsertion,
...that would be a bug.
Currently, that bug doesn't exist in the recvfrom() code path, but *does* appear to exist in the tpacket code path - and that code path also runs the filter before the SLL header is constructed. That should be fixed.
Something like this?
Index: libpcap-1.1.1/pcap-linux.c
===================================================================
@@ -3469,23 +3476,6 @@ pcap_read_linux_mmap(pcap_t *handle, intreturn-1;}-/* run filter on received packet-*Ifthekernelfilteringisenabledweneedtorunthe-*filteruntilalltheframespresentintothering-*atfiltercreationtimeareprocessed.-*Insuchcasemd.use_bpfisusedasacounterforthe-*packetweneedtofilter.-*Note:alternativelyitcouldbepossibletostopapplying-*thefilterwhentheringbecameempty,butitcanpossibly-*happenalotlater...*/-bp=(unsignedchar*)h.raw+tp_mac;-run_bpf=(!handle->md.use_bpf)||-((handle->md.use_bpf>1)&&handle->md.use_bpf--);-if(run_bpf&&handle->fcode.bf_insns&&-(bpf_filter(handle->fcode.bf_insns,bp,-tp_len,tp_snaplen)==0))-gotoskip;-/**Dochecksbasedonpacketdirection.*/
@@ -3582,6 +3576,23 @@ pcap_read_linux_mmap(pcap_t *handle, int}#endif+/* run filter on received packet+*Ifthekernelfilteringisenabledweneedtorunthe+*filteruntilalltheframespresentintothering+*atfiltercreationtimeareprocessed.+*Insuchcasemd.use_bpfisusedasacounterforthe+*packetweneedtofilter.+*Note:alternativelyitcouldbepossibletostopapplying+*thefilterwhentheringbecameempty,butitcanpossibly+*happenalotlater...*/+bp=(unsignedchar*)h.raw+tp_mac;+run_bpf=(!handle->md.use_bpf)||+((handle->md.use_bpf>1)&&handle->md.use_bpf--);+if(run_bpf&&handle->fcode.bf_insns&&+(bpf_filter(handle->fcode.bf_insns,bp,+tp_len,tp_snaplen)==0))+gotoskip;+/**Theonlywaytotellthekerneltocutoffthe*packetatasnapshotlengthiswithafilterprogram;
From: Ani Sinha <hidden> Date: 2012-12-06 21:22:09
On Sat, Nov 17, 2012 at 3:33 PM, Eric W. Biederman
[off-list ref] wrote:
the vlan header in packets as we receive them.
The code is correct except for the case of packets in vlan 0. Currently
the packet reconstruction is ambiguous. The most recent kernels have
a TP_STATUS_VLAN_VALID flag that can be checked to see if the packet was
in vlan 0 or if there was no vlan at all. libpcap probably should be
taught how to handle TP_STATUS_VLAN_VALID so that it can get the vlan 0
handling correct.
May be this?
Index: libpcap-1.1.1/pcap-linux.c
===================================================================
@@ -1486,7 +1487,13 @@ pcap_read_packet(pcap_t *handle, pcap_hacontinue;aux=(structtpacket_auxdata*)CMSG_DATA(cmsg);-if(aux->tp_vlan_tci==0)+#if defined(TP_STATUS_VLAN_VALID)+if(!(aux->tp_vlan_tci&TP_STATUS_VLAN_VALID))+#else+if(aux->tp_vlan_tci==0)/* this is ambigious but without the+
TP_STATUS_VLAN_VALID flag, there is
+ nothing that we can do */
+#endif
continue;
len = packet_len > iov.iov_len ? iov.iov_len : packet_len;
@@ -3565,7 +3555,11 @@ pcap_read_linux_mmap(pcap_t *handle, int } #ifdef HAVE_TPACKET2- if (handle->md.tp_version == TPACKET_V2 && h.h2->tp_vlan_tci &&+#if defined(TP_STATUS_VLAN_VALID)+ if (handle->md.tp_version == TPACKET_V2 &&
From: Eric W. Biederman <hidden> Date: 2012-12-06 22:19:10
Ani Sinha [off-list ref] writes:
On Sat, Nov 17, 2012 at 3:33 PM, Eric W. Biederman
[off-list ref] wrote:
quoted
the vlan header in packets as we receive them.
The code is correct except for the case of packets in vlan 0. Currently
the packet reconstruction is ambiguous. The most recent kernels have
a TP_STATUS_VLAN_VALID flag that can be checked to see if the packet was
in vlan 0 or if there was no vlan at all. libpcap probably should be
taught how to handle TP_STATUS_VLAN_VALID so that it can get the vlan 0
handling correct.
May be this?
Two things.
- TP_STATUS_VLAN_VALID lives in the tp_status field not the tp_vlan_tci field.
- To work on older kernels with binaries compiled with newer headers you
first want to test for tp_vlan_tci == 0 and then look at the status field for
TP_STATUS_VALID.
Which means the tests need to look something like:
- if (aux->tp_vlan_tci == 0)
+#if defined(TP_STATUS_VLAN_VALID)
+ if ((aux->tp_vlan_tci == 0) && !(aux->tp_status & TP_STATUS_VLAN_VALID))
+#else
+ if (aux->tp_vlan_tci == 0) /* this is ambigious but without the
+
TP_STATUS_VLAN_VALID flag, there is
+ nothing that we can do */
+#endif
#ifdef HAVE_TPACKET2
- if (handle->md.tp_version == TPACKET_V2 && h.h2->tp_vlan_tci &&
+ if (handle->md.tp_version == TPACKET_V2 &&
+#if defined(TP_STATUS_VLAN_VALID)
+ (h.h2->tp_vlan_tci || (h.h2->tp_status & TP_STATUS_VALID)) &&
+#else
+ h.h2->tp_vlan_tci &&
+#endif
Eric
From: Ani Sinha <hidden> Date: 2012-12-06 22:40:06
On Thu, Dec 6, 2012 at 2:19 PM, Eric W. Biederman [off-list ref] wrote:
Ani Sinha [off-list ref] writes:
quoted
On Sat, Nov 17, 2012 at 3:33 PM, Eric W. Biederman
[off-list ref] wrote:
quoted
the vlan header in packets as we receive them.
The code is correct except for the case of packets in vlan 0. Currently
the packet reconstruction is ambiguous. The most recent kernels have
a TP_STATUS_VLAN_VALID flag that can be checked to see if the packet was
in vlan 0 or if there was no vlan at all. libpcap probably should be
taught how to handle TP_STATUS_VLAN_VALID so that it can get the vlan 0
handling correct.
May be this?
Two things.
- TP_STATUS_VLAN_VALID lives in the tp_status field not the tp_vlan_tci field.
- To work on older kernels with binaries compiled with newer headers you
first want to test for tp_vlan_tci == 0 and then look at the status field for
TP_STATUS_VALID.
yes you are right Eric on both counts. I will resend the patch again in a bit.
ani
From: Ani Sinha <hidden> Date: 2012-12-07 00:55:00
On Thu, Dec 6, 2012 at 2:19 PM, Eric W. Biederman [off-list ref] wrote:
Ani Sinha [off-list ref] writes:
quoted
quoted
May be this?
Two things.
- TP_STATUS_VLAN_VALID lives in the tp_status field not the tp_vlan_tci field.
- To work on older kernels with binaries compiled with newer headers you
first want to test for tp_vlan_tci == 0 and then look at the status field for
TP_STATUS_VALID.
callback, u_char *userdata)
continue;
aux = (struct tpacket_auxdata *)CMSG_DATA(cmsg);
- if (aux->tp_vlan_tci == 0)
+#if defined(TP_STATUS_VLAN_VALID)
+ if ((aux->tp_vlan_tci == 0) ||
!(aux->tp_status & TP_STATUS_VLAN_VALID))
+#else
+ if (aux->tp_vlan_tci == 0) /* this is ambigious but without the
+
TP_STATUS_VLAN_VALID flag, there is
+ nothing that we can do */
+#endif
continue;
len = packet_len > iov.iov_len ? iov.iov_len : packet_len;
@@ -3936,7 +3926,12 @@ pcap_read_linux_mmap(pcap_t *handle, int
From: Eric W. Biederman <hidden> Date: 2012-12-07 01:03:50
Ani Sinha [off-list ref] writes:
On Thu, Dec 6, 2012 at 2:19 PM, Eric W. Biederman [off-list ref] wrote:
quoted
Ani Sinha [off-list ref] writes:
quoted
quoted
May be this?
Two things.
- TP_STATUS_VLAN_VALID lives in the tp_status field not the tp_vlan_tci field.
- To work on older kernels with binaries compiled with newer headers you
first want to test for tp_vlan_tci == 0 and then look at the status field for
TP_STATUS_VALID.
trying again :
The patch is whitespace damaged. And one of your test is using ||
instead of &&
Eric
callback, u_char *userdata)
continue;
aux = (struct tpacket_auxdata *)CMSG_DATA(cmsg);
- if (aux->tp_vlan_tci == 0)
+#if defined(TP_STATUS_VLAN_VALID)
+ if ((aux->tp_vlan_tci == 0) ||
!(aux->tp_status & TP_STATUS_VLAN_VALID))
The test should be && not ||.
quoted hunk
+#else
+ if (aux->tp_vlan_tci == 0) /* this is ambigious but without the
+
TP_STATUS_VLAN_VALID flag, there is
+ nothing that we can do */
+#endif
continue;
len = packet_len > iov.iov_len ? iov.iov_len : packet_len;
@@ -3936,7 +3926,12 @@ pcap_read_linux_mmap(pcap_t *handle, int
From: Ani Sinha <hidden> Date: 2012-12-07 01:31:31
The patch is whitespace damaged. And one of your test is using ||
instead of &&
OK, using alpine now :-)
The test should be && not ||.
aargh! I am retarded! Fixed. Hopefully 3rd time is a charm :-)
ani
pcap-linux.c | 50 +++++++++++++++++++++++++++++++-------------------
1 files changed, 31 insertions(+), 19 deletions(-)
@@ -1543,7 +1544,13 @@ pcap_read_packet(pcap_t *handle, pcap_handler callback, u_char *userdata)continue;aux=(structtpacket_auxdata*)CMSG_DATA(cmsg);-if(aux->tp_vlan_tci==0)+#if defined(TP_STATUS_VLAN_VALID)+if((aux->tp_vlan_tci==0)&&!(aux->tp_status&TP_STATUS_VLAN_VALID))+#else+if(aux->tp_vlan_tci==0)/* this is ambigious but without the+TP_STATUS_VLAN_VALIDflag,thereis+nothingthatwecando*/+#endifcontinue;len=packet_len>iov.iov_len?iov.iov_len:packet_len;
From: Eric W. Biederman <hidden> Date: 2012-12-07 01:41:09
Ani Sinha [off-list ref] writes:
quoted
The patch is whitespace damaged. And one of your test is using ||
instead of &&
OK, using alpine now :-)
quoted
The test should be && not ||.
aargh! I am retarded! Fixed. Hopefully 3rd time is a charm :-)
Acked-by: "Eric W. Biederman" <redacted>
The code looks ok here. I don't know what format the tcpdump folks
want patches in. The typically format is an email with [PATCH] in
the subject.
You indentation now comes through clear.
It is a bit odd that you are indenting with spaces instead of tabs
in a file that is indented with tab. Again libpcap isn't my code so I
don't care but someone else might.
Eric
@@ -1543,7 +1544,13 @@ pcap_read_packet(pcap_t *handle, pcap_handler callback, u_char *userdata)continue;aux=(structtpacket_auxdata*)CMSG_DATA(cmsg);-if(aux->tp_vlan_tci==0)+#if defined(TP_STATUS_VLAN_VALID)+if((aux->tp_vlan_tci==0)&&!(aux->tp_status&TP_STATUS_VLAN_VALID))+#else+if(aux->tp_vlan_tci==0)/* this is ambigious but without the+TP_STATUS_VLAN_VALIDflag,thereis+nothingthatwecando*/+#endifcontinue;len=packet_len>iov.iov_len?iov.iov_len:packet_len;
From: Michael Richardson <hidden> Date: 2012-12-07 01:59:13
quoted
quoted
quoted
quoted
"Eric" == Eric W Biederman [off-list ref] writes:
Eric> It is a bit odd that you are indenting with spaces instead of tabs
Eric> in a file that is indented with tab. Again libpcap isn't my code so I
Eric> don't care but someone else might.
tabs are hysterical raisens.
Send two patches
1) tab->spaces
2) your patch.
github preferred :-)
but, I'll process this patch as is.
--
] He who is tired of Weird Al is tired of life! | firewalls [
] Michael Richardson, Sandelman Software Works, Ottawa, ON |net architect[
] mcr@sandelman.ottawa.on.ca http://www.sandelman.ottawa.on.ca/ |device driver[
Kyoto Plus: watch the video <http://www.youtube.com/watch?v=kzx1ycLXQSE>
then sign the petition.
_______________________________________________
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
From: Ani Sinha <hidden> Date: 2012-12-11 00:11:38
On Thu, 6 Dec 2012, Michael Richardson wrote:
Date: Thu, 6 Dec 2012 17:59:13
From: Michael Richardson <redacted>
To: Eric W. Biederman <redacted>
Cc: ani@aristanetworks.com, tcpdump-workers@lists.tcpdump.org,
netdev@vger.kernel.org, Francesco Ruggeri [off-list ref]
Subject: Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
quoted
quoted
quoted
quoted
quoted
"Eric" == Eric W Biederman [off-list ref] writes:
Eric> It is a bit odd that you are indenting with spaces instead of tabs
Eric> in a file that is indented with tab. Again libpcap isn't my code so I
Eric> don't care but someone else might.
Here's an updated patch. I have converted spaces to tabs consistent with
rest of the file. I have also set up github and sent a pull request
:
commit b977ebd9d9608bb8a8e4927e7a6286bdfd6b94a8
Author: Ani Sinha [off-list ref]
Date: Mon Dec 10 14:58:15 2012 -0800
Linux kernel 3.0 uses TP_STATUS_VLAN_VALID flag in packet
auxilliary data aux->tp_status to indicate a packet tagged
with a valid vlan ID 0 with another packet that is not
vlan tagged. Use this flag to check for the presence of
a vlan tagged packet. Also be careful not to cause any
breakage when libpcap is compiled against a newer kernel
(>=3.0) and used on top of an older kernel that does not
use this flag.
Signed-off-by: Ani Sinha [off-list ref]
@@ -1543,7 +1544,13 @@ pcap_read_packet(pcap_t *handle, pcap_handler callback, u_char *userdata)continue;aux=(structtpacket_auxdata*)CMSG_DATA(cmsg);-if(aux->tp_vlan_tci==0)+#if defined(TP_STATUS_VLAN_VALID)+if((aux->tp_vlan_tci==0)&&!(aux->tp_status&TP_STATUS_VLAN_VALID))+#else+if(aux->tp_vlan_tci==0)/* this is ambigious but without the+TP_STATUS_VLAN_VALIDflag,thereis+nothingthatwecando*/+#endifcontinue;len=packet_len>iov.iov_len?iov.iov_len:packet_len;
From: Ani Sinha <hidden> Date: 2012-12-11 22:36:45
It is possible to test for the presence of support of the new vlan bpf
extensions by attempting to load a filter that uses them. As only valid
filters can be loaded, old kernels that do not support filtering of vlan
tags will fail to load the a test filter with uses them.
Unfortunately I do not see this. The sk_chk_filter() does not have a
default in the case statement and the check will not detect an unknown
instruction. It will fail when the filter is run and as far as I can see,
the packet will be dropped. Something like this might help?
@@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen)return-EINVAL;/* Some instructions need special checks */switch(code){+/* for unknown instruction, return EINVAL */+default:return-EINVAL;caseBPF_S_ALU_DIV_K:/* check for division by zero */if(ftest->k==0)
From: Eric Dumazet <hidden> Date: 2012-12-11 23:04:22
On Tue, 2012-12-11 at 14:36 -0800, Ani Sinha wrote:
quoted hunk
quoted
It is possible to test for the presence of support of the new vlan bpf
extensions by attempting to load a filter that uses them. As only valid
filters can be loaded, old kernels that do not support filtering of vlan
tags will fail to load the a test filter with uses them.
Unfortunately I do not see this. The sk_chk_filter() does not have a
default in the case statement and the check will not detect an unknown
instruction. It will fail when the filter is run and as far as I can see,
the packet will be dropped. Something like this might help?
@@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen)return-EINVAL;/* Some instructions need special checks */switch(code){+/* for unknown instruction, return EINVAL */+default:return-EINVAL;caseBPF_S_ALU_DIV_K:/* check for division by zero */if(ftest->k==0)
This patch is wrong.
Check lines 546, 547, 548 where we do the check for unknown instructions
code = codes[code];
if (!code)
return -EINVAL;
If you want to test ANCILLARY possible values, its already too late, as
old kernels wont use any patch anyway.
From: Stephen Hemminger <hidden> Date: 2012-12-11 23:18:01
On Tue, 11 Dec 2012 14:36:33 -0800 (PST)
Ani Sinha [off-list ref] wrote:
quoted hunk
quoted
It is possible to test for the presence of support of the new vlan bpf
extensions by attempting to load a filter that uses them. As only valid
filters can be loaded, old kernels that do not support filtering of vlan
tags will fail to load the a test filter with uses them.
Unfortunately I do not see this. The sk_chk_filter() does not have a
default in the case statement and the check will not detect an unknown
instruction. It will fail when the filter is run and as far as I can see,
the packet will be dropped. Something like this might help?
@@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen)return-EINVAL;/* Some instructions need special checks */switch(code){+/* for unknown instruction, return EINVAL */+default:return-EINVAL;caseBPF_S_ALU_DIV_K:/* check for division by zero */if(ftest->k==0)
Did you test this? I think it will blow up for some existing instructions
like BPF_S_ALU_XOR_X or any of the other non-special instructions.
Also it is not formatted correctly for the kernel programming style.
ERROR: space prohibited before that ':' (ctx:WxW)
#86: FILE: net/core/filter.c:552:
+ default : return -EINVAL;
^
From: Ani Sinha <hidden> Date: 2012-12-12 00:46:05
On Tue, Dec 11, 2012 at 3:04 PM, Eric Dumazet [off-list ref] wrote:
On Tue, 2012-12-11 at 14:36 -0800, Ani Sinha wrote:
quoted
quoted
It is possible to test for the presence of support of the new vlan bpf
extensions by attempting to load a filter that uses them. As only valid
filters can be loaded, old kernels that do not support filtering of vlan
tags will fail to load the a test filter with uses them.
Unfortunately I do not see this. The sk_chk_filter() does not have a
default in the case statement and the check will not detect an unknown
instruction. It will fail when the filter is run and as far as I can see,
the packet will be dropped. Something like this might help?
@@ -548,6 +548,8 @@ int sk_chk_filter(struct sock_filter *filter, unsigned int flen)return-EINVAL;/* Some instructions need special checks */switch(code){+/* for unknown instruction, return EINVAL */+default:return-EINVAL;caseBPF_S_ALU_DIV_K:/* check for division by zero */if(ftest->k==0)
This patch is wrong.
yes I generated this patch wrong.
Check lines 546, 547, 548 where we do the check for unknown instructions
code = codes[code];
if (!code)
return -EINVAL;
yepph it's OK here.
If you want to test ANCILLARY possible values, its already too late, as
old kernels wont use any patch anyway.
yepph, I was looking at possible ancilliary values. Basically this
case statement :
#define ANCILLARY(CODE) case SKF_AD_OFF + SKF_AD_##CODE: \
code = BPF_S_ANC_##CODE; \
break
switch (ftest->k) {
ANCILLARY(PROTOCOL);
ANCILLARY(PKTTYPE);
ANCILLARY(IFINDEX);
ANCILLARY(NLATTR);
ANCILLARY(NLATTR_NEST);
ANCILLARY(MARK);
ANCILLARY(QUEUE);
ANCILLARY(HATYPE);
ANCILLARY(RXHASH);
ANCILLARY(CPU);
ANCILLARY(ALU_XOR_X);
ANCILLARY(VLAN_TAG);
ANCILLARY(VLAN_TAG_PRESENT);
}
_______________________________________________
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
From: Ani Sinha <hidden> Date: 2012-12-12 00:50:27
On Tue, Dec 11, 2012 at 3:04 PM, Eric Dumazet [off-list ref] wrote:
On Tue, 2012-12-11 at 14:36 -0800, Ani Sinha wrote:
quoted
quoted
It is possible to test for the presence of support of the new vlan bpf
If you want to test ANCILLARY possible values, its already too late, as
old kernels wont use any patch anyway.
So basically this means that if we generate a filter with these
special negative offset values and expect that the kernel will
complain if it does not recognize the newer values then we would be
wrong. And you are right. Old kernels never knew about them and the
code wasn't written in a way to return EINVAL if it didn't recognize a
special negative anciliary offset value.
From: Ani Sinha <hidden> Date: 2012-12-12 21:53:19
unsigned int netdev_8021q_inskb = 1;
...
{
.ctl_name = NET_CORE_8021q_INSKB,
.procname = "netdev_8021q_inskb",
.data = &netdev_8021q_inskb,
.maxlen = sizeof(int),
.mode = 0444,
.proc_handler = proc_dointvec
},
would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.
Does this work? This is just an experimental patch and by no means final.
I just want to have an idea what everyone thought about it. Once we debate
and discusss, I can cook up a final patch that would be worth commiting.
Also instead of having this /proc interface, we can perhaps check for a
specific
kernel version that :
(a) has the vlan tag info in the skb metadata (as opposed to in the packet
itself)
(b) has the following patch that adds the capability to generate a filter
based on the tag value :
commit f3335031b9452baebfe49b8b5e55d3fe0c4677d1
Author: Eric Dumazet [off-list ref]
Date: Sat Oct 27 02:26:17 2012 +0000
net: filter: add vlan tag access
WE need both of the above two things for the userland to generate a filter
code that compares vlan tag values in the skb metadata. For kernels that
has the vlan tag in
the skb metadata but does not have the above commit (b), there is nothing
that can be done. For older kernels that had the vlan tag info in the
packet itself, the filter code can be generated differently to look at
specific offsets within the packet (something that libpcap does
currently).
We have already ruled out the idea of generating a filter and trying to
load and see if that fails (see previous emails on this thread).
Hope this makes sense.
@@ -41,6 +41,8 @@#include<linux/seccomp.h>#include<linux/if_vlan.h>+boolsysctl_8021q_inskb=1;+/* No hurry in this branch**Exportedforthebpfjitloadhelper.
From: Ani Sinha <hidden> Date: 2012-12-12 22:16:28
+ Eric B.
On Wed, Dec 12, 2012 at 1:53 PM, Ani Sinha [off-list ref] wrote:
quoted hunk
quoted
unsigned int netdev_8021q_inskb = 1;
...
{
.ctl_name = NET_CORE_8021q_INSKB,
.procname = "netdev_8021q_inskb",
.data = &netdev_8021q_inskb,
.maxlen = sizeof(int),
.mode = 0444,
.proc_handler = proc_dointvec
},
would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.
Does this work? This is just an experimental patch and by no means final.
I just want to have an idea what everyone thought about it. Once we debate
and discusss, I can cook up a final patch that would be worth commiting.
Also instead of having this /proc interface, we can perhaps check for a
specific
kernel version that :
(a) has the vlan tag info in the skb metadata (as opposed to in the packet
itself)
(b) has the following patch that adds the capability to generate a filter
based on the tag value :
commit f3335031b9452baebfe49b8b5e55d3fe0c4677d1
Author: Eric Dumazet [off-list ref]
Date: Sat Oct 27 02:26:17 2012 +0000
net: filter: add vlan tag access
WE need both of the above two things for the userland to generate a filter
code that compares vlan tag values in the skb metadata. For kernels that
has the vlan tag in
the skb metadata but does not have the above commit (b), there is nothing
that can be done. For older kernels that had the vlan tag info in the
packet itself, the filter code can be generated differently to look at
specific offsets within the packet (something that libpcap does
currently).
We have already ruled out the idea of generating a filter and trying to
load and see if that fails (see previous emails on this thread).
Hope this makes sense.
@@ -41,6 +41,8 @@#include<linux/seccomp.h>#include<linux/if_vlan.h>+boolsysctl_8021q_inskb=1;+/* No hurry in this branch**Exportedforthebpfjitloadhelper.
From: Daniel Borkmann <hidden> Date: 2012-12-13 08:36:07
On 12/12/2012 10:53 PM, Ani Sinha wrote:
quoted
unsigned int netdev_8021q_inskb = 1;
...
{
.ctl_name = NET_CORE_8021q_INSKB,
.procname = "netdev_8021q_inskb",
.data = &netdev_8021q_inskb,
.maxlen = sizeof(int),
.mode = 0444,
.proc_handler = proc_dointvec
},
would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.
Does this work? This is just an experimental patch and by no means final.
I just want to have an idea what everyone thought about it. Once we debate
and discusss, I can cook up a final patch that would be worth commiting.
Also instead of having this /proc interface, we can perhaps check for a
specific
kernel version that :
(a) has the vlan tag info in the skb metadata (as opposed to in the packet
itself)
(b) has the following patch that adds the capability to generate a filter
based on the tag value :
commit f3335031b9452baebfe49b8b5e55d3fe0c4677d1
Author: Eric Dumazet [off-list ref]
Date: Sat Oct 27 02:26:17 2012 +0000
net: filter: add vlan tag access
WE need both of the above two things for the userland to generate a filter
code that compares vlan tag values in the skb metadata. For kernels that
has the vlan tag in
the skb metadata but does not have the above commit (b), there is nothing
that can be done. For older kernels that had the vlan tag info in the
packet itself, the filter code can be generated differently to look at
specific offsets within the packet (something that libpcap does
currently).
We have already ruled out the idea of generating a filter and trying to
load and see if that fails (see previous emails on this thread).
Hope this makes sense.
I think it doesn't. Because then you are obviously considering adding one
procfs file into /proc/sys/net/core/ *for each* feature that is added into
the ancillary ops which cannot be the right way ...
@@ -41,6 +41,8 @@#include<linux/seccomp.h>#include<linux/if_vlan.h>+boolsysctl_8021q_inskb=1;+/* No hurry in this branch**Exportedforthebpfjitloadhelper.
From: Ani Sinha <hidden> Date: 2012-12-13 17:34:05
On Thu, Dec 13, 2012 at 12:35 AM, Daniel Borkmann [off-list ref] wrote:
On 12/12/2012 10:53 PM, Ani Sinha wrote:
quoted
quoted
unsigned int netdev_8021q_inskb = 1;
...
{
.ctl_name = NET_CORE_8021q_INSKB,
.procname = "netdev_8021q_inskb",
.data = &netdev_8021q_inskb,
.maxlen = sizeof(int),
.mode = 0444,
.proc_handler = proc_dointvec
},
would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.
I think it doesn't. Because then you are obviously considering adding one
procfs file into /proc/sys/net/core/ *for each* feature that is added into
the ancillary ops which cannot be the right way ...
We had already brought up this topic previously in the same thread. A
suggestion was made to add that proc entry and no one from netdev
responded to it saying that it did not make any sense. Therefore
before I went ahead and made the fixes in libpcap, I wanted to run
this by your guys again to make sure we are still on the same page.
I do agree that instead of a /proc entry, we should check for a kenrel
version >= X where X is the upstream version that first started
supporting all the features needed by libpcap for vlan filtering. This
is not a compile time check but a run time one. Does anyone see any
issues with this? Is there any long term implications of this, like if
you backport patches to an older long term supported kernel? Are there
other better ways to do this, like may be returning feature bits from
an ioctl call? This is something we need to deal with on a continuous
basis as we keep supporting newer AUX fields and libpcap and other
user land code needs to make use of it. At the same time, they need to
handle backward compatibility issues with older kernels.
Thanks
From: Daniel Borkmann <hidden> Date: 2012-12-13 21:49:47
On Thu, Dec 13, 2012 at 6:34 PM, Ani Sinha [off-list ref] wrote:
On Thu, Dec 13, 2012 at 12:35 AM, Daniel Borkmann [off-list ref] wrote:
quoted
On 12/12/2012 10:53 PM, Ani Sinha wrote:
quoted
quoted
unsigned int netdev_8021q_inskb = 1;
...
{
.ctl_name = NET_CORE_8021q_INSKB,
.procname = "netdev_8021q_inskb",
.data = &netdev_8021q_inskb,
.maxlen = sizeof(int),
.mode = 0444,
.proc_handler = proc_dointvec
},
would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.
I think it doesn't. Because then you are obviously considering adding one
procfs file into /proc/sys/net/core/ *for each* feature that is added into
the ancillary ops which cannot be the right way ...
We had already brought up this topic previously in the same thread. A
suggestion was made to add that proc entry and no one from netdev
responded to it saying that it did not make any sense. Therefore
before I went ahead and made the fixes in libpcap, I wanted to run
this by your guys again to make sure we are still on the same page.
Well, not everyone on netdev might be following this thread (resp.
following fully). The best way to get responses for a patch is to go
through the normal patch submission process on netdev, and if you like
to request for comments, then mark it as RFC in the subject. This way,
people will know and likely comment on it if it makes sense or not.
I do agree that instead of a /proc entry, we should check for a kenrel
version >= X where X is the upstream version that first started
supporting all the features needed by libpcap for vlan filtering. This
is not a compile time check but a run time one. Does anyone see any
issues with this? Is there any long term implications of this, like if
you backport patches to an older long term supported kernel? Are there
other better ways to do this, like may be returning feature bits from
an ioctl call? This is something we need to deal with on a continuous
basis as we keep supporting newer AUX fields and libpcap and other
user land code needs to make use of it. At the same time, they need to
handle backward compatibility issues with older kernels.
As Eric mentioned earlier, for now there seems not to be a reliable
way to get to know which ops are present and which not. It's not
really nice, but if you want to make use of those new (ANC*) features,
probably checking kernel version might be the only way if I'm not
missing something. Now net-next is closed, but if it reopens, I'll
submit a version 2 of my patch where you've been CC'd to. If it gets
in, then at least it's for sure that since kernel <xyz> this kind of
feature test is present.
From: Ani Sinha <hidden> Date: 2012-12-13 22:07:46
On Thu, Dec 13, 2012 at 1:49 PM, Daniel Borkmann
[off-list ref] wrote:
Well, not everyone on netdev might be following this thread (resp.
following fully). The best way to get responses for a patch is to go
through the normal patch submission process on netdev, and if you like
to request for comments, then mark it as RFC in the subject. This way,
people will know and likely comment on it if it makes sense or not.
OK good to know.
As Eric mentioned earlier, for now there seems not to be a reliable
way to get to know which ops are present and which not. It's not
really nice, but if you want to make use of those new (ANC*) features,
probably checking kernel version might be the only way if I'm not
missing something. Now net-next is closed, but if it reopens, I'll
submit a version 2 of my patch where you've been CC'd to. If it gets
in, then at least it's for sure that since kernel <xyz> this kind of
feature test is present.
thanks, yes, I believe we do need some sort of validation on the
ancilliary features.
From: David Laight <hidden> Date: 2012-12-17 09:57:56
quoted
I do agree that instead of a /proc entry, we should check for a kenrel
version >= X where X is the upstream version that first started
supporting all the features needed by libpcap for vlan filtering. This
is not a compile time check but a run time one. Does anyone see any
issues with this? Is there any long term implications of this, like if
you backport patches to an older long term supported kernel? Are there
other better ways to do this, like may be returning feature bits from
an ioctl call? This is something we need to deal with on a continuous
basis as we keep supporting newer AUX fields and libpcap and other
user land code needs to make use of it. At the same time, they need to
handle backward compatibility issues with older kernels.
As Eric mentioned earlier, for now there seems not to be a reliable
way to get to know which ops are present and which not. It's not
really nice, but if you want to make use of those new (ANC*) features,
probably checking kernel version might be the only way if I'm not
missing something. Now net-next is closed, but if it reopens, I'll
submit a version 2 of my patch where you've been CC'd to. If it gets
in, then at least it's for sure that since kernel <xyz> this kind of
feature test is present.
How are you going to tell whether a feature is present in a non-Linux
kernel ?
Testing kernel versions is somewhat suboptimal as support
could be patched into a much older kernel (maybe not for
this but ...)
David
From: Guy Harris <hidden> Date: 2012-12-17 10:35:33
On Dec 17, 2012, at 1:50 AM, "David Laight" [off-list ref] wrote:
How are you going to tell whether a feature is present in a non-Linux
kernel ?
The Linux memory-mapped capture mechanism is not present in a non-Linux kernel, so all the libpcap work involved here would, if necessary on other platforms, have to be done differently on those platforms. Those platforms would have to have their own mechanisms to indicate whether any changes to filter code, processing of VLAN tags supplied out of band, etc. would need to be done.
The same would apply to other additional features of the Linux memory-mapped capture mechanism that require changes in libpcap. (Ideally, those changes would only require changes in order to use them, and would not break existing userland code, including but not limited to libpcap - your reply was to Daniel Borkmann, who is, I believe, the originator of netsniff-ng:
http://netsniff-ng.org
which has its own code using PF_PACKET sockets.)
_______________________________________________
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
From: Daniel Borkmann <hidden> Date: 2012-12-17 11:08:50
On Mon, Dec 17, 2012 at 11:35 AM, Guy Harris [off-list ref] wrote:
On Dec 17, 2012, at 1:50 AM, "David Laight" [off-list ref] wrote:
quoted
How are you going to tell whether a feature is present in a non-Linux
kernel ?
The Linux memory-mapped capture mechanism is not present in a non-Linux kernel, so all the libpcap work involved here would, if necessary on other platforms, have to be done differently on those platforms. Those platforms would have to have their own mechanisms to indicate whether any changes to filter code, processing of VLAN tags supplied out of band, etc. would need to be done.
The same would apply to other additional features of the Linux memory-mapped capture mechanism that require changes in libpcap.
From: Ani Sinha <hidden> Date: 2012-12-17 19:49:16
On Mon, Dec 17, 2012 at 2:35 AM, Guy Harris [off-list ref] wrote:
On Dec 17, 2012, at 1:50 AM, "David Laight" [off-list ref] wrote:
quoted
How are you going to tell whether a feature is present in a non-Linux
kernel ?
The Linux memory-mapped capture mechanism is not present in a non-Linux kernel, so all the libpcap work involved here would, if necessary on other platforms, have to be done >differently on those platforms. Those platforms would have to have their own mechanisms to indicate whether any changes to filter code, processing of VLAN tags supplied out of >band, etc. would need to be done.
Actually lib-pcap has these pcap-<platform>.c files that are kind of
like platform specific drivers that plug into platform independent
code like gencode.c or bpf_filter.c. These platform specific drivers
are responsible for getting packets from the kernel and running
filters (kernel or userland) on it. So all linux specific code to get
a packet and packet metadata from the kernel can neatly reside in
pcap-linux.c.
Unfortunately though, in this specific problem involving filtering
with vlan tags, both code generation (gentags.c) and code running the
filter (bpf_filter.c) will have to be aware of linux specific
semantics. Due to the issues that Bill had explained earlier in the
thread, we can not rely on post processing before installing the
kernel filter. Therefore, we need to generate a filter that can be
directly installed in the kernel. For the same reason, bpf_filter()
code also needs to change - be aware of linux specific semantics.