netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] net: Add helper for padding short Ethernet frames
@ 2014-12-03 16:17 Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 1/6] net: Add functions for handling padding frame and adding to length Alexander Duyck
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:17 UTC (permalink / raw)
  To: netdev; +Cc: davem

This patch series adds a pair of helpers to pad short Ethernet frames.  The
general idea is to clean up a number of code paths that were all writing
their own versions of the same or similar function.

An added advantage is that this will help to discourage introducing new
bugs as in at least one case I found the skb->len had been updated, but the
tail pointer update was overlooked.

v2: Added skb_put_padto for cases where length is not ETH_ZLEN
    Updated intel drivers and emulex driver to use skb_put_padto
    Updated eth_skb_pad to use skb_put_padto

---

Alexander Duyck (6):
      net: Add functions for handling padding frame and adding to length
      ethernet/intel: Use eth_skb_pad and skb_put_padto helpers
      emulex: Use skb_put_padto instead of skb_padto() and skb->len assignment
      niu: Use eth_skb_pad helper
      myri10ge: use eth_skb_pad helper
      r8169: Use eth_skb_pad function


 drivers/net/ethernet/emulex/benet/be_main.c       |    3 +--
 drivers/net/ethernet/intel/e1000/e1000_main.c     |    8 ++-----
 drivers/net/ethernet/intel/e1000e/netdev.c        |    8 ++-----
 drivers/net/ethernet/intel/fm10k/fm10k_main.c     |   11 +++-------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c       |    8 ++-----
 drivers/net/ethernet/intel/igb/igb_main.c         |   19 ++++-------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   19 ++++-------------
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   11 +++-------
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c  |   15 ++++---------
 drivers/net/ethernet/realtek/r8169.c              |   12 ++---------
 drivers/net/ethernet/sun/niu.c                    |    9 ++------
 include/linux/etherdevice.h                       |   12 +++++++++++
 include/linux/skbuff.h                            |   24 ++++++++++++++++++++-
 13 files changed, 67 insertions(+), 92 deletions(-)

--

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

* [PATCH v2 1/6] net: Add functions for handling padding frame and adding to length
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
@ 2014-12-03 16:17 ` Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 2/6] ethernet/intel: Use eth_skb_pad and skb_put_padto helpers Alexander Duyck
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:17 UTC (permalink / raw)
  To: netdev; +Cc: davem

This patch adds two new helper functions skb_put_padto and eth_skb_pad.
These functions deviate from the standard skb_pad or skb_padto in that they
will also update the length and tail pointers so that they reflect the
padding added to the frame.

The eth_skb_pad helper is meant to be used with Ethernet devices to update
either Rx or Tx frames so that they report the correct size.  The
skb_put_padto helper is meant to be used primarily in the transmit path for
network devices that need frames to be padded up to some minimum size and
don't wish to simply update the length somewhere external to the frame.

The motivation behind this is that there are a number of implementations
throughout the network device drivers that are all doing the same thing,
but each a little bit differently and as a result several implementations
contain bugs such as updating the length without updating the tail offset
and other similar issues.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 include/linux/etherdevice.h |   12 ++++++++++++
 include/linux/skbuff.h      |   24 +++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 733980f..41c891d 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -392,4 +392,16 @@ static inline unsigned long compare_ether_header(const void *a, const void *b)
 #endif
 }
 
+/**
+ * eth_skb_pad - Pad buffer to mininum number of octets for Ethernet frame
+ * @skb: Buffer to pad
+ *
+ * An Ethernet frame should have a minimum size of 60 bytes.  This function
+ * takes short frames and pads them with zeros up to the 60 byte limit.
+ */
+static inline int eth_skb_pad(struct sk_buff *skb)
+{
+	return skb_put_padto(skb, ETH_ZLEN);
+}
+
 #endif	/* _LINUX_ETHERDEVICE_H */
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7691ad5..d1e25750 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2461,7 +2461,6 @@ static inline int skb_cow_head(struct sk_buff *skb, unsigned int headroom)
  *	is untouched. Otherwise it is extended. Returns zero on
  *	success. The skb is freed on error.
  */
- 
 static inline int skb_padto(struct sk_buff *skb, unsigned int len)
 {
 	unsigned int size = skb->len;
@@ -2470,6 +2469,29 @@ static inline int skb_padto(struct sk_buff *skb, unsigned int len)
 	return skb_pad(skb, len - size);
 }
 
+/**
+ *	skb_put_padto - increase size and pad an skbuff up to a minimal size
+ *	@skb: buffer to pad
+ *	@len: minimal length
+ *
+ *	Pads up a buffer to ensure the trailing bytes exist and are
+ *	blanked. If the buffer already contains sufficient data it
+ *	is untouched. Otherwise it is extended. Returns zero on
+ *	success. The skb is freed on error.
+ */
+static inline int skb_put_padto(struct sk_buff *skb, unsigned int len)
+{
+	unsigned int size = skb->len;
+
+	if (unlikely(size < len)) {
+		len -= size;
+		if (skb_pad(skb, len))
+			return -ENOMEM;
+		__skb_put(skb, len);
+	}
+	return 0;
+}
+
 static inline int skb_add_data(struct sk_buff *skb,
 			       char __user *from, int copy)
 {

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

* [PATCH v2 2/6] ethernet/intel: Use eth_skb_pad and skb_put_padto helpers
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 1/6] net: Add functions for handling padding frame and adding to length Alexander Duyck
@ 2014-12-03 16:17 ` Alexander Duyck
  2014-12-03 19:25   ` Jeff Kirsher
  2014-12-03 16:17 ` [PATCH v2 3/6] emulex: Use skb_put_padto instead of skb_padto() and skb->len assignment Alexander Duyck
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:17 UTC (permalink / raw)
  To: netdev; +Cc: Jeff Kirsher, davem

Update the Intel Ethernet drivers to use eth_skb_pad() and skb_put_padto
instead of doing their own implementations of the function.

Also this cleans up two other spots where skb_pad was called but the length
and tail pointers were being manipulated directly instead of just having
the padding length added via __skb_put.

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c     |    8 ++------
 drivers/net/ethernet/intel/e1000e/netdev.c        |    8 ++------
 drivers/net/ethernet/intel/fm10k/fm10k_main.c     |   11 +++--------
 drivers/net/ethernet/intel/i40e/i40e_txrx.c       |    8 ++------
 drivers/net/ethernet/intel/igb/igb_main.c         |   19 +++++--------------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   19 +++++--------------
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   11 +++--------
 7 files changed, 22 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 24f3986..862d198 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -3136,12 +3136,8 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	 * packets may get corrupted during padding by HW.
 	 * To WA this issue, pad all small packets manually.
 	 */
-	if (skb->len < ETH_ZLEN) {
-		if (skb_pad(skb, ETH_ZLEN - skb->len))
-			return NETDEV_TX_OK;
-		skb->len = ETH_ZLEN;
-		skb_set_tail_pointer(skb, ETH_ZLEN);
-	}
+	if (eth_skb_pad(skb))
+		return NETDEV_TX_OK;
 
 	mss = skb_shinfo(skb)->gso_size;
 	/* The controller does a simple calculation to
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 370cfa2..88936aa 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5554,12 +5554,8 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	/* The minimum packet size with TCTL.PSP set is 17 bytes so
 	 * pad skb in order to meet this minimum size requirement
 	 */
-	if (unlikely(skb->len < 17)) {
-		if (skb_pad(skb, 17 - skb->len))
-			return NETDEV_TX_OK;
-		skb->len = 17;
-		skb_set_tail_pointer(skb, 17);
-	}
+	if (skb_put_padto(skb, 17))
+		return NETDEV_TX_OK;
 
 	mss = skb_shinfo(skb)->gso_size;
 	if (mss) {
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 8c8deec..a05d228 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -578,14 +578,9 @@ static bool fm10k_cleanup_headers(struct fm10k_ring *rx_ring,
 	if (skb_is_nonlinear(skb))
 		fm10k_pull_tail(rx_ring, rx_desc, skb);
 
-	/* if skb_pad returns an error the skb was freed */
-	if (unlikely(skb->len < 60)) {
-		int pad_len = 60 - skb->len;
-
-		if (skb_pad(skb, pad_len))
-			return true;
-		__skb_put(skb, pad_len);
-	}
+	/* if eth_skb_pad returns an error the skb was freed */
+	if (eth_skb_pad(skb))
+		return true;
 
 	return false;
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 3195d82..04b4414 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2399,12 +2399,8 @@ netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	/* hardware can't handle really short frames, hardware padding works
 	 * beyond this point
 	 */
-	if (unlikely(skb->len < I40E_MIN_TX_LEN)) {
-		if (skb_pad(skb, I40E_MIN_TX_LEN - skb->len))
-			return NETDEV_TX_OK;
-		skb->len = I40E_MIN_TX_LEN;
-		skb_set_tail_pointer(skb, I40E_MIN_TX_LEN);
-	}
+	if (skb_put_padto(skb, I40E_MIN_TX_LEN))
+		return NETDEV_TX_OK;
 
 	return i40e_xmit_frame_ring(skb, tx_ring);
 }
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6261399..36467cf 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5091,12 +5091,8 @@ static netdev_tx_t igb_xmit_frame(struct sk_buff *skb,
 	/* The minimum packet size with TCTL.PSP set is 17 so pad the skb
 	 * in order to meet this minimum size requirement.
 	 */
-	if (unlikely(skb->len < 17)) {
-		if (skb_pad(skb, 17 - skb->len))
-			return NETDEV_TX_OK;
-		skb->len = 17;
-		skb_set_tail_pointer(skb, 17);
-	}
+	if (skb_put_padto(skb, 17))
+		return NETDEV_TX_OK;
 
 	return igb_xmit_frame_ring(skb, igb_tx_queue_mapping(adapter, skb));
 }
@@ -6847,14 +6843,9 @@ static bool igb_cleanup_headers(struct igb_ring *rx_ring,
 	if (skb_is_nonlinear(skb))
 		igb_pull_tail(rx_ring, rx_desc, skb);
 
-	/* if skb_pad returns an error the skb was freed */
-	if (unlikely(skb->len < 60)) {
-		int pad_len = 60 - skb->len;
-
-		if (skb_pad(skb, pad_len))
-			return true;
-		__skb_put(skb, pad_len);
-	}
+	/* if eth_skb_pad returns an error the skb was freed */
+	if (eth_skb_pad(skb))
+		return true;
 
 	return false;
 }
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 23d72a2..702cc64 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1755,14 +1755,9 @@ static bool ixgbe_cleanup_headers(struct ixgbe_ring *rx_ring,
 		return false;
 
 #endif
-	/* if skb_pad returns an error the skb was freed */
-	if (unlikely(skb->len < 60)) {
-		int pad_len = 60 - skb->len;
-
-		if (skb_pad(skb, pad_len))
-			return true;
-		__skb_put(skb, pad_len);
-	}
+	/* if eth_skb_pad returns an error the skb was freed */
+	if (eth_skb_pad(skb))
+		return true;
 
 	return false;
 }
@@ -7248,12 +7243,8 @@ static netdev_tx_t __ixgbe_xmit_frame(struct sk_buff *skb,
 	 * The minimum packet size for olinfo paylen is 17 so pad the skb
 	 * in order to meet this minimum size requirement.
 	 */
-	if (unlikely(skb->len < 17)) {
-		if (skb_pad(skb, 17 - skb->len))
-			return NETDEV_TX_OK;
-		skb->len = 17;
-		skb_set_tail_pointer(skb, 17);
-	}
+	if (skb_put_padto(skb, 17))
+		return NETDEV_TX_OK;
 
 	tx_ring = ring ? ring : adapter->tx_ring[skb->queue_mapping];
 
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 755f71f..465d6a8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -612,14 +612,9 @@ static bool ixgbevf_cleanup_headers(struct ixgbevf_ring *rx_ring,
 	if (skb_is_nonlinear(skb))
 		ixgbevf_pull_tail(rx_ring, skb);
 
-	/* if skb_pad returns an error the skb was freed */
-	if (unlikely(skb->len < 60)) {
-		int pad_len = 60 - skb->len;
-
-		if (skb_pad(skb, pad_len))
-			return true;
-		__skb_put(skb, pad_len);
-	}
+	/* if eth_skb_pad returns an error the skb was freed */
+	if (eth_skb_pad(skb))
+		return true;
 
 	return false;
 }

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

* [PATCH v2 3/6] emulex: Use skb_put_padto instead of skb_padto() and skb->len assignment
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 1/6] net: Add functions for handling padding frame and adding to length Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 2/6] ethernet/intel: Use eth_skb_pad and skb_put_padto helpers Alexander Duyck
@ 2014-12-03 16:17 ` Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 4/6] niu: Use eth_skb_pad helper Alexander Duyck
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:17 UTC (permalink / raw)
  To: netdev; +Cc: davem

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index e0ab767..9461ad8 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1017,9 +1017,8 @@ static struct sk_buff *be_xmit_workarounds(struct be_adapter *adapter,
 	 * to pad short packets (<= 32 bytes) to a 36-byte length.
 	 */
 	if (unlikely(!BEx_chip(adapter) && skb->len <= 32)) {
-		if (skb_padto(skb, 36))
+		if (skb_put_padto(skb, 36))
 			return NULL;
-		skb->len = 36;
 	}
 
 	if (BEx_chip(adapter) || lancer_chip(adapter)) {

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

* [PATCH v2 4/6] niu: Use eth_skb_pad helper
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
                   ` (2 preceding siblings ...)
  2014-12-03 16:17 ` [PATCH v2 3/6] emulex: Use skb_put_padto instead of skb_padto() and skb->len assignment Alexander Duyck
@ 2014-12-03 16:17 ` Alexander Duyck
  2014-12-03 16:17 ` [PATCH v2 5/6] myri10ge: use " Alexander Duyck
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:17 UTC (permalink / raw)
  To: netdev; +Cc: davem

Replace the standard layout for padding an ethernet frame with the
eth_skb_pad call.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/sun/niu.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index 904fd1a..4aaa324 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -6651,13 +6651,8 @@ static netdev_tx_t niu_start_xmit(struct sk_buff *skb,
 		return NETDEV_TX_BUSY;
 	}
 
-	if (skb->len < ETH_ZLEN) {
-		unsigned int pad_bytes = ETH_ZLEN - skb->len;
-
-		if (skb_pad(skb, pad_bytes))
-			goto out;
-		skb_put(skb, pad_bytes);
-	}
+	if (eth_skb_pad(skb))
+		goto out;
 
 	len = sizeof(struct tx_pkt_hdr) + 15;
 	if (skb_headroom(skb) < len) {

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

* [PATCH v2 5/6] myri10ge: use eth_skb_pad helper
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
                   ` (3 preceding siblings ...)
  2014-12-03 16:17 ` [PATCH v2 4/6] niu: Use eth_skb_pad helper Alexander Duyck
@ 2014-12-03 16:17 ` Alexander Duyck
  2014-12-03 18:56   ` Sergei Shtylyov
  2014-12-03 16:18 ` [PATCH v2 6/6] r8169: Use eth_skb_pad function Alexander Duyck
  2014-12-09  1:48 ` [PATCH v2 0/6] net: Add helper for padding short Ethernet frames David Miller
  6 siblings, 1 reply; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:17 UTC (permalink / raw)
  To: netdev; +Cc: Hyong-Youb Kim, davem

Update myri10ge to use eth_skb_pad helper.  This also corrects a minor
issue as the driver was updating length without updating the tail pointer.

Cc: Hyong-Youb Kim <hykim@myri.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 9e7e3f1..af09905 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -2913,16 +2913,11 @@ again:
 		flags |= MXGEFW_FLAGS_SMALL;
 
 		/* pad frames to at least ETH_ZLEN bytes */
-		if (unlikely(skb->len < ETH_ZLEN)) {
-			if (skb_padto(skb, ETH_ZLEN)) {
-				/* The packet is gone, so we must
-				 * return 0 */
-				ss->stats.tx_dropped += 1;
-				return NETDEV_TX_OK;
-			}
-			/* adjust the len to account for the zero pad
-			 * so that the nic can know how long it is */
-			skb->len = ETH_ZLEN;
+		if (eth_skb_pad(skb)) {
+			/* The packet is gone, so we must
+			 * return 0 */
+			ss->stats.tx_dropped += 1;
+			return NETDEV_TX_OK;
 		}
 	}
 

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

* [PATCH v2 6/6] r8169: Use eth_skb_pad function
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
                   ` (4 preceding siblings ...)
  2014-12-03 16:17 ` [PATCH v2 5/6] myri10ge: use " Alexander Duyck
@ 2014-12-03 16:18 ` Alexander Duyck
  2014-12-09  1:48 ` [PATCH v2 0/6] net: Add helper for padding short Ethernet frames David Miller
  6 siblings, 0 replies; 10+ messages in thread
From: Alexander Duyck @ 2014-12-03 16:18 UTC (permalink / raw)
  To: netdev; +Cc: Realtek linux nic maintainers, davem

Replace rtl_skb_pad with eth_skb_pad since they do the same thing.

Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/realtek/r8169.c |   12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index cf154f7..e9b44fc 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6832,14 +6832,6 @@ err_out:
 	return -EIO;
 }
 
-static bool rtl_skb_pad(struct sk_buff *skb)
-{
-	if (skb_padto(skb, ETH_ZLEN))
-		return false;
-	skb_put(skb, ETH_ZLEN - skb->len);
-	return true;
-}
-
 static bool rtl_test_hw_pad_bug(struct rtl8169_private *tp, struct sk_buff *skb)
 {
 	return skb->len < ETH_ZLEN && tp->mac_version == RTL_GIGA_MAC_VER_34;
@@ -6980,7 +6972,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp,
 		u8 ip_protocol;
 
 		if (unlikely(rtl_test_hw_pad_bug(tp, skb)))
-			return skb_checksum_help(skb) == 0 && rtl_skb_pad(skb);
+			return !(skb_checksum_help(skb) || eth_skb_pad(skb));
 
 		if (transport_offset > TCPHO_MAX) {
 			netif_warn(tp, tx_err, tp->dev,
@@ -7015,7 +7007,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp,
 		opts[1] |= transport_offset << TCPHO_SHIFT;
 	} else {
 		if (unlikely(rtl_test_hw_pad_bug(tp, skb)))
-			return rtl_skb_pad(skb);
+			return !eth_skb_pad(skb);
 	}
 
 	return true;

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

* Re: [PATCH v2 5/6] myri10ge: use eth_skb_pad helper
  2014-12-03 16:17 ` [PATCH v2 5/6] myri10ge: use " Alexander Duyck
@ 2014-12-03 18:56   ` Sergei Shtylyov
  0 siblings, 0 replies; 10+ messages in thread
From: Sergei Shtylyov @ 2014-12-03 18:56 UTC (permalink / raw)
  To: Alexander Duyck, netdev; +Cc: Hyong-Youb Kim, davem

Hello.

On 12/03/2014 07:17 PM, Alexander Duyck wrote:

> Update myri10ge to use eth_skb_pad helper.  This also corrects a minor
> issue as the driver was updating length without updating the tail pointer.

> Cc: Hyong-Youb Kim <hykim@myri.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
>   drivers/net/ethernet/myricom/myri10ge/myri10ge.c |   15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)

> diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> index 9e7e3f1..af09905 100644
> --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> @@ -2913,16 +2913,11 @@ again:
>   		flags |= MXGEFW_FLAGS_SMALL;
>
>   		/* pad frames to at least ETH_ZLEN bytes */
> -		if (unlikely(skb->len < ETH_ZLEN)) {
> -			if (skb_padto(skb, ETH_ZLEN)) {
> -				/* The packet is gone, so we must
> -				 * return 0 */
> -				ss->stats.tx_dropped += 1;
> -				return NETDEV_TX_OK;
> -			}
> -			/* adjust the len to account for the zero pad
> -			 * so that the nic can know how long it is */
> -			skb->len = ETH_ZLEN;
> +		if (eth_skb_pad(skb)) {
> +			/* The packet is gone, so we must
> +			 * return 0 */

    Time to fix the comment style, perhaps? The preferred one for the 
networking code is:

/* bla
  * bla
  */

> +			ss->stats.tx_dropped += 1;

    Hm, why not 'ss->stats.tx_dropped++'?

> +			return NETDEV_TX_OK;
>   		}
>   	}

WBR, Sergei

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

* Re: [PATCH v2 2/6] ethernet/intel: Use eth_skb_pad and skb_put_padto helpers
  2014-12-03 16:17 ` [PATCH v2 2/6] ethernet/intel: Use eth_skb_pad and skb_put_padto helpers Alexander Duyck
@ 2014-12-03 19:25   ` Jeff Kirsher
  0 siblings, 0 replies; 10+ messages in thread
From: Jeff Kirsher @ 2014-12-03 19:25 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: netdev, davem

[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]

On Wed, 2014-12-03 at 08:17 -0800, Alexander Duyck wrote:
> Update the Intel Ethernet drivers to use eth_skb_pad() and
> skb_put_padto
> instead of doing their own implementations of the function.
> 
> Also this cleans up two other spots where skb_pad was called but the
> length
> and tail pointers were being manipulated directly instead of just
> having
> the padding length added via __skb_put.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c     |    8 ++------
>  drivers/net/ethernet/intel/e1000e/netdev.c        |    8 ++------
>  drivers/net/ethernet/intel/fm10k/fm10k_main.c     |   11 +++--------
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c       |    8 ++------
>  drivers/net/ethernet/intel/igb/igb_main.c         |   19
> +++++--------------
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   19
> +++++--------------
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   11 +++--------
>  7 files changed, 22 insertions(+), 62 deletions(-)

Since this is a part of a series, no need to break it up, by having me
only pull in this one patch.

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 0/6] net: Add helper for padding short Ethernet frames
  2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
                   ` (5 preceding siblings ...)
  2014-12-03 16:18 ` [PATCH v2 6/6] r8169: Use eth_skb_pad function Alexander Duyck
@ 2014-12-09  1:48 ` David Miller
  6 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-12-09  1:48 UTC (permalink / raw)
  To: alexander.h.duyck; +Cc: netdev

From: Alexander Duyck <alexander.h.duyck@redhat.com>
Date: Wed, 03 Dec 2014 08:17:26 -0800

> This patch series adds a pair of helpers to pad short Ethernet frames.  The
> general idea is to clean up a number of code paths that were all writing
> their own versions of the same or similar function.
> 
> An added advantage is that this will help to discourage introducing new
> bugs as in at least one case I found the skb->len had been updated, but the
> tail pointer update was overlooked.
> 
> v2: Added skb_put_padto for cases where length is not ETH_ZLEN
>     Updated intel drivers and emulex driver to use skb_put_padto
>     Updated eth_skb_pad to use skb_put_padto

Series applied, thanks Alex.

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

end of thread, other threads:[~2014-12-09  1:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-03 16:17 [PATCH v2 0/6] net: Add helper for padding short Ethernet frames Alexander Duyck
2014-12-03 16:17 ` [PATCH v2 1/6] net: Add functions for handling padding frame and adding to length Alexander Duyck
2014-12-03 16:17 ` [PATCH v2 2/6] ethernet/intel: Use eth_skb_pad and skb_put_padto helpers Alexander Duyck
2014-12-03 19:25   ` Jeff Kirsher
2014-12-03 16:17 ` [PATCH v2 3/6] emulex: Use skb_put_padto instead of skb_padto() and skb->len assignment Alexander Duyck
2014-12-03 16:17 ` [PATCH v2 4/6] niu: Use eth_skb_pad helper Alexander Duyck
2014-12-03 16:17 ` [PATCH v2 5/6] myri10ge: use " Alexander Duyck
2014-12-03 18:56   ` Sergei Shtylyov
2014-12-03 16:18 ` [PATCH v2 6/6] r8169: Use eth_skb_pad function Alexander Duyck
2014-12-09  1:48 ` [PATCH v2 0/6] net: Add helper for padding short Ethernet frames David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).