Re: AESNI driver and fpu_kern KPI

On Tue, May 18, 2010 at 05:30:19PM +0200, Pawel Jakub Dawidek wrote:
> On Sat, May 15, 2010 at 01:04:01PM +0300, Kostik Belousov wrote:
> > Hello,
> > 
> > please find at http://people.freebsd.org/~kib/misc/aesni.1.patch the
> > combined patch, containing the fpu_kern KPI and Intel AESNI crypto(9)
> > driver.  I did development and some testing on the hardware generously
> > provided by Sentex Communications to Netperf cluster.
> 
> Nice work. Few comments:
> 
> - Could you modify this chunk in padlock.c:
> 
> +               td = curthread;
> +               error = fpu_kern_enter(td, &ses->ses_fpu_ctx);
> +               if (error != 0)
> +                       goto out;
>                 error = padlock_hash_setup(ses, macini);
> +               fpu_kern_leave(td, &ses->ses_fpu_ctx);
> +       out:
> 
>   To something without goto, eg.:
> 
> 		td = curthread;
> 		error = fpu_kern_enter(td, &ses->ses_fpu_ctx);
> 		if (error == 0) {
> 			error = padlock_hash_setup(ses, macini);
> 			fpu_kern_leave(td, &ses->ses_fpu_ctx);
> 		}
Done.

> 
> - I see that in sys/dev/random/nehemiah.c you don't check for return
>   value of fpu_kern_enter(). That's the only place where you ignore it.
>   Is that intended?
No, thank you, fixed.

> 
> - Unfortunately the driver in its current version can't be used with
>   IPsec and with GELI where authentication is enabled. This is because
>   the driver doesn't support sessions where both encryption and
>   authentication is defined. Do you have plans to change it?
>   I saw that you based crypto(9) bits on padlock, which does support
>   sessions with authentication by calculating hashes in software.
My goal was to develop fpu_kern_enter() KPI. I used the AESNI as an
opportunity to test the KPI in real application. I may consider adding
software-implemented authentification sometime later. I would not object
if anybody do this instead of me.

Since you are there, I want to confirm that you do not have objections
against your copyright left in aesni.c. The file was copied from
padlock.c and I felt that removing the copyright is wrong.

Updated patch, that also includes some other changes, mainly additions
to fpu_kern() KPI, that were discussed/requested by Fabien Thomas,
is at http://people.freebsd.org/~kib/misc/aesni.2.patch.

Thank you for your comments.