Hello!
On 09/12/2017 04:04 PM, Simon Horman wrote:
Add support for RX checksum offload. This is enabled by default and
may be disabled and re-enabled using ethtool:
# ethtool -K eth0 rx off
# ethtool -K eth0 rx on
The RAVB provides a simple checksumming scheme which appears to be
completely compatible with CHECKSUM_COMPLETE: a 1's complement sum of
Hm, the gen2/3 manuals say calculation doesn't involve bit inversion...
all packet data after the L2 header is appended to packet data; this may
be trivially read by the driver and used to update the skb accordingly.
>
In terms of performance throughput is close to gigabit line-rate both with
and without RX checksum offload enabled. Perf output, however, appears to
indicate that significantly less time is spent in do_csum(). This is as
expected.
[...]
By inspection this also appears to be compatible with the ravb found
on R-Car Gen 2 SoCs, however, this patch is currently untested on such
hardware.
I probably won't be able to test it on gen2 too...
Signed-off-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>
I'm generally OK with the patch but have some questions/comments below...
---
drivers/net/ethernet/renesas/ravb_main.c | 58 +++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index fdf30bfa403b..7c6438cd7de7 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
@@ -1842,6 +1859,41 @@ static int ravb_do_ioctl(struct net_device *ndev, struct ifreq *req, int cmd)
return phy_mii_ioctl(phydev, req, cmd);
}
+static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
+{
+ struct ravb_private *priv = netdev_priv(ndev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ /* Disable TX and RX */
+ ravb_rcv_snd_disable(ndev);
+
+ /* Modify RX Checksum setting */
+ if (enable)
+ ravb_modify(ndev, ECMR, 0, ECMR_RCSC);
Please use ECMR_RCSC as the 3rd argument too to conform the common driver
style.
+ else
+ ravb_modify(ndev, ECMR, ECMR_RCSC, 0);
This *if* can easily be folded into a single ravb_modify() call...
[...]
@@ -2004,6 +2057,9 @@ static int ravb_probe(struct platform_device *pdev)
if (!ndev)
return -ENOMEM;
+ ndev->features |= NETIF_F_RXCSUM;
+ ndev->hw_features |= ndev->features;
Hum, both fields are 0 before this? Then why not use '=' instead of '|='?
Even if not, why not just use the same value as both the rvalues?
[...]
MBR, Sergei
