--- /dev/null
+From foo@baz Sat Nov 7 04:02:26 PM CET 2020
+From: Claudiu Manoil <claudiu.manoil@nxp.com>
+Date: Tue, 20 Oct 2020 20:36:05 +0300
+Subject: gianfar: Account for Tx PTP timestamp in the skb headroom
+
+From: Claudiu Manoil <claudiu.manoil@nxp.com>
+
+[ Upstream commit d6a076d68c6b5d6a5800f3990a513facb7016dea ]
+
+When PTP timestamping is enabled on Tx, the controller
+inserts the Tx timestamp at the beginning of the frame
+buffer, between SFD and the L2 frame header. This means
+that the skb provided by the stack is required to have
+enough headroom otherwise a new skb needs to be created
+by the driver to accommodate the timestamp inserted by h/w.
+Up until now the driver was relying on the second option,
+using skb_realloc_headroom() to create a new skb to accommodate
+PTP frames. Turns out that this method is not reliable, as
+reallocation of skbs for PTP frames along with the required
+overhead (skb_set_owner_w, consume_skb) is causing random
+crashes in subsequent skb_*() calls, when multiple concurrent
+TCP streams are run at the same time on the same device
+(as seen in James' report).
+Note that these crashes don't occur with a single TCP stream,
+nor with multiple concurrent UDP streams, but only when multiple
+TCP streams are run concurrently with the PTP packet flow
+(doing skb reallocation).
+This patch enforces the first method, by requesting enough
+headroom from the stack to accommodate PTP frames, and so avoiding
+skb_realloc_headroom() & co, and the crashes no longer occur.
+There's no reason not to set needed_headroom to a large enough
+value to accommodate PTP frames, so in this regard this patch
+is a fix.
+
+Reported-by: James Jurack <james.jurack@ametek.com>
+Fixes: bee9e58c9e98 ("gianfar:don't add FCB length to hard_header_len")
+Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
+Link: https://lore.kernel.org/r/20201020173605.1173-1-claudiu.manoil@nxp.com
+Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/net/ethernet/freescale/gianfar.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+--- a/drivers/net/ethernet/freescale/gianfar.c
++++ b/drivers/net/ethernet/freescale/gianfar.c
+@@ -1385,7 +1385,7 @@ static int gfar_probe(struct platform_de
+
+ if (dev->features & NETIF_F_IP_CSUM ||
+ priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER)
+- dev->needed_headroom = GMAC_FCB_LEN;
++ dev->needed_headroom = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
+
+ /* Initializing some of the rx/tx queue level parameters */
+ for (i = 0; i < priv->num_tx_queues; i++) {
--- /dev/null
+From foo@baz Sat Nov 7 04:02:26 PM CET 2020
+From: Claudiu Manoil <claudiu.manoil@nxp.com>
+Date: Thu, 29 Oct 2020 10:10:56 +0200
+Subject: gianfar: Replace skb_realloc_headroom with skb_cow_head for PTP
+
+From: Claudiu Manoil <claudiu.manoil@nxp.com>
+
+[ Upstream commit d145c9031325fed963a887851d9fa42516efd52b ]
+
+When PTP timestamping is enabled on Tx, the controller
+inserts the Tx timestamp at the beginning of the frame
+buffer, between SFD and the L2 frame header. This means
+that the skb provided by the stack is required to have
+enough headroom otherwise a new skb needs to be created
+by the driver to accommodate the timestamp inserted by h/w.
+Up until now the driver was relying on skb_realloc_headroom()
+to create new skbs to accommodate PTP frames. Turns out that
+this method is not reliable in this context at least, as
+skb_realloc_headroom() for PTP frames can cause random crashes,
+mostly in subsequent skb_*() calls, when multiple concurrent
+TCP streams are run at the same time with the PTP flow
+on the same device (as seen in James' report). I also noticed
+that when the system is loaded by sending multiple TCP streams,
+the driver receives cloned skbs in large numbers.
+skb_cow_head() instead proves to be stable in this scenario,
+and not only handles cloned skbs too but it's also more efficient
+and widely used in other drivers.
+The commit introducing skb_realloc_headroom in the driver
+goes back to 2009, commit 93c1285c5d92
+("gianfar: reallocate skb when headroom is not enough for fcb").
+For practical purposes I'm referencing a newer commit (from 2012)
+that brings the code to its current structure (and fixes the PTP
+case).
+
+Fixes: 9c4886e5e63b ("gianfar: Fix invalid TX frames returned on error queue when time stamping")
+Reported-by: James Jurack <james.jurack@ametek.com>
+Suggested-by: Jakub Kicinski <kuba@kernel.org>
+Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
+Link: https://lore.kernel.org/r/20201029081057.8506-1-claudiu.manoil@nxp.com
+Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/net/ethernet/freescale/gianfar.c | 12 ++----------
+ 1 file changed, 2 insertions(+), 10 deletions(-)
+
+--- a/drivers/net/ethernet/freescale/gianfar.c
++++ b/drivers/net/ethernet/freescale/gianfar.c
+@@ -2353,20 +2353,12 @@ static int gfar_start_xmit(struct sk_buf
+ fcb_len = GMAC_FCB_LEN + GMAC_TXPAL_LEN;
+
+ /* make space for additional header when fcb is needed */
+- if (fcb_len && unlikely(skb_headroom(skb) < fcb_len)) {
+- struct sk_buff *skb_new;
+-
+- skb_new = skb_realloc_headroom(skb, fcb_len);
+- if (!skb_new) {
++ if (fcb_len) {
++ if (unlikely(skb_cow_head(skb, fcb_len))) {
+ dev->stats.tx_errors++;
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
+ }
+-
+- if (skb->sk)
+- skb_set_owner_w(skb_new, skb->sk);
+- dev_consume_skb_any(skb);
+- skb = skb_new;
+ }
+
+ /* total number of fragments in the SKB */
projects / thirdparty / kernel / stable-queue.git / commitdiff
