netdev - Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute

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: <jgebk37r4xs6w4526hjc5u6r7oudanb5ce7v4xlaw2tcswtycx@cvmxkwxvkpek>
Date: Wed, 29 Oct 2025 15:20:41 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Ivan Vecera <ivecera@...hat.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, 
	Michal Schmidt <mschmidt@...hat.com>, Petr Oros <poros@...hat.com>, 
	Prathosh Satish <Prathosh.Satish@...rochip.com>, Vadim Fedorenko <vadim.fedorenko@...ux.dev>, 
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>, Jonathan Corbet <corbet@....net>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Donald Hunter <donald.hunter@...il.com>, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute

Wed, Oct 29, 2025 at 08:44:52AM +0100, ivecera@...hat.com wrote:
>Hi Kuba,
>
>On 10/29/25 2:39 AM, Jakub Kicinski wrote:
>> On Fri, 24 Oct 2025 16:49:26 +0200 Ivan Vecera wrote:
>> > +      -
>> > +        name: phase-adjust-gran
>> > +        type: s32
>> > +        doc: |
>> > +          Granularity of phase adjustment, in picoseconds. The value of
>> > +          phase adjustment must be a multiple of this granularity.
>> 
>> Do we need this to be signed?
>> 
>To have it unsigned brings a need to use explicit type casting in the core
>and driver's code. The phase adjustment can be both positive and
>negative it has to be signed. The granularity specifies that adjustment
>has to be multiple of granularity value so the core checks for zero
>remainder (this patch) and the driver converts the given adjustment
>value using division by the granularity.
>
>If we would have phase-adjust-gran and corresponding structure fields
>defined as u32 then we have to explicitly cast the granularity to s32
>because for:

I prefer cast. The uapi should be clear. There is not point of having
negative granularity.


>
><snip>
>s32 phase_adjust, remainder;
>u32 phase_gran = 1000;
>
>phase_adjust = 5000;
>remainder = phase_adjust % phase_gran;
>/* remainder = 0 -> OK for positive adjust */
>
>phase_adjust = -5000;
>remainder = phase_adjust % phase_gran;
>/* remainder = 296
> * Wrong for negative adjustment because phase_adjust is casted to u32
> * prior division -> 2^32 - 5000 = 4294962296.
> * 4294962296 % 1000 = 296
> */
>
> remainder = phase_adjust % (s32)phase_gran;
> /* remainder = 0
>  * Now OK because phase_adjust remains to be s32
>  */
></snip>
>
>Similarly for division in the driver code if the granularity would be
>u32.
>
>So I have proposed phase adjustment granularity to be s32 to avoid these
>explicit type castings and potential bugs in drivers.

Cast in dpll core, no?


>
>Thanks,
>Ivan
>