netdev - Re: [PATCH V1 net-next] net: stmmac: should not modify RX descriptor when STMMAC resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 May 2021 14:38:28 +0100
From:   Jon Hunter <jonathanh@...dia.com>
To:     Joakim Zhang <qiangqing.zhang@....com>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "peppe.cavallaro@...com" <peppe.cavallaro@...com>,
        "alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "treding@...dia.com" <treding@...dia.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH V1 net-next] net: stmmac: should not modify RX descriptor
 when STMMAC resume


On 27/05/2021 14:13, Joakim Zhang wrote:

...

>> So as previously mentioned this still causing a regression when resuming from
>> suspend on Jetson TX2 platform. I am not sure why you are still attempting to
>> push this patch as-is when it causes a complete failure for another platform. I
>> am quite disappointed that you are ignoring the issue we have reported :-(
> I first pushed the RFC and discussed about the issue, I think this patch trigger a potential issue at your side. 
> IMHO, you may need try to find the root case why this patch make regression on your platform.

I am always happy to test patches, but I am not sure it is completely
fair to ask us to debug an issue your patch is introducing. Yes this
could be uncovering an underlying issue with the driver, but that does
not mean we can introduce regressions.

TBH where are the maintainers of this driver? We really need their
inputs to help us figure this out.

>> To summarise we do not see any issues with suspend on Jetson TX2 without
>> this patch. I have stressed suspend on this board doing 2000 suspend iterations
>> and so no issues. However, this patch completely breaks resuming from
>> suspend for us. Therefore, I don't see how we can merge this.
> If you read the commit message, you should know you can't reproduce this issue if your DMA bit width is 32 bits.
> Please take the commit message seriously, do you admit this is a real bug? After the analysis, this may a common issue.

I did, but it seems that you are OK to break devices with 32-bit DMAs.
If it breaks Jetson then it could also break others but you don't know
that yet. I am not saying there is not a bug, but I am saying that you
cannot attempt to fix it, by causing regressions for other platforms.

>> Given that this fixes a problem, that appears to be specific to your platform,
>> why do you not implement this in away such that this is only done for your
>> platform?
> I really don't know how to take your case into account, let us add a flag in "struct plat_stmmacenet_data" to 
> separate different cases? If maintainer agree with this, I can do.
> 
> I agree keep this patch doesn't merge until a way also fit your platform. If there is still no good solutions, I will try to add
> a specific flag in platform data.

OK, so I am not sure why you are trying to push this patch again without
coming to a resolution with known issues this change is causing. And
that is the problem I have with this.

Jon

-- 
nvpublic