Re: libpcap linux mmap patch

[Guy Harris <guy () alum mit edu>]

Alexander 'Leo' Bergolth wrote:
> Hi!
>
> On 01/31/2008 02:39 PM, Abeni Paolo wrote:
> > on Thu 1/31/2008 10:37 AM Alexander 'Leo' Bergolth wrote:
> > > I just gave your new linux mmap patch a try
> >
> > thanks for the review :-)
>
> Bad news...
> I've also tested it on debian etch (kernel 2.6.22-2-686).
> Unfortunately, the same patch seems to cause some displacement of the 
> frames, starting with the first captured frame, when using the "any" 
> interface.

At least as I read tpacket_rcv() in 2.6.22.6's net/packet/af_packet.c, 
for SOCK_DGRAM sockets (which are what are used on the "any" interface, 
as well as on some other interfaces, e.g. PPP interfaces), it leaves 16 
bytes worth of extra space in front of the packet data:

        if (sk->sk_type == SOCK_DGRAM) {
                macoff = netoff = TPACKET_ALIGN(TPACKET_HDRLEN) + 16;
        } else {
                unsigned maclen = skb_network_offset(skb);
                netoff = TPACKET_ALIGN(TPACKET_HDRLEN + (maclen < 16 ? 16 : maclen));
                macoff = netoff - maclen;
        }

and Paolo's code appears to construct the SLL header on top of those 16 
reserved bytes.  The SLL header is 16 bytes long, so that *should* work.

*However*, unless I'm missing something, a pointer to the payload after 
the SLL header is being passed to the callback; pcap_read_linux_mmap() does:

        bp = (unsigned char*)thdr + thdr->tp_mac;

which, for SOCK_DGRAM sockets, causes "bp" to point to the packet 
payload, not to the 16-byte reserved area before the packet payload, and 
then later does

                /* if required build in place the sll header*/
                if (handle->md.cooked) {
			struct sll_header *hdrp = (struct sll_header *)((char *)bp - 
sizeof(struct sll_header));

                        hdrp->sll_pkttype = map_packet_type_to_sll_type(
                                                        sll->sll_pkttype);
                        hdrp->sll_hatype = htons(sll->sll_hatype);
                        hdrp->sll_halen = htons(sll->sll_halen);
                        memcpy(hdrp->sll_addr, sll->sll_addr, SLL_ADDRLEN);
                        hdrp->sll_protocol = sll->sll_protocol;

                        /* update packet len */
                        pcaphdr.caplen += SLL_HDR_LEN;
                        pcaphdr.len += SLL_HDR_LEN;
                }

where "hdrp" is set to point to the beginning of the reserved space in 
which it constructs the SLL header, and then does

                callback(user, &pcaphdr, bp);

without modifying "bp".

Perhaps if it instead does something such as:

                /* if required build in place the sll header*/
                if (handle->md.cooked) {
                        struct sll_header *hdrp;

                        bp -= sizeof(struct sll_header);
                        hdrp = (struct sll_header *)bp;

                        hdrp->sll_pkttype = map_packet_type_to_sll_type(
                                                        sll->sll_pkttype);
                        hdrp->sll_hatype = htons(sll->sll_hatype);
                        hdrp->sll_halen = htons(sll->sll_halen);
                        memcpy(hdrp->sll_addr, sll->sll_addr, SLL_ADDRLEN);
                        hdrp->sll_protocol = sll->sll_protocol;

                        /* update packet len */
                        pcaphdr.caplen += SLL_HDR_LEN;
                        pcaphdr.len += SLL_HDR_LEN;
                }

that would work.  I'll try it out on my Ubuntu "machine" (it's made by a 
company named "VMware" :-)).
-
This is the tcpdump-workers list.
Visit https://cod.sandelman.ca/ to unsubscribe.