| Message ID | CAOxq_8OC6zUm93mCHD5daxDmuYmmbUdgy0DnjfQdBKky_2t4nQ@mail.gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
Ani Sinha <ani@aristanetworks.com> writes: > On Sat, Nov 17, 2012 at 3:33 PM, Eric W. Biederman > <ebiederm@xmission.com> 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? 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 6, 2012 at 2:19 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: > Ani Sinha <ani@aristanetworks.com> writes: > >> On Sat, Nov 17, 2012 at 3:33 PM, Eric W. Biederman >> <ebiederm@xmission.com> 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? > > 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ani Sinha <ani@aristanetworks.com> writes: > On Thu, Dec 6, 2012 at 2:19 PM, Eric W. Biederman <ebiederm@xmission.com> wrote: >> Ani Sinha <ani@aristanetworks.com> writes: >>>> >>> >>> 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 > pcap-linux.c | 50 +++++++++++++++++++++++++++++++------------------- > 1 files changed, 31 insertions(+), 19 deletions(-) > > diff --git a/pcap-linux.c b/pcap-linux.c > index a42c3ac..8e355d3 100644 > --- a/pcap-linux.c > +++ b/pcap-linux.c > @@ -133,6 +133,7 @@ static const char rcsid[] _U_ = > #include <sys/utsname.h> > #include <sys/mman.h> > #include <linux/if.h> > +#include <linux/if_packet.h> > #include <netinet/in.h> > #include <linux/if_ether.h> > #include <net/if_arp.h> > @@ -1543,7 +1544,13 @@ pcap_read_packet(pcap_t *handle, pcap_handler > 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 ||. > +#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 > max_packets, pcap_handler callback, > } > > #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_VLAN_VALID)) && > +#else > + h.h2->tp_vlan_tci && > +#endif > handle->md.vlan_offset != -1 && > tp_snaplen >= (unsigned int) handle->md.vlan_offset) { > struct vlan_tag *tag; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: libpcap-1.1.1/pcap-linux.c =================================================================== --- libpcap-1.1.1.orig/pcap-linux.c +++ libpcap-1.1.1/pcap-linux.c @@ -132,6 +132,7 @@ static const char rcsid[] _U_ = #include <sys/utsname.h> #include <sys/mman.h> #include <linux/if.h> +#include <linux/if_packet.h> #include <netinet/in.h> #include <linux/if_ether.h> #include <net/if_arp.h> @@ -1486,7 +1487,13 @@ pcap_read_packet(pcap_t *handle, pcap_ha 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 & 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;