netdev - Re: [PATCH net v2 2/2] net/sched: act_gate: fix configuration of the periodic timer

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: <20200615.180022.2063479179425015644.davem@davemloft.net>
Date:   Mon, 15 Jun 2020 18:00:22 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     olteanv@...il.com
Cc:     dcaratti@...hat.com, Po.Liu@....com, xiyou.wangcong@...il.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net v2 2/2] net/sched: act_gate: fix configuration of
 the periodic timer

From: Vladimir Oltean <olteanv@...il.com>
Date: Tue, 16 Jun 2020 00:55:50 +0300

> On Mon, 15 Jun 2020 at 22:33, Davide Caratti <dcaratti@...hat.com> wrote:
>>
>> assigning a dummy value of 'clock_id' to avoid cancellation of the cycle
>> timer before its initialization was a temporary solution, and we still
>> need to handle the case where act_gate timer parameters are changed by
>> commands like the following one:
>>
>>  # tc action replace action gate <parameters>
>>
>> the fix consists in the following items:
>>
>> 1) remove the workaround assignment of 'clock_id', and init the list of
>>    entries before the first error path after IDR atomic check/allocation
>> 2) validate 'clock_id' earlier: there is no need to do IDR atomic
>>    check/allocation if we know that 'clock_id' is a bad value
>> 3) use a dedicated function, 'gate_setup_timer()', to ensure that the
>>    timer is cancelled and re-initialized on action overwrite, and also
>>    ensure we initialize the timer in the error path of tcf_gate_init()
>>
>> v2: avoid 'goto' in gate_setup_timer (thanks to Cong Wang)
>>
> 
> The change log is put under the 3 '---' characters for a reason: it is
> relevant only to reviewers, and git automatically trims it when
> applying the patch. The way it is now, the commit message would
> contain this line about "v2 ...".

I completely disagree and I ask submitters of networking changes to keep
the changelog in the commit message.

Later people will look at this commit and ask "why didn't they do X
or Y" and if the changelog shows that the submitter was asked not to
do X or Y that is useful information.