linux-kernel - RE: [PATCH V2 net-next] net: fec: add CBS 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: <DB9PR04MB81062C8AA6FDBEA110094A2F88A19@DB9PR04MB8106.eurprd04.prod.outlook.com>
Date:   Fri, 17 Feb 2023 02:18:10 +0000
From:   Wei Fang <wei.fang@....com>
To:     Alexander Lobakin <aleksander.lobakin@...el.com>
CC:     Shenwei Wang <shenwei.wang@....com>,
        Clark Wang <xiaoning.wang@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "simon.horman@...igine.com" <simon.horman@...igine.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V2 net-next] net: fec: add CBS offload support



> -----Original Message-----
> From: Alexander Lobakin <aleksander.lobakin@...el.com>
> Sent: 2023年2月16日 23:28
> To: Wei Fang <wei.fang@....com>
> Cc: Shenwei Wang <shenwei.wang@....com>; Clark Wang
> <xiaoning.wang@....com>; davem@...emloft.net; edumazet@...gle.com;
> kuba@...nel.org; pabeni@...hat.com; simon.horman@...igine.com;
> andrew@...n.ch; netdev@...r.kernel.org; dl-linux-imx <linux-imx@....com>;
> linux-kernel@...r.kernel.org
> Subject: Re: [PATCH V2 net-next] net: fec: add CBS offload support
> 
> From: Wei Fang <wei.fang@....com>
> Date: Thu, 16 Feb 2023 13:03:37 +0000
> 
> >
> >> -----Original Message-----
> >> From: Alexander Lobakin <alexandr.lobakin@...el.com>
> >> Sent: 2023年2月15日 0:49
> >> To: Wei Fang <wei.fang@....com>
> >> Cc: Shenwei Wang <shenwei.wang@....com>; Clark Wang
> >> <xiaoning.wang@....com>; davem@...emloft.net;
> edumazet@...gle.com;
> >> kuba@...nel.org; pabeni@...hat.com; simon.horman@...igine.com;
> >> andrew@...n.ch; netdev@...r.kernel.org; dl-linux-imx
> >> <linux-imx@....com>; linux-kernel@...r.kernel.org
> >> Subject: Re: [PATCH V2 net-next] net: fec: add CBS offload support
> >>
> >> From: Wei Fang <wei.fang@....com>
> >> Date: Tue, 14 Feb 2023 09:34:09 +0000
> 
> [...]
> >>>> Oh okay. Then rounddown_pow_of_two() is what you're looking for.
> >>>>
> >>>> 	power = rounddown_pow_of_two(idle_slope);
> >>>>
> >>>> Or even just use one variable, @idle_slope.
> >>>>
> >>> Thanks for the reminder, I think I should use roundup_pow_of_two().
> >>
> >> But your code does what rounddown_pow_of_two() does, not roundup.
> >> Imagine that you have 0b1111, then your code will turn it into
> >> 0b1000, not 0b10000. Or am I missing something?
> >>
> > 0b1111 is nearest to 0b10000, so it should be turned into 0x10000.
> 
> fls() + BIT() won't give you the *nearest* pow-2, have you checked what your
> code does return? Check with 0xff and then 0x101 and you'll be surprised, it
> doesn't work that way.
> 
My real intention is, if x >= 1.5 * 2 ^ (fls(x) - 1), then x = 2 ^ fls(x), otherwise,
x = 2 ^ (fls(x) - 1). Anyway, I'll check the final implementation can meet my
expectation.

> I'd highly suggest you introducing not only round_closest(), but also
> round_closest_pow_of_two(), as your driver might not be the sole user of such
> generic functionality.
> 
Yes, I agree with you, it's better to use the generic functionality.

> >
> >>>
> >>>>> +	idle_slope = DIV_ROUND_CLOSEST(idle_slope, power) * power;
> >>>>> +
> >>>>> +	return idle_slope;
> >>>>
> >>>> You can return DIV_ROUND_ ... right away, without assignning first.
> >>>> Also, I'm thinking of that this might be a generic helper. We have
> >>>> roundup() and rounddown(), this could be something like
> "round_closest()"?
> [...]
> 
> Thanks,
> Olek