skbuff truesize incorrect.

My questions are:

1) Which buffer size does truesize represent, is it the total buffer or just the data related to the relevant skb?

2) If truesize is for the skb it is contained within should it be updated during the call to skb_trim?

3) Why does the truesize default to 32960?

From: Jim Baxter <[email protected]>
Date: Thu, 22 May 2014 20:07:41 +0100

Quoted 9 lines

My questions are:

1) Which buffer size does truesize represent, is it the total buffer or just the data related to the relevant skb?

2) If truesize is for the skb it is contained within should it be updated during the call to skb_trim?

3) Why does the truesize default to 32960?

skb->truesize represents the total amount of memory that the SKB is holding up.

This is the size of the sk_buff metadata plus all of the buffer memory.

For example, if the buffer is using 16K buffer yet only 1500 of it is actually used for the packet, 16K is what should be plugged into the equation to compute truesize.

Otherwise it would be easy to compromise a system by sending lots of 1 byte packets to a socket, even though the actual memory consumed by that "1 byte" packet is significantly larger.

OK, so it is the value of the memory that has been allocated for the SKB. If there are multiple clones for an skb all pointing at the same data, will that distort the memory used when they all have the same truesize?

If there are multiple clones for an skb all pointing at the same data, will that distort the memory used when they all have the same truesize?

From: Jim Baxter <[email protected]>
Date: Thu, 22 May 2014 21:21:55 +0100

Quoted 2 lines

If there are multiple clones for an skb all pointing at the same data, will that distort the memory used when they all have the same truesize?

What is distorted about it?

Each clone references exactly that much backing memory.

Quoted 8 lines

From: Jim Baxter <[email protected]>
Date: Thu, 22 May 2014 21:21:55 +0100

Quoted 2 lines

If there are multiple clones for an skb all pointing at the same data, will that distort the memory used when they all have the same truesize?

What is distorted about it?

Quoted 2 lines

Each clone references exactly that much backing memory.

Thank you for you help, I think distorted was the wrong word.

I think I am confused what truesize is used for it looks like it is used to deallocate memory, is this correct?

Quoted 8 lines

From: Jim Baxter <[email protected]>
Date: Thu, 22 May 2014 21:21:55 +0100

Quoted 2 lines

If there are multiple clones for an skb all pointing at the same data, will that distort the memory used when they all have the same truesize?

What is distorted about it?

Quoted 2 lines

Each clone references exactly that much backing memory.

Thank you for you help, I think distorted was the wrong word.

I think I am confused what truesize is used for it looks like it is used to deallocate memory, is this correct?

Hi, I was hoping you can help me with some questions.

I have been investigating a network issue with bursts of network traffic over USB CDC-NCM, the issue is that the kernel is dropping packets because sk_rcvqueues_full() returns true due to skb2->truesize is always 32960 instead of SKB_TRUESIZE(skb2->len) which is about 1800.

The code I am trying to fix is this code below, it is splitting a set of multiple network packets compressed into a single 16k packet into individual skb's and sending them up the network stack.

    skb2 = skb_clone(skb, GFP_ATOMIC);
    if (skb2 == NULL)
        goto err;

    if (!skb_pull(skb2, index)) {
        ret = -EOVERFLOW;
        goto err;
    }

    skb_trim(skb2, dg_len - crc_len);

My questions are:

1) Which buffer size does truesize represent, is it the total buffer or just the data related to the relevant skb?

2) If truesize is for the skb it is contained within should it be updated during the call to skb_trim?

3) Why does the truesize default to 32960?

Thank you for any help,
Jim Baxter.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 5 lines · Jim Baxter

    skb2 = skb_clone(skb, GFP_ATOMIC);
    if (skb2 == NULL)
        goto err;

Quoted 4 lines

    if (!skb_pull(skb2, index)) {
        ret = -EOVERFLOW;
        goto err;
    }

This assumes that you original 16K packet is linear. Is that always the case?

This is the very same problem that I ran into with SCTP since it has similar code in it. You can play games with truesize manually, but you have to be very careful here.

-vlad

I now think that the correct solution here is to create a new smaller skb and copy the data from the sub packets into it.

Quoted 2 lines · Jim Baxter

I now think that the correct solution here is to create a new smaller skb and copy the data from the sub packets into it.

For low speed devices, this is indeed the best way.

(this is called copybreak in some nic drivers)

I have been investigating a network issue with bursts of network traffic over USB CDC-NCM, the issue is that the kernel is dropping packets because sk_rcvqueues_full() returns true due to skb2->truesize is always 32960 instead of SKB_TRUESIZE(skb2->len) which is about 1800.

The code I am trying to fix is this code below, it is splitting a set of multiple network packets compressed into a single 16k packet into individual skb's and sending them up the network stack.

I would set rx_max (rx_urb_size) to SKB_MAX_HEAD(0) so that you do not use high order allocations.

Quoted 3 lines

I would set rx_max (rx_urb_size) to SKB_MAX_HEAD(0) so that you do not use high order allocations.

Correction, that would need SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN), because drivers/net/usb/usbnet.c calls __netdev_alloc_skb_ip_align().

From: Eric Dumazet <[email protected]>
Date: Thu, 22 May 2014 14:03:21 -0700

Quoted 5 lines · Eric Dumazet

Quoted 3 lines

I would set rx_max (rx_urb_size) to SKB_MAX_HEAD(0) so that you do not use high order allocations.

Quoted 2 lines

Correction, that would need SKB_MAX_HEAD(NET_SKB_PAD + NET_IP_ALIGN), because drivers/net/usb/usbnet.c calls __netdev_alloc_skb_ip_align().

I seem to recall that this driver has a lot of strange buffering restrictions and that Bjorn Mork was talking a lot about this recently.

Bjorn people are getting really burnt because the driver ends up having a skb->truesize of 32K for the buffers it allocates on receive and this chokes up TCP and SCTP because the socket memory limits are hitting earlier than they should.

We've just in the past few postings been discussing whether the just copy every packet into a more appropriately sized buffer, and it isn't clear if that's a good idea of the data rates handled here.

But although the problem is the same, I believe the driver in question isn't the one I have been looking at recently. The posted code snippet was from the NCM gadget driver (drivers/usb/gadget/f_ncm.c), isn't that right Jim?

Yes, judging by this discussion I guess we should unconditionally copy instead of cloning in these drivers. We'll always have bad payload/truesize ratio for cloned skbs, often less than 1/10 even for max size payload.

Actually, I thought we already did copy in the host cdc_ncm driver. But I was wrong. I was thinking of the cdc_mbim driver (which is different enough to have its own implementation of this part of the rx code). The cdc_ncm driver is cloning and the cdc_mbim driver is copying. So we're not even consistent...

I'll create and test a patch for the cdc_ncm host driver unless someone else wants to do that. I haven't really played with the gadget driver before, so I'd prefer if someone knowing it (Jim maybe?) could take care of it. If not, then I can always make an attempt using dummy_hcd to test it.

BTW, wrt the data rates: These drivers are USB class drivers and we should really think of *all* possible rates, even future ones. This is not limited to 480 Mbps USB2. AFAICS, there isn't anything preventing the gadget driver from being used with e.g. a USB3380 controller to create a 5 Gbps NCM device. I'm sure the future will bring us even faster USB devices. The drivers will be the same. Which is sort of beautiful and scaring at the same time :-)

But I assume the bad payload/truesize ratio is the most important factor here, so we should still copy?

Quoted 5 lines

I'll create and test a patch for the cdc_ncm host driver unless someone else wants to do that. I haven't really played with the gadget driver before, so I'd prefer if someone knowing it (Jim maybe?) could take care of it. If not, then I can always make an attempt using dummy_hcd to test it.

I can create a patch for the host driver, I will issue the gadget patch first to resolve any issues, the fix would be similar.

From 4c7431cd046a6972e153e23249ad490a3692f9a3 Mon Sep 17 00:00:00 2001

Quoted 5 lines · Jim Baxter

Quoted 5 lines

I'll create and test a patch for the cdc_ncm host driver unless someone else wants to do that. I haven't really played with the gadget driver before, so I'd prefer if someone knowing it (Jim maybe?) could take care of it. If not, then I can always make an attempt using dummy_hcd to test it.

Quoted 2 lines

I can create a patch for the host driver, I will issue the gadget patch first to resolve any issues, the fix would be similar.

Well, I couldn't help myself. I just had to test it. The attached patch works for me, briefly tested with an Ericsson H5321gw NCM device. I have no ideas about the performance impact as that modem is limited to 21 Mbps HSDPA.

Quoted 5 lines · Jim Baxter

Quoted 5 lines

I'll create and test a patch for the cdc_ncm host driver unless someone else wants to do that. I haven't really played with the gadget driver before, so I'd prefer if someone knowing it (Jim maybe?) could take care of it. If not, then I can always make an attempt using dummy_hcd to test it.

Quoted 2 lines

I can create a patch for the host driver, I will issue the gadget patch first to resolve any issues, the fix would be similar.

Well, I couldn't help myself. I just had to test it. The attached patch works for me, briefly tested with an Ericsson H5321gw NCM device. I have no ideas about the performance impact as that modem is limited to 21 Mbps HSDPA.

If you are measuring performance with the likes of netperf, you should be able to get an idea of the performance effect from the change in service demand (CPU consumed per unit of work) even if the maximum throughput remains capped.

Quoted 10 lines

I have been investigating a network issue with bursts of network traffic over USB CDC-NCM, the issue is that the kernel is dropping packets because sk_rcvqueues_full() returns true due to skb2->truesize is always 32960 instead of SKB_TRUESIZE(skb2->len) which is about 1800.

The code I am trying to fix is this code below, it is splitting a set of multiple network packets compressed into a single 16k packet into individual skb's and sending them up the network stack.

if skb are allocated with 16k = 16384 bytes, adding the struct skb_shared_info overhead and rounding up to power of two gives 32768 bytes.

Chances to be able to allocate 32KB contiguous memory are slim after a while.

I would set rx_max (rx_urb_size) to SKB_MAX_HEAD(0) so that you do not use high order allocations.

What is the exact 'overhead' using ~4K instead of 16K exactly on this hardware ?

Since USB bulk data are terminated by a short fragment there is actually no need for the URB be long enough for the full message. Provided the URB are multiples of the USB message size (1k for USB 3) the message can be received into multiple URB - the driver just has to be willing to merge URB buffers (as well as split them) when generating the ethernet frames.

What the driver needs to do us allocate URB with 2k (or 4k) buffers and only allocate the skb when processing the receive data. Unfortunately this is a major rework of usb_net.c

Note that some of the usb ethernet drivers allocate large skb then lie about the truesize.

David Laight <[email protected]> writes:

Quoted 2 lines

Note that some of the usb ethernet drivers allocate large skb then lie about the truesize.

Hmm, which drivers are these?

From: Bjørn Mork [mailto:[email protected]]

Quoted 1 line

David Laight <[email protected]> writes:

...

Quoted 3 lines

Quoted 2 lines

Note that some of the usb ethernet drivers allocate large skb then lie about the truesize.

Quoted 1 line

Hmm, which drivers are these?

$ grep truesize linux/drivers/net/usb/*

asix_88179_178a, smsc95xx, sr9700.

David

What are the side effects of changing the truesize, if the original uncloned skb has the full truesize then isn't the potential memory usage still counted for the avoidance of OOM?

Quoted 3 lines · Jim Baxter

What are the side effects of changing the truesize, if the original uncloned skb has the full truesize then isn't the potential memory usage still counted for the avoidance of OOM?

Nope. This can be disastrous.

A malicious remote peer can crash your host by sending specially cooked TCP messages.

Send messages with one byte of payload, and out of order so that they cant be consumed by receiver, and cant be coalesced/collapsed.

If you claim the true size is sizeof(sk_buff) + 512, TCP stack will accumulate these messages in out of order queue, and will not bother with them, unless you hit sk_rcvbuf limit.

But in reality these messages uses sizeof(sk_buff) + 32768 bytes.

Divide your physical memory by 32768 : How many such messages will fit in memory before the host crashes ?

I've seen that kind of attacks in real cases.

Even the fast clones sk_buff mismatch can be noticed. Luckily a 10% error has no severe impact.

TCP stack uses fast clones, and current stack gives them a truesize of 2048 + sizeof(sk_buff), while it really should be 2048 + 2*sizeof(sk_buff)

Luckily, GSO/TSO tends to reduce the error, as skbs overhead is lower.

TCP stack uses fast clones, and current stack gives them a truesize of 2048 + sizeof(sk_buff), while it really should be 2048 + 2*sizeof(sk_buff)

Luckily, GSO/TSO tends to reduce the error, as skbs overhead is lower.

From: Eric Dumazet
...

Quoted 5 lines

TCP stack uses fast clones, and current stack gives them a truesize of 2048 + sizeof(sk_buff), while it really should be 2048 + 2*sizeof(sk_buff)

Luckily, GSO/TSO tends to reduce the error, as skbs overhead is lower.

Doesn't that affect the tx side - where the truesize doesn't matter as much?

The hardware will put multiple ethernet frames into a single USB bulk data message. To handle this the driver generates a URB that is (hopefully) long enough for the longest USB message (typically 32k is assumed to be enough). The URB that usb_net generated have the data in a linear skb - which then has a large 'truesize'.

Since USB bulk data are terminated by a short fragment there is actually no need for the URB be long enough for the full message. Provided the URB are multiples of the USB message size (1k for USB 3) the message can be received into multiple URB - the driver just has to be willing to merge URB buffers (as well as split them) when generating the ethernet frames.

What the driver needs to do us allocate URB with 2k (or 4k) buffers and only allocate the skb when processing the receive data. Unfortunately this is a major rework of usb_net.c

Note that some of the usb ethernet drivers allocate large skb then lie about the truesize.

From: David Laight <[email protected]>
Date: Fri, 23 May 2014 08:52:13 +0000

Quoted 6 lines

The hardware will put multiple ethernet frames into a single USB bulk data message. To handle this the driver generates a URB that is (hopefully) long enough for the longest USB message (typically 32k is assumed to be enough). The URB that usb_net generated have the data in a linear skb - which then has a large 'truesize'.

Quoted 6 lines

Since USB bulk data are terminated by a short fragment there is actually no need for the URB be long enough for the full message. Provided the URB are multiples of the USB message size (1k for USB 3) the message can be received into multiple URB - the driver just has to be willing to merge URB buffers (as well as split them) when generating the ethernet frames.

I think we could take a less invasive approach.

Use whatever order page is needed for that 32K chunk for the URB, but split up the compound page so that the individual pages can be accounted for separately.

Hook up the pages into the SKB frag array, and only account PAGE_SIZE into the SKB truesize for the individual pages actually used from the superpage.

This will largely retain the URB allocation and processing scheme, yet at the same time dramatically decrease the truesize overrage factor.

True, you still need to sort out how to handle multiple ethernet frames in the same 4k page, and arbitrary page boundaries within a frame.