| Message ID | 20220326172051.14722-1-thepaulodoom@thepaulodoom.com |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: aes_generic: fixed styling warnings | expand |
On Sat, 26 Mar 2022 at 18:48, Paul Lemmermann <thepaulodoom@thepaulodoom.com> wrote: > > Fixed all styling warnings from the checkpatch.pl script. > > Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com> Did you test this code after 'fixing' it? > --- > crypto/aes_generic.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/crypto/aes_generic.c b/crypto/aes_generic.c > index 27ab27931..322e345ac 100644 > --- a/crypto/aes_generic.c > +++ b/crypto/aes_generic.c > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0+ > /* > * Cryptographic API. > * > @@ -56,7 +57,7 @@ > #include <asm/byteorder.h> > #include <asm/unaligned.h> > > -static inline u8 byte(const u32 x, const unsigned n) > +static inline u8 byte(const u32 x, const unsigned int n) > { > return x >> (n << 3); > } > @@ -325,6 +326,7 @@ __visible const u32 crypto_ft_tab[4][256] ____cacheline_aligned = { > 0x7bcbb0b0, 0xa8fc5454, 0x6dd6bbbb, 0x2c3a1616, > } > }; > +EXPORT_SYMBOL_GPL(crypto_ft_tab); > > static const u32 crypto_fl_tab[4][256] ____cacheline_aligned = { > { > @@ -853,6 +855,7 @@ __visible const u32 crypto_it_tab[4][256] ____cacheline_aligned = { > 0x7b6184cb, 0xd570b632, 0x48745c6c, 0xd04257b8, > } > }; > +EXPORT_SYMBOL_GPL(crypto_it_tab); > > static const u32 crypto_il_tab[4][256] ____cacheline_aligned = { > { > @@ -1118,8 +1121,6 @@ static const u32 crypto_il_tab[4][256] ____cacheline_aligned = { > } > }; > > -EXPORT_SYMBOL_GPL(crypto_ft_tab); > -EXPORT_SYMBOL_GPL(crypto_it_tab); > > /** > * crypto_aes_set_key - Set the AES key. > @@ -1144,34 +1145,34 @@ EXPORT_SYMBOL_GPL(crypto_aes_set_key); > > /* encrypt a block of text */ > > -#define f_rn(bo, bi, n, k) do { \ > +#define f_rn(bo, bi, n, k) while (0) { \ > bo[n] = crypto_ft_tab[0][byte(bi[n], 0)] ^ \ > crypto_ft_tab[1][byte(bi[(n + 1) & 3], 1)] ^ \ > crypto_ft_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ > crypto_ft_tab[3][byte(bi[(n + 3) & 3], 3)] ^ *(k + n); \ > -} while (0) > +} > > -#define f_nround(bo, bi, k) do {\ > +#define f_nround(bo, bi, k) while (0) {\ > f_rn(bo, bi, 0, k); \ > f_rn(bo, bi, 1, k); \ > f_rn(bo, bi, 2, k); \ > f_rn(bo, bi, 3, k); \ > k += 4; \ > -} while (0) > +} > > -#define f_rl(bo, bi, n, k) do { \ > +#define f_rl(bo, bi, n, k) while (0) { \ > bo[n] = crypto_fl_tab[0][byte(bi[n], 0)] ^ \ > crypto_fl_tab[1][byte(bi[(n + 1) & 3], 1)] ^ \ > crypto_fl_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ > crypto_fl_tab[3][byte(bi[(n + 3) & 3], 3)] ^ *(k + n); \ > -} while (0) > +} > > -#define f_lround(bo, bi, k) do {\ > +#define f_lround(bo, bi, k) while (0) {\ > f_rl(bo, bi, 0, k); \ > f_rl(bo, bi, 1, k); \ > f_rl(bo, bi, 2, k); \ > f_rl(bo, bi, 3, k); \ > -} while (0) > +} > > static void crypto_aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) > { > @@ -1214,12 +1215,12 @@ static void crypto_aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) > > /* decrypt a block of text */ > > -#define i_rn(bo, bi, n, k) do { \ > +#define i_rn(bo, bi, n, k) while (0) { \ > bo[n] = crypto_it_tab[0][byte(bi[n], 0)] ^ \ > crypto_it_tab[1][byte(bi[(n + 3) & 3], 1)] ^ \ > crypto_it_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ > crypto_it_tab[3][byte(bi[(n + 1) & 3], 3)] ^ *(k + n); \ > -} while (0) > +} > > #define i_nround(bo, bi, k) do {\ > i_rn(bo, bi, 0, k); \ > @@ -1229,19 +1230,19 @@ static void crypto_aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) > k += 4; \ > } while (0) > > -#define i_rl(bo, bi, n, k) do { \ > +#define i_rl(bo, bi, n, k) while (0) { \ > bo[n] = crypto_il_tab[0][byte(bi[n], 0)] ^ \ > crypto_il_tab[1][byte(bi[(n + 3) & 3], 1)] ^ \ > crypto_il_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ > crypto_il_tab[3][byte(bi[(n + 1) & 3], 3)] ^ *(k + n); \ > -} while (0) > +} > > -#define i_lround(bo, bi, k) do {\ > +#define i_lround(bo, bi, k) while (0) {\ > i_rl(bo, bi, 0, k); \ > i_rl(bo, bi, 1, k); \ > i_rl(bo, bi, 2, k); \ > i_rl(bo, bi, 3, k); \ > -} while (0) > +} > > static void crypto_aes_decrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) > { > -- > 2.35.1 >
(please keep the cc's) On Mon, 28 Mar 2022 at 00:46, Paul Lemmermann <thepaulodoom@thepaulodoom.com> wrote: > > On Sun, Mar 27, 2022 at 01:41:19PM +0200, Ard Biesheuvel wrote: > > On Sat, 26 Mar 2022 at 18:48, Paul Lemmermann > > <thepaulodoom@thepaulodoom.com> wrote: > > > > > > Fixed all styling warnings from the checkpatch.pl script. > > > > > > Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com> > > > > Did you test this code after 'fixing' it? > > > No, I did not. Now that I scrutinized it a bit more, I realized the > kernel coding conventions. Sorry about that, this is my first patch. In that case, welcome! This is not about coding conventions. This is about correctness. For instance, > > > > > > -#define f_nround(bo, bi, k) do {\ > > > +#define f_nround(bo, bi, k) while (0) {\ > > > f_rn(bo, bi, 0, k); \ > > > f_rn(bo, bi, 1, k); \ > > > f_rn(bo, bi, 2, k); \ > > > f_rn(bo, bi, 3, k); \ > > > k += 4; \ > > > -} while (0) > > > +} > > > Why are you making this change, and why do you think it produces the same result? > Can you remove everything in the patch past the section with line > 1144, or do I have to resubit the patch? > checkpatch.pl is a useful tool for finding style issues, but please use it with care. And changing decades old code just to fix issues reported by checkpatch.pl is really just pointless churn. So let's just drop this patch altogether, shall we? If you're interested in helping out, please have a look at the staging/ tree - there is a lot of code there that needs cleaning up. Thanks, Ard.
On Mon, Mar 28, 2022 at 09:39:14AM +0200, Ard Biesheuvel wrote: > (please keep the cc's) > > On Mon, 28 Mar 2022 at 00:46, Paul Lemmermann > <thepaulodoom@thepaulodoom.com> wrote: > > > > On Sun, Mar 27, 2022 at 01:41:19PM +0200, Ard Biesheuvel wrote: > > > On Sat, 26 Mar 2022 at 18:48, Paul Lemmermann > > > <thepaulodoom@thepaulodoom.com> wrote: > > > > > > > > Fixed all styling warnings from the checkpatch.pl script. > > > > > > > > Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com> > > > > > > Did you test this code after 'fixing' it? > > > > > No, I did not. Now that I scrutinized it a bit more, I realized the > > kernel coding conventions. Sorry about that, this is my first patch. > > In that case, welcome! > > This is not about coding conventions. This is about correctness. > > For instance, > > > > > > > > > -#define f_nround(bo, bi, k) do {\ > > > > +#define f_nround(bo, bi, k) while (0) {\ > > > > f_rn(bo, bi, 0, k); \ > > > > f_rn(bo, bi, 1, k); \ > > > > f_rn(bo, bi, 2, k); \ > > > > f_rn(bo, bi, 3, k); \ > > > > k += 4; \ > > > > -} while (0) > > > > +} > > > > > > Why are you making this change, and why do you think it produces the > same result? > > > Can you remove everything in the patch past the section with line > > 1144, or do I have to resubit the patch? > > > > checkpatch.pl is a useful tool for finding style issues, but please > use it with care. And changing decades old code just to fix issues > reported by checkpatch.pl is really just pointless churn. > > So let's just drop this patch altogether, shall we? If you're > interested in helping out, please have a look at the staging/ tree - > there is a lot of code there that needs cleaning up. > Yes, we can drop the patch. Thank you so much for your help and support. Looking forward to contributing more to the Linux kernel. Thanks, Paul
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: 8505b026c91a3e6d6c562a1fe46c2b8c20835de8 ("[PATCH] crypto: aes_generic: fixed styling warnings")
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Lemmermann/crypto-aes_generic-fixed-styling-warnings/20220327-014837
base: https://git.kernel.org/cgit/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link: https://lore.kernel.org/linux-crypto/20220326172051.14722-1-thepaulodoom@thepaulodoom.com
in testcase: hwsim
version: hwsim-x86_64-717e5d7-1_20220325
with following parameters:
test: group-36
ucode: 0x21
on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz with 8G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <oliver.sang@intel.com>
2022-04-07 14:59:07 ./run-tests.py fils_sk_hlp_oom
DEV: wlan0: 02:00:00:00:00:00
DEV: wlan1: 02:00:00:00:01:00
DEV: wlan2: 02:00:00:00:02:00
APDEV: wlan3
APDEV: wlan4
START fils_sk_hlp_oom 1/1
Test: FILS SK HLP and hostapd OOM
Starting AP as-erp
Starting AP wlan3 (old add_ap argument type)
Connect STA wlan0 to AP
Connection timed out
Traceback (most recent call last):
File "./run-tests.py", line 531, in main
t(dev, apdev, params)
File "/lkp/benchmarks/hwsim/tests/hwsim/test_fils.py", line 919, in test_fils_sk_hlp_oom
dev[0].wait_connected()
File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 1411, in wait_connected
raise Exception(error)
Exception: Connection timed out
FAIL fils_sk_hlp_oom 10.282646 2022-04-07 14:59:18.241254
passed 0 test case(s)
skipped 0 test case(s)
failed tests: fils_sk_hlp_oom
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
On Mon, Apr 11, 2022 at 06:34:21PM +0800, kernel test robot wrote: > > > Greeting, > > FYI, we noticed the following commit (built with gcc-9): > > commit: 8505b026c91a3e6d6c562a1fe46c2b8c20835de8 ("[PATCH] crypto: aes_generic: fixed styling warnings") According to the following emails, it was agreed to drop this patch: https://lore.kernel.org/linux-crypto/CAMj1kXHCR1nD24WDnYpD4Nu23x9+hw+=7EXOpq7y7m9LDk2J0w@mail.gmail.com/ https://lore.kernel.org/linux-crypto/20220328125137.bsbvroyxcjw6rl5m@hp-amd-paul/ Regards, Paul > url: https://github.com/intel-lab-lkp/linux/commits/Paul-Lemmermann/crypto-aes_generic-fixed-styling-warnings/20220327-014837 > base: https://git.kernel.org/cgit/linux/kernel/git/herbert/cryptodev-2.6.git master > patch link: https://lore.kernel.org/linux-crypto/20220326172051.14722-1-thepaulodoom@thepaulodoom.com > > in testcase: hwsim > version: hwsim-x86_64-717e5d7-1_20220325 > with following parameters: > > test: group-36 > ucode: 0x21 > > > > on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz with 8G memory > > caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): > > > > > If you fix the issue, kindly add following tag > Reported-by: kernel test robot <oliver.sang@intel.com> > > > > 2022-04-07 14:59:07 ./run-tests.py fils_sk_hlp_oom > DEV: wlan0: 02:00:00:00:00:00 > DEV: wlan1: 02:00:00:00:01:00 > DEV: wlan2: 02:00:00:00:02:00 > APDEV: wlan3 > APDEV: wlan4 > START fils_sk_hlp_oom 1/1 > Test: FILS SK HLP and hostapd OOM > Starting AP as-erp > Starting AP wlan3 (old add_ap argument type) > Connect STA wlan0 to AP > Connection timed out > Traceback (most recent call last): > File "./run-tests.py", line 531, in main > t(dev, apdev, params) > File "/lkp/benchmarks/hwsim/tests/hwsim/test_fils.py", line 919, in test_fils_sk_hlp_oom > dev[0].wait_connected() > File "/lkp/benchmarks/hwsim/tests/hwsim/wpasupplicant.py", line 1411, in wait_connected > raise Exception(error) > Exception: Connection timed out > FAIL fils_sk_hlp_oom 10.282646 2022-04-07 14:59:18.241254 > passed 0 test case(s) > skipped 0 test case(s) > failed tests: fils_sk_hlp_oom > > > > To reproduce: > > git clone https://github.com/intel/lkp-tests.git > cd lkp-tests > sudo bin/lkp install job.yaml # job file is attached in this email > bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run > sudo bin/lkp run generated-yaml-file > > # if come across any failure that blocks the test, > # please remove ~/.lkp and /lkp dir to run from a clean state. > > > > -- > 0-DAY CI Kernel Test Service > https://01.org/lkp >
diff --git a/crypto/aes_generic.c b/crypto/aes_generic.c index 27ab27931..322e345ac 100644 --- a/crypto/aes_generic.c +++ b/crypto/aes_generic.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * Cryptographic API. * @@ -56,7 +57,7 @@ #include <asm/byteorder.h> #include <asm/unaligned.h> -static inline u8 byte(const u32 x, const unsigned n) +static inline u8 byte(const u32 x, const unsigned int n) { return x >> (n << 3); } @@ -325,6 +326,7 @@ __visible const u32 crypto_ft_tab[4][256] ____cacheline_aligned = { 0x7bcbb0b0, 0xa8fc5454, 0x6dd6bbbb, 0x2c3a1616, } }; +EXPORT_SYMBOL_GPL(crypto_ft_tab); static const u32 crypto_fl_tab[4][256] ____cacheline_aligned = { { @@ -853,6 +855,7 @@ __visible const u32 crypto_it_tab[4][256] ____cacheline_aligned = { 0x7b6184cb, 0xd570b632, 0x48745c6c, 0xd04257b8, } }; +EXPORT_SYMBOL_GPL(crypto_it_tab); static const u32 crypto_il_tab[4][256] ____cacheline_aligned = { { @@ -1118,8 +1121,6 @@ static const u32 crypto_il_tab[4][256] ____cacheline_aligned = { } }; -EXPORT_SYMBOL_GPL(crypto_ft_tab); -EXPORT_SYMBOL_GPL(crypto_it_tab); /** * crypto_aes_set_key - Set the AES key. @@ -1144,34 +1145,34 @@ EXPORT_SYMBOL_GPL(crypto_aes_set_key); /* encrypt a block of text */ -#define f_rn(bo, bi, n, k) do { \ +#define f_rn(bo, bi, n, k) while (0) { \ bo[n] = crypto_ft_tab[0][byte(bi[n], 0)] ^ \ crypto_ft_tab[1][byte(bi[(n + 1) & 3], 1)] ^ \ crypto_ft_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ crypto_ft_tab[3][byte(bi[(n + 3) & 3], 3)] ^ *(k + n); \ -} while (0) +} -#define f_nround(bo, bi, k) do {\ +#define f_nround(bo, bi, k) while (0) {\ f_rn(bo, bi, 0, k); \ f_rn(bo, bi, 1, k); \ f_rn(bo, bi, 2, k); \ f_rn(bo, bi, 3, k); \ k += 4; \ -} while (0) +} -#define f_rl(bo, bi, n, k) do { \ +#define f_rl(bo, bi, n, k) while (0) { \ bo[n] = crypto_fl_tab[0][byte(bi[n], 0)] ^ \ crypto_fl_tab[1][byte(bi[(n + 1) & 3], 1)] ^ \ crypto_fl_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ crypto_fl_tab[3][byte(bi[(n + 3) & 3], 3)] ^ *(k + n); \ -} while (0) +} -#define f_lround(bo, bi, k) do {\ +#define f_lround(bo, bi, k) while (0) {\ f_rl(bo, bi, 0, k); \ f_rl(bo, bi, 1, k); \ f_rl(bo, bi, 2, k); \ f_rl(bo, bi, 3, k); \ -} while (0) +} static void crypto_aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) { @@ -1214,12 +1215,12 @@ static void crypto_aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) /* decrypt a block of text */ -#define i_rn(bo, bi, n, k) do { \ +#define i_rn(bo, bi, n, k) while (0) { \ bo[n] = crypto_it_tab[0][byte(bi[n], 0)] ^ \ crypto_it_tab[1][byte(bi[(n + 3) & 3], 1)] ^ \ crypto_it_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ crypto_it_tab[3][byte(bi[(n + 1) & 3], 3)] ^ *(k + n); \ -} while (0) +} #define i_nround(bo, bi, k) do {\ i_rn(bo, bi, 0, k); \ @@ -1229,19 +1230,19 @@ static void crypto_aes_encrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) k += 4; \ } while (0) -#define i_rl(bo, bi, n, k) do { \ +#define i_rl(bo, bi, n, k) while (0) { \ bo[n] = crypto_il_tab[0][byte(bi[n], 0)] ^ \ crypto_il_tab[1][byte(bi[(n + 3) & 3], 1)] ^ \ crypto_il_tab[2][byte(bi[(n + 2) & 3], 2)] ^ \ crypto_il_tab[3][byte(bi[(n + 1) & 3], 3)] ^ *(k + n); \ -} while (0) +} -#define i_lround(bo, bi, k) do {\ +#define i_lround(bo, bi, k) while (0) {\ i_rl(bo, bi, 0, k); \ i_rl(bo, bi, 1, k); \ i_rl(bo, bi, 2, k); \ i_rl(bo, bi, 3, k); \ -} while (0) +} static void crypto_aes_decrypt(struct crypto_tfm *tfm, u8 *out, const u8 *in) {
Fixed all styling warnings from the checkpatch.pl script. Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com> --- crypto/aes_generic.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-)