[PATCH 1/2] staging: et131x: drop packet when error occurs in et131x_tx()

[PATCH 1/2] staging: et131x: drop packet when error occurs in et131x_tx()

[ZHAO Gang]

Drop packet instead of return NETDEV_TX_BUSY when tx failed.

Signed-off-by: ZHAO Gang <gamerh2o@gmail.com>
---

move function send_packet's work directly to et131x_tx(), also make
some changes to improve readability.

 drivers/staging/et131x/et131x.c | 84 +++++++++++------------------------------
 1 file changed, 22 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index d9446c4..ca3332e 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -3125,55 +3125,6 @@ static int send_packet(struct sk_buff *skb, struct et131x_adapter *adapter)
 	return 0;
 }
 
-/* et131x_send_packets - This function is called by the OS to send packets
- * @skb: the packet(s) to send
- * @netdev:device on which to TX the above packet(s)
- *
- * Return 0 in almost all cases; non-zero value in extreme hard failure only
- */
-static int et131x_send_packets(struct sk_buff *skb, struct net_device *netdev)
-{
-	int status = 0;
-	struct et131x_adapter *adapter = netdev_priv(netdev);
-
-	/* Send these packets
-	 *
-	 * NOTE: The Linux Tx entry point is only given one packet at a time
-	 * to Tx, so the PacketCount and it's array used makes no sense here
-	 */
-
-	/* TCB is not available */
-	if (adapter->tx_ring.used >= NUM_TCB) {
-		/* NOTE: If there's an error on send, no need to queue the
-		 * packet under Linux; if we just send an error up to the
-		 * netif layer, it will resend the skb to us.
-		 */
-		status = -ENOMEM;
-	} else {
-		/* We need to see if the link is up; if it's not, make the
-		 * netif layer think we're good and drop the packet
-		 */
-		if ((adapter->flags & FMP_ADAPTER_FAIL_SEND_MASK) ||
-					!netif_carrier_ok(netdev)) {
-			dev_kfree_skb_any(skb);
-			skb = NULL;
-
-			adapter->net_stats.tx_dropped++;
-		} else {
-			status = send_packet(skb, adapter);
-			if (status != 0 && status != -ENOMEM) {
-				/* On any other error, make netif think we're
-				 * OK and drop the packet
-				 */
-				dev_kfree_skb_any(skb);
-				skb = NULL;
-				adapter->net_stats.tx_dropped++;
-			}
-		}
-	}
-	return status;
-}
-
 /* free_send_packet - Recycle a struct tcb
  * @adapter: pointer to our adapter
  * @tcb: pointer to struct tcb
@@ -4540,12 +4491,9 @@ static void et131x_multicast(struct net_device *netdev)
 /* et131x_tx - The handler to tx a packet on the device
  * @skb: data to be Tx'd
  * @netdev: device on which data is to be Tx'd
- *
- * Returns 0 on success, errno on failure (as defined in errno.h)
  */
-static int et131x_tx(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t et131x_tx(struct sk_buff *skb, struct net_device *netdev)
 {
-	int status = 0;
 	struct et131x_adapter *adapter = netdev_priv(netdev);
 
 	/* stop the queue if it's getting full */
@@ -4556,17 +4504,29 @@ static int et131x_tx(struct sk_buff *skb, struct net_device *netdev)
 	/* Save the timestamp for the TX timeout watchdog */
 	netdev->trans_start = jiffies;
 
-	/* Call the device-specific data Tx routine */
-	status = et131x_send_packets(skb, netdev);
+	/* TCB is not available */
+	if (adapter->tx_ring.used >= NUM_TCB)
+		goto drop;
 
-	/* Check status and manage the netif queue if necessary */
-	if (status != 0) {
-		if (status == -ENOMEM)
-			status = NETDEV_TX_BUSY;
-		else
-			status = NETDEV_TX_OK;
+	/* We need to see if the link is up; if it's not, make the
+	 * netif layer think we're good and drop the packet
+	 */
+	if ((adapter->flags & FMP_ADAPTER_FAIL_SEND_MASK) ||
+	    !netif_carrier_ok(netdev)) {
+		goto drop;
+	} else {
+		if (!send_packet(skb, adapter))
+			goto out;
+		/* On error (send_packet returns != 0), make netif
+		 * think we're OK and drop the packet
+		 */
 	}
-	return status;
+
+drop:
+	dev_kfree_skb_any(skb);
+	adapter->net_stats.tx_dropped++;
+out:
+	return NETDEV_TX_OK;
 }
 
 /* et131x_tx_timeout - Timeout handler
-- 
1.8.3.1

[PATCH 2/2] staging: et131x: improve code readability

[ZHAO Gang]

Signed-off-by: ZHAO Gang <gamerh2o@gmail.com>
---
 drivers/staging/et131x/et131x.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index ca3332e..8c2a4a5 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -1822,6 +1822,9 @@ static void et131x_config_rx_dma_regs(struct et131x_adapter *adapter)
 		u32 __iomem *base_hi;
 		u32 __iomem *base_lo;
 
+		struct fbr_lookup *fbr;
+		fbr = rx_local->fbr[id];
+
 		if (id == 0) {
 			num_des = &rx_dma->fbr0_num_des;
 			full_offset = &rx_dma->fbr0_full_offset;
@@ -1837,12 +1840,10 @@ static void et131x_config_rx_dma_regs(struct et131x_adapter *adapter)
 		}
 
 		/* Now's the best time to initialize FBR contents */
-		fbr_entry =
-		    (struct fbr_desc *) rx_local->fbr[id]->ring_virtaddr;
-		for (entry = 0;
-		     entry < rx_local->fbr[id]->num_entries; entry++) {
-			fbr_entry->addr_hi = rx_local->fbr[id]->bus_high[entry];
-			fbr_entry->addr_lo = rx_local->fbr[id]->bus_low[entry];
+		fbr_entry = (struct fbr_desc *)fbr->ring_virtaddr;
+		for (entry = 0; entry < fbr->num_entries; entry++) {
+			fbr_entry->addr_hi = fbr->bus_high[entry];
+			fbr_entry->addr_lo = fbr->bus_low[entry];
 			fbr_entry->word2 = entry;
 			fbr_entry++;
 		}
@@ -1850,19 +1851,16 @@ static void et131x_config_rx_dma_regs(struct et131x_adapter *adapter)
 		/* Set the address and parameters of Free buffer ring 1 and 0
 		 * into the 1310's registers
 		 */
-		writel(upper_32_bits(rx_local->fbr[id]->ring_physaddr),
-		       base_hi);
-		writel(lower_32_bits(rx_local->fbr[id]->ring_physaddr),
-		       base_lo);
-		writel(rx_local->fbr[id]->num_entries - 1, num_des);
+		writel(upper_32_bits(fbr->ring_physaddr), base_hi);
+		writel(lower_32_bits(fbr->ring_physaddr), base_lo);
+		writel(fbr->num_entries - 1, num_des);
 		writel(ET_DMA10_WRAP, full_offset);
 
 		/* This variable tracks the free buffer ring 1 full position,
 		 * so it has to match the above.
 		 */
-		rx_local->fbr[id]->local_full = ET_DMA10_WRAP;
-		writel(((rx_local->fbr[id]->num_entries *
-					LO_MARK_PERCENT_FOR_RX) / 100) - 1,
+		fbr->local_full = ET_DMA10_WRAP;
+		writel((fbr->num_entries * LO_MARK_PERCENT_FOR_RX / 100) - 1,
 		       min_des);
 	}
 
-- 
1.8.3.1

Re: [PATCH 2/2] staging: et131x: improve code readability

[Mark Einon]

On 3 November 2013 01:41, ZHAO Gang <gamerh2o@gmail.com> wrote:
> Signed-off-by: ZHAO Gang <gamerh2o@gmail.com>

Acked-by: Mark Einon <mark.einon@gmail.com>

Re: [PATCH 1/2] staging: et131x: drop packet when error occurs in et131x_tx()

[Mark Einon]

On 3 November 2013 01:40, ZHAO Gang <gamerh2o@gmail.com> wrote:
> Drop packet instead of return NETDEV_TX_BUSY when tx failed.
>
> Signed-off-by: ZHAO Gang <gamerh2o@gmail.com>

Hi,

Looks good. If you could also send a patch to remove this item from the
et131x/README TODO list, then I'm happy to add:

Acked-by: Mark Einon <mark.einon@gmail.com>

Cheers,

Mark