Thread: [Ocf-linux-users] crypto_dispatch update for 20080917
Brought to you by:
david-m
Menu
▾
▴
ocf-linux-users
|
From: Kennedy, B. <bre...@in...> - 2009-04-29 15:59:18
|
Hi All,
I am having trouble understanding the update to the crypto_dispatch function in crypto.c for ocf-linux-20080917 from ocf-linux-2071215:
2007 code:
if (!cap->cc_qblocked) {
result = crypto_invoke(cap, crp, 0);
if (result != ERESTART)
return (result);
2008 code:
result = crypto_invoke(cap, crp, 0);
CRYPTO_Q_LOCK();
if (result != ERESTART)
crypto_drivers[hid].cc_qblocked = 0;
In the 2008 code an error is not returned to cryptodev.c if crypto_invoke returns an error code.
Should we now always call crypto_done() if there is a fail in the driver invoke function?
I ask because I've tried this but am getting a soft lockup when using crypto-tools and testing some fail case scenarios if I set the crp->etype before calling crypto-done.
Best Regards,
Brendan
------------------------------------------------
Brendan Kennedy
Intel Shannon Ltd.,
Ireland.
Email: bre...@in...<mailto:bre...@in...>
------------------------------------------------
---------------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
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.
|
|
From: David M. <Dav...@se...> - 2009-04-29 23:03:33
|
Jivin Kennedy, Brendan lays it down ...
> Hi All,
>
> I am having trouble understanding the update to the crypto_dispatch function in crypto.c for ocf-linux-20080917 from ocf-linux-2071215:
>
> 2007 code:
> if (!cap->cc_qblocked) {
> result = crypto_invoke(cap, crp, 0);
> if (result != ERESTART)
> return (result);
>
> 2008 code:
> result = crypto_invoke(cap, crp, 0);
> CRYPTO_Q_LOCK();
> if (result != ERESTART)
> crypto_drivers[hid].cc_qblocked = 0;
>
> In the 2008 code an error is not returned to cryptodev.c if crypto_invoke returns an error code.
> Should we now always call crypto_done() if there is a fail in the driver invoke function?
>
> I ask because I've tried this but am getting a soft lockup when using crypto-tools and testing some fail case scenarios if I set the crp->etype before calling crypto-done.
Yeah, I can't explain it either and I made that change. It was part of a
SMP/locking problem and it looks like I lost the result code. Here is a
quick patch (untested :-) that I think will return the functionality that
was lost. Let me know how it goes.
Out of interest, what is the condition error that the driver you are using
is reporting ? EINVAL perhaps ?
Thanks for tracking this down,
Cheers,
Davidm
Index: ocf/crypto.c
===================================================================
RCS file: ocf/crypto.c,v
retrieving revision 1.42
diff -u -r1.42 crypto.c
--- ocf/crypto.c 31 Mar 2009 00:41:14 -0000 1.42
+++ ocf/crypto.c 29 Apr 2009 23:01:08 -0000
@@ -838,13 +838,15 @@
*/
list_add(&crp->crp_next, &crp_q);
cryptostats.cs_blocks++;
+ result = 0;
} else if (result == -1) {
TAILQ_INSERT_TAIL(&crp_q, crp, crp_next);
+ result = 0;
}
if (crp_sleep)
wake_up_interruptible(&cryptoproc_wait);
CRYPTO_Q_UNLOCK();
- return 0;
+ return result;
}
/*
--
David McCullough, dav...@se..., Ph:+61 734352815
McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
|
|
From: Kennedy, B. <bre...@in...> - 2009-04-30 08:22:47
|
Thanks David,
So is that patch going to be in the next release? :-) Just it definitely changes how error cases need to be handled in the drivers...
Yep, it's EINVAL or EFAULT. The fault code seems to only matter if it's ERESTART though??
Regards,
Brendan
-----Original Message-----
From: David McCullough [mailto:Dav...@se...]
Sent: 30 April 2009 00:03
To: Kennedy, Brendan
Cc: ocf...@li...
Subject: Re: [Ocf-linux-users] crypto_dispatch update for 20080917
Jivin Kennedy, Brendan lays it down ...
> Hi All,
>
> I am having trouble understanding the update to the crypto_dispatch function in crypto.c for ocf-linux-20080917 from ocf-linux-2071215:
>
> 2007 code:
> if (!cap->cc_qblocked) {
> result = crypto_invoke(cap, crp, 0);
> if (result != ERESTART)
> return (result);
>
> 2008 code:
> result = crypto_invoke(cap, crp, 0);
> CRYPTO_Q_LOCK();
> if (result != ERESTART)
> crypto_drivers[hid].cc_qblocked = 0;
>
> In the 2008 code an error is not returned to cryptodev.c if crypto_invoke returns an error code.
> Should we now always call crypto_done() if there is a fail in the driver invoke function?
>
> I ask because I've tried this but am getting a soft lockup when using crypto-tools and testing some fail case scenarios if I set the crp->etype before calling crypto-done.
Yeah, I can't explain it either and I made that change. It was part of a
SMP/locking problem and it looks like I lost the result code. Here is a
quick patch (untested :-) that I think will return the functionality that
was lost. Let me know how it goes.
Out of interest, what is the condition error that the driver you are using
is reporting ? EINVAL perhaps ?
Thanks for tracking this down,
Cheers,
Davidm
Index: ocf/crypto.c
===================================================================
RCS file: ocf/crypto.c,v
retrieving revision 1.42
diff -u -r1.42 crypto.c
--- ocf/crypto.c 31 Mar 2009 00:41:14 -0000 1.42
+++ ocf/crypto.c 29 Apr 2009 23:01:08 -0000
@@ -838,13 +838,15 @@
*/
list_add(&crp->crp_next, &crp_q);
cryptostats.cs_blocks++;
+ result = 0;
} else if (result == -1) {
TAILQ_INSERT_TAIL(&crp_q, crp, crp_next);
+ result = 0;
}
if (crp_sleep)
wake_up_interruptible(&cryptoproc_wait);
CRYPTO_Q_UNLOCK();
- return 0;
+ return result;
}
/*
--
David McCullough, dav...@se..., Ph:+61 734352815
McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
---------------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
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.
|
|
From: David M. <Dav...@se...> - 2009-04-30 11:08:45
|
Jivin Kennedy, Brendan lays it down ...
> Thanks David,
>
> So is that patch going to be in the next release? :-) Just it definitely
> changes how error cases need to be handled in the drivers...
If it fixes the problem you are seeing I will commit it and it will be in
the next release :-) I should do one really, it has the 2.6.29 updates
etc in there now and quite a few more little updates.
> Yep, it's EINVAL or EFAULT. The fault code seems to only matter if it's
> ERESTART though??
Basically I just wanted to know what a driver was doing that would return
an error other than full. EINVAL sprang to mind if the kery sizes or
something similar didn't match.
I was concerned if it was something that would normally happen, as I
haven't been seeing it myself.
Thanks again,
Cheers,
Davidm
> -----Original Message-----
> From: David McCullough [mailto:Dav...@se...]
> Sent: 30 April 2009 00:03
> To: Kennedy, Brendan
> Cc: ocf...@li...
> Subject: Re: [Ocf-linux-users] crypto_dispatch update for 20080917
>
>
> Jivin Kennedy, Brendan lays it down ...
> > Hi All,
> >
> > I am having trouble understanding the update to the crypto_dispatch function in crypto.c for ocf-linux-20080917 from ocf-linux-2071215:
> >
> > 2007 code:
> > if (!cap->cc_qblocked) {
> > result = crypto_invoke(cap, crp, 0);
> > if (result != ERESTART)
> > return (result);
> >
> > 2008 code:
> > result = crypto_invoke(cap, crp, 0);
> > CRYPTO_Q_LOCK();
> > if (result != ERESTART)
> > crypto_drivers[hid].cc_qblocked = 0;
> >
> > In the 2008 code an error is not returned to cryptodev.c if crypto_invoke returns an error code.
> > Should we now always call crypto_done() if there is a fail in the driver invoke function?
> >
> > I ask because I've tried this but am getting a soft lockup when using crypto-tools and testing some fail case scenarios if I set the crp->etype before calling crypto-done.
>
> Yeah, I can't explain it either and I made that change. It was part of a
> SMP/locking problem and it looks like I lost the result code. Here is a
> quick patch (untested :-) that I think will return the functionality that
> was lost. Let me know how it goes.
>
> Out of interest, what is the condition error that the driver you are using
> is reporting ? EINVAL perhaps ?
>
> Thanks for tracking this down,
>
> Cheers,
> Davidm
>
> Index: ocf/crypto.c
> ===================================================================
> RCS file: ocf/crypto.c,v
> retrieving revision 1.42
> diff -u -r1.42 crypto.c
> --- ocf/crypto.c 31 Mar 2009 00:41:14 -0000 1.42
> +++ ocf/crypto.c 29 Apr 2009 23:01:08 -0000
> @@ -838,13 +838,15 @@
> */
> list_add(&crp->crp_next, &crp_q);
> cryptostats.cs_blocks++;
> + result = 0;
> } else if (result == -1) {
> TAILQ_INSERT_TAIL(&crp_q, crp, crp_next);
> + result = 0;
> }
> if (crp_sleep)
> wake_up_interruptible(&cryptoproc_wait);
> CRYPTO_Q_UNLOCK();
> - return 0;
> + return result;
> }
>
> /*
>
>
> --
> David McCullough, dav...@se..., Ph:+61 734352815
> McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
> ---------------------------------------------------------------------
> Intel Shannon Limited
> Registered in Ireland
> Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
> 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.
>
--
David McCullough, dav...@se..., Ph:+61 734352815
McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
|
|
From: Kennedy, B. <bre...@in...> - 2009-05-05 07:04:59
|
Hi David,
Thanks, that patch seems to be working as expected for some of my fail cases. However, the reason I have taken so long to get back to you (sorry!) is that I have encountered another problem, but I think it's in my own code. Anyway, I can't run a full set of tests on the change until it is fixed :/
Regards,
Brendan
-----Original Message-----
From: David McCullough [mailto:Dav...@se...]
Sent: 30 April 2009 12:09
To: Kennedy, Brendan
Cc: ocf...@li...
Subject: Re: [Ocf-linux-users] crypto_dispatch update for 20080917
Jivin Kennedy, Brendan lays it down ...
> Thanks David,
>
> So is that patch going to be in the next release? :-) Just it definitely
> changes how error cases need to be handled in the drivers...
If it fixes the problem you are seeing I will commit it and it will be in
the next release :-) I should do one really, it has the 2.6.29 updates
etc in there now and quite a few more little updates.
> Yep, it's EINVAL or EFAULT. The fault code seems to only matter if it's
> ERESTART though??
Basically I just wanted to know what a driver was doing that would return
an error other than full. EINVAL sprang to mind if the kery sizes or
something similar didn't match.
I was concerned if it was something that would normally happen, as I
haven't been seeing it myself.
Thanks again,
Cheers,
Davidm
> -----Original Message-----
> From: David McCullough [mailto:Dav...@se...]
> Sent: 30 April 2009 00:03
> To: Kennedy, Brendan
> Cc: ocf...@li...
> Subject: Re: [Ocf-linux-users] crypto_dispatch update for 20080917
>
>
> Jivin Kennedy, Brendan lays it down ...
> > Hi All,
> >
> > I am having trouble understanding the update to the crypto_dispatch function in crypto.c for ocf-linux-20080917 from ocf-linux-2071215:
> >
> > 2007 code:
> > if (!cap->cc_qblocked) {
> > result = crypto_invoke(cap, crp, 0);
> > if (result != ERESTART)
> > return (result);
> >
> > 2008 code:
> > result = crypto_invoke(cap, crp, 0);
> > CRYPTO_Q_LOCK();
> > if (result != ERESTART)
> > crypto_drivers[hid].cc_qblocked = 0;
> >
> > In the 2008 code an error is not returned to cryptodev.c if crypto_invoke returns an error code.
> > Should we now always call crypto_done() if there is a fail in the driver invoke function?
> >
> > I ask because I've tried this but am getting a soft lockup when using crypto-tools and testing some fail case scenarios if I set the crp->etype before calling crypto-done.
>
> Yeah, I can't explain it either and I made that change. It was part of a
> SMP/locking problem and it looks like I lost the result code. Here is a
> quick patch (untested :-) that I think will return the functionality that
> was lost. Let me know how it goes.
>
> Out of interest, what is the condition error that the driver you are using
> is reporting ? EINVAL perhaps ?
>
> Thanks for tracking this down,
>
> Cheers,
> Davidm
>
> Index: ocf/crypto.c
> ===================================================================
> RCS file: ocf/crypto.c,v
> retrieving revision 1.42
> diff -u -r1.42 crypto.c
> --- ocf/crypto.c 31 Mar 2009 00:41:14 -0000 1.42
> +++ ocf/crypto.c 29 Apr 2009 23:01:08 -0000
> @@ -838,13 +838,15 @@
> */
> list_add(&crp->crp_next, &crp_q);
> cryptostats.cs_blocks++;
> + result = 0;
> } else if (result == -1) {
> TAILQ_INSERT_TAIL(&crp_q, crp, crp_next);
> + result = 0;
> }
> if (crp_sleep)
> wake_up_interruptible(&cryptoproc_wait);
> CRYPTO_Q_UNLOCK();
> - return 0;
> + return result;
> }
>
> /*
>
>
> --
> David McCullough, dav...@se..., Ph:+61 734352815
> McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
> ---------------------------------------------------------------------
> Intel Shannon Limited
> Registered in Ireland
> Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
> 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.
>
--
David McCullough, dav...@se..., Ph:+61 734352815
McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
---------------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
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.
|
|
From: David M. <Dav...@se...> - 2009-05-05 23:17:10
|
Jivin Kennedy, Brendan lays it down ...
> Hi David,
>
> Thanks, that patch seems to be working as expected for some of my
> fail cases. However, the reason I have taken so long to get back to
> you (sorry!) is that I have encountered another problem, but I think
> it's in my own code. Anyway, I can't run a full set of tests on the
> change until it is fixed :/
I think I'll commit it, all the other drivers are broken for failure cases
at the moment so better to improve things on that front :-)
Thanks,
Davidm
> -----Original Message-----
> From: David McCullough [mailto:Dav...@se...]
> Sent: 30 April 2009 12:09
> To: Kennedy, Brendan
> Cc: ocf...@li...
> Subject: Re: [Ocf-linux-users] crypto_dispatch update for 20080917
>
>
> Jivin Kennedy, Brendan lays it down ...
> > Thanks David,
> >
> > So is that patch going to be in the next release? :-) Just it definitely
> > changes how error cases need to be handled in the drivers...
>
> If it fixes the problem you are seeing I will commit it and it will be in
> the next release :-) I should do one really, it has the 2.6.29 updates
> etc in there now and quite a few more little updates.
>
> > Yep, it's EINVAL or EFAULT. The fault code seems to only matter if it's
> > ERESTART though??
>
> Basically I just wanted to know what a driver was doing that would return
> an error other than full. EINVAL sprang to mind if the kery sizes or
> something similar didn't match.
>
> I was concerned if it was something that would normally happen, as I
> haven't been seeing it myself.
>
> Thanks again,
>
> Cheers,
> Davidm
>
> > -----Original Message-----
> > From: David McCullough [mailto:Dav...@se...]
> > Sent: 30 April 2009 00:03
> > To: Kennedy, Brendan
> > Cc: ocf...@li...
> > Subject: Re: [Ocf-linux-users] crypto_dispatch update for 20080917
> >
> >
> > Jivin Kennedy, Brendan lays it down ...
> > > Hi All,
> > >
> > > I am having trouble understanding the update to the crypto_dispatch function in crypto.c for ocf-linux-20080917 from ocf-linux-2071215:
> > >
> > > 2007 code:
> > > if (!cap->cc_qblocked) {
> > > result = crypto_invoke(cap, crp, 0);
> > > if (result != ERESTART)
> > > return (result);
> > >
> > > 2008 code:
> > > result = crypto_invoke(cap, crp, 0);
> > > CRYPTO_Q_LOCK();
> > > if (result != ERESTART)
> > > crypto_drivers[hid].cc_qblocked = 0;
> > >
> > > In the 2008 code an error is not returned to cryptodev.c if crypto_invoke returns an error code.
> > > Should we now always call crypto_done() if there is a fail in the driver invoke function?
> > >
> > > I ask because I've tried this but am getting a soft lockup when using crypto-tools and testing some fail case scenarios if I set the crp->etype before calling crypto-done.
> >
> > Yeah, I can't explain it either and I made that change. It was part of a
> > SMP/locking problem and it looks like I lost the result code. Here is a
> > quick patch (untested :-) that I think will return the functionality that
> > was lost. Let me know how it goes.
> >
> > Out of interest, what is the condition error that the driver you are using
> > is reporting ? EINVAL perhaps ?
> >
> > Thanks for tracking this down,
> >
> > Cheers,
> > Davidm
> >
> > Index: ocf/crypto.c
> > ===================================================================
> > RCS file: ocf/crypto.c,v
> > retrieving revision 1.42
> > diff -u -r1.42 crypto.c
> > --- ocf/crypto.c 31 Mar 2009 00:41:14 -0000 1.42
> > +++ ocf/crypto.c 29 Apr 2009 23:01:08 -0000
> > @@ -838,13 +838,15 @@
> > */
> > list_add(&crp->crp_next, &crp_q);
> > cryptostats.cs_blocks++;
> > + result = 0;
> > } else if (result == -1) {
> > TAILQ_INSERT_TAIL(&crp_q, crp, crp_next);
> > + result = 0;
> > }
> > if (crp_sleep)
> > wake_up_interruptible(&cryptoproc_wait);
> > CRYPTO_Q_UNLOCK();
> > - return 0;
> > + return result;
> > }
> >
> > /*
> >
> >
> > --
> > David McCullough, dav...@se..., Ph:+61 734352815
> > McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
> > ---------------------------------------------------------------------
> > Intel Shannon Limited
> > Registered in Ireland
> > Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
> > 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.
> >
>
> --
> David McCullough, dav...@se..., Ph:+61 734352815
> McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
> ---------------------------------------------------------------------
> Intel Shannon Limited
> Registered in Ireland
> Registered Office: One Spencer Dock, North Wall Quay, Dublin 1
> 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.
>
--
David McCullough, dav...@se..., Ph:+61 734352815
McAfee - SnapGear http://www.snapgear.com http://www.uCdot.org
|
Thanks for helping keep SourceForge clean.
X