Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 30 Mar 2015 11:11:29 +0200
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: [GSoC] John the Ripper support for PHC finalists

On 2015-03-30 09:24, Solar Designer wrote:
> On Mon, Mar 30, 2015 at 01:56:03AM +0200, Agnieszka Bielec wrote:
>> I have added OpenCL and  I have fixed almost all things commented
>> by magnumripper https://github.com/Lucife-r/JohnTheRipper

> [solar@...er src]$ ../run/john -te -form=pomelo-opencl -dev=5
> Device 5: GeForce GTX TITAN
> Options used: -I ../run/kernels -cl-mad-enable -cl-nv-verbose -DDEVICE_INFO=4114 -D_OPENCL_COMPILER -DDEV_VER_MAJOR=319 -DDEV_VER_MINOR=60 
> Build log: :162:15: error: 'long long' type is not supported
>         state_size = 1ULL << (13 + m_cost);     //m_cost=3 is max

Agnieszka, please note that in OpenCL an int is always 32-bit and long
is always 64-bit. Long long is not used but is reserved; it may be used
in the future eg. for 128-bit.

So when porting C code, basically ULL should always be UL and you can
replace all "long long" with long. On the same note, some reference code
use long where they really only need 32-bit (because that's what you
need to use to be really portable) and replacing that with int in a
kernel can give a very significant speed boost.

Even after fixing that, my Apple driver failed compiling your kernel
because you pass build_opts uninitialized [in init()], giving random
garbage data as compiler options. What you should do is this right
before opencl_init():

+  sprintf(build_opts, "-DBINARY_SIZE=%d -DSALT_SIZE=%d -DMEM_SIZE=%d",
+          BINARY_SIZE, SALT_SIZE, MEM_SIZE);

And a corresponding change to the kernel:

-#define BINARY_SIZE     257
-#define SALT_SIZE       32
-#define MEM_SIZE        131072
+// BINARY_SIZE, SALT_SIZE and MEM_SIZE is passed with -D during build

Also, your auto-tune settings are totally wrong, possibly ending up in
suboptimal work sizes. I will fix them and submit a patch.

BTW I also see this build warning (please build with "make -s" and do
care about all warnings):
opencl_pomelo_fmt_plug.c:88:12: warning: 'length_cipher' defined but not
used [-Wunused-variable]
 static int length_cipher;
            ^

And I also saw this notice from our autoconf dependency magic:
Warning: pomelo_fmt_plug.c includes "omp.h" but that file is not found.

You should use <omp.h>, not "omp.h".

Finally: Please set up your editor to discard trailing whitespace. We
hate whitespace noise in the repo because it will eventually result in
problems (for example, if I edit your kernel and save the changes, MY
editor will strip all trailing whitespace and if I commit that, there
will be lots of no-op changes - and "git blame" will point to *me* for
things I did not write). For the same reason it's important that you use
"tab to indent, space to align" but you seem to do that already - good!

magnum


Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ