| Message ID | 20220428082858.545176-1-joel@jms.id.au (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: ftgmac100: Disable hardware checksum on AST2600 | expand |
> Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property") > Reported-by: David Wilder <wilder@us.ibm.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > Net maintainers, if no one has a counter proposal I would like this > merged as a fix. Please give Dylan from Aspeed a chance to reply before > applying the patch. What has phy-handle got to do with this? You might want to add an explanation why you picked that as a Fixes: commit, if it is in fact correct. Andrew
On Fri, 29 Apr 2022 at 01:02, Andrew Lunn <andrew@lunn.ch> wrote: > > > Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property") > > Reported-by: David Wilder <wilder@us.ibm.com> > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > --- > > Net maintainers, if no one has a counter proposal I would like this > > merged as a fix. Please give Dylan from Aspeed a chance to reply before > > applying the patch. > > What has phy-handle got to do with this? You might want to add an > explanation why you picked that as a Fixes: commit, if it is in fact > correct. If you have a look at that commit, you can see that was where ast2600 support was added to the driver.
On Fri, Apr 29, 2022 at 01:35:43AM +0000, Joel Stanley wrote: > On Fri, 29 Apr 2022 at 01:02, Andrew Lunn <andrew@lunn.ch> wrote: > > > > > Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property") > > > Reported-by: David Wilder <wilder@us.ibm.com> > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > > --- > > > Net maintainers, if no one has a counter proposal I would like this > > > merged as a fix. Please give Dylan from Aspeed a chance to reply before > > > applying the patch. > > > > What has phy-handle got to do with this? You might want to add an > > explanation why you picked that as a Fixes: commit, if it is in fact > > correct. > > If you have a look at that commit, you can see that was where ast2600 > support was added to the driver. O.K, so please do add an explanation, because it is not obvious. I'm partially to blame, i should of asked for that patch to be split in two. Andrew
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index caf48023f8ea..5231818943c6 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -1928,6 +1928,11 @@ static int ftgmac100_probe(struct platform_device *pdev) /* AST2400 doesn't have working HW checksum generation */ if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac"))) netdev->hw_features &= ~NETIF_F_HW_CSUM; + + /* AST2600 tx checksum with NCSI is broken */ + if (priv->use_ncsi && of_device_is_compatible(np, "aspeed,ast2600-mac")) + netdev->hw_features &= ~NETIF_F_HW_CSUM; + if (np && of_get_property(np, "no-hw-checksum", NULL)) netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM); netdev->features |= netdev->hw_features;
The AST2600 when using the i210 NIC over NC-SI has been observed to produce incorrect checksum results with specific MTU values. This was first observed when sending data across a long distance set of networks. On a local network, the following test was performed using a 1MB file of random data. On the receiver run this script: #!/bin/bash while [ 1 ]; do # Zero the stats nstat -r > /dev/null nc -l 9899 > test-file # Check for checksum errors TcpInCsumErrors=$(nstat | grep TcpInCsumErrors) if [ -z "$TcpInCsumErrors" ]; then echo No TcpInCsumErrors else echo TcpInCsumErrors = $TcpInCsumErrors fi done On an AST2600 system: # nc <IP of receiver host> 9899 < test-file The test was repeated with various MTU values: # ip link set mtu 1410 dev eth0 The observed results: 1500 - good 1434 - bad 1400 - good 1410 - bad 1420 - good The test was repeated after disabling tx checksumming: # ethtool -K eth0 tx-checksumming off And all MTU values tested resulted in transfers without error. An issue with the driver cannot be ruled out, however there has been no bug discovered so far. David has done the work to take the original bug report of slow data transfer between long distance connections and triaged it down to this test case. Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property") Reported-by: David Wilder <wilder@us.ibm.com> Signed-off-by: Joel Stanley <joel@jms.id.au> --- Net maintainers, if no one has a counter proposal I would like this merged as a fix. Please give Dylan from Aspeed a chance to reply before applying the patch. drivers/net/ethernet/faraday/ftgmac100.c | 5 +++++ 1 file changed, 5 insertions(+)