netdev - Re: [PATCH net-next v9 2/2] net: ti: icssg_prueth: add TAPRIO offload support

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]

Message-ID: <f81178c5-313e-47f6-bfe0-7e21a7f89a90@ti.com>
Date: Fri, 7 Jun 2024 10:29:12 +0530
From: "Anwar, Md Danish" <a0501179@...com>
To: Vladimir Oltean <vladimir.oltean@....com>,
        MD Danish Anwar
	<danishanwar@...com>
CC: Jan Kiszka <jan.kiszka@...mens.com>,
        Dan Carpenter
	<dan.carpenter@...aro.org>,
        Andrew Lunn <andrew@...n.ch>, Simon Horman
	<horms@...nel.org>,
        Diogo Ivo <diogo.ivo@...mens.com>,
        Wolfram Sang
	<wsa+renesas@...g-engineering.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Niklas Schnelle <schnelle@...ux.ibm.com>,
        Vignesh Raghavendra
	<vigneshr@...com>,
        Richard Cochran <richardcochran@...il.com>,
        Roger Quadros
	<rogerq@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski
	<kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
        "David S. Miller"
	<davem@...emloft.net>,
        <linux-arm-kernel@...ts.infradead.org>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <srk@...com>,
        Jacob
 Keller <jacob.e.keller@...el.com>,
        Roger Quadros <rogerq@...com>
Subject: Re: [PATCH net-next v9 2/2] net: ti: icssg_prueth: add TAPRIO offload
 support



On 6/3/2024 7:35 PM, Vladimir Oltean wrote:
> On Mon, Jun 03, 2024 at 04:51:00PM +0300, Vladimir Oltean wrote:
>>>>> +static void tas_reset(struct prueth_emac *emac)
>>>>> +{
>>>>> +	struct tas_config *tas = &emac->qos.tas.config;
>>>>> +	int i;
>>>>> +
>>>>> +	for (i = 0; i < TAS_MAX_NUM_QUEUES; i++)
>>>>> +		tas->max_sdu_table.max_sdu[i] = 2048;
>>>>
>>>> Macro + short comment for the magic number, please.
>>>>
>>>
>>> Sure I will add it. Each elements in this array is a 2 byte value
>>> showing the maximum length of frame to be allowed through each gate.
>>
>> Is the queueMaxSDU[] array active even with the TAS being in the reset
>> state? Does this configuration have any impact upon the device MTU?
>> I don't know why 2048 was chosen.
> 
> Another comment here is: in the tc-taprio UAPI, a max-sdu value of 0
> is special and means "no maxSDU limit for this TX queue". You are
> programming the values from taprio straight away to hardware, so,
> assuming there's no bug there, it means that the hardware also
> understands 0 to mean "no maxSDU limit".
> 

I discussed this with the firmware team. They are not treating 0 as
something special (""no maxSDU limit for this TX queue"). They have
limit on every queue. Driver needs to handle the max-sdu size carefully.

> If so, then during tas_reset(), after which the TAS should be disabled,
> why aren't you also using 0 as a default value, but 2048?

As using 0 doesn't mean anything special in firmware. The default value
during reset is kept as the max supported value.

There's also one thing missing here, the max-sdu table in firmware is
updated (by calling tas_update_maxsdu_table()) only once by driver
during tas_reset(). The firmware table should also be updated once
before triggering the list change so that the firmware would know what
are the max-sdu value that user has requested.

If a user request max-sdu as 0 0 0 80 for 4 queues. The driver will
update these values to firmware as PRUETH_MAX_MTU, PRUETH_MAX_MTU,
PRUETH_MAX_MTU, 80.


-- 
Thanks and Regards,
Md Danish Anwar