From 9c0c112422a2a6b06fcddcaf21957676490cebba Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Dec 2014 08:17:33 -0800 Subject: net: Add functions for handling padding frame and adding to length 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 Signed-off-by: David S. Miller --- 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 733980fce8e3..41c891d05f04 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 7691ad5b4771..d1e2575000b9 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) { -- cgit From a94d9e224e3c48f57559183582c6410e7acf1d8b Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Dec 2014 08:17:39 -0800 Subject: ethernet/intel: Use eth_skb_pad and skb_put_padto helpers 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 Signed-off-by: Alexander Duyck Acked-by: Jeff Kirsher Signed-off-by: David S. Miller --- 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 24f3986cfae2..862d1989ae1c 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 370cfa275ddb..88936aa0029d 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 73457ede53ec..91516aed373e 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 3195d82e4942..04b441460bbd 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 3c0221620c9d..f04ad13f7159 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)); } @@ -6850,14 +6846,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 82d418729dd4..fbd52924ee34 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -1774,14 +1774,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; } @@ -7334,12 +7329,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 3b0ddf757fb6..62a0d8e0f17d 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c @@ -616,14 +616,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; } -- cgit From 74b6939de3aea269627537e5178c973c3f123690 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Dec 2014 08:17:46 -0800 Subject: emulex: Use skb_put_padto instead of skb_padto() and skb->len assignment Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- 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 e0ab7673afe7..9461ad8d837b 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)) { -- cgit From 28f7936cdf7d445570b214123f45866d3f6aa836 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Dec 2014 08:17:52 -0800 Subject: niu: Use eth_skb_pad helper Replace the standard layout for padding an ethernet frame with the eth_skb_pad call. Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- 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 904fd1ab5f6e..4aaa3240453a 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) { -- cgit From b0b9f33334d0c6b212b02a7cfc4a2f9910abf7ca Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Dec 2014 08:17:58 -0800 Subject: myri10ge: use eth_skb_pad helper 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 Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- 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 9e7e3f1dce3e..af099057f0e9 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; } } -- cgit From 207c5f448f385536e0bbf81bfc3556a919b205e9 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Dec 2014 08:18:04 -0800 Subject: r8169: Use eth_skb_pad function Replace rtl_skb_pad with eth_skb_pad since they do the same thing. Cc: Realtek linux nic maintainers Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- 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 56280c1fc0ed..b9c2f33b463d 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -6836,14 +6836,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; @@ -6984,7 +6976,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, @@ -7019,7 +7011,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; -- cgit