linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] net: ftgmac100: Disable hardware checksum on AST2600
@ 2022-04-28  8:28 Joel Stanley
  2022-04-29  1:02 ` Andrew Lunn
  0 siblings, 1 reply; 4+ messages in thread
From: Joel Stanley @ 2022-04-28  8:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni,
	Benjamin Herrenschmidt, Dylan Hung, David Wilder
  Cc: openbmc, netdev, linux-kernel, David Wilder

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(+)

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;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: ftgmac100: Disable hardware checksum on AST2600
  2022-04-28  8:28 [PATCH net] net: ftgmac100: Disable hardware checksum on AST2600 Joel Stanley
@ 2022-04-29  1:02 ` Andrew Lunn
  2022-04-29  1:35   ` Joel Stanley
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Lunn @ 2022-04-29  1:02 UTC (permalink / raw)
  To: Joel Stanley
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni,
	Benjamin Herrenschmidt, Dylan Hung, David Wilder, openbmc,
	netdev, linux-kernel, David Wilder

> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: ftgmac100: Disable hardware checksum on AST2600
  2022-04-29  1:02 ` Andrew Lunn
@ 2022-04-29  1:35   ` Joel Stanley
  2022-04-29  1:40     ` Andrew Lunn
  0 siblings, 1 reply; 4+ messages in thread
From: Joel Stanley @ 2022-04-29  1:35 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni,
	Benjamin Herrenschmidt, Dylan Hung, David Wilder,
	OpenBMC Maillist, Networking, Linux Kernel Mailing List,
	David Wilder

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: ftgmac100: Disable hardware checksum on AST2600
  2022-04-29  1:35   ` Joel Stanley
@ 2022-04-29  1:40     ` Andrew Lunn
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Lunn @ 2022-04-29  1:40 UTC (permalink / raw)
  To: Joel Stanley
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni,
	Benjamin Herrenschmidt, Dylan Hung, David Wilder,
	OpenBMC Maillist, Networking, Linux Kernel Mailing List,
	David Wilder

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-29  1:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-28  8:28 [PATCH net] net: ftgmac100: Disable hardware checksum on AST2600 Joel Stanley
2022-04-29  1:02 ` Andrew Lunn
2022-04-29  1:35   ` Joel Stanley
2022-04-29  1:40     ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).