Re: [Ocf-linux-users] crypto_newsession / crypto_freesession
Brought to you by:
david-m
Menu
▾
▴
Re: [Ocf-linux-users] crypto_newsession / crypto_freesession
|
From: David M. <Dav...@se...> - 2009-01-30 04:00:49
|
Jivin Pirasenna Thiyagarajan lays it down ... > David: > Here is what I'm trying achieve. In my driver I'm adding an abstract layer > to interface with OCF. There is an opaque structure that hosts the crypto > fields, which in OCF-speak is the session. So, in the kernel space, instead > of creating a table to maintain the mapping between session id and the > appropriate crypto operation, I'm trying to return the address. > > I explored your idea of using offset address as session id. That would work > in Linux 32-bit mode since the addressable space in kernel is less than a > gigabyte. Whereas, in 64-bit mode, I cannot make an assumption of having > memory obtained from the OS to be within a 32-bit offset. Ok, then it is not "on crypto chip" memory but host system memory obtained through one of the kernel allocator functions. It sounds like the HW doesn't have a classic "divided up" ring buffer but just chains arbitary descriptors together ? > I'm expecting to use the session id as an opaque handler to the OCF > interface and the driver may use it to dereference the operation however it > may see fit. > > In a multi-processor scenario, it would eliminate lock to access the session > table structure safely. Are you doing this to make finding the "session" info easy, and everything you need to know about a session is contained within the memory pointed to by that address ? That I can understand. Especially if you want unlimited sessions. That way when I ask to do something on session N, you just dereference N inside your driver to find out context for that session. Do you have infomation that is not stored in the memory at the "address" ? How does the HW find out about a new session ? Do you just link it into a list of sessions ? Does the HW have a session limit ? If the address is only used by the driver, it sounds reasonable, if you want to use the address outside of your driver for anything else then it sounds quite risky :-) > Any comments is definitely welcome. Looking at an OCF driver (forget cryptodev, thats different), newsession returns a 32 bit session id, but freesession takes a 64 bit id, that doesn't make much sense in any way :-) I just checked the current FreeBSD source and the API hasn't changed in that respect. I wouldn't like to deviate from that too much, but I can't say I am completely tied to it either. Having to recompile doesn't bother me much either as this is a source only thing anyway. Cheers, Davidm > On Thu, Jan 29, 2009 at 4:39 PM, David McCullough < > Dav...@se...> wrote: > > > > > Jivin Pirasenna Thiyagarajan lays it down ... > > > Here is one option, that does not require interface change and requires > > no > > > modification in ocf-compat.h. It would require recompiling existing > > drivers > > > though. > > > > > > Since sid is always passed as u_int64_t * for create_newsession, > > internally > > > in cryptop structure crp_sid may be setup as a 2 entry 64 byte array. The > > > nid defintion in crypto.c:1017 may be defined the same way. The first > > > 64-bytes are left for driver to put whatever value they want. The second > > > 64-byte is left for OCF to store flags and anything else that is required > > > for administration. > > > > Ok, I think I am missing something here. Are you trying to pass the > > address back to userspace by any chance ? > > > > > > > crypto_newsession in crytpodev.c would be modified to store the flags in > > the > > > second 64-byte field. > > > The macros CRYPTO_SESID2HID, CRYPTO_SESID2CAPS and CRYPTO_SESID2LID will > > be > > > modified to harvest the information from correct index. > > > crypto_freesession sends only the first 64-byte data as the argument > > value > > > to the function. So, driver can use it as it is. > > > > > > Let me know if you think there are any loose ends or incompatibilities. > > I > > > can send you a patch with these changes after testing it out with > > software > > > driver using cryptotest. > > > > IMO it is sounding a little messy at this point, but that may be > > because I don't quite understand what you are trying to do and how it > > will be used. > > > > Did you consider returning an offset rather than a full pointer as I > > suggested below ? Is it even possible in your context ? > > > > If you think the patch is simple enough to do perhaps it will help me > > understand what you need better ;-) > > > > Cheers, > > Davidm > > > > > > > On Wed, Jan 28, 2009 at 10:03 PM, David McCullough < > > > Dav...@se...> wrote: > > > > > > > > > > > Jivin Pirasenna Thiyagarajan lays it down ... > > > > > I'm writing a driver and attempting it to make it OCF compatible. I > > was > > > > > intending on passing the virtual memory address I allocate for driver > > > > data > > > > > structures for a given operation as the session id. To avoid > > 32/64bit > > > > > issues, having the crypto_newsession pass in u_int64_t * would make > > it > > > > > convenient. Otherwise, I'll have to keep a mapping table and look up > > > > > routines appropriately (unnecessary computation cycles). > > > > > > > > Hmm, sounds fair, only I think that crypto.c messes with the 64 bit > > > > version a fair bit before handing it to the driver. > > > > > > > > Check crypto.c:crypto_newsession to see this in action. > > > > > > > > The 64bit version has the driver id + the drivers session id and the > > > > drivers capability flags all merged into it. > > > > > > > > It's probably unlikely your card with have more than 4GB of memory, or > > > > maybe it does. You could just return the "offset" into it's memory for > > > > that session. Saves a lookup table (baseaddr + offset = address). > > > > > > > > > From the looks of it, it seems just like an interface definition > > issue, > > > > > since as I pointed out, from within the ioctl, it does use u_int64_t > > > > > internally. > > > > > > > > Unfortunately I think it's not that simple, happy to consider something > > > > though if you have ideas. > > > > > > > > Are you planning to do a FreeBSD driver as well ? If so it wouldn't > > hurt > > > > to > > > > check if it's still the same. Otherwise I'll get around to checking in > > > > time ;-) > > > > > > > > Cheers, > > > > Davidm > > > > > > > > > > > > > On Wed, Jan 28, 2009 at 3:58 PM, David McCullough < > > > > > Dav...@se...> wrote: > > > > > > > > > > > > > > > > > Jivin Pirasenna Thiyagarajan lays it down ... > > > > > > > I've used OCF patch and attempting to use the cryptodev > > interface. > > > > In > > > > > > > ocf-compat.h, I see crypto_newsession's session ID is passed in > > as a > > > > > > > u_int32_t *, whereas, the crypto_freesession takes the session id > > as > > > > > > > u_int64_t. It seems like this is in error. Looks like sid data > > type > > > > > > should > > > > > > > be u_int64_t *sidp. In cryptodev.c where it is used, the sid is > > > > defined > > > > > > as > > > > > > > u_int64_t in cryptodev_ioctl. > > > > > > > > > > > > I had to look twice at that too now that you point it out. > > > > > > > > > > > > The trick is that device_method_t structure and resulting > > > > > > CRYPTODEV_NEWSESSION and cryptodev_newsession used for it are > > > > > > part of the interface for OCF drivers. This is how OCF > > communicates > > > > > > with it's crypto drivers (ie., safe, hifn7751, cryptosoft etc). > > > > > > > > > > > > cryptodev, the user interface driver, doesn't talk to the crypto > > > > > > drivers, it uses the OCF API's: > > > > > > > > > > > > crypto_newsession > > > > > > crypto_freesession > > > > > > > > > > > > and they are both u_int64_t. > > > > > > > > > > > > Are you having a problem with cryptodev or did you just notice this > > > > oddity > > > > > > in the code ? > > > > > > > > > > > > I admit the naming is confusing, but we inherited that with that > > last > > > > > > round of BSD updates. Unless you are writing a driver, ignore the > > > > section > > > > > > at the top of ocf-compat.h, in fact, ignore as much of > > ocf-compat.h > > > > > > as you can every chance you get :-) It's where we hide the older > > OS > > > > > > support and some BSD compat things to try and keep the driver > > source as > > > > > > close to the original as practical. > > > > > > > > > > > > Cheers, > > > > > > Davidm > > > > > > > > > > > > -- > > > > > > David McCullough, dav...@se..., Ph:+61 > > > > > > 734352815 > > > > > > Secure Computing - SnapGear http://www.uCdot.org > > > > > > http://www.snapgear.com > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Online Gallery: http://www.deptons.com > > > > > You comments and ratings are very welcome!! > > > > > > > > -- > > > > David McCullough, dav...@se..., Ph:+61 > > > > 734352815 > > > > Secure Computing - SnapGear http://www.uCdot.org > > > > http://www.snapgear.com > > > > > > > > > > > > > > > > -- > > > Online Gallery: http://www.deptons.com > > > You comments and ratings are very welcome!! > > > > -- > > David McCullough, dav...@se..., Ph:+61 > > 734352815 > > Secure Computing - SnapGear http://www.uCdot.org > > http://www.snapgear.com > > > > > > -- > Online Gallery: http://www.deptons.com > You comments and ratings are very welcome!! -- David McCullough, dav...@se..., Ph:+61 734352815 Secure Computing - SnapGear http://www.uCdot.org http://www.snapgear.com |
