HI Evgeniy: On Tue, May 22, 2007 at 12:58:29PM +0000, Evgeniy Polyakov wrote: > > This is preliminary driver for HIFN 795x crypto accelerator chip. Thanks for the working on this! > Likely it is not even a request for testing, since I see at least one > problem with current approach: what to do when crypto hardware queue is > full and no new packets can arrive? Current code just returns an error > -EINVAL if there is real error and -EBUSY if queue is full. Each device should have a crypto_queue dedicated to it to handle this situation. So when the hardware queue is full you start filling up the software crypto_queue using crypto_enqueue_request. When that becomes full the caller either gets an error or it can enqueue one last request and then block by setting the CRYPTO_TFM_REQ_MAY_BACKLOG flag. In either case it'll get back a -EBUSY error. When your hardware queue is drained you should try to refill it by calling crypto_dequeue_request. > Due to problems with interrupt storms and possible adapter freeze > (sorry, but HIFN spec I have really sucks, so likely it is programming > error, but who knows) I added special watchdog, which fires if after > predefined timeout sessions which are supposed to be completed are not. > In that case callback is invoked with -EBUSY error. Yes we do need watchdogs for all hardware devices to handle situations like this. Feel free to add helpers to the API to aid drivers in handling this. > This driver supports old-style crypto_alg with "aes" string only, and I > would like to rise a discussion of the needs to support several > structures for cbc(aes), ecb(aes) and so on, since some hardware > supports plenty of modes, and allocating set of structures for each > hardware adapter found in the system would be an overkill. It was an explicit design decision to avoid using bitmasks. Just as we use strings as the unique key to identify algorithms rather than integers as that provides the freedom for expansion, we now use strings to describe cipher modes rather than bitmasks. There are numerous new cipher modes in recent years and there is no way we're going back to describing these using bitmasks again. As to allocating an object for each algorithm that you support being an overkill, are you concerned about the data size? That shouldn't be an issue because you'd only have one such object per algorithm per device and they really aren't that big. If you're worried about duplicate code then we can probably look at providing helpers to eliminate as much of that as possible. Have a look at padlock/s390 for example. They handle these in a fairly sane way. > Current driver only supports AES ECB encrypt/decrypt, since I do not > know how to detect operation mode in runtime (a question). For each incoming request you have an associated tfm object which has a link to the algorithm object. The algorithm object provides you the info you need to know which algorithm to use and the tfm object provides the session-specific information which would be the key for ciphers. > Another issue unknown issue is a possibility to call setkey() the same > time encrypt/decrypt is called. As far as I can see it can not be done, > but I may be wrong, if so, small changes are needed in hifn_setkey > (mainly operation must be done under dev->lock). Indeed users should not call setkey while there are still outstanding operations. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt - To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
