[PATCH net-next] drivers/net/wan/hdlc_fr: Improvements to the code of pvc_xmit

[PATCH net-next] drivers/net/wan/hdlc_fr: Improvements to the code of pvc_xmit

[Xie He]

1. Keep the code for the normal (non-error) flow at the lowest
indentation level. And use "goto drop" for all error handling.

2. Replace code that pads short Ethernet frames with a "__skb_pad" call.

3. Change "dev_kfree_skb" to "kfree_skb" in error handling code.
"kfree_skb" is the correct function to call when dropping an skb due to
an error. "dev_kfree_skb", which is an alias of "consume_skb", is for
dropping skbs normally (not due to an error).

Cc: Krzysztof Halasa <khc@pm.waw.pl>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Xie He <xie.he.0141@gmail.com>
---
 drivers/net/wan/hdlc_fr.c | 54 +++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index 3a44dad87602..4dfdbca54296 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -416,38 +416,36 @@ static netdev_tx_t pvc_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct pvc_device *pvc = dev->ml_priv;
 
-	if (pvc->state.active) {
-		if (dev->type == ARPHRD_ETHER) {
-			int pad = ETH_ZLEN - skb->len;
-			if (pad > 0) { /* Pad the frame with zeros */
-				int len = skb->len;
-				if (skb_tailroom(skb) < pad)
-					if (pskb_expand_head(skb, 0, pad,
-							     GFP_ATOMIC)) {
-						dev->stats.tx_dropped++;
-						dev_kfree_skb(skb);
-						return NETDEV_TX_OK;
-					}
-				skb_put(skb, pad);
-				memset(skb->data + len, 0, pad);
-			}
-		}
-		skb->dev = dev;
-		if (!fr_hard_header(&skb, pvc->dlci)) {
-			dev->stats.tx_bytes += skb->len;
-			dev->stats.tx_packets++;
-			if (pvc->state.fecn) /* TX Congestion counter */
-				dev->stats.tx_compressed++;
-			skb->dev = pvc->frad;
-			skb->protocol = htons(ETH_P_HDLC);
-			skb_reset_network_header(skb);
-			dev_queue_xmit(skb);
-			return NETDEV_TX_OK;
+	if (!pvc->state.active)
+		goto drop;
+
+	if (dev->type == ARPHRD_ETHER) {
+		int pad = ETH_ZLEN - skb->len;
+
+		if (pad > 0) { /* Pad the frame with zeros */
+			if (__skb_pad(skb, pad, false))
+				goto drop;
+			skb_put(skb, pad);
 		}
 	}
 
+	skb->dev = dev;
+	if (fr_hard_header(&skb, pvc->dlci))
+		goto drop;
+
+	dev->stats.tx_bytes += skb->len;
+	dev->stats.tx_packets++;
+	if (pvc->state.fecn) /* TX Congestion counter */
+		dev->stats.tx_compressed++;
+	skb->dev = pvc->frad;
+	skb->protocol = htons(ETH_P_HDLC);
+	skb_reset_network_header(skb);
+	dev_queue_xmit(skb);
+	return NETDEV_TX_OK;
+
+drop:
 	dev->stats.tx_dropped++;
-	dev_kfree_skb(skb);
+	kfree_skb(skb);
 	return NETDEV_TX_OK;
 }
 
-- 
2.25.1

Re: [PATCH net-next] drivers/net/wan/hdlc_fr: Improvements to the code of pvc_xmit

[David Miller]

From: Xie He <xie.he.0141@gmail.com>
Date: Sat,  3 Oct 2020 15:41:05 -0700

> 1. Keep the code for the normal (non-error) flow at the lowest
> indentation level. And use "goto drop" for all error handling.
> 
> 2. Replace code that pads short Ethernet frames with a "__skb_pad" call.
> 
> 3. Change "dev_kfree_skb" to "kfree_skb" in error handling code.
> "kfree_skb" is the correct function to call when dropping an skb due to
> an error. "dev_kfree_skb", which is an alias of "consume_skb", is for
> dropping skbs normally (not due to an error).
> 
> Cc: Krzysztof Halasa <khc@pm.waw.pl>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Xie He <xie.he.0141@gmail.com>

Applied, thank you.