* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
[not found] <20190910213734.3112330-1-vijaykhemka@fb.com>
@ 2019-09-10 22:05 ` Florian Fainelli
2019-09-10 22:13 ` Vijay Khemka
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Florian Fainelli @ 2019-09-10 22:05 UTC (permalink / raw)
To: Vijay Khemka, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, joel, linux-aspeed, sdasari
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 <vijaykhemka@fb.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-10 22:05 ` [PATCH] ftgmac100: Disable HW checksum generation on AST2500 Florian Fainelli
@ 2019-09-10 22:13 ` Vijay Khemka
2019-09-10 22:48 ` Vijay Khemka
2019-09-11 14:48 ` Joel Stanley
2 siblings, 0 replies; 15+ messages in thread
From: Vijay Khemka @ 2019-09-10 22:13 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, joel, linux-aspeed, Sai Dasari
On 9/10/19, 3:05 PM, "Florian Fainelli" <f.fainelli@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?
Yes IPv4 works. Let me test with your suggestion and will update patch.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-10 22:05 ` [PATCH] ftgmac100: Disable HW checksum generation on AST2500 Florian Fainelli
2019-09-10 22:13 ` Vijay Khemka
@ 2019-09-10 22:48 ` Vijay Khemka
2019-09-10 23:07 ` Vijay Khemka
2019-09-11 14:48 ` Joel Stanley
2 siblings, 1 reply; 15+ messages in thread
From: Vijay Khemka @ 2019-09-10 22:48 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, joel, linux-aspeed, Sai Dasari
On 9/10/19, 3:05 PM, "Florian Fainelli" <f.fainelli@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?
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 <vijaykhemka@fb.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-10 22:48 ` Vijay Khemka
@ 2019-09-10 23:07 ` Vijay Khemka
2019-09-11 18:30 ` Vijay Khemka
0 siblings, 1 reply; 15+ messages in thread
From: Vijay Khemka @ 2019-09-10 23:07 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, Sai Dasari, linux-aspeed
On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
On 9/10/19, 3:05 PM, "Florian Fainelli" <f.fainelli@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?
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 <vijaykhemka@fb.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-10 22:05 ` [PATCH] ftgmac100: Disable HW checksum generation on AST2500 Florian Fainelli
2019-09-10 22:13 ` Vijay Khemka
2019-09-10 22:48 ` Vijay Khemka
@ 2019-09-11 14:48 ` Joel Stanley
2019-09-11 17:44 ` Vijay Khemka
2019-10-09 4:37 ` Benjamin Herrenschmidt
2 siblings, 2 replies; 15+ messages in thread
From: Joel Stanley @ 2019-09-11 14:48 UTC (permalink / raw)
To: Florian Fainelli, Benjamin Herrenschmidt
Cc: Vijay Khemka, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, Linux Kernel Mailing List,
openbmc @ lists . ozlabs . org, linux-aspeed, Sai Dasari
Hi Ben,
On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <f.fainelli@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 <vijaykhemka@fb.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-11 14:48 ` Joel Stanley
@ 2019-09-11 17:44 ` Vijay Khemka
2019-10-09 4:37 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 15+ messages in thread
From: Vijay Khemka @ 2019-09-11 17:44 UTC (permalink / raw)
To: Joel Stanley, Florian Fainelli, Benjamin Herrenschmidt
Cc: David S. Miller, YueHaibing, Andrew Lunn, Kate Stewart,
Mauro Carvalho Chehab, Luis Chamberlain, Thomas Gleixner, netdev,
Linux Kernel Mailing List, openbmc @ lists . ozlabs . org,
linux-aspeed, Sai Dasari
On 9/11/19, 7:49 AM, "Joel Stanley" <joel@jms.id.au> wrote:
Hi Ben,
On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <f.fainelli@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 <vijaykhemka@fb.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-10 23:07 ` Vijay Khemka
@ 2019-09-11 18:30 ` Vijay Khemka
2019-09-11 18:34 ` Florian Fainelli
0 siblings, 1 reply; 15+ messages in thread
From: Vijay Khemka @ 2019-09-11 18:30 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, Sai Dasari, linux-aspeed
On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
On 9/10/19, 3:05 PM, "Florian Fainelli" <f.fainelli@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?
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 <vijaykhemka@fb.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-11 18:30 ` Vijay Khemka
@ 2019-09-11 18:34 ` Florian Fainelli
2019-09-11 18:50 ` Vijay Khemka
0 siblings, 1 reply; 15+ messages in thread
From: Florian Fainelli @ 2019-09-11 18:34 UTC (permalink / raw)
To: Vijay Khemka, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, Sai Dasari, linux-aspeed
On 9/11/19 11:30 AM, Vijay Khemka wrote:
>
>
> On 9/10/19, 4:08 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
>
>
>
> On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
>
>
>
> On 9/10/19, 3:05 PM, "Florian Fainelli" <f.fainelli@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?
>
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-11 18:34 ` Florian Fainelli
@ 2019-09-11 18:50 ` Vijay Khemka
0 siblings, 0 replies; 15+ messages in thread
From: Vijay Khemka @ 2019-09-11 18:50 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, linux-kernel
Cc: openbmc @ lists . ozlabs . org, Sai Dasari, linux-aspeed
On 9/11/19, 11:34 AM, "Florian Fainelli" <f.fainelli@gmail.com> 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" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
>
>
>
> On 9/10/19, 3:50 PM, "Linux-aspeed on behalf of Vijay Khemka" <linux-aspeed-bounces+vijaykhemka=fb.com@lists.ozlabs.org on behalf of vijaykhemka@fb.com> wrote:
>
>
>
> On 9/10/19, 3:05 PM, "Florian Fainelli" <f.fainelli@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?
>
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-09-11 14:48 ` Joel Stanley
2019-09-11 17:44 ` Vijay Khemka
@ 2019-10-09 4:37 ` Benjamin Herrenschmidt
2019-10-09 18:20 ` Oskar Senft
` (2 more replies)
1 sibling, 3 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2019-10-09 4:37 UTC (permalink / raw)
To: Joel Stanley, Florian Fainelli
Cc: Vijay Khemka, David S. Miller, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev, Linux Kernel Mailing List,
openbmc @ lists . ozlabs . org, linux-aspeed, Sai Dasari
On Wed, 2019-09-11 at 14:48 +0000, Joel Stanley wrote:
> Hi Ben,
>
> On Tue, 10 Sep 2019 at 22:05, Florian Fainelli <f.fainelli@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 <vijaykhemka@fb.com>
> > > ---
> > > 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-10-09 4:37 ` Benjamin Herrenschmidt
@ 2019-10-09 18:20 ` Oskar Senft
[not found] ` <CABoTLcTNwNTua9Neuw5cuFn0Nuz1E6UAakqfkLp1rirbwoQo=w@mail.gmail.com>
2019-10-10 19:15 ` Vijay Khemka
2 siblings, 0 replies; 15+ messages in thread
From: Oskar Senft @ 2019-10-09 18:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Joel Stanley, Florian Fainelli, Kate Stewart, Andrew Lunn,
linux-aspeed, netdev, openbmc @ lists . ozlabs . org, YueHaibing,
Linux Kernel Mailing List, David S. Miller, Luis Chamberlain,
Mauro Carvalho Chehab, Thomas Gleixner, Vijay Khemka
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
<benh@kernel.crashing.org> 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 <f.fainelli@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 <vijaykhemka@fb.com>
> > > > ---
> > > > 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.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
[not found] ` <CABoTLcTNwNTua9Neuw5cuFn0Nuz1E6UAakqfkLp1rirbwoQo=w@mail.gmail.com>
@ 2019-10-10 0:09 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2019-10-10 0:09 UTC (permalink / raw)
To: Oskar Senft, Florian Fainelli
Cc: Joel Stanley, Kate Stewart, Andrew Lunn, linux-aspeed, netdev,
openbmc @ lists . ozlabs . org, YueHaibing,
Linux Kernel Mailing List, David S. Miller, Luis Chamberlain,
Mauro Carvalho Chehab, Thomas Gleixner, Vijay Khemka
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 <
> benh@kernel.crashing.org> 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 <
> > f.fainelli@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 <vijaykhemka@fb.com>
> > > > > ---
> > > > > 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.
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-10-09 4:37 ` Benjamin Herrenschmidt
2019-10-09 18:20 ` Oskar Senft
[not found] ` <CABoTLcTNwNTua9Neuw5cuFn0Nuz1E6UAakqfkLp1rirbwoQo=w@mail.gmail.com>
@ 2019-10-10 19:15 ` Vijay Khemka
2019-10-11 3:10 ` Benjamin Herrenschmidt
2 siblings, 1 reply; 15+ messages in thread
From: Vijay Khemka @ 2019-10-10 19:15 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Joel Stanley, Florian Fainelli
Cc: David S. Miller, YueHaibing, Andrew Lunn, Kate Stewart,
Mauro Carvalho Chehab, Luis Chamberlain, Thomas Gleixner, netdev,
Linux Kernel Mailing List, openbmc @ lists . ozlabs . org,
linux-aspeed, Sai Dasari
On 10/8/19, 9:37 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> 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 <f.fainelli@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 <vijaykhemka@fb.com>
> > > ---
> > > 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-10-10 19:15 ` Vijay Khemka
@ 2019-10-11 3:10 ` Benjamin Herrenschmidt
2019-10-11 21:28 ` Vijay Khemka
0 siblings, 1 reply; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2019-10-11 3:10 UTC (permalink / raw)
To: Vijay Khemka, Joel Stanley, Florian Fainelli
Cc: David S. Miller, YueHaibing, Andrew Lunn, Kate Stewart,
Mauro Carvalho Chehab, Luis Chamberlain, Thomas Gleixner, netdev,
Linux Kernel Mailing List, openbmc @ lists . ozlabs . org,
linux-aspeed, Sai Dasari
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
2019-10-11 3:10 ` Benjamin Herrenschmidt
@ 2019-10-11 21:28 ` Vijay Khemka
0 siblings, 0 replies; 15+ messages in thread
From: Vijay Khemka @ 2019-10-11 21:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Joel Stanley, Florian Fainelli
Cc: David S. Miller, YueHaibing, Andrew Lunn, Kate Stewart,
Mauro Carvalho Chehab, Luis Chamberlain, Thomas Gleixner, netdev,
Linux Kernel Mailing List, openbmc @ lists . ozlabs . org,
linux-aspeed, Sai Dasari
On 10/10/19, 8:11 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org> 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-10-11 21:45 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190910213734.3112330-1-vijaykhemka@fb.com>
2019-09-10 22:05 ` [PATCH] ftgmac100: Disable HW checksum generation on AST2500 Florian Fainelli
2019-09-10 22:13 ` Vijay Khemka
2019-09-10 22:48 ` Vijay Khemka
2019-09-10 23:07 ` Vijay Khemka
2019-09-11 18:30 ` Vijay Khemka
2019-09-11 18:34 ` Florian Fainelli
2019-09-11 18:50 ` Vijay Khemka
2019-09-11 14:48 ` Joel Stanley
2019-09-11 17:44 ` Vijay Khemka
2019-10-09 4:37 ` Benjamin Herrenschmidt
2019-10-09 18:20 ` Oskar Senft
[not found] ` <CABoTLcTNwNTua9Neuw5cuFn0Nuz1E6UAakqfkLp1rirbwoQo=w@mail.gmail.com>
2019-10-10 0:09 ` Benjamin Herrenschmidt
2019-10-10 19:15 ` Vijay Khemka
2019-10-11 3:10 ` Benjamin Herrenschmidt
2019-10-11 21:28 ` Vijay Khemka
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).