linux-kernel - Re: [PATCH v2 3/5] thermal: core: Remove old uapi generic netlink

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: <3fa94347-8392-5324-da0a-91ff97f6ecfc@linaro.org>
Date:   Wed, 1 Jul 2020 14:13:52 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Amit Kucheria <amit.kucheria@...aro.org>,
        Linux PM list <linux-pm@...r.kernel.org>
Cc:     Zhang Rui <rui.zhang@...el.com>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Ram Chandrasekar <rkumbako@...eaurora.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/5] thermal: core: Remove old uapi generic netlink

On 01/07/2020 14:10, Amit Kucheria wrote:
> On Wed, Jul 1, 2020 at 3:15 PM Daniel Lezcano <daniel.lezcano@...aro.org> wrote:
>>
>> On 01/07/2020 11:33, Amit Kucheria wrote:
>>> On Wed, Jul 1, 2020 at 2:56 PM Daniel Lezcano <daniel.lezcano@...aro.org> wrote:
>>>>
>>>> On 30/06/2020 13:47, Amit Kucheria wrote:
>>>>> On Thu, Jun 25, 2020 at 8:15 PM Daniel Lezcano
>>>>> <daniel.lezcano@...aro.org> wrote:
>>>>>>
> 
>>>>>>  /* Adding event notification support elements */
>>>>>>  #define THERMAL_GENL_FAMILY_NAME                "thermal_event"
>>>>>> -#define THERMAL_GENL_VERSION                    0x01
>>>>>> +#define THERMAL_GENL_VERSION                    0x02
>>>>>
>>>>> This hunk should be removed since you set version back to 1 in the
>>>>> next patch and we don't actually intend to bump the version yet.
>>>>
>>>> Well, I've been very strict here for git-bisecting.
>>>>
>>>> I move to V2 because of the removal, but when adding the new genetlink
>>>> code, the family name changed, so we returned back to the V1 as it is a
>>>> new genetlink thermal brand.
>>>
>>> I don't understand the move to v2 for an empty skeleton UAPI. For the
>>> purposes of bisection, couldn't you just remove all the v1 UAPI (w/o
>>> bumping to v2) and then add a new UAPI in the next patch?
>>>
>>>> The name is change because it is no longer event based but also sampling
>>>> and commands.
>>>
>>> In this case, just to avoid any confusion, the new UAPI could be v2
>>> making the transition clear in case of bisection.
>>>
>>> I'm afraid the v1->v2->v1 is a bit more confusing.
>>
>> Let me elaborate a bit:
>>
>> Why there is this patch ?
>> - By removing this code first, the next patch will just contain
>> additions, I thought it would be clearer
>>
>> Why increase the version here ?
>> - Code must continue to compile and as the 'thermal_event' family is now
>> different from V1, the version is changed
>>
>> Why the version goes to V1 in the next patch ?
>> - The family name is changed as it is not doing event only, so it is a
>> new netlink thermal protocol and we begin at V1
>>
>> So the main reason of this patch is to be very strict in the iteration
>> changes. May be it is too much, in this case I can merge this patch with
>> 4/5, the old netlink protocol removal will be lost in the addition of
>> the new protocol. I'm fine with that if you think it is simpler.
> 
> Considering that there are no users of v1 currently, it feels a bit
> over engineered, IMHO.
> 
> Also, the new UAPI doesn't need to begin at v1. Just having it start
> at v2 will avoid this confusion, no?

Ok, I will merge both patches but I will keep the V1 because the netlink
protocol is a new one.


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog