[PATCH] patch to OpenCrypto framework

[Kennedy, Brendan]

Hi Patrick,

You are right to an extent - hardware drivers generally require physically contiguous memory to work with, however virtually contiguous allocations under a page size should not cause a problem (unless the allocation crosses a virtual page boundary).
From the man page, malloc gives page boundary aligned memory so long as the requested size is less than a page, so I guess the best workaround would be to have something like:

if(get_page_size() < crp_desc->packet_size){
        malloc(packet_size);
}else
        contigmalloc(packetsize);
}

The same check can be done when it is time to free the memory. What do you think? It could be even nicer if malloc would take a flag to give physically contiguous memory!

As for GELI/IPSEC (not sure about Kerberos, but grepping for crypto_dispatch the FreeBSD sources should find each app where Opencrypto is used):

IPSEC: racoon uses cryptodev and the ipsec stack does encrypt/decrypts on packet fragments which tend to be less than the kernel page size (this is worth checking for connections with MTU > PAGE_SIZE)
GELI: I suppose there could be an issue if the file system block size is greater than the kernel page size, however for optimal performance these two numbers tend to be aligned. There still should be a check before crypto_dispatch is called however.

I think it is best to do this check at allocation time, rather than forcing drivers to do it. It would be a serious overhead for them to have to do another allocation and memory copy in the data path.

Best Regards,
Brendan


-----Original Message-----
From: Patrick Lamaiziere [mailto:patfbsd at davenulle.org] 
Sent: Saturday, February 27, 2010 9:52 PM
To: freebsd-drivers at freebsd.org
Cc: Kennedy, Brendan; Philip at freebsd.org
Subject: Re: [PATCH] patch to OpenCrypto framework

Le Wed, 24 Feb 2010 16:50:35 +0000,
"Kennedy, Brendan" <brendan.kennedy at intel.com> a écrit :

> Hi Philip, All,

Hello,

> This OpenCrypto patch does a number of updates:
> 
> 1) It updates Cryptodev to allocate contiguous memory blocks – a
> requirement for some hardware drivers

I don't see the benefit for this one -changing malloc(9) to
contigmalloc(9)- it introduces a small (but useless) overhead
to existing drivers (I tried with glxsb : we lose around 5%).

IMO, if this is required for some drivers, this is not
the good place to do this. A crypto driver can receive data from
somewhere else than cryptodev: ipsec, geli, kerberos (at least)

Best regards.
--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.