From: Ioana Radulescu <ruxandra.radulescu@nxp.com>
To: <gregkh@linuxfoundation.org>
CC: <devel@driverdev.osuosl.org>, <linux-kernel@vger.kernel.org>,
        <bogdan.purcareata@nxp.com>
Subject: [PATCH 2/6] staging: fsl-dpaa2/eth: Don't set netdev->needed_headroom
Date: Fri, 8 Dec 2017 06:47:54 -0600
Message-ID: <20171208124758.2732-3-ruxandra.radulescu@nxp.com>
In-Reply-To: <20171208124758.2732-1-ruxandra.radulescu@nxp.com>
References: <20171208124758.2732-1-ruxandra.radulescu@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2017 12:47:57.0623
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: b42fb22b-297e-403a-d257-08d53e39e832
X-MS-Exchange-CrossTenant-Id: 5afe0b00-7697-4969-b663-5eab37d5f47e
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=5afe0b00-7697-4969-b663-5eab37d5f47e;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO2PR03MB2358
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4050
Lines: 93

Commit 4b2d9fe87950 ("staging: fsl-dpaa2/eth: Extra headroom in RX
buffers") tried to avoid the performance penalty of doing skb
reallocations in the network stack for IP forwarded frames between
two DPAA2 Ethernet interfaces. This led to a (too) complicated
formula that relies on the stack's internal implementation.

Instead, it's safer and easier to just not request any guarantee
from the stack. We already double check in the driver the required
headroom size of egress frames and realloc the skb if needed, so
we don't need to add any extra code.

On forwarding between two of our own interfaces, there is no
functional change; for traffic forwarded from a different device or
generated on the core, skb realloc operations are moved from the stack
to our driver, with no visible impact on performance.

Signed-off-by: Ioana Radulescu <ruxandra.radulescu@nxp.com>
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 24 ++----------------------
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h |  2 +-
 2 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index c8a8e3a..04db65c 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -449,8 +449,7 @@ static int build_single_fd(struct dpaa2_eth_priv *priv,
 	struct sk_buff **skbh;
 	dma_addr_t addr;
 
-	buffer_start = PTR_ALIGN(skb->data - priv->tx_data_offset -
-				 DPAA2_ETH_TX_BUF_ALIGN,
+	buffer_start = PTR_ALIGN(skb->data - dpaa2_eth_needed_headroom(priv),
 				 DPAA2_ETH_TX_BUF_ALIGN);
 
 	/* PTA from egress side is passed as is to the confirmation side so
@@ -571,7 +570,7 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff *skb, struct net_device *net_dev)
 	percpu_stats = this_cpu_ptr(priv->percpu_stats);
 	percpu_extras = this_cpu_ptr(priv->percpu_extras);
 
-	if (unlikely(skb_headroom(skb) < dpaa2_eth_needed_headroom(priv))) {
+	if (skb_headroom(skb) < dpaa2_eth_needed_headroom(priv)) {
 		struct sk_buff *ns;
 
 		ns = skb_realloc_headroom(skb, dpaa2_eth_needed_headroom(priv));
@@ -2273,7 +2272,6 @@ static int netdev_init(struct net_device *net_dev)
 {
 	struct device *dev = net_dev->dev.parent;
 	struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
-	u16 rx_headroom, req_headroom;
 	u8 bcast_addr[ETH_ALEN];
 	u8 num_queues;
 	int err;
@@ -2292,24 +2290,6 @@ static int netdev_init(struct net_device *net_dev)
 		return err;
 	}
 
-	/* Reserve enough space to align buffer as per hardware requirement;
-	 * NOTE: priv->tx_data_offset MUST be initialized at this point.
-	 */
-	net_dev->needed_headroom = dpaa2_eth_needed_headroom(priv);
-
-	/* If headroom guaranteed by hardware in the Rx frame buffer is
-	 * smaller than the Tx headroom required by the stack, issue a
-	 * one time warning. This will most likely mean skbs forwarded to
-	 * another DPAA2 network interface will get reallocated, with a
-	 * significant performance impact.
-	 */
-	req_headroom = LL_RESERVED_SPACE(net_dev) - ETH_HLEN;
-	rx_headroom = ALIGN(DPAA2_ETH_RX_HWA_SIZE +
-			    dpaa2_eth_rx_head_room(priv), priv->rx_buf_align);
-	if (req_headroom > rx_headroom)
-		dev_info_once(dev, "Required headroom (%d) greater than available (%d)\n",
-			      req_headroom, rx_headroom);
-
 	/* Set MTU limits */
 	net_dev->min_mtu = 68;
 	net_dev->max_mtu = DPAA2_ETH_MAX_MTU;
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index 3a4e939..63b09d1 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -371,7 +371,7 @@ static inline unsigned int dpaa2_eth_buf_raw_size(struct dpaa2_eth_priv *priv)
 static inline
 unsigned int dpaa2_eth_needed_headroom(struct dpaa2_eth_priv *priv)
 {
-	return priv->tx_data_offset + DPAA2_ETH_TX_BUF_ALIGN - HH_DATA_MOD;
+	return priv->tx_data_offset + DPAA2_ETH_TX_BUF_ALIGN;
 }
 
 /* Extra headroom space requested to hardware, in order to make sure there's
-- 
2.7.4