[PATCH] ucc_geth: Add support for skb recycling

[PATCH] ucc_geth: Add support for skb recycling

[Anton Vorontsov]

We can reclaim transmitted skbs to use in the receive path, so-called
skb recycling support.

Also reorder ucc_geth_poll() steps, so that we'll clean tx ring firstly,
thus maybe reclaim some skbs for rx.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/net/ucc_geth.c |   40 ++++++++++++++++++++++++++++------------
 drivers/net/ucc_geth.h |    2 ++
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 40c6eba..beaa329 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -209,9 +209,10 @@ static struct sk_buff *get_new_skb(struct ucc_geth_private *ugeth,
 {
 	struct sk_buff *skb = NULL;
 
-	skb = dev_alloc_skb(ugeth->ug_info->uf_info.max_rx_buf_length +
-				  UCC_GETH_RX_DATA_BUF_ALIGNMENT);
-
+	skb = __skb_dequeue(&ugeth->rx_recycle);
+	if (!skb)
+		skb = dev_alloc_skb(ugeth->ug_info->uf_info.max_rx_buf_length +
+				    UCC_GETH_RX_DATA_BUF_ALIGNMENT);
 	if (skb == NULL)
 		return NULL;
 
@@ -1986,6 +1987,8 @@ static void ucc_geth_memclean(struct ucc_geth_private *ugeth)
 		iounmap(ugeth->ug_regs);
 		ugeth->ug_regs = NULL;
 	}
+
+	skb_queue_purge(&ugeth->rx_recycle);
 }
 
 static void ucc_geth_set_multi(struct net_device *dev)
@@ -2202,6 +2205,8 @@ static int ucc_struct_init(struct ucc_geth_private *ugeth)
 		return -ENOMEM;
 	}
 
+	skb_queue_head_init(&ugeth->rx_recycle);
+
 	return 0;
 }
 
@@ -3208,8 +3213,10 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
 			if (netif_msg_rx_err(ugeth))
 				ugeth_err("%s, %d: ERROR!!! skb - 0x%08x",
 					   __func__, __LINE__, (u32) skb);
-			if (skb)
-				dev_kfree_skb_any(skb);
+			if (skb) {
+				skb->data = skb->head + NET_SKB_PAD;
+				__skb_queue_head(&ugeth->rx_recycle, skb);
+			}
 
 			ugeth->rx_skbuff[rxQ][ugeth->skb_currx[rxQ]] = NULL;
 			dev->stats.rx_dropped++;
@@ -3267,6 +3274,8 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
 
 	/* Normal processing. */
 	while ((bd_status & T_R) == 0) {
+		struct sk_buff *skb;
+
 		/* BD contains already transmitted buffer.   */
 		/* Handle the transmitted buffer and release */
 		/* the BD to be used with the current frame  */
@@ -3276,9 +3285,16 @@ static int ucc_geth_tx(struct net_device *dev, u8 txQ)
 
 		dev->stats.tx_packets++;
 
-		/* Free the sk buffer associated with this TxBD */
-		dev_kfree_skb(ugeth->
-				  tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]]);
+		skb = ugeth->tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]];
+
+		if (skb_queue_len(&ugeth->rx_recycle) < RX_BD_RING_LEN &&
+			     skb_recycle_check(skb,
+				    ugeth->ug_info->uf_info.max_rx_buf_length +
+				    UCC_GETH_RX_DATA_BUF_ALIGNMENT))
+			__skb_queue_head(&ugeth->rx_recycle, skb);
+		else
+			dev_kfree_skb(skb);
+
 		ugeth->tx_skbuff[txQ][ugeth->skb_dirtytx[txQ]] = NULL;
 		ugeth->skb_dirtytx[txQ] =
 		    (ugeth->skb_dirtytx[txQ] +
@@ -3307,16 +3323,16 @@ static int ucc_geth_poll(struct napi_struct *napi, int budget)
 
 	ug_info = ugeth->ug_info;
 
-	howmany = 0;
-	for (i = 0; i < ug_info->numQueuesRx; i++)
-		howmany += ucc_geth_rx(ugeth, i, budget - howmany);
-
 	/* Tx event processing */
 	spin_lock(&ugeth->lock);
 	for (i = 0; i < ug_info->numQueuesTx; i++)
 		ucc_geth_tx(ugeth->ndev, i);
 	spin_unlock(&ugeth->lock);
 
+	howmany = 0;
+	for (i = 0; i < ug_info->numQueuesRx; i++)
+		howmany += ucc_geth_rx(ugeth, i, budget - howmany);
+
 	if (howmany < budget) {
 		napi_complete(napi);
 		setbits32(ugeth->uccf->p_uccm, UCCE_RX_EVENTS | UCCE_TX_EVENTS);
diff --git a/drivers/net/ucc_geth.h b/drivers/net/ucc_geth.h
index 195ab26..cfb31af 100644
--- a/drivers/net/ucc_geth.h
+++ b/drivers/net/ucc_geth.h
@@ -1212,6 +1212,8 @@ struct ucc_geth_private {
 	/* index of the first skb which hasn't been transmitted yet. */
 	u16 skb_dirtytx[NUM_TX_QUEUES];
 
+	struct sk_buff_head rx_recycle;
+
 	struct ugeth_mii_info *mii_info;
 	struct phy_device *phydev;
 	phy_interface_t phy_interface;
-- 
1.6.3.3

Re: [PATCH] ucc_geth: Add support for skb recycling

[Rick Jones]

Anton Vorontsov wrote:
> We can reclaim transmitted skbs to use in the receive path, so-called
> skb recycling support.
> 
> Also reorder ucc_geth_poll() steps, so that we'll clean tx ring firstly,
> thus maybe reclaim some skbs for rx.

Admittedly, all the world is not TCP, but a big chunk is, so are you likely to 
have reference counts go to zero on the tx queue for anything other than small 
standalone TCP ACK segments?

rick jones

Re: [PATCH] ucc_geth: Add support for skb recycling

[Anton Vorontsov]

On Tue, Jul 07, 2009 at 11:47:38AM -0700, Rick Jones wrote:
> Anton Vorontsov wrote:
> >We can reclaim transmitted skbs to use in the receive path, so-called
> >skb recycling support.
> >
> >Also reorder ucc_geth_poll() steps, so that we'll clean tx ring firstly,
> >thus maybe reclaim some skbs for rx.
> 
> Admittedly, all the world is not TCP, but a big chunk is, so are you
> likely to have reference counts go to zero on the tx queue for
> anything other than small standalone TCP ACK segments?

That's a generic question wrt skb recycling, right? Whether we can
always recycle transmitted skbs. No, sometimes (or mostly) we can't.

Initially, I was quite puzzled by this support... looking at how
gianfar driver works (it has the same support as of 0fd56bb5be6455d0),
I noticed that skb_recycle_check() always returns 0, and so we
don't recycle the skbs.

Though, things change when the kernel starts packets forwarding,
*then* skb recycling path actually triggers.

Lennert (skb recycling author) hints us that the gain is indeed
in forwarding/routing workload:

http://kerneltrap.org/mailarchive/linux-netdev/2008/9/28/3433514

Hope I understood everything correctly. :-)


Thanks!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH] ucc_geth: Add support for skb recycling

[Rick Jones]

Anton Vorontsov wrote:
> On Tue, Jul 07, 2009 at 11:47:38AM -0700, Rick Jones wrote:
>>Admittedly, all the world is not TCP, but a big chunk is, so are you
>>likely to have reference counts go to zero on the tx queue for
>>anything other than small standalone TCP ACK segments?
> 
> 
> That's a generic question wrt skb recycling, right? Whether we can
> always recycle transmitted skbs. No, sometimes (or mostly) we can't.
> 
> Initially, I was quite puzzled by this support... looking at how
> gianfar driver works (it has the same support as of 0fd56bb5be6455d0),
> I noticed that skb_recycle_check() always returns 0, and so we
> don't recycle the skbs.
> 
> Though, things change when the kernel starts packets forwarding,
> *then* skb recycling path actually triggers.
> 
> Lennert (skb recycling author) hints us that the gain is indeed
> in forwarding/routing workload:
> 
> http://kerneltrap.org/mailarchive/linux-netdev/2008/9/28/3433514
 >
> 
> Hope I understood everything correctly. :-)

Given the text reads:

  This gives a nice increase in the maximum loss-free packet forwarding
  rate in routing workloads.

Your understanding is probably correct.  Might have been "nice" :) to get a 
definition of a "nice increase" though :)

rick jones

Re: [PATCH] ucc_geth: Add support for skb recycling

[David Miller]

From: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Tue, 7 Jul 2009 22:38:42 +0400

> We can reclaim transmitted skbs to use in the receive path, so-called
> skb recycling support.
> 
> Also reorder ucc_geth_poll() steps, so that we'll clean tx ring firstly,
> thus maybe reclaim some skbs for rx.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Applied to net-next-2.6