avoid unnecessary alignement overhead in skb->data allocation.

net/core/skbuff.c struct sk_buff *__alloc_skb(unsigned int · line 162

Quoted 1 line

net/core/skbuff.c +1 -0

+	if ((1UL << order) > size + sizeof(void *) + sizeof(struct skb_shared_info)) {

get_order() returns a PAGE_SIZE order not a byte one. So this test here at the end is incorrect. It should probably be something like "if ((PAGE_SIZE << order) > ..."

I don't know if I want to eat an entire extra allocation for every SKB just to handle broken e1000 cards that can't be bothered to support non-power-of-2 receive buffer sizes and a proper MTU setting.

I guess we might have to, but this is extremely unfortunate. :-/

I don't know if I want to eat an entire extra allocation for every SKB just to handle broken e1000 cards that can't be bothered to support non-power-of-2 receive buffer sizes and a proper MTU setting.

I guess we might have to, but this is extremely unfortunate. :-/

Quoted 6 lines · David Miller

I don't know if I want to eat an entire extra allocation for every SKB just to handle broken e1000 cards that can't be bothered to support non-power-of-2 receive buffer sizes and a proper MTU setting.

I guess we might have to, but this is extremely unfortunate. :-/

I'd hope not. Apparently they are capable of putting data into individual pages and chaining them together. The only problem

Quoted 3 lines

I'd hope not. Apparently they are capable of putting data into individual pages and chaining them together. The only problem

Unfortunately not all chips are capable to do this.

Quoted 5 lines · Evgeniy Polyakov

Quoted 3 lines

I'd hope not. Apparently they are capable of putting data into individual pages and chaining them together. The only problem

Unfortunately not all chips are capable to do this.

No not all chips are capable of header-splitting. However, from what Jesse was saying it sounded as if all (or most?) chips are capable of storing data cross pages.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Only if they form contiguous region?
Jesse, is it possible for every e1000 chip to split frame into several page-sized chunks i.e. create some kind of receiving scatter-gather?

Quoted 4 lines · Evgeniy Polyakov

Only if they form contiguous region?
Jesse, is it possible for every e1000 chip to split frame into several page-sized chunks i.e. create some kind of receiving scatter-gather?

Actually, it was Chris Leech who raised this possibility:

: Yes, e1000 devices will spill over and use multiple buffers for a : single frame. We've been trying to find a good way to use multiple : buffers to take care of these allocation problems. The structure of : the sk_buff does not make it easy. Or should I say that it's the : limitation that drivers are not allowed to chain together multiple : sk_buffs to represent a single frame that does not make it easy.

Perhaps he can enlighten us.

also, eth_type_trans wants skb->data to point to header, which would require us to memcpy data from a page back to skb->data.

LPE controls whether long packet reception is permitted. Hardware discards long packets if LPE is 0. A long packet is one longer than 1522 bytes. If LPE is 1, the maximum packet size that the device can receive is 16384 bytes.

So there is no place at the end of skb for additional pointer. And new question arises - until what Jesse suggested is implemented in some way, do we need to store a pointer to shared info inside skb and allocate it from cache in case it does no fit into aligned buffer (in case of e1000 it happens all the time exept 1500 MTU)? David, Herbert?

Quoted 7 lines · Evgeniy Polyakov

So there is no place at the end of skb for additional pointer. And new question arises - until what Jesse suggested is implemented in some way, do we need to store a pointer to shared info inside skb and allocate it from cache in case it does no fit into aligned buffer (in case of e1000 it happens all the time exept 1500 MTU)? David, Herbert?

I'm not sure whether this is really worth it. After all, the only order of allocation that's really likely to succeed is 0. So going from order 3 to order 2 probably doesn't make that big a difference.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Attached patch allows to avoid unnecessary alignment overhead in skb->data allocation.
Main idea is to allocate struct skb_shared_info from cache when addition of sizeof(struct skb_shared_info) ens up in different order allocation than initial size order.
This allows to solve problem with 4k allocations for 1500 MTU and 32k allocations for 9k jumbo frames for some chips.
Patch was not tested, so if idea worth it I will complete it.

Quoted 9 lines · Evgeniy Polyakov

Attached patch allows to avoid unnecessary alignment overhead in skb->data allocation.
Main idea is to allocate struct skb_shared_info from cache when addition of sizeof(struct skb_shared_info) ens up in different order allocation than initial size order.
This allows to solve problem with 4k allocations for 1500 MTU and 32k allocations for 9k jumbo frames for some chips.
Patch was not tested, so if idea worth it I will complete it.

I thought the Intel guys were saying that their NIC could write the full 16KB which means it it's unsafe to use the last four bytes, no?

Cheers,

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Well, theirs comments in code say, that maximum allowed frame size is 0x3f00, so there is a little place at the end to put there a pointer, so I allocate size + sizeof(void *).
If they actually can eat all 16k, then we need a pointer somewhere in the skb for shared_info, since 16k + sizeof(void *) will be aligned to 32k.

Hello.

Attached patch allows to avoid unnecessary alignment overhead in skb->data allocation.
Main idea is to allocate struct skb_shared_info from cache when addition of sizeof(struct skb_shared_info) ens up in different order allocation than initial size order.
This allows to solve problem with 4k allocations for 1500 MTU and 32k allocations for 9k jumbo frames for some chips.
Patch was not tested, so if idea worth it I will complete it.

Signed-off-by Evgeniy Polyakov <[email protected]>

(quoted diff) +1 -0

+	if ((1UL << order) > size + sizeof(void *) + sizeof(struct

skb_shared_info)) { + data = ____kmalloc(size + sizeof(struct
skb_shared_info), gfp_mask); + if (!data)

net/core/skbuff.c struct sk_buff *__alloc_skb(unsigned int · line 170

Quoted 9 lines

(quoted diff) +1 -0

+	if ((1UL << order) > size + sizeof(void *) + sizeof(struct

skb_shared_info)) { + data = ____kmalloc(size + sizeof(struct
skb_shared_info), gfp_mask); + if (!data)

net/core/skbuff.c +6 -0

+			goto nodata;
+		memset(skb, 0, offsetof(struct sk_buff, truesize));
+	} else {
+		unsigned long *ptr;
+
+		data = ____kmalloc(size, gfp_mask);

You certainly want to kmalloc(size + sizeof(void *)) here, dont you ?

Eric