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

2020-10-04 Thread David Miller
From: Xie He 
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 
> Cc: Stephen Hemminger 
> Signed-off-by: Xie He 

Applied, thank you.


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

2020-10-03 Thread 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 
Cc: Stephen Hemminger 
Signed-off-by: Xie He 
---
 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(, 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(, 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