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

2019-10-11 Thread Vijay Khemka


On 10/10/19, 8:11 PM, "Benjamin Herrenschmidt"  
wrote:

On Thu, 2019-10-10 at 19:15 +, 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.






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

2019-10-10 Thread Benjamin Herrenschmidt
On Thu, 2019-10-10 at 19:15 +, 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.




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

2019-10-10 Thread Vijay Khemka


On 10/8/19, 9:37 PM, "Benjamin Herrenschmidt"  wrote:

On Wed, 2019-09-11 at 14:48 +, Joel Stanley wrote:
> Hi Ben,
> 
> On Tue, 10 Sep 2019 at 22:05, Florian Fainelli 
> 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 
> > > ---
> > >  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.





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

2019-10-09 Thread Benjamin Herrenschmidt
On Wed, 2019-10-09 at 14:18 -0400, Oskar Senft wrote:
> 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?

I wouldn't rely on the NC-SI NIC for UDP/TCP checksums. We should be
providing it with well formed traffic.

Cheers,
Ben.

> Oskar.
> 
> On Wed, Oct 9, 2019 at 12:38 AM Benjamin Herrenschmidt <
> b...@kernel.crashing.org> wrote:
> > On Wed, 2019-09-11 at 14:48 +, Joel Stanley wrote:
> > > Hi Ben,
> > > 
> > > On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <
> > f.faine...@gmail.com>
> > > 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 
> > > > > ---
> > > > >  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.
> > 



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

2019-10-09 Thread Oskar Senft
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
 wrote:
>
> On Wed, 2019-09-11 at 14:48 +, Joel Stanley wrote:
> > Hi Ben,
> >
> > On Tue, 10 Sep 2019 at 22:05, Florian Fainelli 
> > 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 
> > > > ---
> > > >  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.
>


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

2019-10-08 Thread Benjamin Herrenschmidt
On Wed, 2019-09-11 at 14:48 +, Joel Stanley wrote:
> Hi Ben,
> 
> On Tue, 10 Sep 2019 at 22:05, Florian Fainelli 
> 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 
> > > ---
> > >  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.



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

2019-09-11 Thread Vijay Khemka


On 9/11/19, 11:34 AM, "Florian Fainelli"  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" 
 wrote:
> 
> 
> 
> On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" 
 wrote:
> 
> 
> 
> On 9/10/19, 3:05 PM, "Florian Fainelli"  
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




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

2019-09-11 Thread Florian Fainelli
On 9/11/19 11:30 AM, Vijay Khemka wrote:
> 
> 
> On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" 
>  vijaykhe...@fb.com> wrote:
> 
> 
> 
> On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" 
>  vijaykhe...@fb.com> wrote:
> 
> 
> 
> On 9/10/19, 3:05 PM, "Florian Fainelli"  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


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

2019-09-11 Thread Vijay Khemka


On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" 
 wrote:



On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" 
 wrote:



On 9/10/19, 3:05 PM, "Florian Fainelli"  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 
> ---
>  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








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

2019-09-11 Thread Vijay Khemka


On 9/11/19, 7:49 AM, "Joel Stanley"  wrote:

Hi Ben,

On Tue, 10 Sep 2019 at 22:05, Florian Fainelli  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 
> > ---
> >  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




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

2019-09-11 Thread Joel Stanley
Hi Ben,

On Tue, 10 Sep 2019 at 22:05, Florian Fainelli  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 
> > ---
> >  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


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

2019-09-10 Thread Vijay Khemka


On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" 
 wrote:



On 9/10/19, 3:05 PM, "Florian Fainelli"  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 
> ---
>  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






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

2019-09-10 Thread Vijay Khemka


On 9/10/19, 3:05 PM, "Florian Fainelli"  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 
> ---
>  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




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

2019-09-10 Thread Vijay Khemka


On 9/10/19, 3:05 PM, "Florian Fainelli"  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 
> ---
>  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




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

2019-09-10 Thread Florian Fainelli
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 
> ---
>  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