add netdev_alloc_skb

PATCH In review Christoph Hellwig 2 participants 7 messages Jul 26, 2006 – Jul 28, 2006

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Christoph Hellwig

author

2006-07-26 11:31 UTC #0 lore raw

PATCH 1/3 add netdev_alloc_skb

Add a dev_alloc_skb variant that takes a struct net_device * paramater. For now that paramater is unused, but I'll use it to allocate the skb from node-local memory in a follow-up patch. Also there have been some other plans mentioned on the list that can use it.

Signed-off-by Christoph Hellwig <[email protected]>

include/linux/skbuff.h +22 -0

Index: linux-2.6/include/linux/skbuff.h
===================================================================
@@ -1104,6 +1104,28 @@
 	return __dev_alloc_skb(length, GFP_ATOMIC);
 }
 
+extern struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
+		unsigned int length, gfp_t gfp_mask);
+
+/**
+ *	netdev_alloc_skb - allocate an skbuff for rx on a specific device
+ *	@dev: network device to receive on
+ *	@length: length to allocate
+ *
+ *	Allocate a new &sk_buff and assign it a usage count of one. The
+ *	buffer has unspecified headroom built in. Users should allocate
+ *	the headroom they think they need without accounting for the
+ *	built in space. The built in space is used for optimisations.
+ *
+ *	%NULL is returned in there is no free memory. Although this function
+ *	allocates memory it can be called from an interrupt.
+ */
+static inline struct sk_buff *netdev_alloc_skb(struct net_device *dev,
+		unsigned int length)
+{
+	return __netdev_alloc_skb(dev, length, GFP_ATOMIC);
+}
+
 /**
  *	skb_cow - copy header of skb when it is required
  *	@skb: buffer to cow

net/core/skbuff.c +24 -0

Index: linux-2.6/net/core/skbuff.c
===================================================================
@@ -256,6 +256,29 @@
 	goto out;
 }
 
+/**
+ *	__netdev_alloc_skb - allocate an skbuff for rx on a specific device
+ *	@dev: network device to receive on
+ *	@length: length to allocate
+ *	@gfp_mask: get_free_pages mask, passed to alloc_skb
+ *
+ *	Allocate a new &sk_buff and assign it a usage count of one. The
+ *	buffer has unspecified headroom built in. Users should allocate
+ *	the headroom they think they need without accounting for the
+ *	built in space. The built in space is used for optimisations.
+ *
+ *	%NULL is returned in there is no free memory.
+ */
+struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
+		unsigned int length, gfp_t gfp_mask)
+{
+	struct sk_buff *skb;
+
+	skb = alloc_skb(length + NET_SKB_PAD, gfp_mask);
+	if (likely(skb))
+		skb_reserve(skb, NET_SKB_PAD);
+	return skb;
+}
 
 static void skb_drop_list(struct sk_buff **listp)
 {

@@ -2042,6 +2065,7 @@
 EXPORT_SYMBOL(kfree_skb);
 EXPORT_SYMBOL(__pskb_pull_tail);
 EXPORT_SYMBOL(__alloc_skb);
+EXPORT_SYMBOL(__netdev_alloc_skb);
 EXPORT_SYMBOL(pskb_copy);
 EXPORT_SYMBOL(pskb_expand_head);
 EXPORT_SYMBOL(skb_checksum);

David Miller

maintainer

2006-07-26 22:24 UTC #1 lore raw

From: Christoph Hellwig <[email protected]>
Date: Wed, 26 Jul 2006 13:31:36 +0200

Quoted 2 lines

+ * %NULL is returned in there is no free memory. Although this function
+ * allocates memory it can be called from an interrupt.

...

Quoted 1 line

+ * %NULL is returned in there is no free memory.

Looks like a typo in both cases, I think you mean "NULL is returned _if_ there" not "in there"

I think I can queue this up for 2.6.19 if you fix this typo, but I'm not going to slap the deprecated marker on dev_alloc_skb() until all of the in-tree cases are being taken care of. It will just be a bunch of pointless noise, and at worse it will push the remaining conversions on some poor soul such as Andrew :)

Christoph Hellwig

author

2006-07-28 07:09 UTC #2 lore raw

Quoted 10 lines · David Miller

From: Christoph Hellwig <[email protected]>
Date: Wed, 26 Jul 2006 13:31:36 +0200
Quoted 2 lines
+ * %NULL is returned in there is no free memory. Although this function
+ * allocates memory it can be called from an interrupt.
...
Quoted 1 line
+ * %NULL is returned in there is no free memory.
Looks like a typo in both cases, I think you mean
"NULL is returned _if_ there" not "in there"

Yes, Joe Perches already pointed that out in private. This typo is copy and pasted from the dev_alloc_skb kerneldoc, so this patch fixes up all four occurances:

Signed-off-by Christoph Hellwig <[email protected]>

include/linux/skbuff.h +3 -3

Index: linux-2.6/include/linux/skbuff.h
===================================================================
@@ -1076,7 +1076,7 @@
  *	the headroom they think they need without accounting for the
  *	built in space. The built in space is used for optimisations.
  *
- *	%NULL is returned in there is no free memory.
+ *	%NULL is returned if there is no free memory.
  */
 static inline struct sk_buff *__dev_alloc_skb(unsigned int length,
 					      gfp_t gfp_mask)

@@ -1096,7 +1096,7 @@
  *	the headroom they think they need without accounting for the
  *	built in space. The built in space is used for optimisations.
  *
- *	%NULL is returned in there is no free memory. Although this function
+ *	%NULL is returned if there is no free memory. Although this function
  *	allocates memory it can be called from an interrupt.
  */
 static inline struct sk_buff *dev_alloc_skb(unsigned int length)

@@ -1117,7 +1117,7 @@
  *	the headroom they think they need without accounting for the
  *	built in space. The built in space is used for optimisations.
  *
- *	%NULL is returned in there is no free memory. Although this function
+ *	%NULL is returned if there is no free memory. Although this function
  *	allocates memory it can be called from an interrupt.
  */
 static inline struct sk_buff *netdev_alloc_skb(struct net_device *dev,

net/core/skbuff.c +1 -1

Index: linux-2.6/net/core/skbuff.c
===================================================================
@@ -267,7 +267,7 @@
  *	the headroom they think they need without accounting for the
  *	built in space. The built in space is used for optimisations.
  *
- *	%NULL is returned in there is no free memory.
+ *	%NULL is returned if there is no free memory.
  */
 struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
 		unsigned int length, gfp_t gfp_mask)

David Miller

maintainer

2006-07-28 08:24 UTC #3 lore raw

From: Christoph Hellwig <[email protected]>
Date: Fri, 28 Jul 2006 09:08:51 +0200

Quoted 5 lines

Yes, Joe Perches already pointed that out in private. This typo is copy and pasted from the dev_alloc_skb kerneldoc, so this patch fixes up all four occurances:
Signed-off-by Christoph Hellwig <[email protected]>

Please respin your original patch with this fixup, thanks a lot Christoph.

Christoph Hellwig

author

2006-07-28 08:27 UTC #4 lore raw

Quoted 11 lines · David Miller

From: Christoph Hellwig <[email protected]>
Date: Fri, 28 Jul 2006 09:08:51 +0200
Quoted 5 lines
Yes, Joe Perches already pointed that out in private. This typo is copy and pasted from the dev_alloc_skb kerneldoc, so this patch fixes up all four occurances:
Signed-off-by Christoph Hellwig <[email protected]>
Please respin your original patch with this fixup, thanks a lot Christoph.

We need a fixup patch for the existing wrong comments on dev_alloc_skb anyway - but if you prefer to respin the netdev_alloc_skb and have another patch to fixup the original comments I can do that aswell.

David Miller

maintainer

2006-07-28 08:28 UTC #5 lore raw

From: Christoph Hellwig <[email protected]>
Date: Fri, 28 Jul 2006 10:27:12 +0200

Quoted 3 lines

We need a fixup patch for the existing wrong comments on dev_alloc_skb anyway - but if you prefer to respin the netdev_alloc_skb and have another patch to fixup the original comments I can do that aswell.

I intend to, for everyone's convenience, put the addition of netdev_alloc_skb() into 2.6.18.

Christoph Hellwig

author

2006-07-28 08:46 UTC #6 lore raw

Quoted 9 lines · David Miller

From: Christoph Hellwig <[email protected]>
Date: Fri, 28 Jul 2006 10:27:12 +0200
Quoted 3 lines
We need a fixup patch for the existing wrong comments on dev_alloc_skb anyway - but if you prefer to respin the netdev_alloc_skb and have another patch to fixup the original comments I can do that aswell.
I intend to, for everyone's convenience, put the addition of netdev_alloc_skb() into 2.6.18.

Here is the requested fixed up version:

include/linux/skbuff.h +22 -0

Index: linux-2.6/include/linux/skbuff.h
===================================================================
@@ -1104,6 +1104,28 @@
 	return __dev_alloc_skb(length, GFP_ATOMIC);
 }
 
+extern struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
+		unsigned int length, gfp_t gfp_mask);
+
+/**
+ *	netdev_alloc_skb - allocate an skbuff for rx on a specific device
+ *	@dev: network device to receive on
+ *	@length: length to allocate
+ *
+ *	Allocate a new &sk_buff and assign it a usage count of one. The
+ *	buffer has unspecified headroom built in. Users should allocate
+ *	the headroom they think they need without accounting for the
+ *	built in space. The built in space is used for optimisations.
+ *
+ *	%NULL is returned if there is no free memory. Although this function
+ *	allocates memory it can be called from an interrupt.
+ */
+static inline struct sk_buff *netdev_alloc_skb(struct net_device *dev,
+		unsigned int length)
+{
+	return __netdev_alloc_skb(dev, length, GFP_ATOMIC);
+}
+
 /**
  *	skb_cow - copy header of skb when it is required
  *	@skb: buffer to cow

net/core/skbuff.c +24 -0

Index: linux-2.6/net/core/skbuff.c
===================================================================
@@ -256,6 +256,29 @@
 	goto out;
 }
 
+/**
+ *	__netdev_alloc_skb - allocate an skbuff for rx on a specific device
+ *	@dev: network device to receive on
+ *	@length: length to allocate
+ *	@gfp_mask: get_free_pages mask, passed to alloc_skb
+ *
+ *	Allocate a new &sk_buff and assign it a usage count of one. The
+ *	buffer has unspecified headroom built in. Users should allocate
+ *	the headroom they think they need without accounting for the
+ *	built in space. The built in space is used for optimisations.
+ *
+ *	%NULL is returned if there is no free memory.
+ */
+struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
+		unsigned int length, gfp_t gfp_mask)
+{
+	struct sk_buff *skb;
+
+	skb = alloc_skb(length + NET_SKB_PAD, gfp_mask);
+	if (likely(skb))
+		skb_reserve(skb, NET_SKB_PAD);
+	return skb;
+}
 
 static void skb_drop_list(struct sk_buff **listp)
 {

@@ -2042,6 +2065,7 @@
 EXPORT_SYMBOL(kfree_skb);
 EXPORT_SYMBOL(__pskb_pull_tail);
 EXPORT_SYMBOL(__alloc_skb);
+EXPORT_SYMBOL(__netdev_alloc_skb);
 EXPORT_SYMBOL(pskb_copy);
 EXPORT_SYMBOL(pskb_expand_head);
 EXPORT_SYMBOL(skb_checksum);