2019-09-10 22:08:02

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500

On 9/10/19 2:37 PM, Vijay Khemka wrote:
> HW checksum generation is not working for AST2500, specially with IPV6
> over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> it works perfectly fine with IPV6.
>
> Verified with IPV6 enabled and can do ssh.

How about IPv4, do these packets have problem? If not, can you continue
advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?

>
> Signed-off-by: Vijay Khemka <[email protected]>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 030fed65393e..591c9725002b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> if (priv->use_ncsi)
> netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>
> - /* AST2400 doesn't have working HW checksum generation */
> - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> + of_device_is_compatible(np, "aspeed,ast2500-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);
>


--
Florian


2019-09-10 22:20:26

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 9/10/19, 3:05 PM, "Florian Fainelli" <[email protected]> wrote:

On 9/10/19 2:37 PM, Vijay Khemka wrote:
> HW checksum generation is not working for AST2500, specially with IPV6
> over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> it works perfectly fine with IPV6.
>
> Verified with IPV6 enabled and can do ssh.

How about IPv4, do these packets have problem? If not, can you continue
advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?

Yes IPv4 works. Let me test with your suggestion and will update patch.

>
> Signed-off-by: Vijay Khemka <[email protected]>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 030fed65393e..591c9725002b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> if (priv->use_ncsi)
> netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>
> - /* AST2400 doesn't have working HW checksum generation */
> - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> + of_device_is_compatible(np, "aspeed,ast2500-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);
>


--
Florian


2019-09-10 22:55:15

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 9/10/19, 3:05 PM, "Florian Fainelli" <[email protected]> wrote:

On 9/10/19 2:37 PM, Vijay Khemka wrote:
> HW checksum generation is not working for AST2500, specially with IPV6
> over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> it works perfectly fine with IPV6.
>
> Verified with IPV6 enabled and can do ssh.

How about IPv4, do these packets have problem? If not, can you continue
advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?

I changed code from (netdev->hw_features &= ~NETIF_F_HW_CSUM) to
(netdev->hw_features &= ~NETIF_F_ IPV6_CSUM). And it is not working.
Don't know why. IPV4 works without any change but IPv6 needs HW_CSUM
Disabled.

>
> Signed-off-by: Vijay Khemka <[email protected]>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 030fed65393e..591c9725002b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> if (priv->use_ncsi)
> netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>
> - /* AST2400 doesn't have working HW checksum generation */
> - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> + of_device_is_compatible(np, "aspeed,ast2500-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);
>


--
Florian


2019-09-10 23:13:00

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:



On 9/10/19, 3:05 PM, "Florian Fainelli" <[email protected]> wrote:

On 9/10/19 2:37 PM, Vijay Khemka wrote:
> HW checksum generation is not working for AST2500, specially with IPV6
> over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> it works perfectly fine with IPV6.
>
> Verified with IPV6 enabled and can do ssh.

How about IPv4, do these packets have problem? If not, can you continue
advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?

I changed code from (netdev->hw_features &= ~NETIF_F_HW_CSUM) to
(netdev->hw_features &= ~NETIF_F_ IPV6_CSUM). And it is not working.
Don't know why. IPV4 works without any change but IPv6 needs HW_CSUM
Disabled.

Now I changed to
netdev->hw_features &= (~NETIF_F_HW_CSUM) | NETIF_F_IP_CSUM;
And it works.

>
> Signed-off-by: Vijay Khemka <[email protected]>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 030fed65393e..591c9725002b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> if (priv->use_ncsi)
> netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>
> - /* AST2400 doesn't have working HW checksum generation */
> - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> + of_device_is_compatible(np, "aspeed,ast2500-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);
>


--
Florian




2019-09-11 14:51:19

by Joel Stanley

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500

Hi Ben,

On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <[email protected]> wrote:
>
> On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > HW checksum generation is not working for AST2500, specially with IPV6
> > over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> > it works perfectly fine with IPV6.
> >
> > Verified with IPV6 enabled and can do ssh.
>
> How about IPv4, do these packets have problem? If not, can you continue
> advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
>
> >
> > Signed-off-by: Vijay Khemka <[email protected]>
> > ---
> > drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> > index 030fed65393e..591c9725002b 100644
> > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> > if (priv->use_ncsi)
> > netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> >
> > - /* AST2400 doesn't have working HW checksum generation */
> > - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> > + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> > + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> > + of_device_is_compatible(np, "aspeed,ast2500-mac")))

Do you recall under what circumstances we need to disable hardware checksumming?

Cheers,

Joel

> > 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);
> >
>
>
> --
> Florian

2019-09-11 17:47:45

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 9/11/19, 7:49 AM, "Joel Stanley" <[email protected]> wrote:

Hi Ben,

On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <[email protected]> wrote:
>
> On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > HW checksum generation is not working for AST2500, specially with IPV6
> > over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> > it works perfectly fine with IPV6.
> >
> > Verified with IPV6 enabled and can do ssh.
>
> How about IPv4, do these packets have problem? If not, can you continue
> advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
>
> >
> > Signed-off-by: Vijay Khemka <[email protected]>
> > ---
> > drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> > index 030fed65393e..591c9725002b 100644
> > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> > if (priv->use_ncsi)
> > netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> >
> > - /* AST2400 doesn't have working HW checksum generation */
> > - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> > + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> > + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> > + of_device_is_compatible(np, "aspeed,ast2500-mac")))

Do you recall under what circumstances we need to disable hardware checksumming?
Mainly, TCP packets over IPV6 getting dropped. After disabling it was working.

Cheers,

Joel

> > 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);
> >
>
>
> --
> Florian


2019-09-11 18:36:43

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:



On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:



On 9/10/19, 3:05 PM, "Florian Fainelli" <[email protected]> wrote:

On 9/10/19 2:37 PM, Vijay Khemka wrote:
> HW checksum generation is not working for AST2500, specially with IPV6
> over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> it works perfectly fine with IPV6.
>
> Verified with IPV6 enabled and can do ssh.

How about IPv4, do these packets have problem? If not, can you continue
advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?

I changed code from (netdev->hw_features &= ~NETIF_F_HW_CSUM) to
(netdev->hw_features &= ~NETIF_F_ IPV6_CSUM). And it is not working.
Don't know why. IPV4 works without any change but IPv6 needs HW_CSUM
Disabled.

Now I changed to
netdev->hw_features &= (~NETIF_F_HW_CSUM) | NETIF_F_IP_CSUM;
And it works.

I investigated more on these features and found that we cannot set NETIF_F_IP_CSUM
While NETIF_F_HW_CSUM is set. So I disabled NETIF_F_HW_CSUM first and enabled
NETIF_F_IP_CSUM in next statement. And it works fine.

But as per line 166 in include/linux/skbuff.h,
* NETIF_F_IP_CSUM and NETIF_F_IPV6_CSUM are being deprecated in favor of
* NETIF_F_HW_CSUM. New devices should use NETIF_F_HW_CSUM to indicate
* checksum offload capability.

Please suggest which of below 2 I should do. As both works for me.
1. Disable completely NETIF_F_HW_CSUM and do nothing. This is original patch.
2. Enable NETIF_F_IP_CSUM in addition to 1. I can have v2 if this is accepted.

>
> Signed-off-by: Vijay Khemka <[email protected]>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 030fed65393e..591c9725002b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> if (priv->use_ncsi)
> netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>
> - /* AST2400 doesn't have working HW checksum generation */
> - if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> + /* AST2400 and AST2500 doesn't have working HW checksum generation */
> + if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> + of_device_is_compatible(np, "aspeed,ast2500-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);
>


--
Florian






2019-09-11 18:37:34

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500

On 9/11/19 11:30 AM, Vijay Khemka wrote:
>
>
> On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:
>
>
>
> On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:
>
>
>
> On 9/10/19, 3:05 PM, "Florian Fainelli" <[email protected]> wrote:
>
> On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > HW checksum generation is not working for AST2500, specially with IPV6
> > over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> > it works perfectly fine with IPV6.
> >
> > Verified with IPV6 enabled and can do ssh.
>
> How about IPv4, do these packets have problem? If not, can you continue
> advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
>
> I changed code from (netdev->hw_features &= ~NETIF_F_HW_CSUM) to
> (netdev->hw_features &= ~NETIF_F_ IPV6_CSUM). And it is not working.
> Don't know why. IPV4 works without any change but IPv6 needs HW_CSUM
> Disabled.
>
> Now I changed to
> netdev->hw_features &= (~NETIF_F_HW_CSUM) | NETIF_F_IP_CSUM;
> And it works.
>
> I investigated more on these features and found that we cannot set NETIF_F_IP_CSUM
> While NETIF_F_HW_CSUM is set. So I disabled NETIF_F_HW_CSUM first and enabled
> NETIF_F_IP_CSUM in next statement. And it works fine.
>
> But as per line 166 in include/linux/skbuff.h,
> * NETIF_F_IP_CSUM and NETIF_F_IPV6_CSUM are being deprecated in favor of
> * NETIF_F_HW_CSUM. New devices should use NETIF_F_HW_CSUM to indicate
> * checksum offload capability.
>
> Please suggest which of below 2 I should do. As both works for me.
> 1. Disable completely NETIF_F_HW_CSUM and do nothing. This is original patch.
> 2. Enable NETIF_F_IP_CSUM in addition to 1. I can have v2 if this is accepted.

Sounds like 2 would leave the option of offloading IPv4 checksum
offload, so that would be a better middle group than flat out disable
checksum offload for both IPv4 and IPv6, no?
--
Florian

2019-09-11 18:57:27

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 9/11/19, 11:34 AM, "Florian Fainelli" <[email protected]> wrote:

On 9/11/19 11:30 AM, Vijay Khemka wrote:
>
>
> On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:
>
>
>
> On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <[email protected] on behalf of [email protected]> wrote:
>
>
>
> On 9/10/19, 3:05 PM, "Florian Fainelli" <[email protected]> wrote:
>
> On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > HW checksum generation is not working for AST2500, specially with IPV6
> > over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> > it works perfectly fine with IPV6.
> >
> > Verified with IPV6 enabled and can do ssh.
>
> How about IPv4, do these packets have problem? If not, can you continue
> advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
>
> I changed code from (netdev->hw_features &= ~NETIF_F_HW_CSUM) to
> (netdev->hw_features &= ~NETIF_F_ IPV6_CSUM). And it is not working.
> Don't know why. IPV4 works without any change but IPv6 needs HW_CSUM
> Disabled.
>
> Now I changed to
> netdev->hw_features &= (~NETIF_F_HW_CSUM) | NETIF_F_IP_CSUM;
> And it works.
>
> I investigated more on these features and found that we cannot set NETIF_F_IP_CSUM
> While NETIF_F_HW_CSUM is set. So I disabled NETIF_F_HW_CSUM first and enabled
> NETIF_F_IP_CSUM in next statement. And it works fine.
>
> But as per line 166 in include/linux/skbuff.h,
> * NETIF_F_IP_CSUM and NETIF_F_IPV6_CSUM are being deprecated in favor of
> * NETIF_F_HW_CSUM. New devices should use NETIF_F_HW_CSUM to indicate
> * checksum offload capability.
>
> Please suggest which of below 2 I should do. As both works for me.
> 1. Disable completely NETIF_F_HW_CSUM and do nothing. This is original patch.
> 2. Enable NETIF_F_IP_CSUM in addition to 1. I can have v2 if this is accepted.

Sounds like 2 would leave the option of offloading IPv4 checksum
offload, so that would be a better middle group than flat out disable
checksum offload for both IPv4 and IPv6, no?
Sounds good, I will have v2 after lot more testing.
--
Florian


2019-10-09 04:40:07

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500

On Wed, 2019-09-11 at 14:48 +0000, Joel Stanley wrote:
> Hi Ben,
>
> On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <[email protected]>
> wrote:
> >
> > On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > > HW checksum generation is not working for AST2500, specially with
> > > IPV6
> > > over NCSI. All TCP packets with IPv6 get dropped. By disabling
> > > this
> > > it works perfectly fine with IPV6.
> > >
> > > Verified with IPV6 enabled and can do ssh.
> >
> > How about IPv4, do these packets have problem? If not, can you
> > continue
> > advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
> >
> > >
> > > Signed-off-by: Vijay Khemka <[email protected]>
> > > ---
> > > drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
> > > b/drivers/net/ethernet/faraday/ftgmac100.c
> > > index 030fed65393e..591c9725002b 100644
> > > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > > @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct
> > > platform_device *pdev)
> > > if (priv->use_ncsi)
> > > netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> > >
> > > - /* AST2400 doesn't have working HW checksum generation */
> > > - if (np && (of_device_is_compatible(np, "aspeed,ast2400-
> > > mac")))
> > > + /* AST2400 and AST2500 doesn't have working HW checksum
> > > generation */
> > > + if (np && (of_device_is_compatible(np, "aspeed,ast2400-
> > > mac") ||
> > > + of_device_is_compatible(np, "aspeed,ast2500-
> > > mac")))
>
> Do you recall under what circumstances we need to disable hardware
> checksumming?

Any news on this ? AST2400 has no HW checksum logic in HW, AST2500
should work for IPV4 fine, we should only selectively disable it for
IPV6.

Can you do an updated patch ?

Cheers,
Ben.

2019-10-09 18:22:00

by Oskar Senft

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500

Does HW in the AST2500 actually perform the HW checksum calculation,
or would that be the responsibility of the NIC that it's talking to
via NC-SI?

(Sorry for the double posting! I had HTML mode enabled by default
which causes the e-mail to be dropped in some places)


On Wed, Oct 9, 2019 at 12:38 AM Benjamin Herrenschmidt
<[email protected]> wrote:
>
> On Wed, 2019-09-11 at 14:48 +0000, Joel Stanley wrote:
> > Hi Ben,
> >
> > On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <[email protected]>
> > wrote:
> > >
> > > On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > > > HW checksum generation is not working for AST2500, specially with
> > > > IPV6
> > > > over NCSI. All TCP packets with IPv6 get dropped. By disabling
> > > > this
> > > > it works perfectly fine with IPV6.
> > > >
> > > > Verified with IPV6 enabled and can do ssh.
> > >
> > > How about IPv4, do these packets have problem? If not, can you
> > > continue
> > > advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
> > >
> > > >
> > > > Signed-off-by: Vijay Khemka <[email protected]>
> > > > ---
> > > > drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
> > > > b/drivers/net/ethernet/faraday/ftgmac100.c
> > > > index 030fed65393e..591c9725002b 100644
> > > > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > > > @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct
> > > > platform_device *pdev)
> > > > if (priv->use_ncsi)
> > > > netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> > > >
> > > > - /* AST2400 doesn't have working HW checksum generation */
> > > > - if (np && (of_device_is_compatible(np, "aspeed,ast2400-
> > > > mac")))
> > > > + /* AST2400 and AST2500 doesn't have working HW checksum
> > > > generation */
> > > > + if (np && (of_device_is_compatible(np, "aspeed,ast2400-
> > > > mac") ||
> > > > + of_device_is_compatible(np, "aspeed,ast2500-
> > > > mac")))
> >
> > Do you recall under what circumstances we need to disable hardware
> > checksumming?
>
> Any news on this ? AST2400 has no HW checksum logic in HW, AST2500
> should work for IPV4 fine, we should only selectively disable it for
> IPV6.
>
> Can you do an updated patch ?
>
> Cheers,
> Ben.
>

2019-10-10 19:17:19

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 10/8/19, 9:37 PM, "Benjamin Herrenschmidt" <[email protected]> wrote:

On Wed, 2019-09-11 at 14:48 +0000, Joel Stanley wrote:
> Hi Ben,
>
> On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <[email protected]>
> wrote:
> >
> > On 9/10/19 2:37 PM, Vijay Khemka wrote:
> > > HW checksum generation is not working for AST2500, specially with
> > > IPV6
> > > over NCSI. All TCP packets with IPv6 get dropped. By disabling
> > > this
> > > it works perfectly fine with IPV6.
> > >
> > > Verified with IPV6 enabled and can do ssh.
> >
> > How about IPv4, do these packets have problem? If not, can you
> > continue
> > advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?
> >
> > >
> > > Signed-off-by: Vijay Khemka <[email protected]>
> > > ---
> > > drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
> > > b/drivers/net/ethernet/faraday/ftgmac100.c
> > > index 030fed65393e..591c9725002b 100644
> > > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > > @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct
> > > platform_device *pdev)
> > > if (priv->use_ncsi)
> > > netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> > >
> > > - /* AST2400 doesn't have working HW checksum generation */
> > > - if (np && (of_device_is_compatible(np, "aspeed,ast2400-
> > > mac")))
> > > + /* AST2400 and AST2500 doesn't have working HW checksum
> > > generation */
> > > + if (np && (of_device_is_compatible(np, "aspeed,ast2400-
> > > mac") ||
> > > + of_device_is_compatible(np, "aspeed,ast2500-
> > > mac")))
>
> Do you recall under what circumstances we need to disable hardware
> checksumming?

Any news on this ? AST2400 has no HW checksum logic in HW, AST2500
should work for IPV4 fine, we should only selectively disable it for
IPV6.

Ben, I have already sent v2 for this with requested change which only disable
for IPV6 in AST2500. I can send it again.

Can you do an updated patch ?

Cheers,
Ben.



2019-10-11 03:14:17

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500

On Thu, 2019-10-10 at 19:15 +0000, Vijay Khemka wrote:
> Any news on this ? AST2400 has no HW checksum logic in HW, AST2500
> should work for IPV4 fine, we should only selectively disable it for
> IPV6.
>
> Ben, I have already sent v2 for this with requested change which only disable
> for IPV6 in AST2500. I can send it again.

I didn't see it, did you CC me ? I maintain that driver...

Cheers,
Ben.


2019-10-11 22:33:47

by Vijay Khemka

[permalink] [raw]
Subject: Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500



On 10/10/19, 8:11 PM, "Benjamin Herrenschmidt" <[email protected]> wrote:

On Thu, 2019-10-10 at 19:15 +0000, Vijay Khemka wrote:
> Any news on this ? AST2400 has no HW checksum logic in HW, AST2500
> should work for IPV4 fine, we should only selectively disable it for
> IPV6.
>
> Ben, I have already sent v2 for this with requested change which only disable
> for IPV6 in AST2500. I can send it again.

I didn't see it, did you CC me ? I maintain that driver...

Let me send again via git send-email.

Cheers,
Ben.