CODEWITHSUNDEEP
clinuxnetwork-programmingkernel
Cody
Cody

Reputation: 145

Extracting data from struct sk_buff

I'm attempting to extract data from a struct sk_buff, but have not received the output I am expecting. The frame in question is 34 bytes; a 14-byte Ethernet header wrapped around an 8-byte (experimental protocol) header:

struct monitoring_hdr {
    u8      version;
    u8      type;
    u8      reserved;
    u8      haddr_len;

    u32     clock;
} __packed;

After this header, there are two, variable-length hardware addresses (their lengths are dictated by the haddr_len field above). In the example here, they are both 6 bytes long.

The following code extracts the header (the struct) correctly, but not the two MAC addresses that follow.

Sender side:

    ...
    skb = alloc_skb(mtu, GFP_ATOMIC);
    if (unlikely(!skb))
            return;
    skb_reserve(skb, ll_hlen);
    skb_reset_network_header(skb);
    nwp = (struct monitoring_hdr *)skb_put(skb, hdr_len);
    /* ... Set up fields in struct monitoring_hdr ... */
    memcpy(skb_put(skb, dev->addr_len), src, dev->addr_len);
    memcpy(skb_put(skb, dev->addr_len), dst, dev->addr_len);
    ...

Receiver side:

   ...
   skb_reset_network_header(skb);
   nwp = (struct monitoring_hdr *)skb_network_header(skb);

   src = skb_pull(skb, nwp->haddr_len);
   dst = skb_pull(skb, nwp->haddr_len);
   ...

Expected output:

I used tcpdump to capture the packet in question on the wire, and saw this (it was actually padded to 60 bytes by the sender's NIC, which I've omitted):

0000 | 00 90 f5 c6 44 5b 00 0e  c6 89 04 2f c0 df 01 03
0010 | 00 06 d0 ba 8c 88 00 0e  c6 89 04 2f 00 90 f5 c6
0020 | 44 5b

The first 14 bytes is the Ethernet header. The following 8 bytes (starting with 01 and ending with 88) should be the bytes put into the struct monitoring_hdr, which executes correctly. Then, I am expecting the following MAC addresses to be found:

src = 00 0e c6 89 04 2f
dst = 00 90 f5 c6 44 5b

Actual output:

However, the data that I receive is shifted two bytes to the left:

src = 8c 88 00 0e c6 89
dst = 04 2f 00 90 f5 c6

Can anyone see a logical flaw in above code? Or is there a better way to do this? I've also tried skb_pull in place of skb_network_header on the receiving side, but that resulted in a kernel panic.

Thanks in advance for any help.

SOLUTION:

The pointer to the first byte of the data in the sk_buff was not being pointed to by src as it should have been. I ended up using the following:

   ...
   skb_reset_network_header(skb);
   nwp = (struct monitoring_hdr *)skb_network_header(skb);
   skb_pull(skb, offsetof(struct monitoring_hdr, haddrs_begin));

   src = skb->data;
   dst = skb_pull(skb, nwp->haddr_len);
   ...

Upvotes: 2

Views: 15533

Answers (1)

Ben Kelly
Ben Kelly

Reputation: 1344

Looking at the skbuff.h header, the functions you are using look like this:

static inline void skb_reset_network_header(struct sk_buff *skb)
{
        skb->network_header = skb->data - skb->head;
}

static inline unsigned char *skb_network_header(const struct sk_buff *skb)
{
        return skb->head + skb->network_header;
}

extern unsigned char *skb_pull(struct sk_buff *skb, unsigned int len);
static inline unsigned char *__skb_pull(struct sk_buff *skb, unsigned int len)
{
        skb->len -= len;
        BUG_ON(skb->len < skb->data_len);
        return skb->data += len;
}

So first, I would try printing out skb->data and skb->head to make sure they are referencing the parts of the packet you expect them to. Since you are using a custom protocol here, perhaps there is a bug in the header processing code which is causing skb->data to be set incorrectly.

Also, looking at the definitions of sky_network_header and skb_pull makes me think perhaps you are using them incorrectly. Shouldn't the first 6-byte addr be at the location pointed to be the return value of skb_network_header()? It looks like that function adds the length of the header block to the head of the buffer, which should result in a pointer to your first data value.

Similarly, it looks like skb_pull() adds the length of the field you pass in and returns the pointer to the next byte. So you probably want something more like this:

src = skb_network_header(skb);
dst = skb_pull(skb, nwp->haddr_len);

I hope that helps. I'm sorry that this is not an exact answer.

Upvotes: 3

Related Questions