Re: [Ocf-linux-users] crypto_newsession / crypto_freesession

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

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.

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.

Any comments is definitely welcome.

Thanks,
Piras

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!!