netdev - Re: [PATCH net 1/6] igc: Rename qbv_enable to taprio_offload

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: <275f1916-3f23-45e5-ae4d-a5d47e75e452@linutronix.de>
Date: Tue, 11 Jul 2023 09:51:34 +0200
From: Florian Kauer <florian.kauer@...utronix.de>
To: Leon Romanovsky <leon@...nel.org>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
 netdev@...r.kernel.org, kurt@...utronix.de, vinicius.gomes@...el.com,
 muhammad.husaini.zulkifli@...el.com, tee.min.tan@...ux.intel.com,
 aravindhan.gunasekaran@...el.com, sasha.neftin@...el.com,
 Naama Meir <naamax.meir@...ux.intel.com>
Subject: Re: [PATCH net 1/6] igc: Rename qbv_enable to taprio_offload_enable

On 11.07.23 09:32, Leon Romanovsky wrote:
> On Tue, Jul 11, 2023 at 09:18:31AM +0200, Florian Kauer wrote:
>> Hi Leon,
>>
>> On 11.07.23 09:01, Leon Romanovsky wrote:
>>> On Mon, Jul 10, 2023 at 09:34:58AM -0700, Tony Nguyen wrote:
>>>> From: Florian Kauer <florian.kauer@...utronix.de>
>>>>
>>>> In the current implementation the flags adapter->qbv_enable
>>>> and IGC_FLAG_TSN_QBV_ENABLED have a similar name, but do not
>>>> have the same meaning. The first one is used only to indicate
>>>> taprio offload (i.e. when igc_save_qbv_schedule was called),
>>>> while the second one corresponds to the Qbv mode of the hardware.
>>>> However, the second one is also used to support the TX launchtime
>>>> feature, i.e. ETF qdisc offload. This leads to situations where
>>>> adapter->qbv_enable is false, but the flag IGC_FLAG_TSN_QBV_ENABLED
>>>> is set. This is prone to confusion.
>>>>
>>>> The rename should reduce this confusion. Since it is a pure
>>>> rename, it has no impact on functionality.
>>>
>>> And shouldn't be sent to net, but to net-next.> 
>>> Thanks
>>
>> In principle I fully agree that sole renames are not intended for net.
>> But in this case the rename is tightly coupled with the other patches
>> of the series, not only due to overlapping code changes, but in particular
>> because the naming might very likely be one root cause of the regressions.
> 
> I understand the intention, but your second patch showed that rename was
> premature.
> 
> Thanks

The second patch does not touch the rename in igc.h and igc_tsn.c...
(and the latter is from the context probably the most relevant one)
But I see what you mean. I am fine with both squashing and keeping it separate,
but I have no idea how the preferred process is since this
is already so far through the pipeline...

Thanks,
Florian