Received frame from QMC contains the CRC. Upper layers don't need this CRC and tcpdump mentioned trailing junk data due to this CRC presence.
As some other HDLC driver, simply discard this CRC.
Fixes: d0f2258e79fd ("net: wan: Add support for QMC HDLC") Cc: stable@vger.kernel.org Signed-off-by: Herve Codina herve.codina@bootlin.com --- drivers/net/wan/fsl_qmc_hdlc.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c index 64b4bfa6fea7..8fcfbde31a1c 100644 --- a/drivers/net/wan/fsl_qmc_hdlc.c +++ b/drivers/net/wan/fsl_qmc_hdlc.c @@ -250,6 +250,7 @@ static void qmc_hcld_recv_complete(void *context, size_t length, unsigned int fl struct qmc_hdlc_desc *desc = context; struct net_device *netdev; struct qmc_hdlc *qmc_hdlc; + size_t crc_size; int ret;
netdev = desc->netdev; @@ -268,15 +269,26 @@ static void qmc_hcld_recv_complete(void *context, size_t length, unsigned int fl if (flags & QMC_RX_FLAG_HDLC_CRC) /* CRC error */ netdev->stats.rx_crc_errors++; kfree_skb(desc->skb); - } else { - netdev->stats.rx_packets++; - netdev->stats.rx_bytes += length; + goto re_queue; + }
- skb_put(desc->skb, length); - desc->skb->protocol = hdlc_type_trans(desc->skb, netdev); - netif_rx(desc->skb); + /* Discard the CRC */ + crc_size = qmc_hdlc->is_crc32 ? 4 : 2; + if (length < crc_size) { + netdev->stats.rx_length_errors++; + kfree_skb(desc->skb); + goto re_queue; } + length -= crc_size; + + netdev->stats.rx_packets++; + netdev->stats.rx_bytes += length; + + skb_put(desc->skb, length); + desc->skb->protocol = hdlc_type_trans(desc->skb, netdev); + netif_rx(desc->skb);
+re_queue: /* Re-queue a transfer using the same descriptor */ ret = qmc_hdlc_recv_queue(qmc_hdlc, desc, desc->dma_size); if (ret) {
On Tue, Jul 30, 2024 at 08:31:33AM +0200, Herve Codina wrote:
Received frame from QMC contains the CRC. Upper layers don't need this CRC and tcpdump mentioned trailing junk data due to this CRC presence.
As some other HDLC driver, simply discard this CRC.
It might be nice to specifically site an example. But yes, I see this pattern in hdlc_rx_done().
Fixes: d0f2258e79fd ("net: wan: Add support for QMC HDLC") Cc: stable@vger.kernel.org Signed-off-by: Herve Codina herve.codina@bootlin.com
drivers/net/wan/fsl_qmc_hdlc.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
The above notwithstanding, this looks good to me.
Reviewed-by: Simon Horman horms@kernel.org
From: Simon Horman
Sent: 31 July 2024 09:45
On Tue, Jul 30, 2024 at 08:31:33AM +0200, Herve Codina wrote:
Received frame from QMC contains the CRC. Upper layers don't need this CRC and tcpdump mentioned trailing junk data due to this CRC presence.
As some other HDLC driver, simply discard this CRC.
It might be nice to specifically site an example. But yes, I see this pattern in hdlc_rx_done().
Pretty much the only reason you'd want the received CRC is to do error recovery assuming a single short error burst. Not that I've ever seen a driver contain the required code. 'Back of envelope' calculation: 32bit crc, 2kbyte frame, so 18bits of crc per data bit, sqrt for 'birthday paradox' - I bet you can recover 8-bit error bursts.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hello:
This patch was applied to netdev/net.git (main) by Jakub Kicinski kuba@kernel.org:
On Tue, 30 Jul 2024 08:31:33 +0200 you wrote:
Received frame from QMC contains the CRC. Upper layers don't need this CRC and tcpdump mentioned trailing junk data due to this CRC presence.
As some other HDLC driver, simply discard this CRC.
Fixes: d0f2258e79fd ("net: wan: Add support for QMC HDLC") Cc: stable@vger.kernel.org Signed-off-by: Herve Codina herve.codina@bootlin.com
[...]
Here is the summary with links: - [net,v1] net: wan: fsl_qmc_hdlc: Discard received CRC https://git.kernel.org/netdev/net/c/e549360069b4
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org