diff mbox series

[1/2] stmmac: introduce flag to dynamically disable TX offload for rockchip devices

Message ID 20190401181840.31255-1-papadakospan@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] stmmac: introduce flag to dynamically disable TX offload for rockchip devices | expand

Commit Message

Leonidas P. Papadakos April 1, 2019, 6:18 p.m. UTC
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>

Some rockchip boards exhibit an issue where tx checksumming does not work with
packets larger than 1498.

This is bad for network stability.

The previous approach was using force_thresh_dma_mode in the board dts, which
does more than we need.

Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
 include/linux/stmmac.h                                | 1 +
 3 files changed, 7 insertions(+)

Comments

Heiko Stübner April 1, 2019, 6:31 p.m. UTC | #1
Hi Leonidas,

Am Montag, 1. April 2019, 20:18:40 CEST schrieb Leonidas P. Papadakos:
> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
> 
> Some rockchip boards exhibit an issue where tx checksumming does not work with
> packets larger than 1498.
> 
> This is bad for network stability.
> 
> The previous approach was using force_thresh_dma_mode in the board dts, which
> does more than we need.
> 
> Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 3031f2bf1..807cf5826 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -519,6 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>  		pr_warn("force_sf_dma_mode is ignored if force_thresh_dma_mode is set.");
>  	}
>  
> +	plat->bugged_tx_coe = of_property_read_bool(np, "rockchip,bugged_tx_coe");
> +

you're adding a new devicetree property, so please
(1) also update the binding doc in
    Documentation/devicetree/bindings/net/rockchip-dwmac.txt
(2) Cc devicetree maintainers and lists (via get_maintainer.pl once
    you've added the binding change) to let them look at it.


Thanks
Heiko

>  	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
>  
>  	plat->axi = stmmac_axi_setup(pdev);
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 4335bd771..60c411f43 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -162,6 +162,7 @@ struct plat_stmmacenet_data {
>  	int pmt;
>  	int force_sf_dma_mode;
>  	int force_thresh_dma_mode;
> +	int bugged_tx_coe;
>  	int riwt_off;
>  	int max_speed;
>  	int maxmtu;
>
Robin Murphy April 1, 2019, 6:54 p.m. UTC | #2
On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
> 
> Some rockchip boards exhibit an issue where tx checksumming does not work with
> packets larger than 1498.

Is it really a board-level problem? I'm no networking expert, but the 
nature of the workaround suggests this is more likely to be some 
inherent limitation of the IP block in the SoC, rather than something to 
do with how the external pins get wired up. Does anyone have an RK3328 
or RK3399 board that provably *does* checksum large packets correctly?

> This is bad for network stability.
> 
> The previous approach was using force_thresh_dma_mode in the board dts, which
> does more than we need.

If indeed it is a SoC-level thing (or at least we want to treat it as 
such), then couldn't we just hang it off the existing SoC-specific 
compatibles in dwmac-rk.c and avoid the need for a new DT property at 
all? After all, that's precisely why SoC-specific compatibles are a 
thing in the first place.

Robin.

> Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
>   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
>   include/linux/stmmac.h                                | 1 +
>   3 files changed, 7 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6a2e1031a..4552147e9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3660,6 +3660,10 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
>   	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
>   		features &= ~NETIF_F_CSUM_MASK;
>   
> +	/* Including very small MTUs of 1498 for Rockchip devices */
> +	if (priv->plat->bugged_tx_coe && (dev->mtu > ETH_DATA_LEN - 2))
> +		features &= ~NETIF_F_CSUM_MASK;
> +
>   	/* Disable tso if asked by ethtool */
>   	if ((priv->plat->tso_en) && (priv->dma_cap.tsoen)) {
>   		if (features & NETIF_F_TSO)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 3031f2bf1..807cf5826 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -519,6 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>   		pr_warn("force_sf_dma_mode is ignored if force_thresh_dma_mode is set.");
>   	}
>   
> +	plat->bugged_tx_coe = of_property_read_bool(np, "rockchip,bugged_tx_coe");
> +
>   	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
>   
>   	plat->axi = stmmac_axi_setup(pdev);
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 4335bd771..60c411f43 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -162,6 +162,7 @@ struct plat_stmmacenet_data {
>   	int pmt;
>   	int force_sf_dma_mode;
>   	int force_thresh_dma_mode;
> +	int bugged_tx_coe;
>   	int riwt_off;
>   	int max_speed;
>   	int maxmtu;
>
Heiko Stübner April 1, 2019, 7:06 p.m. UTC | #3
Am Montag, 1. April 2019, 20:54:45 CEST schrieb Robin Murphy:
> On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
> > From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
> > 
> > Some rockchip boards exhibit an issue where tx checksumming does not work with
> > packets larger than 1498.
> 
> Is it really a board-level problem? I'm no networking expert, but the 
> nature of the workaround suggests this is more likely to be some 
> inherent limitation of the IP block in the SoC, rather than something to 
> do with how the external pins get wired up. Does anyone have an RK3328 
> or RK3399 board that provably *does* checksum large packets correctly?

I don't have that many rk3399-boards with actual ethernet and even only
the rock64 from rk3328-land, but at least my rk3399-firefly also seems
affected by this.

But so far the rk3399-puma board from Theobroma did not show that ethernet
issue for me, so I've added two Theobroma people who may or may not tell
if they've also seen that issue.

> 
> > This is bad for network stability.
> > 
> > The previous approach was using force_thresh_dma_mode in the board dts, which
> > does more than we need.
> 
> If indeed it is a SoC-level thing (or at least we want to treat it as 
> such), then couldn't we just hang it off the existing SoC-specific 
> compatibles in dwmac-rk.c and avoid the need for a new DT property at 
> all? After all, that's precisely why SoC-specific compatibles are a 
> thing in the first place.
> 
> Robin.
> 
> > Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>
> > ---
> >   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
> >   drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
> >   include/linux/stmmac.h                                | 1 +
> >   3 files changed, 7 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6a2e1031a..4552147e9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3660,6 +3660,10 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
> >   	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
> >   		features &= ~NETIF_F_CSUM_MASK;
> >   
> > +	/* Including very small MTUs of 1498 for Rockchip devices */
> > +	if (priv->plat->bugged_tx_coe && (dev->mtu > ETH_DATA_LEN - 2))
> > +		features &= ~NETIF_F_CSUM_MASK;
> > +
> >   	/* Disable tso if asked by ethtool */
> >   	if ((priv->plat->tso_en) && (priv->dma_cap.tsoen)) {
> >   		if (features & NETIF_F_TSO)
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > index 3031f2bf1..807cf5826 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> > @@ -519,6 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> >   		pr_warn("force_sf_dma_mode is ignored if force_thresh_dma_mode is set.");
> >   	}
> >   
> > +	plat->bugged_tx_coe = of_property_read_bool(np, "rockchip,bugged_tx_coe");
> > +
> >   	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
> >   
> >   	plat->axi = stmmac_axi_setup(pdev);
> > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> > index 4335bd771..60c411f43 100644
> > --- a/include/linux/stmmac.h
> > +++ b/include/linux/stmmac.h
> > @@ -162,6 +162,7 @@ struct plat_stmmacenet_data {
> >   	int pmt;
> >   	int force_sf_dma_mode;
> >   	int force_thresh_dma_mode;
> > +	int bugged_tx_coe;
> >   	int riwt_off;
> >   	int max_speed;
> >   	int maxmtu;
> > 
>
Philipp Tomsich April 1, 2019, 7:12 p.m. UTC | #4
+ Christoph.

> On 01.04.2019, at 21:06, Heiko Stübner <heiko@sntech.de> wrote:
> 
> Am Montag, 1. April 2019, 20:54:45 CEST schrieb Robin Murphy:
>> On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
>>> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
>>> 
>>> Some rockchip boards exhibit an issue where tx checksumming does not work with
>>> packets larger than 1498.
>> 
>> Is it really a board-level problem? I'm no networking expert, but the 
>> nature of the workaround suggests this is more likely to be some 
>> inherent limitation of the IP block in the SoC, rather than something to 
>> do with how the external pins get wired up. Does anyone have an RK3328 
>> or RK3399 board that provably *does* checksum large packets correctly?
> 
> I don't have that many rk3399-boards with actual ethernet and even only
> the rock64 from rk3328-land, but at least my rk3399-firefly also seems
> affected by this.
> 
> But so far the rk3399-puma board from Theobroma did not show that ethernet
> issue for me, so I've added two Theobroma people who may or may not tell
> if they've also seen that issue.
> 
>> 
>>> This is bad for network stability.
>>> 
>>> The previous approach was using force_thresh_dma_mode in the board dts, which
>>> does more than we need.
>> 
>> If indeed it is a SoC-level thing (or at least we want to treat it as 
>> such), then couldn't we just hang it off the existing SoC-specific 
>> compatibles in dwmac-rk.c and avoid the need for a new DT property at 
>> all? After all, that's precisely why SoC-specific compatibles are a 
>> thing in the first place.
>> 
>> Robin.
>> 
>>> Signed-off-by: Leonidas P. Papadakos <papadakospan@gmail.com>
>>> ---
>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     | 4 ++++
>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
>>>  include/linux/stmmac.h                                | 1 +
>>>  3 files changed, 7 insertions(+)
>>> 
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index 6a2e1031a..4552147e9 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -3660,6 +3660,10 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
>>>  	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
>>>  		features &= ~NETIF_F_CSUM_MASK;
>>> 
>>> +	/* Including very small MTUs of 1498 for Rockchip devices */
>>> +	if (priv->plat->bugged_tx_coe && (dev->mtu > ETH_DATA_LEN - 2))
>>> +		features &= ~NETIF_F_CSUM_MASK;
>>> +
>>>  	/* Disable tso if asked by ethtool */
>>>  	if ((priv->plat->tso_en) && (priv->dma_cap.tsoen)) {
>>>  		if (features & NETIF_F_TSO)
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index 3031f2bf1..807cf5826 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -519,6 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>>>  		pr_warn("force_sf_dma_mode is ignored if force_thresh_dma_mode is set.");
>>>  	}
>>> 
>>> +	plat->bugged_tx_coe = of_property_read_bool(np, "rockchip,bugged_tx_coe");
>>> +
>>>  	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
>>> 
>>>  	plat->axi = stmmac_axi_setup(pdev);
>>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>>> index 4335bd771..60c411f43 100644
>>> --- a/include/linux/stmmac.h
>>> +++ b/include/linux/stmmac.h
>>> @@ -162,6 +162,7 @@ struct plat_stmmacenet_data {
>>>  	int pmt;
>>>  	int force_sf_dma_mode;
>>>  	int force_thresh_dma_mode;
>>> +	int bugged_tx_coe;
>>>  	int riwt_off;
>>>  	int max_speed;
>>>  	int maxmtu;
>>> 
>> 
> 
> 
> 
>
Jose Abreu April 2, 2019, 7:59 a.m. UTC | #5
From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Date: Mon, Apr 01, 2019 at 20:12:21

> + Christoph.
> 
> > On 01.04.2019, at 21:06, Heiko Stübner <heiko@sntech.de> wrote:
> > 
> > Am Montag, 1. April 2019, 20:54:45 CEST schrieb Robin Murphy:
> >> On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
> >>> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
> >>> 
> >>> Some rockchip boards exhibit an issue where tx checksumming does not work 
> with
> >>> packets larger than 1498.
> >> 
> >> Is it really a board-level problem? I'm no networking expert, but the 
> >> nature of the workaround suggests this is more likely to be some 
> >> inherent limitation of the IP block in the SoC, rather than something to 
> >> do with how the external pins get wired up. Does anyone have an RK3328 
> >> or RK3399 board that provably *does* checksum large packets correctly?
> > 
> > I don't have that many rk3399-boards with actual ethernet and even only
> > the rock64 from rk3328-land, but at least my rk3399-firefly also seems
> > affected by this.
> > 
> > But so far the rk3399-puma board from Theobroma did not show that ethernet
> > issue for me, so I've added two Theobroma people who may or may not tell
> > if they've also seen that issue.
> > 
> >> 
> >>> This is bad for network stability.
> >>> 
> >>> The previous approach was using force_thresh_dma_mode in the board dts, 
> which
> >>> does more than we need.
> >> 
> >> If indeed it is a SoC-level thing (or at least we want to treat it as 
> >> such), then couldn't we just hang it off the existing SoC-specific 
> >> compatibles in dwmac-rk.c and avoid the need for a new DT property at 
> >> all? After all, that's precisely why SoC-specific compatibles are a 
> >> thing in the first place.
> >> 

This can happen when FIFO size + PBL settings are not big enough for COE.

Can you please share the above settings ?

Thanks,
Jose Miguel Abreu
Robin Murphy April 2, 2019, 11:49 a.m. UTC | #6
On 02/04/2019 08:59, Jose Abreu wrote:
> From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Date: Mon, Apr 01, 2019 at 20:12:21
> 
>> + Christoph.
>>
>>> On 01.04.2019, at 21:06, Heiko Stübner <heiko@sntech.de> wrote:
>>>
>>> Am Montag, 1. April 2019, 20:54:45 CEST schrieb Robin Murphy:
>>>> On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
>>>>> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
>>>>>
>>>>> Some rockchip boards exhibit an issue where tx checksumming does not work
>> with
>>>>> packets larger than 1498.
>>>>
>>>> Is it really a board-level problem? I'm no networking expert, but the
>>>> nature of the workaround suggests this is more likely to be some
>>>> inherent limitation of the IP block in the SoC, rather than something to
>>>> do with how the external pins get wired up. Does anyone have an RK3328
>>>> or RK3399 board that provably *does* checksum large packets correctly?
>>>
>>> I don't have that many rk3399-boards with actual ethernet and even only
>>> the rock64 from rk3328-land, but at least my rk3399-firefly also seems
>>> affected by this.
>>>
>>> But so far the rk3399-puma board from Theobroma did not show that ethernet
>>> issue for me, so I've added two Theobroma people who may or may not tell
>>> if they've also seen that issue.
>>>
>>>>
>>>>> This is bad for network stability.
>>>>>
>>>>> The previous approach was using force_thresh_dma_mode in the board dts,
>> which
>>>>> does more than we need.
>>>>
>>>> If indeed it is a SoC-level thing (or at least we want to treat it as
>>>> such), then couldn't we just hang it off the existing SoC-specific
>>>> compatibles in dwmac-rk.c and avoid the need for a new DT property at
>>>> all? After all, that's precisely why SoC-specific compatibles are a
>>>> thing in the first place.
>>>>
> 
> This can happen when FIFO size + PBL settings are not big enough for COE.
> 
> Can you please share the above settings ?

Can the FIFO size be discovered by dumping registers, or does someone 
from Rockchip need to look up the IP configuration details?

FWIW, taking a look at the RK3399 TRM, this (p788) jumps out:

"PBL
...
For TxFIFO, valid PBL range in full duplex mode and duplex mode is
128 or less.
For RxFIFO, valid PBL range in full duplex mode is all."


Does that suggest that it's worth fiddling with the "snps,txpbl" value 
in DT?

Thanks,
Robin.
Jose Abreu April 2, 2019, 11:53 a.m. UTC | #7
From: Robin Murphy <robin.murphy@arm.com>
Date: Tue, Apr 02, 2019 at 12:49:36

> On 02/04/2019 08:59, Jose Abreu wrote:
> > From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> > Date: Mon, Apr 01, 2019 at 20:12:21
> > 
> >> + Christoph.
> >>
> >>> On 01.04.2019, at 21:06, Heiko Stübner <heiko@sntech.de> wrote:
> >>>
> >>> Am Montag, 1. April 2019, 20:54:45 CEST schrieb Robin Murphy:
> >>>> On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
> >>>>> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
> >>>>>
> >>>>> Some rockchip boards exhibit an issue where tx checksumming does not 
> work
> >> with
> >>>>> packets larger than 1498.
> >>>>
> >>>> Is it really a board-level problem? I'm no networking expert, but the
> >>>> nature of the workaround suggests this is more likely to be some
> >>>> inherent limitation of the IP block in the SoC, rather than something to
> >>>> do with how the external pins get wired up. Does anyone have an RK3328
> >>>> or RK3399 board that provably *does* checksum large packets correctly?
> >>>
> >>> I don't have that many rk3399-boards with actual ethernet and even only
> >>> the rock64 from rk3328-land, but at least my rk3399-firefly also seems
> >>> affected by this.
> >>>
> >>> But so far the rk3399-puma board from Theobroma did not show that 
> ethernet
> >>> issue for me, so I've added two Theobroma people who may or may not tell
> >>> if they've also seen that issue.
> >>>
> >>>>
> >>>>> This is bad for network stability.
> >>>>>
> >>>>> The previous approach was using force_thresh_dma_mode in the board dts,
> >> which
> >>>>> does more than we need.
> >>>>
> >>>> If indeed it is a SoC-level thing (or at least we want to treat it as
> >>>> such), then couldn't we just hang it off the existing SoC-specific
> >>>> compatibles in dwmac-rk.c and avoid the need for a new DT property at
> >>>> all? After all, that's precisely why SoC-specific compatibles are a
> >>>> thing in the first place.
> >>>>
> > 
> > This can happen when FIFO size + PBL settings are not big enough for COE.
> > 
> > Can you please share the above settings ?
> 
> Can the FIFO size be discovered by dumping registers, or does someone 
> from Rockchip need to look up the IP configuration details?
> 
> FWIW, taking a look at the RK3399 TRM, this (p788) jumps out:
> 
> "PBL
> ...
> For TxFIFO, valid PBL range in full duplex mode and duplex mode is
> 128 or less.
> For RxFIFO, valid PBL range in full duplex mode is all."
> 
> 
> Does that suggest that it's worth fiddling with the "snps,txpbl" value 
> in DT?

Yes, please try with PBL = 0x1 and no-pbl-x8. Performance will be lower
but at least we will know if that’s the cause.

Thanks,
Jose Miguel Abreu
Robin Murphy April 2, 2019, 10:08 p.m. UTC | #8
On 2019-04-02 12:53 pm, Jose Abreu wrote:
> From: Robin Murphy <robin.murphy@arm.com>
> Date: Tue, Apr 02, 2019 at 12:49:36
> 
>> On 02/04/2019 08:59, Jose Abreu wrote:
>>> From: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> Date: Mon, Apr 01, 2019 at 20:12:21
>>>
>>>> + Christoph.
>>>>
>>>>> On 01.04.2019, at 21:06, Heiko Stübner <heiko@sntech.de> wrote:
>>>>>
>>>>> Am Montag, 1. April 2019, 20:54:45 CEST schrieb Robin Murphy:
>>>>>> On 01/04/2019 19:18, Leonidas P. Papadakos wrote:
>>>>>>> From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
>>>>>>>
>>>>>>> Some rockchip boards exhibit an issue where tx checksumming does not
>> work
>>>> with
>>>>>>> packets larger than 1498.
>>>>>>
>>>>>> Is it really a board-level problem? I'm no networking expert, but the
>>>>>> nature of the workaround suggests this is more likely to be some
>>>>>> inherent limitation of the IP block in the SoC, rather than something to
>>>>>> do with how the external pins get wired up. Does anyone have an RK3328
>>>>>> or RK3399 board that provably *does* checksum large packets correctly?
>>>>>
>>>>> I don't have that many rk3399-boards with actual ethernet and even only
>>>>> the rock64 from rk3328-land, but at least my rk3399-firefly also seems
>>>>> affected by this.
>>>>>
>>>>> But so far the rk3399-puma board from Theobroma did not show that
>> ethernet
>>>>> issue for me, so I've added two Theobroma people who may or may not tell
>>>>> if they've also seen that issue.
>>>>>
>>>>>>
>>>>>>> This is bad for network stability.
>>>>>>>
>>>>>>> The previous approach was using force_thresh_dma_mode in the board dts,
>>>> which
>>>>>>> does more than we need.
>>>>>>
>>>>>> If indeed it is a SoC-level thing (or at least we want to treat it as
>>>>>> such), then couldn't we just hang it off the existing SoC-specific
>>>>>> compatibles in dwmac-rk.c and avoid the need for a new DT property at
>>>>>> all? After all, that's precisely why SoC-specific compatibles are a
>>>>>> thing in the first place.
>>>>>>
>>>
>>> This can happen when FIFO size + PBL settings are not big enough for COE.
>>>
>>> Can you please share the above settings ?
>>
>> Can the FIFO size be discovered by dumping registers, or does someone
>> from Rockchip need to look up the IP configuration details?
>>
>> FWIW, taking a look at the RK3399 TRM, this (p788) jumps out:
>>
>> "PBL
>> ...
>> For TxFIFO, valid PBL range in full duplex mode and duplex mode is
>> 128 or less.
>> For RxFIFO, valid PBL range in full duplex mode is all."
>>
>>
>> Does that suggest that it's worth fiddling with the "snps,txpbl" value
>> in DT?
> 
> Yes, please try with PBL = 0x1 and no-pbl-x8. Performance will be lower
> but at least we will know if that’s the cause.

OK, this looks promising - I've never encountered the problem 
'naturally' myself, but I managed to contrive a test setup with my 
RK3399 board wired directly to a laptop running an iperf3 server. As 
standard with an MTU of 1500, I get ~940Mbps as expected; with MTU 
bumped up to 1550 at both ends, the client grinds to a halt after ~100KB 
sent with a dozen or so retries, and the server reports 0 bytes 
received; with "snps,no-pbl-x8" present and the MTU still at 1550, it's 
back to ~940Mbps with 0 retries again.

I'll experiment further with txpbl, and my other boards, as and when I 
have time, but at this point I'm already pretty much convinced you're 
right about that Tx FIFO.

Cheers,
Robin.
Leonidas P. Papadakos April 2, 2019, 10:48 p.m. UTC | #9
I can confirm that snps,no-pbl-x8, currently set by uboot for the fdt
basically fixes the tx-checksumming issue.
I get 700~800 Mbps myself but that might come down to the setup.
You could say it affects performance slightly, as I saw in a previous 
email.

Without the setting The rate consistently drops to 0.
ethtool reports tx-checksumming is on in both cases

Pretty cool!
Does that give you guys an idea on how to tackle it "officially"?
Jose Abreu April 3, 2019, 7:55 a.m. UTC | #10
From: Leonidas P. Papadakos <papadakospan@gmail.com>
Date: Tue, Apr 02, 2019 at 23:48:16

> I can confirm that snps,no-pbl-x8, currently set by uboot for the fdt
> basically fixes the tx-checksumming issue.
> I get 700~800 Mbps myself but that might come down to the setup.
> You could say it affects performance slightly, as I saw in a previous 
> email.
> 
> Without the setting The rate consistently drops to 0.
> ethtool reports tx-checksumming is on in both cases
> 
> Pretty cool!
> Does that give you guys an idea on how to tackle it "officially"?
> 
> 
> 

Thank you all for testing !

It's been on my todo list to cook up a patch that limits PBL according to
FIFO size but I didn't manage to have the time yet so for now it would
be better if you fix it by submitting the DT bindings changes.

Thanks,
Jose Miguel Abreu
Leonidas P. Papadakos April 3, 2019, 3:35 p.m. UTC | #11
Στις Τετ, 3 Απρ, 2019 at 10:55 ΠΜ, ο/η Jose Abreu 
<jose.abreu@synopsys.com> έγραψε:
> From: Leonidas P. Papadakos <papadakospan@gmail.com>
> Date: Tue, Apr 02, 2019 at 23:48:16
> 
>>  I can confirm that snps,no-pbl-x8, currently set by uboot for the 
>> fdt
>>  basically fixes the tx-checksumming issue.
>>  I get 700~800 Mbps myself but that might come down to the setup.
>>  You could say it affects performance slightly, as I saw in a 
>> previous
>>  email.
>> 
>>  Without the setting The rate consistently drops to 0.
>>  ethtool reports tx-checksumming is on in both cases
>> 
>>  Pretty cool!
>>  Does that give you guys an idea on how to tackle it "officially"?
>> 
>> 
>> 
> 
> Thank you all for testing !
> 
> It's been on my todo list to cook up a patch that limits PBL 
> according to
> FIFO size but I didn't manage to have the time yet so for now it would
> be better if you fix it by submitting the DT bindings changes.
> 
> Thanks,
> Jose Miguel Abreu

If snps,no-pbl-x8 does indeed have a performace hit, should the 
workaround be to
a) turn tx checksumming off or
b) snps,no-pbl-x8 set?

I'll test to see the difference
Leonidas P. Papadakos April 3, 2019, 3:55 p.m. UTC | #12
> If snps,no-pbl-x8 does indeed have a performace hit, should the 
> workaround be to
> a) turn tx checksumming off or
> b) snps,no-pbl-x8 set?
> 
> I'll test to see the difference

Update: It really seems like snps,no-pbl-x8 is the better option.
So I say, replace snps,force_thresh_dma_mode with it.
Robin Murphy April 3, 2019, 4:12 p.m. UTC | #13
On 03/04/2019 16:55, Leonidas P. Papadakos wrote:
> 
>> If snps,no-pbl-x8 does indeed have a performace hit, should the 
>> workaround be to
>> a) turn tx checksumming off or
>> b) snps,no-pbl-x8 set?
>>
>> I'll test to see the difference
> 
> Update: It really seems like snps,no-pbl-x8 is the better option.
> So I say, replace snps,force_thresh_dma_mode with it.

Yes, I would expect software checksumming to have a much more noticeable 
impact (in fact I've already been trying to get round to benchmarking 
some arm64 checksum optimisations on my RK3328 precisely because of this 
issue).

If I'm interpreting the register descriptions in the Rockchip TRMs 
correctly, it seems like no-pbl-x8 is a relatively big hammer and there 
should still be room to tune things a bit closer to the maximum limits - 
I'll have another play this evening to see if I've understood things right.

Robin.
Jose Abreu April 5, 2019, 10:24 a.m. UTC | #14
From: Robin Murphy <robin.murphy@arm.com>
Date: Wed, Apr 03, 2019 at 17:12:03

> Yes, I would expect software checksumming to have a much more noticeable 
> impact (in fact I've already been trying to get round to benchmarking 
> some arm64 checksum optimisations on my RK3328 precisely because of this 
> issue).
> 

Can you share the optimizations ? 
Leonidas P. Papadakos April 5, 2019, 5:58 p.m. UTC | #15
> You can play around different PBL values without using the no-pbl-x8 
> option.
> 
> I would start with PBL=0x1 and then going up. If PBL=0x1 does not 
> work then
> add the no-pbl-x8 option and start with PBL=0x20 and keep decreasing.
> 
> Thanks,
> Jose Miguel Abreu
> 

Okay, so setting snps,txpbl = <0x1>; seems to work. Is that our best 
value or do higher values yield better performance?
Leonidas P. Papadakos April 5, 2019, 6:14 p.m. UTC | #16
Seems like snps,txpbl = <0x20> gives me better performance
Robin Murphy April 5, 2019, 6:29 p.m. UTC | #17
On 05/04/2019 19:14, Leonidas P. Papadakos wrote:
> 
> Seems like snps,txpbl = <0x20> gives me better performance

What's your overall setup? So far with my RK3328 box I've found that 
with an IPv4 link and an MTU of 2000 I have to dial PBL right down to 1 
or 2 (with no-pbl-x8 as well) to prevent Tx stalling, and with the MTU 
bumped up to 3000 it just won't play at any PBL setting...

Robin.
Leonidas P. Papadakos April 5, 2019, 6:38 p.m. UTC | #18
> What's your overall setup? So far with my RK3328 box I've found that 
> with an IPv4 link and an MTU of 2000 I have to dial PBL right down to 
> 1 or 2 (with no-pbl-x8 as well) to prevent Tx stalling, and with the 
> MTU bumped up to 3000 it just won't play at any PBL setting...
> 
> Robin.

I'm using a Libre Computer Renegade with RK3328 SoC(maybe it's a board 
specific thing?)
I'm using txpbl as from what I can gather, the rx has no issues, no 
need to change it.

When it comes to rx,
running the server (iperf3 -s) on the Renegade and iperf3 -c 
<renegade_ip> on my laptop,
I get about 940 Mbps. It's always been like that, satisfactory.

When it comes to tx (the one all the fuss is about)
I eventually find that just with snps,txpbl = <0x21>, and no other pbl 
thing set I get about 905 Mbps on average sent to my laptop, now 
running the iperf3 server.

I must say I haven't touched the MTUs but I never needed to as the 
issue presented itself in normal usage. Simple iperf tests will fail 
after same kilobytes if no tweaks are applied.
Leonidas P. Papadakos April 11, 2019, 9:09 p.m. UTC | #19
At this point I've settled on snps,txpbl = <0x20> by itself.
If I increase the MTU from the default of 1500 I get a stack trace and 
link reset almost immediately:
(https://pastebin.com/raw/5JBtfWei)
whether TX Checksumming is ON or OFF.

That said, with the default MTU, I get better speeds when TX 
Checksumming is on and the PBL tweak is set.

Is there a better option in the horizon for the near future?
At least for the Renegade (the only board I have to test) it can serve 
as a temporary workaround.
Should I make a patch to replace force_thresh_dma_mode with txbpl 
<0x20> for the Renegade specifically?

In any case I would be happy to help as much as I can to figure out if 
it's a board specific thing, or SoC, or even an issue of the Ethernet 
device itself.
Jose Abreu April 12, 2019, 7:35 a.m. UTC | #20
From: Leonidas P. Papadakos <papadakospan@gmail.com>
Date: Thu, Apr 11, 2019 at 22:09:30

> 
> At this point I've settled on snps,txpbl = <0x20> by itself.
> If I increase the MTU from the default of 1500 I get a stack trace and 
> link reset almost immediately:
> (https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_raw_5JBtfWei&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=4VBEjla4XZR8-m8v84wV5TnhbHyIvTniApjuk2XMoS8&s=piZ9ECO216D0lthteXLpCiOvlHwJhq-x_pBKChT274Y&e=)
> whether TX Checksumming is ON or OFF.

Can you please share the stacktrace here ? I can't access pastebin due to 
corporate policy.

If it's a queue timeout then please share your "dmesg | grep -i stmmac" 
since boot.

> 
> That said, with the default MTU, I get better speeds when TX 
> Checksumming is on and the PBL tweak is set.
> 
> Is there a better option in the horizon for the near future?
> At least for the Renegade (the only board I have to test) it can serve 
> as a temporary workaround.
> Should I make a patch to replace force_thresh_dma_mode with txbpl 
> <0x20> for the Renegade specifically?
> 
> In any case I would be happy to help as much as I can to figure out if 
> it's a board specific thing, or SoC, or even an issue of the Ethernet 
> device itself.

This is not a workaround neither an issue. It's well stablished that PBL 
setting interferes with COE so one must choose a setting that depends on 
FIFO size. I would like to make it automatic in the driver but I didn't 
have the time to submit a patch yet, sorry.

Thanks,
Jose Miguel Abreu
Leonidas P. Papadakos April 12, 2019, 11:13 a.m. UTC | #21
> Can you please share the stacktrace here ? I can't access pastebin 
> due to
> corporate policy.

This happens seconds after I change the MTU. I remember it used to 
happen with 1500 as well, before the gmac tweaks. It makes sense

[  111.111639] NETDEV WATCHDOG: eth0 (rk_gmac-dwmac): transmit queue 0 
timed out
[  111.112374] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:461 
dev_watchdog+0x2b8/0x2c0
[  111.113112] Modules linked in: lz4 lz4_compress zram rockchipdrm 
realtek analogix_dp dw_mipi_dsi dw_hdmi cec rc_core dwmac_rk 
stmmac_platform drm_kms_helper stmmac rtc_rk808 drm rockchip_thermal 
drm_panel_orientation_quirks dw_wdt syscopyarea sysfillrect sysimgblt 
fb_sys_fops ip6t_REJECT nf_reject_ipv6 xt_hl ip6t_rt ipt_REJECT 
xt_recent xt_multiport xt_comment xt_limit xt_addrtype xt_conntrack 
ip6table_filter ip6_tables nf_conntrack_netbios_ns 
nf_conntrack_broadcast nf_nat_ftp nf_nat nf_conntrack_ftp nf_conntrack 
nf_defrag_ipv6 nf_defrag_ipv4 iptable_filter bpfilter tcp_bbr tcp_lp 
nfsd
[  111.117725] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
5.1.0-rc4-1-ARCH #1
[  111.118326] Hardware name: Firefly roc-rk3328-cc (DT)
[  111.118778] pstate: 20000005 (nzCv daif -PAN -UAO)
[  111.119204] pc : dev_watchdog+0x2b8/0x2c0
[  111.119568] lr : dev_watchdog+0x2b8/0x2c0
[  111.119928] sp : ffff000010003d50
[  111.120223] x29: ffff000010003d50 x28: ffff00001166a000
[  111.120695] x27: 0000000000000140 x26: 00000000ffffffff
[  111.121166] x25: 0000000000000000 x24: 0000000000000000
[  111.121635] x23: ffff8000383c4480 x22: 0000000000000001
[  111.122104] x21: ffff000011687000 x20: ffff8000383c4000
[  111.122579] x19: 0000000000000000 x18: 0000000000000010
[  111.123052] x17: 0000000000000000 x16: 0000000000000000
[  111.123520] x15: ffffffffffffffff x14: ffff00001168d6c8
[  111.123995] x13: ffff000090003a67 x12: ffff000010003a6f
[  111.124470] x11: ffff0000116b7000 x10: ffff0000100039f0
[  111.124938] x9 : 00000000ffffffd0 x8 : ffff0000108395d0
[  111.125411] x7 : 0000000000000179 x6 : 0000000000000001
[  111.125885] x5 : 0000000000000000 x4 : 0000000000000001
[  111.126359] x3 : 0000000000000000 x2 : ffff00001168ea58
[  111.126828] x1 : 576938cacdaf1700 x0 : 0000000000000000
[  111.127309] Call trace:
[  111.127533]  dev_watchdog+0x2b8/0x2c0
[  111.127864]  call_timer_fn+0x34/0x170
[  111.128195]  expire_timers.part.5+0xc8/0x158
[  111.128578]  run_timer_softirq+0xd4/0x218
[  111.128940]  __do_softirq+0x130/0x330
[  111.129273]  irq_exit+0xc0/0xd0
[  111.129560]  __handle_domain_irq+0x70/0xc0
[  111.129924]  gic_handle_irq+0x58/0xa8
[  111.130255]  el1_irq+0xb8/0x140
[  111.130542]  arch_cpu_idle+0x3c/0x1c8
[  111.130874]  default_idle_call+0x38/0x40
[  111.131226]  do_idle+0x23c/0x2b8
[  111.131513]  cpu_startup_entry+0x2c/0x30
[  111.131868]  rest_init+0xb8/0xc4
[  111.132161]  arch_call_rest_init+0x14/0x1c
[  111.132529]  start_kernel+0x484/0x4b0
[  111.132862] ---[ end trace 7efe323f823d5938 ]---
[  111.133352] rk_gmac-dwmac ff540000.ethernet eth0: Reset adapter.
[  111.134637] rk_gmac-dwmac ff540000.ethernet eth0: Link is Down
[  111.138412] RTL8211E Gigabit Ethernet stmmac-0:00: attached PHY 
driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=stmmac-0:00, 
irq=POLL)
[  111.151252] rk_gmac-dwmac ff540000.ethernet eth0: No Safety Features 
support found
[  111.151962] rk_gmac-dwmac ff540000.ethernet eth0: PTP not supported 
by HW
[  115.352150] rk_gmac-dwmac ff540000.ethernet eth0: Link is Up - 
1Gbps/Full - flow control rx/tx
[  115.352965] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready

I also have another one that happened while wireguard was loaded:
I brought down eth0. changed the mtu, and brought ot up again.

Απρ 11 16:06:11.411210 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Down
Απρ 11 16:06:38.621145 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:00: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
Απρ 11 16:06:38.641228 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: No Safety Features support found
Απρ 11 16:06:38.642602 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: PTP not supported by HW
Απρ 11 16:06:45.901192 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
Απρ 11 16:06:45.902237 renegade kernel: IPv6: 
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Απρ 11 16:08:41.781166 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Down
Απρ 11 16:08:54.791163 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:00: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
Απρ 11 16:08:54.801388 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: No Safety Features support found
Απρ 11 16:08:54.802780 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: PTP not supported by HW
Απρ 11 16:09:02.061182 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
Απρ 11 16:09:02.062327 renegade kernel: IPv6: 
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Απρ 11 16:09:03.214823 renegade kernel: ------------[ cut here 
]------------
Απρ 11 16:09:03.215300 renegade kernel: refcount_t: underflow; 
use-after-free.
Απρ 11 16:09:03.215472 renegade kernel: WARNING: CPU: 2 PID: 0 at 
lib/refcount.c:190 refcount_sub_and_test_checked+0xc8/0xd8
Απρ 11 16:03:25.240754 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet: TX Checksum insertion supported
Απρ 11 16:03:25.242650 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet: Wake-Up On Lan supported
Απρ 11 16:03:25.244155 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet: Normal descriptors
Απρ 11 16:03:25.247336 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet: Ring mode enabled
Απρ 11 16:03:25.248837 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet: Enable RX Mitigation via HW Watchdog Timer
Απρ 11 16:03:25.351198 renegade kernel: rockchip-drm 
display-subsystem: [drm:rockchip_drm_platform_probe [rockchipdrm]] 
*ERROR* No available vop found for display-subsy>
Απρ 11 16:03:25.391228 renegade kernel: libphy: stmmac: probed
Απρ 11 16:03:25.391708 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:00: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
Απρ 11 16:03:25.393255 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:01: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:01, irq=POLL)
Απρ 11 16:03:25.541243 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:00: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
Απρ 11 16:03:25.561202 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: No Safety Features support found
Απρ 11 16:03:25.562458 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: PTP not supported by HW
Απρ 11 16:03:25.563674 renegade kernel: A link change request failed 
with some changes committed already. Interface eth0 may have been left 
with an inconsistent configu>
Απρ 11 16:03:26.841235 renegade kernel: zram: Added device: zram0
Απρ 11 16:03:26.941263 renegade kernel: random: crng init done
Απρ 11 16:03:26.941801 renegade kernel: random: 7 urandom warning(s) 
missed due to ratelimiting
Απρ 11 16:03:27.571191 renegade kernel: phy 
phy-ff450000.syscon:usb2-phy@100.0: charger = USB_DCP_CHARGER
Απρ 11 16:03:27.921230 renegade kernel: zram0: detected capacity 
change from 0 to 1025568768
Απρ 11 16:03:28.813838 renegade kernel: Adding 1001528k swap on 
/dev/zram0.  Priority:32767 extents:1 across:1001528k SSDscFS
Απρ 11 16:03:32.861210 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
Απρ 11 16:03:32.862527 renegade kernel: IPv6: 
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Απρ 11 16:05:25.932408 renegade kernel: wireguard: loading 
out-of-tree module taints kernel.
Απρ 11 16:05:25.936144 renegade kernel: wireguard: WireGuard 
0.0.20190406 loaded. See www.wireguard.com for information.
Απρ 11 16:05:25.936467 renegade kernel: wireguard: Copyright (C) 
2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
Απρ 11 16:05:26.451886 renegade kernel: NFSD: starting 90-second 
grace period (net f0000041)
Απρ 11 16:05:27.252030 renegade kernel: nf_conntrack: default 
automatic helper assignment has been turned off for security reasons 
and CT-based  firewall rule not found>
Απρ 11 16:06:11.411210 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Down
Απρ 11 16:06:38.621145 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:00: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
Απρ 11 16:06:38.641228 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: No Safety Features support found
Απρ 11 16:06:38.642602 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: PTP not supported by HW
Απρ 11 16:06:45.901192 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
Απρ 11 16:06:45.902237 renegade kernel: IPv6: 
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Απρ 11 16:08:41.781166 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Down
Απρ 11 16:08:54.791163 renegade kernel: RTL8211E Gigabit Ethernet 
stmmac-0:00: attached PHY driver [RTL8211E Gigabit Ethernet] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
Απρ 11 16:08:54.801388 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: No Safety Features support found
Απρ 11 16:08:54.802780 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: PTP not supported by HW
Απρ 11 16:09:02.061182 renegade kernel: rk_gmac-dwmac 
ff540000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
Απρ 11 16:09:02.062327 renegade kernel: IPv6: 
ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Απρ 11 16:09:03.214823 renegade kernel: ------------[ cut here 
]------------
Απρ 11 16:09:03.215300 renegade kernel: refcount_t: underflow; 
use-after-free.
Απρ 11 16:09:03.215472 renegade kernel: WARNING: CPU: 2 PID: 0 at 
lib/refcount.c:190 refcount_sub_and_test_checked+0xc8/0xd8
Απρ 11 16:09:03.215603 renegade kernel: Modules linked in: 
iptable_nat ipt_MASQUERADE wireguard(O) ip6_udp_tunnel udp_tunnel lz4 
lz4_compress zram realtek rockchipdrm a>
Απρ 11 16:09:03.215845 renegade kernel: CPU: 2 PID: 0 Comm: 
swapper/2 Tainted: G           O      5.1.0-rc4-1-ARCH #1
Απρ 11 16:09:03.215959 renegade kernel: Hardware name: Firefly 
roc-rk3328-cc (DT)
Απρ 11 16:09:03.216060 renegade kernel: pstate: 80000005 (Nzcv daif 
-PAN -UAO)
Απρ 11 16:09:03.216160 renegade kernel: pc : 
refcount_sub_and_test_checked+0xc8/0xd8
Απρ 11 16:09:03.216256 renegade kernel: lr : 
refcount_sub_and_test_checked+0xc8/0xd8
Απρ 11 16:09:03.216344 renegade kernel: sp : ffff000010013ca0
Απρ 11 16:09:03.216434 renegade kernel: x29: ffff000010013ca0 x28: 
ffff80003406c8c0
Απρ 11 16:09:03.216550 renegade kernel: x27: ffff80002792d100 x26: 
ffff80003406c8c0
Απρ 11 16:09:03.216648 renegade kernel: x25: 0000000000000060 x24: 
0000000000000000
Απρ 11 16:09:03.216734 renegade kernel: x23: 000000000000014b x22: 
0000000000000001
Απρ 11 16:09:03.216826 renegade kernel: x21: 0000000000000004 x20: 
0000000000000000
Απρ 11 16:09:03.216922 renegade kernel: x19: ffff80002792d100 x18: 
0000000000000010
Απρ 11 16:09:03.217014 renegade kernel: x17: 0000000000000000 x16: 
0000000000000000
Απρ 11 16:09:03.217109 renegade kernel: x15: ffffffffffffffff x14: 
0720072007200720
Απρ 11 16:09:03.217197 renegade kernel: x13: 0720072007200720 x12: 
0720072007200720
Απρ 11 16:09:03.217283 renegade kernel: x11: 0720072007200720 x10: 
0720072007200720
Απρ 11 16:09:03.217427 renegade kernel: x9 : 0720072007200720 x8 : 
072007200720072e
Απρ 11 16:09:03.217537 renegade kernel: x7 : 0000000000000184 x6 : 
0000000000000001
Απρ 11 16:09:03.217637 renegade kernel: x5 : 0000000000000000 x4 : 
0000000000000001
Απρ 11 16:09:03.217740 renegade kernel: x3 : 0000000000000007 x2 : 
0000000000000007
Απρ 11 16:09:03.217833 renegade kernel: x1 : 576938cacdaf1700 x0 : 
0000000000000000
Απρ 11 16:09:03.217928 renegade kernel: Call trace:
Απρ 11 16:09:03.218016 renegade kernel:  
refcount_sub_and_test_checked+0xc8/0xd8

> If it's a queue timeout then please share your "dmesg | grep -i 
> stmmac"
> since boot.

The only time I see it mentioned is basically when the link is brought 
up

$ dmesg | grep -i stmmac
[  +0,134955] libphy: stmmac: probed
[  +0,000352] RTL8211E Gigabit Ethernet stmmac-0:00: attached PHY 
driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=stmmac-0:00, 
irq=POLL)
[  +0,001253] RTL8211E Gigabit Ethernet stmmac-0:01: attached PHY 
driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=stmmac-0:01, 
irq=POLL)
[  +0,256702] RTL8211E Gigabit Ethernet stmmac-0:00: attached PHY 
driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=stmmac-0:00, 
irq=POLL)

> This is not a workaround neither an issue. It's well stablished that 
> PBL
> setting interferes with COE so one must choose a setting that depends 
> on
> FIFO size. I would like to make it automatic in the driver but I 
> didn't
> have the time to submit a patch yet, sorry.
> 
> Thanks,
> Jose Miguel Abreu

When you say FIFO size, is that related to the tx/rx_delay?
Jose Abreu April 15, 2019, 8:15 a.m. UTC | #22
From: Leonidas P. Papadakos <papadakospan@gmail.com>
Date: Fri, Apr 12, 2019 at 12:13:52

> 
> > Can you please share the stacktrace here ? I can't access pastebin 
> > due to
> > corporate policy.
> 
> This happens seconds after I change the MTU. I remember it used to 
> happen with 1500 as well, before the gmac tweaks. It makes sense
>

Notice that incorrect settings of PBL can cause Queue timeout. Can you 
try with PBL = 0x1 and no-pbl-x8 and let me know if the timeout still 
happens?
 
> 
> When you say FIFO size, is that related to the tx/rx_delay?

No, FIFO size is the Queue FIFO size that stores packets after DMA 
fetches them.

Thanks,
Jose Miguel Abreu
Leonidas P. Papadakos April 15, 2019, 9:45 p.m. UTC | #23
> Notice that incorrect settings of PBL can cause Queue timeout. Can you
> try with PBL = 0x1 and no-pbl-x8 and let me know if the timeout still
> happens?

Seems like the timeout (and subsequent stacktrace) doesn't happen when 
using those options.
That said, with those options the maximum transmit to laptop is 185 Mbps
Leonidas P. Papadakos April 15, 2019, 10:19 p.m. UTC | #24
So far with iperf testing

txpbl <0x20>
and no-pbl-x8 gives me the best tx I've seen so far:
914 average, peak 933 Mbps
Jose Abreu April 16, 2019, 8:01 a.m. UTC | #25
From: Leonidas P. Papadakos <papadakospan@gmail.com>
Date: Mon, Apr 15, 2019 at 23:19:19

> 
> So far with iperf testing
> 
> txpbl <0x20>
> and no-pbl-x8 gives me the best tx I've seen so far:
> 914 average, peak 933 Mbps

Cool. If you want to do more tests then the next step is to remove the 
no-pbl-x8 and start with pbl = 0x1, then 0x2, 0x4, 0x8, 0x10, 0x20. 
Without the pblx8 binding then PBL setting will be multiplied by 8.

Do not try to use PBL values different than the above because it can lead 
to undefined behavior.

Anyway, I think the performance values you presented are good.

Thanks,
Jose Miguel Abreu
Leonidas P. Papadakos April 16, 2019, 10:03 a.m. UTC | #26
txpbl <0x4> (without the no-pbl-x8 option) is the last value where I 
can change the MTU without a timeout.
I've also tried <0x20> with and without no-pbl-x8 but in that area MTU 
is a hot potato.

Seems pretty good though, I'll keep poking to see if it's stable.
currently at txbpl <0x4> by iself I get tx max 934 average 916 Mbps

IT does fluctuate at times but mostly there.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6a2e1031a..4552147e9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3660,6 +3660,10 @@  static netdev_features_t stmmac_fix_features(struct net_device *dev,
 	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
 		features &= ~NETIF_F_CSUM_MASK;
 
+	/* Including very small MTUs of 1498 for Rockchip devices */
+	if (priv->plat->bugged_tx_coe && (dev->mtu > ETH_DATA_LEN - 2))
+		features &= ~NETIF_F_CSUM_MASK;
+
 	/* Disable tso if asked by ethtool */
 	if ((priv->plat->tso_en) && (priv->dma_cap.tsoen)) {
 		if (features & NETIF_F_TSO)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 3031f2bf1..807cf5826 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -519,6 +519,8 @@  stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 		pr_warn("force_sf_dma_mode is ignored if force_thresh_dma_mode is set.");
 	}
 
+	plat->bugged_tx_coe = of_property_read_bool(np, "rockchip,bugged_tx_coe");
+
 	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
 
 	plat->axi = stmmac_axi_setup(pdev);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 4335bd771..60c411f43 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -162,6 +162,7 @@  struct plat_stmmacenet_data {
 	int pmt;
 	int force_sf_dma_mode;
 	int force_thresh_dma_mode;
+	int bugged_tx_coe;
 	int riwt_off;
 	int max_speed;
 	int maxmtu;