netdev - [PATCH net] ravb: expand rx descriptor data to accommodate hw checksum

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190110140222.32740-1-horms+renesas@verge.net.au>
Date:   Thu, 10 Jan 2019 15:02:22 +0100
From:   Simon Horman <horms+renesas@...ge.net.au>
To:     David Miller <davem@...emloft.net>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:     Magnus Damm <magnus.damm@...il.com>, netdev@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org,
        Simon Horman <horms+renesas@...ge.net.au>
Subject: [PATCH net] ravb: expand rx descriptor data to accommodate hw checksum

EtherAVB may provide a checksum of packet data appended to packet data. In
order to allow this checksum to be received by the host descriptor data
needs to be enlarged by 2 bytes to accommodate the checksum.

In the case of MTU-sized packets without a VLAN tag the
checksum were already accommodated by virtue of the space reserved for the
VLAN tag. However, a packet of MTU-size with a  VLAN tag consumed all
packet data space provided by a descriptor leaving no space for the
trailing checksum.

This was not detected by the driver which incorrectly used the last two
bytes of packet data as the checksum and truncate the packet by two bytes.
This resulted all such packets being dropped.

A work around is to disable rx checksum offload
 # ethtool -K eth0 rx off

This patch resolves this problem by increasing the size available for
packet data in rx descriptors by two bytes. It also introduces
RAVB_CSUM_LEN to make things a little clearer than "2" sprinkled lightly
over the driver.

Tested on R-Car E3 (r8a77990) ES1.0 based Ebisu-4D board

Fixes: 4d86d3818627 ("ravb: RX checksum offload")
Signed-off-by: Simon Horman <horms+renesas@...ge.net.au>
---
 drivers/net/ethernet/renesas/ravb_main.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

 Change since previous posting: drop RFC designation. I have tested this
 patch and it is ready to bring it to a wider audience is via upstream

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index ffc1ada4e6da..545633128718 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -40,6 +40,8 @@
 		 NETIF_MSG_RX_ERR | \
 		 NETIF_MSG_TX_ERR)

+#define RAVB_CSUM_LEN 2
+
 static const char *ravb_rx_irqs[NUM_RX_QUEUE] = {
 	"ch0", /* RAVB_BE */
 	"ch1", /* RAVB_NC */
@@ -343,7 +345,7 @@ static int ravb_ring_init(struct net_device *ndev, int q)
 	int i;

 	priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) +
-		ETH_HLEN + VLAN_HLEN;
+		ETH_HLEN + VLAN_HLEN + RAVB_CSUM_LEN;

 	/* Allocate RX and TX skb rings */
 	priv->rx_skb[q] = kcalloc(priv->num_rx_ring[q],
@@ -524,13 +526,15 @@ static void ravb_rx_csum(struct sk_buff *skb)
 {
 	u8 *hw_csum;

-	/* The hardware checksum is 2 bytes appended to packet data */
-	if (unlikely(skb->len < 2))
+	/* The hardware checksum is contained in RAVB_CSUM_LEN (2) bytes
+	 * appended to packet data
+	 */
+	if (unlikely(skb->len < RAVB_CSUM_LEN))
 		return;
-	hw_csum = skb_tail_pointer(skb) - 2;
+	hw_csum = skb_tail_pointer(skb) - RAVB_CSUM_LEN;
 	skb->csum = csum_unfold((__force __sum16)get_unaligned_le16(hw_csum));
 	skb->ip_summed = CHECKSUM_COMPLETE;
-	skb_trim(skb, skb->len - 2);
+	skb_trim(skb, skb->len - RAVB_CSUM_LEN);
 }

 /* Packet receive function for Ethernet AVB */
-- 
2.11.0