linux-kernel - Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed changes after linking up

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: Fri, 31 May 2024 16:23:19 +0800
From: xiaolei wang <xiaolei.wang@...driver.com>
To: Vladimir Oltean <olteanv@...il.com>,
        "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, alexandre.torgue@...s.st.com,
        joabreu@...opsys.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, mcoquelin.stm32@...il.com,
        netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [net v2 PATCH] net: stmmac: Update CBS parameters when speed
 changes after linking up


On 5/30/24 22:35, Vladimir Oltean wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Thu, May 30, 2024 at 03:14:48PM +0100, Russell King (Oracle) wrote:
>>> I don't see why the CBS parameters would need to be de-programmed from
>>> hardware on a link down event. Is that some stmmac specific thing?
>> If the driver is having to do computation on the parameters based on
>> the link speed, then when the link speed changes, the parameters
>> no longer match what the kernel _thinks_ those parameters were
>> programmed with.
>>
>> What I'm trying to get over to you is that what you propose causes
>> an inconsistency between how the hardware is _programmed_ to behave
>> for CBS and what the kernel reports the CBS settings are if the
>> link speed changes.
>>
>> It's all very well saying that userspace should basically reconstruct
>> the tc settings when the link changes, but not everyone is aware of
>> that. I'm saying it's a problem if one isn't aware of this issue with
>> this hardware, and one looks at tc qdisc show output, and assumes
>> that reflects what is actually being used when it isn't.
>>
>> It's quality of implmentation - as far as I'm concerned, the kernel
>> should *not* mislead the user like this.
> I was saying that the tc-cbs parameters input into the kernel should
> already have the link speed baked into them:
> portTransmitRate = idleSlope - sendSlope. In theory one could feed any
> data into the kernel, but this is based on the IEEE 802.1Q formulas.
>
> I had missed the fact that there is a calculation dependent on
> priv->speed within tc_setup_cbs(), and I'm sorry for that. I thought
> that the values were passed unaltered down to stmmac_config_cbs(). So
> "make no change to the driver" is no longer my recommendation.
>
> In that case, my recommendation is to do as sja1105_setup_tc_cbs() does:
> replace priv->speed with the portTransmitRate recovered from the tc-cbs
> parameters, and fully expect that when the link speed changes, user
> space comes along and changes those parameters.

I will try it

thanks

xiaolei