LKML Archive on lore.kernel.org
 help / color / Atom feed
* 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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
  2 siblings, 1 reply; 9+ 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] 9+ 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
  0 siblings, 0 replies; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ 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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox