diff mbox series

[net-next] lib8390: Use eth_skb_pad()

Message ID 20201112163134.11880-1-W_Armin@gmx.de
State Superseded
Headers show
Series [net-next] lib8390: Use eth_skb_pad() | expand

Commit Message

Armin Wolf Nov. 12, 2020, 4:31 p.m. UTC 
Use eth_skb_pad() instead of a custom padding solution
and replace associated variables with skb->* expressions.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/net/ethernet/8390/lib8390.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

--
2.20.1

Comments

Jakub Kicinski Nov. 14, 2020, 8:56 p.m. UTC | #1 
On Thu, 12 Nov 2020 17:31:34 +0100 Armin Wolf wrote:
> Use eth_skb_pad() instead of a custom padding solution
> and replace associated variables with skb->* expressions.

These are two separate changes, please split them out to two patches.
Florian Fainelli Nov. 14, 2020, 9:03 p.m. UTC | #2 
On 11/12/2020 8:31 AM, Armin Wolf wrote:
> Use eth_skb_pad() instead of a custom padding solution
> and replace associated variables with skb->* expressions.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---

[snip]

> @@ -407,8 +404,8 @@ static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,
>  	spin_unlock(&ei_local->page_lock);
>  	enable_irq_lockdep_irqrestore(dev->irq, &flags);
>  	skb_tx_timestamp(skb);
> +	dev->stats.tx_bytes += skb->len;
>  	dev_consume_skb_any(skb);
> -	dev->stats.tx_bytes += send_length;

There is nothing wrong here with the existing code and you could avoid
an use-after-free if not careful, I would personally keep the existing
code which appears to be just fine.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/8390/lib8390.c b/drivers/net/ethernet/8390/lib8390.c
index e84021282edf..47e2962eff56 100644
--- a/drivers/net/ethernet/8390/lib8390.c
+++ b/drivers/net/ethernet/8390/lib8390.c
@@ -305,16 +305,13 @@  static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,
 {
 	unsigned long e8390_base = dev->base_addr;
 	struct ei_device *ei_local = netdev_priv(dev);
-	int send_length = skb->len, output_page;
+	int output_page;
 	unsigned long flags;
-	char buf[ETH_ZLEN];
-	char *data = skb->data;
-
-	if (skb->len < ETH_ZLEN) {
-		memset(buf, 0, ETH_ZLEN);	/* more efficient than doing just the needed bits */
-		memcpy(buf, data, skb->len);
-		send_length = ETH_ZLEN;
-		data = buf;
+
+	/* The Hardware does not pad undersized frames */
+	if (eth_skb_pad(skb)) {
+		dev->stats.tx_dropped++;
+		return NETDEV_TX_OK;
 	}

 	/* Mask interrupts from the ethercard.
@@ -347,7 +344,7 @@  static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,

 	if (ei_local->tx1 == 0) {
 		output_page = ei_local->tx_start_page;
-		ei_local->tx1 = send_length;
+		ei_local->tx1 = skb->len;
 		if ((netif_msg_tx_queued(ei_local)) &&
 		    ei_local->tx2 > 0)
 			netdev_dbg(dev,
@@ -355,7 +352,7 @@  static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,
 				   ei_local->tx2, ei_local->lasttx, ei_local->txing);
 	} else if (ei_local->tx2 == 0) {
 		output_page = ei_local->tx_start_page + TX_PAGES/2;
-		ei_local->tx2 = send_length;
+		ei_local->tx2 = skb->len;
 		if ((netif_msg_tx_queued(ei_local)) &&
 		    ei_local->tx1 > 0)
 			netdev_dbg(dev,
@@ -380,11 +377,11 @@  static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,
 	 * trigger the send later, upon receiving a Tx done interrupt.
 	 */

-	ei_block_output(dev, send_length, data, output_page);
+	ei_block_output(dev, skb->len, skb->data, output_page);

 	if (!ei_local->txing) {
 		ei_local->txing = 1;
-		NS8390_trigger_send(dev, send_length, output_page);
+		NS8390_trigger_send(dev, skb->len, output_page);
 		if (output_page == ei_local->tx_start_page) {
 			ei_local->tx1 = -1;
 			ei_local->lasttx = -1;
@@ -407,8 +404,8 @@  static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,
 	spin_unlock(&ei_local->page_lock);
 	enable_irq_lockdep_irqrestore(dev->irq, &flags);
 	skb_tx_timestamp(skb);
+	dev->stats.tx_bytes += skb->len;
 	dev_consume_skb_any(skb);
-	dev->stats.tx_bytes += send_length;

 	return NETDEV_TX_OK;
 }