All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance
@ 2014-06-04  7:39 Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function Fugang Duan
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

$ ethtool -K eth0 sg on
$ ethtool -K eth0 tso on
[  3] local 10.192.242.108 port 35388 connected with 10.192.242.167 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0- 3.0 sec   181 MBytes   506 Mbits/sec
* cpu loading is 30%

$ ethtool -K eth0 sg off
$ ethtool -K eth0 tso off
[  3] local 10.192.242.108 port 52618 connected with 10.192.242.167 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0- 3.0 sec  99.5 MBytes   278 Mbits/sec

FEC HW support IP header and TCP/UDP hw checksum, support multi buffer descriptor transfer
one frame, but don't support HW TSO. And imx6q/dl SOC FEC Gbps speed has HW bus Bandwidth
limitation (400Mbps ~ 700Mbps), imx6sx SOC FEC Gbps speed has no HW bandwidth limitation.

The patch set just enable TSO feature, which is done following the mv643xx_eth driver.

Test result analyze:
imx6dl sabresd board: there have 82% improvement, since imx6dl FEC HW has bandwidth limitation,
		      the performance with SW TSO is a milestone.

Addition test:
imx6sx sdb board:
upstream still don't support imx6sx due to some patches being upstream... they use same FEC IP.
Use the SW TSO patches test imx6sx sdb board in internal kernel tree:
No SW TSO patch: tx bandwidth 840Mbps, cpu loading is 100%.
SW TSO patch:    tx bandwidth 942Mbps, cpu loading is 65%.
It means the patch set have great improvement for imx6sx FEC performance.


V2:
* From Frank Li's suggestion:
	Change the API "fec_enet_txdesc_entry_free" name to "fec_enet_get_free_txdesc_num".
* Summary David Laight and Eric Dumazet's thoughts:
	RX BD entry number change to 256.
* From ezequiel's suggestion:
	Follow the latest TSO fixes from his solution to rework the queue stop/wake-up.
	Avoid unmapping the TSO header buffers.
* From Eric Dumazet's suggestion:
	Avoid more bytes copy, just copying the unaligned part of the payload into first
	descriptor. The suggestion will bring more complex for the driver, and imx6dl FEC
	DMA need 16 bytes alignment, but cpu loading is not problem that cpu loading is
	30%, the current performance is so better. Later chip like imx6sx Gigbit FEC DMA
	support byte alignment, so there don't exist memory copy. So, the V2 version drop
	the suggestion.
	Anyway, thanks for Eric's response and suggestion.

Fugang Duan (6):
  net: fec: Factorize the .xmit transmit function
  net: fec: Enable IP header hardware checksum
  net: fec: Factorize feature setting
  net: fec: Increase buffer descriptor entry number
  net: fec: Add Scatter/gather support
  net: fec: Add software TSO support

 drivers/net/ethernet/freescale/fec.h      |   14 +-
 drivers/net/ethernet/freescale/fec_main.c |  548 ++++++++++++++++++++++++-----
 2 files changed, 461 insertions(+), 101 deletions(-)

-- 
1.7.8

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
@ 2014-06-04  7:39 ` Fugang Duan
  2014-06-04  9:24   ` David Laight
  2014-06-04  7:39 ` [PATCH v2 2/6] net: fec: Enable IP header hardware checksum Fugang Duan
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

Make the code more readable and easy to support other features like
SG, TSO, moving the common transmit function to one api.

And the patch also factorize the getting BD index to it own function.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c |   87 ++++++++++++++++++-----------
 1 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 802be17..32c2276 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -287,6 +287,25 @@ struct bufdesc *fec_enet_get_prevdesc(struct bufdesc *bdp, struct fec_enet_priva
 		return (new_bd < base) ? (new_bd + ring_size) : new_bd;
 }
 
+static inline
+int fec_enet_get_bd_index(struct bufdesc *bdp, struct fec_enet_private *fep)
+{
+	struct bufdesc *base;
+	int index;
+
+	if (bdp >= fep->tx_bd_base)
+		base = fep->tx_bd_base;
+	else
+		base = fep->rx_bd_base;
+
+	if (fep->bufdesc_ex)
+		index = (struct bufdesc_ex *)bdp - (struct bufdesc_ex *)base;
+	else
+		index = bdp - base;
+
+	return index;
+}
+
 static void *swap_buffer(void *bufaddr, int len)
 {
 	int i;
@@ -313,8 +332,7 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
 	return 0;
 }
 
-static netdev_tx_t
-fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+static int txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	const struct platform_device_id *id_entry =
@@ -329,14 +347,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	status = bdp->cbd_sc;
 
-	if (status & BD_ENET_TX_READY) {
-		/* Ooops.  All transmit buffers are full.  Bail out.
-		 * This should not happen, since ndev->tbusy should be set.
-		 */
-		netdev_err(ndev, "tx queue full!\n");
-		return NETDEV_TX_BUSY;
-	}
-
 	/* Protocol checksum off-load for TCP and UDP. */
 	if (fec_enet_clear_csum(skb, ndev)) {
 		dev_kfree_skb_any(skb);
@@ -350,16 +360,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	bufaddr = skb->data;
 	bdp->cbd_datlen = skb->len;
 
-	/*
-	 * On some FEC implementations data must be aligned on
-	 * 4-byte boundaries. Use bounce buffers to copy data
-	 * and get it aligned. Ugh.
-	 */
-	if (fep->bufdesc_ex)
-		index = (struct bufdesc_ex *)bdp -
-			(struct bufdesc_ex *)fep->tx_bd_base;
-	else
-		index = bdp - fep->tx_bd_base;
+	index = fec_enet_get_bd_index(bdp, fep);
 
 	if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
 		memcpy(fep->tx_bounce[index], skb->data, skb->len);
@@ -433,15 +434,43 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	fep->cur_tx = bdp;
 
-	if (fep->cur_tx == fep->dirty_tx)
-		netif_stop_queue(ndev);
-
 	/* Trigger transmission start */
 	writel(0, fep->hwp + FEC_X_DES_ACTIVE);
 
 	return NETDEV_TX_OK;
 }
 
+static netdev_tx_t
+fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	struct bufdesc *bdp;
+	unsigned short	status;
+	int ret;
+
+	/* Fill in a Tx ring entry */
+	bdp = fep->cur_tx;
+
+	status = bdp->cbd_sc;
+
+	if (status & BD_ENET_TX_READY) {
+		/* Ooops.  All transmit buffers are full.  Bail out.
+		 * This should not happen, since ndev->tbusy should be set.
+		 */
+		netdev_err(ndev, "tx queue full!\n");
+		return NETDEV_TX_BUSY;
+	}
+
+	ret = txq_submit_skb(skb, ndev);
+	if (ret == -EBUSY)
+		return NETDEV_TX_BUSY;
+
+	if (fep->cur_tx == fep->dirty_tx)
+		netif_stop_queue(ndev);
+
+	return NETDEV_TX_OK;
+}
+
 /* Init RX & TX buffer descriptors
  */
 static void fec_enet_bd_init(struct net_device *dev)
@@ -770,11 +799,7 @@ fec_enet_tx(struct net_device *ndev)
 		if (bdp == fep->cur_tx)
 			break;
 
-		if (fep->bufdesc_ex)
-			index = (struct bufdesc_ex *)bdp -
-				(struct bufdesc_ex *)fep->tx_bd_base;
-		else
-			index = bdp - fep->tx_bd_base;
+		index = fec_enet_get_bd_index(bdp, fep);
 
 		skb = fep->tx_skbuff[index];
 		dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, skb->len,
@@ -921,11 +946,7 @@ fec_enet_rx(struct net_device *ndev, int budget)
 		pkt_len = bdp->cbd_datlen;
 		ndev->stats.rx_bytes += pkt_len;
 
-		if (fep->bufdesc_ex)
-			index = (struct bufdesc_ex *)bdp -
-				(struct bufdesc_ex *)fep->rx_bd_base;
-		else
-			index = bdp - fep->rx_bd_base;
+		index = fec_enet_get_bd_index(bdp, fep);
 		data = fep->rx_skbuff[index]->data;
 		dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
 					FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/6] net: fec: Enable IP header hardware checksum
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function Fugang Duan
@ 2014-06-04  7:39 ` Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 3/6] net: fec: Factorize feature setting Fugang Duan
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

IP header checksum is calcalated by network layer in default.
To support software TSO, it is better to use HW calculate the
IP header checksum.

FEC hw checksum feature request the checksum field in frame
is zero, otherwise the calculative CRC is not correct.

For segmentated TCP packet, HW calculate the IP header checksum again,
it doesn't bring any impact. For SW TSO, HW calculated checksum bring
better performance.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 32c2276..299f7f5 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -327,6 +327,7 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
 	if (unlikely(skb_cow_head(skb, 0)))
 		return -1;
 
+	ip_hdr(skb)->check = 0;
 	*(__sum16 *)(skb->head + skb->csum_start + skb->csum_offset) = 0;
 
 	return 0;
@@ -408,7 +409,7 @@ static int txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 			 * are done by the kernel
 			 */
 			if (skb->ip_summed == CHECKSUM_PARTIAL)
-				ebdp->cbd_esc |= BD_ENET_TX_PINS;
+				ebdp->cbd_esc |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
 		}
 	}
 
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/6] net: fec: Factorize feature setting
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 2/6] net: fec: Enable IP header hardware checksum Fugang Duan
@ 2014-06-04  7:39 ` Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number Fugang Duan
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

In order to enhance the code readable, let's factorize the
feature list.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 299f7f5..cf3c368 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2097,21 +2097,19 @@ static int fec_enet_init(struct net_device *ndev)
 	writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK);
 	netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT);
 
-	if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN) {
+	if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN)
 		/* enable hw VLAN support */
 		ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
-		ndev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX;
-	}
 
 	if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) {
 		/* enable hw accelerator */
 		ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM
 				| NETIF_F_RXCSUM);
-		ndev->hw_features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM
-				| NETIF_F_RXCSUM);
 		fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
 	}
 
+	ndev->hw_features = ndev->features;
+
 	fec_restart(ndev, 0);
 
 	return 0;
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
                   ` (2 preceding siblings ...)
  2014-06-04  7:39 ` [PATCH v2 3/6] net: fec: Factorize feature setting Fugang Duan
@ 2014-06-04  7:39 ` Fugang Duan
  2014-06-04  9:26   ` David Laight
  2014-06-04  7:39 ` [PATCH v2 5/6] net: fec: Add Scatter/gather support Fugang Duan
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

In order to support SG, software TSO, let's increase BD entry number.

CC: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
CC: Eric Dumazet <eric.dumazet@gmail.com>
CC: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |    6 +++---
 drivers/net/ethernet/freescale/fec_main.c |   21 ++++++++++++---------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 3b8d6d1..798ad88 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -240,14 +240,14 @@ struct bufdesc_ex {
  * the skbuffer directly.
  */
 
-#define FEC_ENET_RX_PAGES	8
+#define FEC_ENET_RX_PAGES	128
 #define FEC_ENET_RX_FRSIZE	2048
 #define FEC_ENET_RX_FRPPG	(PAGE_SIZE / FEC_ENET_RX_FRSIZE)
 #define RX_RING_SIZE		(FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES)
 #define FEC_ENET_TX_FRSIZE	2048
 #define FEC_ENET_TX_FRPPG	(PAGE_SIZE / FEC_ENET_TX_FRSIZE)
-#define TX_RING_SIZE		16	/* Must be power of two */
-#define TX_RING_MOD_MASK	15	/*   for this to work */
+#define TX_RING_SIZE		512	/* Must be power of two */
+#define TX_RING_MOD_MASK	511	/*   for this to work */
 
 #define BD_ENET_RX_INT          0x00800000
 #define BD_ENET_RX_PTP          ((ushort)0x0400)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index cf3c368..9de1660 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -173,10 +173,6 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #endif
 #endif /* CONFIG_M5272 */
 
-#if (((RX_RING_SIZE + TX_RING_SIZE) * 32) > PAGE_SIZE)
-#error "FEC: descriptor ring size constants too large"
-#endif
-
 /* Interrupt events/masks. */
 #define FEC_ENET_HBERR	((uint)0x80000000)	/* Heartbeat error */
 #define FEC_ENET_BABR	((uint)0x40000000)	/* Babbling receiver */
@@ -2061,9 +2057,20 @@ static int fec_enet_init(struct net_device *ndev)
 	const struct platform_device_id *id_entry =
 				platform_get_device_id(fep->pdev);
 	struct bufdesc *cbd_base;
+	int bd_size;
+
+	/* init the tx & rx ring size */
+	fep->tx_ring_size = TX_RING_SIZE;
+	fep->rx_ring_size = RX_RING_SIZE;
+
+	if (fep->bufdesc_ex)
+		bd_size = sizeof(struct bufdesc_ex);
+	else
+		bd_size = sizeof(struct bufdesc);
+	bd_size *= (fep->tx_ring_size + fep->rx_ring_size);
 
 	/* Allocate memory for buffer descriptors. */
-	cbd_base = dma_alloc_coherent(NULL, PAGE_SIZE, &fep->bd_dma,
+	cbd_base = dma_alloc_coherent(NULL, bd_size, &fep->bd_dma,
 				      GFP_KERNEL);
 	if (!cbd_base)
 		return -ENOMEM;
@@ -2077,10 +2084,6 @@ static int fec_enet_init(struct net_device *ndev)
 	/* make sure MAC we just acquired is programmed into the hw */
 	fec_set_mac_address(ndev, NULL);
 
-	/* init the tx & rx ring size */
-	fep->tx_ring_size = TX_RING_SIZE;
-	fep->rx_ring_size = RX_RING_SIZE;
-
 	/* Set receive and transmit descriptor base. */
 	fep->rx_bd_base = cbd_base;
 	if (fep->bufdesc_ex)
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/6] net: fec: Add Scatter/gather support
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
                   ` (3 preceding siblings ...)
  2014-06-04  7:39 ` [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number Fugang Duan
@ 2014-06-04  7:39 ` Fugang Duan
  2014-06-04  7:39 ` [PATCH v2 6/6] net: fec: Add software TSO support Fugang Duan
  2014-06-04 22:28 ` [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance David Miller
  6 siblings, 0 replies; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

Add Scatter/gather support for FEC.
This feature allows to improve outbound throughput performance.

Tested on imx6dl sabresd board:
Running iperf tests shows a 55.4% improvement.

$ ethtool -K eth0 sg off
$ iperf -c 10.192.242.167 -t 3 &
[  3] local 10.192.242.108 port 52618 connected with 10.192.242.167 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0- 3.0 sec  99.5 MBytes   278 Mbits/sec

$ ethtool -K eth0 sg on
$ iperf -c 10.192.242.167 -t 3 &
[  3] local 10.192.242.108 port 52617 connected with 10.192.242.167 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0- 3.0 sec   154 MBytes   432 Mbits/sec

CC: Li Frank <B20596@freescale.com>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |    2 +-
 drivers/net/ethernet/freescale/fec_main.c |  245 ++++++++++++++++++++++-------
 2 files changed, 185 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 798ad88..f321ff1 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -221,7 +221,7 @@ struct bufdesc_ex {
 #define BD_ENET_TX_RCMASK       ((ushort)0x003c)
 #define BD_ENET_TX_UN           ((ushort)0x0002)
 #define BD_ENET_TX_CSL          ((ushort)0x0001)
-#define BD_ENET_TX_STATS        ((ushort)0x03ff)        /* All status bits */
+#define BD_ENET_TX_STATS        ((ushort)0x0fff)        /* All status bits */
 
 /*enhanced buffer descriptor control/status used by Ethernet transmit*/
 #define BD_ENET_TX_INT          0x40000000
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9de1660..0ce8de6 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -302,6 +302,23 @@ int fec_enet_get_bd_index(struct bufdesc *bdp, struct fec_enet_private *fep)
 	return index;
 }
 
+static inline
+int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep)
+{
+	int entries;
+
+	if (fep->bufdesc_ex)
+		entries = (struct bufdesc_ex *)fep->dirty_tx -
+				(struct bufdesc_ex *)fep->cur_tx;
+	else
+		entries = fep->dirty_tx - fep->cur_tx;
+
+	if (fep->cur_tx >= fep->dirty_tx)
+		entries += fep->tx_ring_size;
+
+	return entries;
+}
+
 static void *swap_buffer(void *bufaddr, int len)
 {
 	int i;
@@ -329,20 +346,119 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
 	return 0;
 }
 
-static int txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
+static void
+fec_enet_submit_work(struct bufdesc *bdp, struct fec_enet_private *fep)
+{
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+	struct bufdesc *bdp_pre;
+
+	bdp_pre = fec_enet_get_prevdesc(bdp, fep);
+	if ((id_entry->driver_data & FEC_QUIRK_ERR006358) &&
+	    !(bdp_pre->cbd_sc & BD_ENET_TX_READY)) {
+		fep->delay_work.trig_tx = true;
+		schedule_delayed_work(&(fep->delay_work.delay_work),
+					msecs_to_jiffies(1));
+	}
+}
+
+static int
+fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	const struct platform_device_id *id_entry =
 				platform_get_device_id(fep->pdev);
-	struct bufdesc *bdp, *bdp_pre;
-	void *bufaddr;
-	unsigned short	status;
+	struct bufdesc *bdp = fep->cur_tx;
+	struct bufdesc_ex *ebdp;
+	int nr_frags = skb_shinfo(skb)->nr_frags;
+	int frag, frag_len;
+	unsigned short status;
+	unsigned int estatus = 0;
+	skb_frag_t *this_frag;
 	unsigned int index;
+	void *bufaddr;
+	int i;
 
-	/* Fill in a Tx ring entry */
+	for (frag = 0; frag < nr_frags; frag++) {
+		this_frag = &skb_shinfo(skb)->frags[frag];
+		bdp = fec_enet_get_nextdesc(bdp, fep);
+		ebdp = (struct bufdesc_ex *)bdp;
+
+		status = bdp->cbd_sc;
+		status &= ~BD_ENET_TX_STATS;
+		status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
+		frag_len = skb_shinfo(skb)->frags[frag].size;
+
+		/* Handle the last BD specially */
+		if (frag == nr_frags - 1) {
+			status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST);
+			if (fep->bufdesc_ex) {
+				estatus |= BD_ENET_TX_INT;
+				if (unlikely(skb_shinfo(skb)->tx_flags &
+					SKBTX_HW_TSTAMP && fep->hwts_tx_en))
+					estatus |= BD_ENET_TX_TS;
+			}
+		}
+
+		if (fep->bufdesc_ex) {
+			if (skb->ip_summed == CHECKSUM_PARTIAL)
+				estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
+			ebdp->cbd_bdu = 0;
+			ebdp->cbd_esc = estatus;
+		}
+
+		bufaddr = page_address(this_frag->page.p) + this_frag->page_offset;
+
+		index = fec_enet_get_bd_index(bdp, fep);
+		if (((unsigned long) bufaddr) & FEC_ALIGNMENT ||
+			id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+			memcpy(fep->tx_bounce[index], bufaddr, frag_len);
+			bufaddr = fep->tx_bounce[index];
+
+			if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+				swap_buffer(bufaddr, frag_len);
+		}
+
+		bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
+						frag_len, DMA_TO_DEVICE);
+		if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+			dev_kfree_skb_any(skb);
+			if (net_ratelimit())
+				netdev_err(ndev, "Tx DMA memory map failed\n");
+			goto dma_mapping_error;
+		}
+
+		bdp->cbd_datlen = frag_len;
+		bdp->cbd_sc = status;
+	}
+
+	fep->cur_tx = bdp;
+
+	return 0;
+
+dma_mapping_error:
 	bdp = fep->cur_tx;
+	for (i = 0; i < frag; i++) {
+		bdp = fec_enet_get_nextdesc(bdp, fep);
+		dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
+				bdp->cbd_datlen, DMA_TO_DEVICE);
+	}
+	return NETDEV_TX_OK;
+}
 
-	status = bdp->cbd_sc;
+static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+	int nr_frags = skb_shinfo(skb)->nr_frags;
+	struct bufdesc *bdp, *last_bdp;
+	void *bufaddr;
+	unsigned short status;
+	unsigned short buflen;
+	unsigned int estatus = 0;
+	unsigned int index;
+	int ret;
 
 	/* Protocol checksum off-load for TCP and UDP. */
 	if (fec_enet_clear_csum(skb, ndev)) {
@@ -350,82 +466,83 @@ static int txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 		return NETDEV_TX_OK;
 	}
 
-	/* Clear all of the status flags */
+	/* Fill in a Tx ring entry */
+	bdp = fep->cur_tx;
+	status = bdp->cbd_sc;
 	status &= ~BD_ENET_TX_STATS;
 
 	/* Set buffer length and buffer pointer */
 	bufaddr = skb->data;
-	bdp->cbd_datlen = skb->len;
+	buflen = skb_headlen(skb);
 
 	index = fec_enet_get_bd_index(bdp, fep);
-
-	if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
-		memcpy(fep->tx_bounce[index], skb->data, skb->len);
+	if (((unsigned long) bufaddr) & FEC_ALIGNMENT ||
+		id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+		memcpy(fep->tx_bounce[index], skb->data, buflen);
 		bufaddr = fep->tx_bounce[index];
-	}
-
-	/*
-	 * Some design made an incorrect assumption on endian mode of
-	 * the system that it's running on. As the result, driver has to
-	 * swap every frame going to and coming from the controller.
-	 */
-	if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
-		swap_buffer(bufaddr, skb->len);
 
-	/* Save skb pointer */
-	fep->tx_skbuff[index] = skb;
+		if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+			swap_buffer(bufaddr, buflen);
+	}
 
 	/* Push the data cache so the CPM does not get stale memory
 	 * data.
 	 */
 	bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
-			skb->len, DMA_TO_DEVICE);
+					buflen, DMA_TO_DEVICE);
 	if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
-		bdp->cbd_bufaddr = 0;
-		fep->tx_skbuff[index] = NULL;
 		dev_kfree_skb_any(skb);
 		if (net_ratelimit())
 			netdev_err(ndev, "Tx DMA memory map failed\n");
 		return NETDEV_TX_OK;
 	}
 
+	if (nr_frags) {
+		ret = fec_enet_txq_submit_frag_skb(skb, ndev);
+		if (ret)
+			return ret;
+	} else {
+		status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST);
+		if (fep->bufdesc_ex) {
+			estatus = BD_ENET_TX_INT;
+			if (unlikely(skb_shinfo(skb)->tx_flags &
+				SKBTX_HW_TSTAMP && fep->hwts_tx_en))
+				estatus |= BD_ENET_TX_TS;
+		}
+	}
+
 	if (fep->bufdesc_ex) {
 
 		struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
-		ebdp->cbd_bdu = 0;
+
 		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
-			fep->hwts_tx_en)) {
-			ebdp->cbd_esc = (BD_ENET_TX_TS | BD_ENET_TX_INT);
+			fep->hwts_tx_en))
 			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
-		} else {
-			ebdp->cbd_esc = BD_ENET_TX_INT;
 
-			/* Enable protocol checksum flags
-			 * We do not bother with the IP Checksum bits as they
-			 * are done by the kernel
-			 */
-			if (skb->ip_summed == CHECKSUM_PARTIAL)
-				ebdp->cbd_esc |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
-		}
+		if (skb->ip_summed == CHECKSUM_PARTIAL)
+			estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
+
+		ebdp->cbd_bdu = 0;
+		ebdp->cbd_esc = estatus;
 	}
 
+	last_bdp = fep->cur_tx;
+	index = fec_enet_get_bd_index(last_bdp, fep);
+	/* Save skb pointer */
+	fep->tx_skbuff[index] = skb;
+
+	bdp->cbd_datlen = buflen;
+
 	/* Send it on its way.  Tell FEC it's ready, interrupt when done,
 	 * it's the last BD of the frame, and to put the CRC on the end.
 	 */
-	status |= (BD_ENET_TX_READY | BD_ENET_TX_INTR
-			| BD_ENET_TX_LAST | BD_ENET_TX_TC);
+	status |= (BD_ENET_TX_READY | BD_ENET_TX_TC);
 	bdp->cbd_sc = status;
 
-	bdp_pre = fec_enet_get_prevdesc(bdp, fep);
-	if ((id_entry->driver_data & FEC_QUIRK_ERR006358) &&
-	    !(bdp_pre->cbd_sc & BD_ENET_TX_READY)) {
-		fep->delay_work.trig_tx = true;
-		schedule_delayed_work(&(fep->delay_work.delay_work),
-					msecs_to_jiffies(1));
-	}
+	fec_enet_submit_work(bdp, fep);
 
 	/* If this was the last BD in the ring, start at the beginning again. */
-	bdp = fec_enet_get_nextdesc(bdp, fep);
+	bdp = fec_enet_get_nextdesc(last_bdp, fep);
 
 	skb_tx_timestamp(skb);
 
@@ -434,7 +551,7 @@ static int txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 	/* Trigger transmission start */
 	writel(0, fep->hwp + FEC_X_DES_ACTIVE);
 
-	return NETDEV_TX_OK;
+	return 0;
 }
 
 static netdev_tx_t
@@ -443,6 +560,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	struct bufdesc *bdp;
 	unsigned short	status;
+	int entries_free;
 	int ret;
 
 	/* Fill in a Tx ring entry */
@@ -454,15 +572,17 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		/* Ooops.  All transmit buffers are full.  Bail out.
 		 * This should not happen, since ndev->tbusy should be set.
 		 */
-		netdev_err(ndev, "tx queue full!\n");
+		if (net_ratelimit())
+			netdev_err(ndev, "tx queue full!\n");
 		return NETDEV_TX_BUSY;
 	}
 
-	ret = txq_submit_skb(skb, ndev);
-	if (ret == -EBUSY)
-		return NETDEV_TX_BUSY;
+	ret = fec_enet_txq_submit_skb(skb, ndev);
+	if (ret)
+		return ret;
 
-	if (fep->cur_tx == fep->dirty_tx)
+	entries_free = fec_enet_get_free_txdesc_num(fep);
+	if (entries_free < MAX_SKB_FRAGS + 1)
 		netif_stop_queue(ndev);
 
 	return NETDEV_TX_OK;
@@ -783,6 +903,7 @@ fec_enet_tx(struct net_device *ndev)
 	unsigned short status;
 	struct	sk_buff	*skb;
 	int	index = 0;
+	int	entries;
 
 	fep = netdev_priv(ndev);
 	bdp = fep->dirty_tx;
@@ -799,9 +920,13 @@ fec_enet_tx(struct net_device *ndev)
 		index = fec_enet_get_bd_index(bdp, fep);
 
 		skb = fep->tx_skbuff[index];
-		dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, skb->len,
+		dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, bdp->cbd_datlen,
 				DMA_TO_DEVICE);
 		bdp->cbd_bufaddr = 0;
+		if (!skb) {
+			bdp = fec_enet_get_nextdesc(bdp, fep);
+			continue;
+		}
 
 		/* Check for errors. */
 		if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
@@ -820,7 +945,7 @@ fec_enet_tx(struct net_device *ndev)
 				ndev->stats.tx_carrier_errors++;
 		} else {
 			ndev->stats.tx_packets++;
-			ndev->stats.tx_bytes += bdp->cbd_datlen;
+			ndev->stats.tx_bytes += skb->len;
 		}
 
 		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
@@ -857,15 +982,13 @@ fec_enet_tx(struct net_device *ndev)
 
 		/* Since we have freed up a buffer, the ring is no longer full
 		 */
-		if (fep->dirty_tx != fep->cur_tx) {
-			if (netif_queue_stopped(ndev))
-				netif_wake_queue(ndev);
-		}
+		entries = fec_enet_get_free_txdesc_num(fep);
+		if (entries >= MAX_SKB_FRAGS + 1 && netif_queue_stopped(ndev))
+			netif_wake_queue(ndev);
 	}
 	return;
 }
 
-
 /* During a receive, the cur_rx points to the current incoming buffer.
  * When we update through the ring, if the next incoming buffer has
  * not been given to the system, we just set the empty indicator,
@@ -2107,7 +2230,7 @@ static int fec_enet_init(struct net_device *ndev)
 	if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) {
 		/* enable hw accelerator */
 		ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM
-				| NETIF_F_RXCSUM);
+				| NETIF_F_RXCSUM | NETIF_F_SG);
 		fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
 	}
 
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/6] net: fec: Add software TSO support
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
                   ` (4 preceding siblings ...)
  2014-06-04  7:39 ` [PATCH v2 5/6] net: fec: Add Scatter/gather support Fugang Duan
@ 2014-06-04  7:39 ` Fugang Duan
  2014-06-04 22:28 ` [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance David Miller
  6 siblings, 0 replies; 13+ messages in thread
From: Fugang Duan @ 2014-06-04  7:39 UTC (permalink / raw)
  To: davem
  Cc: netdev, shawn.guo, b38611, R49496, ezequiel.garcia, bhutchings,
	stephen, b20596, David.Laight, eric.dumazet

Add software TSO support for FEC.
This feature allows to improve outbound throughput performance.

Tested on imx6dl sabresd board, running iperf tcp tests shows:
- 16.2% improvement comparing with FEC SG patch
- 82% improvement comparing with NO SG & TSO patch

$ ethtool -K eth0 tso on
$ iperf -c 10.192.242.167 -t 3 &
[  3] local 10.192.242.108 port 35388 connected with 10.192.242.167 port 5001
[ ID] Interval       Transfer     Bandwidth
[  3]  0.0- 3.0 sec   181 MBytes   506 Mbits/sec

During the testing, CPU loading is 30%.
Since imx6dl FEC Bandwidth is limited to SOC system bus bandwidth, the
performance with SW TSO is a milestone.

CC: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
CC: Eric Dumazet <eric.dumazet@gmail.com>
CC: David Laight <David.Laight@ACULAB.COM>
CC: Li Frank <B20596@freescale.com>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |    6 +
 drivers/net/ethernet/freescale/fec_main.c |  254 ++++++++++++++++++++++++++---
 2 files changed, 237 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index f321ff1..e8d4813 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -298,6 +298,12 @@ struct fec_enet_private {
 
 	unsigned short tx_ring_size;
 	unsigned short rx_ring_size;
+	unsigned short tx_stop_threshold;
+	unsigned short tx_wake_threshold;
+
+	/* Software TSO */
+	char *tso_hdrs;
+	dma_addr_t tso_hdrs_dma;
 
 	struct	platform_device *pdev;
 
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 0ce8de6..fba1adf 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -36,6 +36,7 @@
 #include <linux/in.h>
 #include <linux/ip.h>
 #include <net/ip.h>
+#include <net/tso.h>
 #include <linux/tcp.h>
 #include <linux/udp.h>
 #include <linux/icmp.h>
@@ -228,6 +229,15 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #define FEC_PAUSE_FLAG_AUTONEG	0x1
 #define FEC_PAUSE_FLAG_ENABLE	0x2
 
+#define TSO_HEADER_SIZE		128
+/* Max number of allowed TCP segments for software TSO */
+#define FEC_MAX_TSO_SEGS	100
+#define FEC_MAX_SKB_DESCS	(FEC_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS)
+
+#define IS_TSO_HEADER(txq, addr) \
+	((addr >= txq->tso_hdrs_dma) && \
+	(addr < txq->tso_hdrs_dma + txq->tx_ring_size * TSO_HEADER_SIZE))
+
 static int mii_cnt;
 
 static inline
@@ -458,8 +468,17 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 	unsigned short buflen;
 	unsigned int estatus = 0;
 	unsigned int index;
+	int entries_free;
 	int ret;
 
+	entries_free = fec_enet_get_free_txdesc_num(fep);
+	if (entries_free < MAX_SKB_FRAGS + 1) {
+		dev_kfree_skb_any(skb);
+		if (net_ratelimit())
+			netdev_err(ndev, "NOT enough BD for SG!\n");
+		return NETDEV_TX_OK;
+	}
+
 	/* Protocol checksum off-load for TCP and UDP. */
 	if (fec_enet_clear_csum(skb, ndev)) {
 		dev_kfree_skb_any(skb);
@@ -554,35 +573,209 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
 	return 0;
 }
 
-static netdev_tx_t
-fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+static int
+fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev,
+			struct bufdesc *bdp, int index, char *data,
+			int size, bool last_tcp, bool is_last)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
-	struct bufdesc *bdp;
-	unsigned short	status;
-	int entries_free;
-	int ret;
-
-	/* Fill in a Tx ring entry */
-	bdp = fep->cur_tx;
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+	struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
+	unsigned short status;
+	unsigned int estatus = 0;
 
 	status = bdp->cbd_sc;
+	status &= ~BD_ENET_TX_STATS;
 
-	if (status & BD_ENET_TX_READY) {
-		/* Ooops.  All transmit buffers are full.  Bail out.
-		 * This should not happen, since ndev->tbusy should be set.
-		 */
+	status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
+	bdp->cbd_datlen = size;
+
+	if (((unsigned long) data) & FEC_ALIGNMENT ||
+		id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+		memcpy(fep->tx_bounce[index], data, size);
+		data = fep->tx_bounce[index];
+
+		if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+			swap_buffer(data, size);
+	}
+
+	bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, data,
+					size, DMA_TO_DEVICE);
+	if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+		dev_kfree_skb_any(skb);
 		if (net_ratelimit())
-			netdev_err(ndev, "tx queue full!\n");
+			netdev_err(ndev, "Tx DMA memory map failed\n");
 		return NETDEV_TX_BUSY;
 	}
 
-	ret = fec_enet_txq_submit_skb(skb, ndev);
+	if (fep->bufdesc_ex) {
+		if (skb->ip_summed == CHECKSUM_PARTIAL)
+			estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
+		ebdp->cbd_bdu = 0;
+		ebdp->cbd_esc = estatus;
+	}
+
+	/* Handle the last BD specially */
+	if (last_tcp)
+		status |= (BD_ENET_TX_LAST | BD_ENET_TX_TC);
+	if (is_last) {
+		status |= BD_ENET_TX_INTR;
+		if (fep->bufdesc_ex)
+			ebdp->cbd_esc |= BD_ENET_TX_INT;
+	}
+
+	bdp->cbd_sc = status;
+
+	return 0;
+}
+
+static int
+fec_enet_txq_put_hdr_tso(struct sk_buff *skb, struct net_device *ndev,
+			struct bufdesc *bdp, int index)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+	int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
+	struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
+	void *bufaddr;
+	unsigned long dmabuf;
+	unsigned short status;
+	unsigned int estatus = 0;
+
+	status = bdp->cbd_sc;
+	status &= ~BD_ENET_TX_STATS;
+	status |= (BD_ENET_TX_TC | BD_ENET_TX_READY);
+
+	bufaddr = fep->tso_hdrs + index * TSO_HEADER_SIZE;
+	dmabuf = fep->tso_hdrs_dma + index * TSO_HEADER_SIZE;
+	if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) {
+		memcpy(fep->tx_bounce[index], skb->data, hdr_len);
+		bufaddr = fep->tx_bounce[index];
+
+		if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
+			swap_buffer(bufaddr, hdr_len);
+
+		dmabuf = dma_map_single(&fep->pdev->dev, bufaddr,
+					hdr_len, DMA_TO_DEVICE);
+		if (dma_mapping_error(&fep->pdev->dev, dmabuf)) {
+			dev_kfree_skb_any(skb);
+			if (net_ratelimit())
+				netdev_err(ndev, "Tx DMA memory map failed\n");
+			return NETDEV_TX_BUSY;
+		}
+	}
+
+	bdp->cbd_bufaddr = dmabuf;
+	bdp->cbd_datlen = hdr_len;
+
+	if (fep->bufdesc_ex) {
+		if (skb->ip_summed == CHECKSUM_PARTIAL)
+			estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
+		ebdp->cbd_bdu = 0;
+		ebdp->cbd_esc = estatus;
+	}
+
+	bdp->cbd_sc = status;
+
+	return 0;
+}
+
+static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
+	int total_len, data_left;
+	struct bufdesc *bdp = fep->cur_tx;
+	struct tso_t tso;
+	unsigned int index = 0;
+	int ret;
+
+	if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep)) {
+		dev_kfree_skb_any(skb);
+		if (net_ratelimit())
+			netdev_err(ndev, "NOT enough BD for TSO!\n");
+		return NETDEV_TX_OK;
+	}
+
+	/* Protocol checksum off-load for TCP and UDP. */
+	if (fec_enet_clear_csum(skb, ndev)) {
+		dev_kfree_skb_any(skb);
+		return NETDEV_TX_OK;
+	}
+
+	/* Initialize the TSO handler, and prepare the first payload */
+	tso_start(skb, &tso);
+
+	total_len = skb->len - hdr_len;
+	while (total_len > 0) {
+		char *hdr;
+
+		index = fec_enet_get_bd_index(bdp, fep);
+		data_left = min_t(int, skb_shinfo(skb)->gso_size, total_len);
+		total_len -= data_left;
+
+		/* prepare packet headers: MAC + IP + TCP */
+		hdr = fep->tso_hdrs + index * TSO_HEADER_SIZE;
+		tso_build_hdr(skb, hdr, &tso, data_left, total_len == 0);
+		ret = fec_enet_txq_put_hdr_tso(skb, ndev, bdp, index);
+		if (ret)
+			goto err_release;
+
+		while (data_left > 0) {
+			int size;
+
+			size = min_t(int, tso.size, data_left);
+			bdp = fec_enet_get_nextdesc(bdp, fep);
+			index = fec_enet_get_bd_index(bdp, fep);
+			ret = fec_enet_txq_put_data_tso(skb, ndev, bdp, index, tso.data,
+							size, size == data_left,
+							total_len == 0);
+			if (ret)
+				goto err_release;
+
+			data_left -= size;
+			tso_build_data(skb, &tso, size);
+		}
+
+		bdp = fec_enet_get_nextdesc(bdp, fep);
+	}
+
+	/* Save skb pointer */
+	fep->tx_skbuff[index] = skb;
+
+	fec_enet_submit_work(bdp, fep);
+
+	skb_tx_timestamp(skb);
+	fep->cur_tx = bdp;
+
+	/* Trigger transmission start */
+	writel(0, fep->hwp + FEC_X_DES_ACTIVE);
+
+	return 0;
+
+err_release:
+	/* TODO: Release all used data descriptors for TSO */
+	return ret;
+}
+
+static netdev_tx_t
+fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	int entries_free;
+	int ret;
+
+	if (skb_is_gso(skb))
+		ret = fec_enet_txq_submit_tso(skb, ndev);
+	else
+		ret = fec_enet_txq_submit_skb(skb, ndev);
 	if (ret)
 		return ret;
 
 	entries_free = fec_enet_get_free_txdesc_num(fep);
-	if (entries_free < MAX_SKB_FRAGS + 1)
+	if (entries_free <= fep->tx_stop_threshold)
 		netif_stop_queue(ndev);
 
 	return NETDEV_TX_OK;
@@ -903,7 +1096,7 @@ fec_enet_tx(struct net_device *ndev)
 	unsigned short status;
 	struct	sk_buff	*skb;
 	int	index = 0;
-	int	entries;
+	int	entries_free;
 
 	fep = netdev_priv(ndev);
 	bdp = fep->dirty_tx;
@@ -920,8 +1113,9 @@ fec_enet_tx(struct net_device *ndev)
 		index = fec_enet_get_bd_index(bdp, fep);
 
 		skb = fep->tx_skbuff[index];
-		dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, bdp->cbd_datlen,
-				DMA_TO_DEVICE);
+		if (!IS_TSO_HEADER(fep, bdp->cbd_bufaddr))
+			dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
+					bdp->cbd_datlen, DMA_TO_DEVICE);
 		bdp->cbd_bufaddr = 0;
 		if (!skb) {
 			bdp = fec_enet_get_nextdesc(bdp, fep);
@@ -982,9 +1176,11 @@ fec_enet_tx(struct net_device *ndev)
 
 		/* Since we have freed up a buffer, the ring is no longer full
 		 */
-		entries = fec_enet_get_free_txdesc_num(fep);
-		if (entries >= MAX_SKB_FRAGS + 1 && netif_queue_stopped(ndev))
-			netif_wake_queue(ndev);
+		if (netif_queue_stopped(ndev)) {
+			entries_free = fec_enet_get_free_txdesc_num(fep);
+			if (entries_free >= fep->tx_wake_threshold)
+				netif_wake_queue(ndev);
+		}
 	}
 	return;
 }
@@ -2186,6 +2382,9 @@ static int fec_enet_init(struct net_device *ndev)
 	fep->tx_ring_size = TX_RING_SIZE;
 	fep->rx_ring_size = RX_RING_SIZE;
 
+	fep->tx_stop_threshold = FEC_MAX_SKB_DESCS;
+	fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2;
+
 	if (fep->bufdesc_ex)
 		bd_size = sizeof(struct bufdesc_ex);
 	else
@@ -2198,6 +2397,13 @@ static int fec_enet_init(struct net_device *ndev)
 	if (!cbd_base)
 		return -ENOMEM;
 
+	fep->tso_hdrs = dma_alloc_coherent(NULL, fep->tx_ring_size * TSO_HEADER_SIZE,
+						&fep->tso_hdrs_dma, GFP_KERNEL);
+	if (!fep->tso_hdrs) {
+		dma_free_coherent(NULL, bd_size, cbd_base, fep->bd_dma);
+		return -ENOMEM;
+	}
+
 	memset(cbd_base, 0, PAGE_SIZE);
 
 	fep->netdev = ndev;
@@ -2228,9 +2434,11 @@ static int fec_enet_init(struct net_device *ndev)
 		ndev->features |= NETIF_F_HW_VLAN_CTAG_RX;
 
 	if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) {
+		ndev->gso_max_segs = FEC_MAX_TSO_SEGS;
+
 		/* enable hw accelerator */
 		ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM
-				| NETIF_F_RXCSUM | NETIF_F_SG);
+				| NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_TSO);
 		fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
 	}
 
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function
  2014-06-04  7:39 ` [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function Fugang Duan
@ 2014-06-04  9:24   ` David Laight
  0 siblings, 0 replies; 13+ messages in thread
From: David Laight @ 2014-06-04  9:24 UTC (permalink / raw)
  To: 'Fugang Duan', davem
  Cc: netdev, shawn.guo, R49496, ezequiel.garcia, bhutchings, stephen,
	b20596, eric.dumazet

From: Fugang Duan 
> Make the code more readable and easy to support other features like
> SG, TSO, moving the common transmit function to one api.
>
> And the patch also factorize the getting BD index to it own function.

Except that you seem to have added additional conditional clauses
into the common code paths.

	David

> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c |   87 ++++++++++++++++++-----------
>  1 files changed, 54 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 802be17..32c2276 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -287,6 +287,25 @@ struct bufdesc *fec_enet_get_prevdesc(struct bufdesc *bdp, struct fec_enet_priva
>  		return (new_bd < base) ? (new_bd + ring_size) : new_bd;
>  }
> 
> +static inline
> +int fec_enet_get_bd_index(struct bufdesc *bdp, struct fec_enet_private *fep)
> +{
> +	struct bufdesc *base;
> +	int index;
> +
> +	if (bdp >= fep->tx_bd_base)
> +		base = fep->tx_bd_base;
> +	else
> +		base = fep->rx_bd_base;
> +
> +	if (fep->bufdesc_ex)
> +		index = (struct bufdesc_ex *)bdp - (struct bufdesc_ex *)base;
> +	else
> +		index = bdp - base;
> +
> +	return index;
> +}
> +
>  static void *swap_buffer(void *bufaddr, int len)
>  {
>  	int i;
> @@ -313,8 +332,7 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev)
>  	return 0;
>  }
> 
> -static netdev_tx_t
> -fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +static int txq_submit_skb(struct sk_buff *skb, struct net_device *ndev)
>  {
>  	struct fec_enet_private *fep = netdev_priv(ndev);
>  	const struct platform_device_id *id_entry =
> @@ -329,14 +347,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> 
>  	status = bdp->cbd_sc;
> 
> -	if (status & BD_ENET_TX_READY) {
> -		/* Ooops.  All transmit buffers are full.  Bail out.
> -		 * This should not happen, since ndev->tbusy should be set.
> -		 */
> -		netdev_err(ndev, "tx queue full!\n");
> -		return NETDEV_TX_BUSY;
> -	}
> -
>  	/* Protocol checksum off-load for TCP and UDP. */
>  	if (fec_enet_clear_csum(skb, ndev)) {
>  		dev_kfree_skb_any(skb);
> @@ -350,16 +360,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  	bufaddr = skb->data;
>  	bdp->cbd_datlen = skb->len;
> 
> -	/*
> -	 * On some FEC implementations data must be aligned on
> -	 * 4-byte boundaries. Use bounce buffers to copy data
> -	 * and get it aligned. Ugh.
> -	 */
> -	if (fep->bufdesc_ex)
> -		index = (struct bufdesc_ex *)bdp -
> -			(struct bufdesc_ex *)fep->tx_bd_base;
> -	else
> -		index = bdp - fep->tx_bd_base;
> +	index = fec_enet_get_bd_index(bdp, fep);
> 
>  	if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
>  		memcpy(fep->tx_bounce[index], skb->data, skb->len);
> @@ -433,15 +434,43 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> 
>  	fep->cur_tx = bdp;
> 
> -	if (fep->cur_tx == fep->dirty_tx)
> -		netif_stop_queue(ndev);
> -
>  	/* Trigger transmission start */
>  	writel(0, fep->hwp + FEC_X_DES_ACTIVE);
> 
>  	return NETDEV_TX_OK;
>  }
> 
> +static netdev_tx_t
> +fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct fec_enet_private *fep = netdev_priv(ndev);
> +	struct bufdesc *bdp;
> +	unsigned short	status;
> +	int ret;
> +
> +	/* Fill in a Tx ring entry */
> +	bdp = fep->cur_tx;
> +
> +	status = bdp->cbd_sc;
> +
> +	if (status & BD_ENET_TX_READY) {
> +		/* Ooops.  All transmit buffers are full.  Bail out.
> +		 * This should not happen, since ndev->tbusy should be set.
> +		 */
> +		netdev_err(ndev, "tx queue full!\n");
> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	ret = txq_submit_skb(skb, ndev);
> +	if (ret == -EBUSY)
> +		return NETDEV_TX_BUSY;
> +
> +	if (fep->cur_tx == fep->dirty_tx)
> +		netif_stop_queue(ndev);
> +
> +	return NETDEV_TX_OK;
> +}
> +
>  /* Init RX & TX buffer descriptors
>   */
>  static void fec_enet_bd_init(struct net_device *dev)
> @@ -770,11 +799,7 @@ fec_enet_tx(struct net_device *ndev)
>  		if (bdp == fep->cur_tx)
>  			break;
> 
> -		if (fep->bufdesc_ex)
> -			index = (struct bufdesc_ex *)bdp -
> -				(struct bufdesc_ex *)fep->tx_bd_base;
> -		else
> -			index = bdp - fep->tx_bd_base;
> +		index = fec_enet_get_bd_index(bdp, fep);
> 
>  		skb = fep->tx_skbuff[index];
>  		dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, skb->len,
> @@ -921,11 +946,7 @@ fec_enet_rx(struct net_device *ndev, int budget)
>  		pkt_len = bdp->cbd_datlen;
>  		ndev->stats.rx_bytes += pkt_len;
> 
> -		if (fep->bufdesc_ex)
> -			index = (struct bufdesc_ex *)bdp -
> -				(struct bufdesc_ex *)fep->rx_bd_base;
> -		else
> -			index = bdp - fep->rx_bd_base;
> +		index = fec_enet_get_bd_index(bdp, fep);
>  		data = fep->rx_skbuff[index]->data;
>  		dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
>  					FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
> --
> 1.7.8

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number
  2014-06-04  7:39 ` [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number Fugang Duan
@ 2014-06-04  9:26   ` David Laight
  2014-06-04  9:39     ` fugang.duan
  2014-06-05  8:06     ` fugang.duan
  0 siblings, 2 replies; 13+ messages in thread
From: David Laight @ 2014-06-04  9:26 UTC (permalink / raw)
  To: 'Fugang Duan', davem
  Cc: netdev, shawn.guo, R49496, ezequiel.garcia, bhutchings, stephen,
	b20596, eric.dumazet

From: Fugang Duan
> In order to support SG, software TSO, let's increase BD entry number.
> 
> CC: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
> CC: David Laight <David.Laight@ACULAB.COM>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
>  drivers/net/ethernet/freescale/fec.h      |    6 +++---
>  drivers/net/ethernet/freescale/fec_main.c |   21 ++++++++++++---------
>  2 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 3b8d6d1..798ad88 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -240,14 +240,14 @@ struct bufdesc_ex {
>   * the skbuffer directly.
>   */
> 
> -#define FEC_ENET_RX_PAGES	8
> +#define FEC_ENET_RX_PAGES	128
>  #define FEC_ENET_RX_FRSIZE	2048
>  #define FEC_ENET_RX_FRPPG	(PAGE_SIZE / FEC_ENET_RX_FRSIZE)
>  #define RX_RING_SIZE		(FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES)
> ...

You are changing the number of rx descriptors as well.
This isn't mentioned in the description.

And, as I said before, basing this on PAGE_SIZE cannot be right.

	David

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number
  2014-06-04  9:26   ` David Laight
@ 2014-06-04  9:39     ` fugang.duan
  2014-06-05  8:06     ` fugang.duan
  1 sibling, 0 replies; 13+ messages in thread
From: fugang.duan @ 2014-06-04  9:39 UTC (permalink / raw)
  To: David Laight, davem
  Cc: netdev, shawn.guo, Fabio.Estevam, ezequiel.garcia, bhutchings,
	stephen, Frank.Li, eric.dumazet

From: David Laight <David.Laight@ACULAB.COM> Data: Wednesday, June 04, 2014 5:27 PM
> To: Duan Fugang-B38611; davem@davemloft.net
> Cc: netdev@vger.kernel.org; shawn.guo@linaro.org; Estevam Fabio-R49496;
> ezequiel.garcia@free-electrons.com; bhutchings@solarflare.com;
> stephen@networkplumber.org; Li Frank-B20596; eric.dumazet@gmail.com
> Subject: RE: [PATCH v2 4/6] net: fec: Increase buffer descriptor entry
> number
> 
> From: Fugang Duan
> > In order to support SG, software TSO, let's increase BD entry number.
> >
> > CC: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > CC: Eric Dumazet <eric.dumazet@gmail.com>
> > CC: David Laight <David.Laight@ACULAB.COM>
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > ---
> >  drivers/net/ethernet/freescale/fec.h      |    6 +++---
> >  drivers/net/ethernet/freescale/fec_main.c |   21 ++++++++++++-------
> --
> >  2 files changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> > index 3b8d6d1..798ad88 100644
> > --- a/drivers/net/ethernet/freescale/fec.h
> > +++ b/drivers/net/ethernet/freescale/fec.h
> > @@ -240,14 +240,14 @@ struct bufdesc_ex {
> >   * the skbuffer directly.
> >   */
> >
> > -#define FEC_ENET_RX_PAGES	8
> > +#define FEC_ENET_RX_PAGES	128
> >  #define FEC_ENET_RX_FRSIZE	2048
> >  #define FEC_ENET_RX_FRPPG	(PAGE_SIZE / FEC_ENET_RX_FRSIZE)
> >  #define RX_RING_SIZE		(FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES)
> > ...
> 
> You are changing the number of rx descriptors as well.
> This isn't mentioned in the description.
> 
> And, as I said before, basing this on PAGE_SIZE cannot be right.
> 
> 	David
> 
The patch just increase tx and rx BD entry number,  and from previous discuss, we set tx number to 512, rx number to 256.


Thanks,
Andy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance
  2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
                   ` (5 preceding siblings ...)
  2014-06-04  7:39 ` [PATCH v2 6/6] net: fec: Add software TSO support Fugang Duan
@ 2014-06-04 22:28 ` David Miller
  2014-06-05  8:11   ` fugang.duan
  6 siblings, 1 reply; 13+ messages in thread
From: David Miller @ 2014-06-04 22:28 UTC (permalink / raw)
  To: b38611
  Cc: netdev, shawn.guo, R49496, ezequiel.garcia, bhutchings, stephen,
	b20596, David.Laight, eric.dumazet


I think you need to start taking David Laight's feedback more seriously.

He's right, it makes no sense at all the increase the number of RX
descriptors for the sake of a send side feature like TSO.

If you have a another reason to increase the RX descriptor count, do
it as a separate change and with an appropriate commit log message
explaining WHY you are doing this.

I'm not applying this patch series until you start taking the feedback
you are receiving to heart.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number
  2014-06-04  9:26   ` David Laight
  2014-06-04  9:39     ` fugang.duan
@ 2014-06-05  8:06     ` fugang.duan
  1 sibling, 0 replies; 13+ messages in thread
From: fugang.duan @ 2014-06-05  8:06 UTC (permalink / raw)
  To: David Laight, davem
  Cc: netdev, shawn.guo, Fabio.Estevam, ezequiel.garcia, bhutchings,
	stephen, Frank.Li, eric.dumazet

Hi, David,

From: David Laight <David.Laight@ACULAB.COM> Data: Wednesday, June 04, 2014 5:27 PM
> To: Duan Fugang-B38611; davem@davemloft.net
> Cc: netdev@vger.kernel.org; shawn.guo@linaro.org; Estevam Fabio-R49496;
> ezequiel.garcia@free-electrons.com; bhutchings@solarflare.com;
> stephen@networkplumber.org; Li Frank-B20596; eric.dumazet@gmail.com
> Subject: RE: [PATCH v2 4/6] net: fec: Increase buffer descriptor entry
> number
> 
> From: Fugang Duan
> > In order to support SG, software TSO, let's increase BD entry number.
> >
> > CC: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > CC: Eric Dumazet <eric.dumazet@gmail.com>
> > CC: David Laight <David.Laight@ACULAB.COM>
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > ---
> >  drivers/net/ethernet/freescale/fec.h      |    6 +++---
> >  drivers/net/ethernet/freescale/fec_main.c |   21 ++++++++++++---------
> >  2 files changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> > index 3b8d6d1..798ad88 100644
> > --- a/drivers/net/ethernet/freescale/fec.h
> > +++ b/drivers/net/ethernet/freescale/fec.h
> > @@ -240,14 +240,14 @@ struct bufdesc_ex {
> >   * the skbuffer directly.
> >   */
> >
> > -#define FEC_ENET_RX_PAGES	8
> > +#define FEC_ENET_RX_PAGES	128
> >  #define FEC_ENET_RX_FRSIZE	2048
> >  #define FEC_ENET_RX_FRPPG	(PAGE_SIZE / FEC_ENET_RX_FRSIZE)
> >  #define RX_RING_SIZE		(FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES)
> > ...
> 
> You are changing the number of rx descriptors as well.
> This isn't mentioned in the description.
> 
> And, as I said before, basing this on PAGE_SIZE cannot be right.
> 
> 	David

Ok, I agree your advice.
I will remove the change for RX BD entry. After test, the performance has no impact.

My initial purpose of increasing it is for interrupt coalescing patch. 

Thanks,
Andy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance
  2014-06-04 22:28 ` [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance David Miller
@ 2014-06-05  8:11   ` fugang.duan
  0 siblings, 0 replies; 13+ messages in thread
From: fugang.duan @ 2014-06-05  8:11 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, shawn.guo, Fabio.Estevam, ezequiel.garcia, bhutchings,
	stephen, Frank.Li, David.Laight, eric.dumazet

Hi, David,

From: David Miller <davem@davemloft.net> Data: Thursday, June 05, 2014 6:29 AM
> To: Duan Fugang-B38611
> Cc: netdev@vger.kernel.org; shawn.guo@linaro.org; Estevam Fabio-R49496;
> ezequiel.garcia@free-electrons.com; bhutchings@solarflare.com;
> stephen@networkplumber.org; Li Frank-B20596; David.Laight@ACULAB.COM;
> eric.dumazet@gmail.com
> Subject: Re: [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx
> performance
> 
> 
> I think you need to start taking David Laight's feedback more seriously.
> 
> He's right, it makes no sense at all the increase the number of RX
> descriptors for the sake of a send side feature like TSO.
> 
> If you have a another reason to increase the RX descriptor count, do it as
> a separate change and with an appropriate commit log message explaining WHY
> you are doing this.
> 
> I'm not applying this patch series until you start taking the feedback you
> are receiving to heart.

Considering with David Laight's suggestion by previous mail, I decide to remove the RX descriptors number change for the patch set.
After the patch set, I have another patch serial for new feature support like interrupt coalescing that request
RX descriptors number is enough big.  I will generate one separate patch to increase RX BDs entry.

I will send the V3 for the change.

Thanks,
Andy

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-06-05  8:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-04  7:39 [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance Fugang Duan
2014-06-04  7:39 ` [PATCH v2 1/6] net: fec: Factorize the .xmit transmit function Fugang Duan
2014-06-04  9:24   ` David Laight
2014-06-04  7:39 ` [PATCH v2 2/6] net: fec: Enable IP header hardware checksum Fugang Duan
2014-06-04  7:39 ` [PATCH v2 3/6] net: fec: Factorize feature setting Fugang Duan
2014-06-04  7:39 ` [PATCH v2 4/6] net: fec: Increase buffer descriptor entry number Fugang Duan
2014-06-04  9:26   ` David Laight
2014-06-04  9:39     ` fugang.duan
2014-06-05  8:06     ` fugang.duan
2014-06-04  7:39 ` [PATCH v2 5/6] net: fec: Add Scatter/gather support Fugang Duan
2014-06-04  7:39 ` [PATCH v2 6/6] net: fec: Add software TSO support Fugang Duan
2014-06-04 22:28 ` [PATCH v2 0/6] net: fec: Enable Software TSO to improve the tx performance David Miller
2014-06-05  8:11   ` fugang.duan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.