[Openswan dev] Openswan OCF : ----- USE_BATCH mode + ERESTART
David McCullough
david_mccullough at au.securecomputing.com
Thu Dec 7 06:22:03 EST 2006
Jivin Kabir Ahsan-r9aahw lays it down ...
> Hi All
> I am trying to get the USE_BATCH (=1) mode to work for my crypto driver.
> I guess I have found some problem, that is, crypto driver signals
> ERESTART as a function call return (line 1317 below), then the crypto
> thread sets the cc_qblocked to 1 (line 1333 below).
Ok I have fixed this since the last release. A patch is attached.
Here is my description:
Fix a problem where a driver would return ERESTART (full) but
then unblock itself before the upper layer had marked it as blocked. This
caused the code to get stuck in crypto_proc and process no more
requests.
Sorry for the slow response, I'll blame being on vacation :-)
Cheers,
Davidm
> problem where a driver would return ERESTART (full) but then unblock
> itself before the upper layer had marked it as blocked. This caused
> the code to get stuck in crypto_proc and process no more requests.
> 1303 if (submit != NULL) {
> 1304 /* AK added */
> 1305 //if (!( gKeepTrackOfCryptoThread %
> 100)) {
> 1306 // printk("6," );
> 1307 //}
> 1308 list_del(&submit->crp_list);
> 1309 CRYPTO_Q_UNLOCK();
> 1310
> 1311 //trace_set_L1(c,30); /* 30 is the id
> for OCF code */
> 1312 //trace_set_L1(f,7); /* trace point */
> 1313 //trace_log_L1(trace_var(c),
> trace_var(f), 200); /* log */
> 1314
> 1315 result = crypto_invoke(submit, hint);
> 1316 CRYPTO_Q_LOCK();
> 1317 if (result == ERESTART) {
> 1318
> 1319 /* AK added */
> 1320 //if (!(
> gKeepTrackOfCryptoThread % 100)) {
> 1321 // printk("7," );
> 1322 //}
> 1323 /*
> 1324 * The driver ran out of
> resources, mark the
> 1325 * driver ``blocked'' for
> cryptop's and put
> 1326 * the request back in the
> queue. It would
> 1327 * best to put the request back
> where we got
> 1328 * it but that's hard so for now
> we put it
> 1329 * at the front. This should be
> ok; putting
> 1330 * it at the end does not work.
> 1331 */
> 1332 /* XXX validate sid again? */
> 1333
> crypto_drivers[CRYPTO_SESID2HID(submit->crp_sid)].cc_qblocked = 1
> ;
> 1334 list_add(&submit->crp_list,
> &crp_q);
> 1335 cryptostats.cs_blocks++;
> 1336 }
> 1337 }
>
>
> Because the crypto hardware is not able to cope up with the request now
> it makes sense for the crypto thread to go to sleep and later be
> awakened by the crypto driver. But I guess this thread doesn't go to
> sleep because the wait_on_event condition argument is not turning out to
> be false. Here is the code segment. Line 1414 should put the thread to
> sleep but it does not because the submit queue is not empty, when crypto
> driver returned ERESTART.
>
> 1375 if (submit == NULL && krp == NULL) {
> 1376 /* AK added */
> 1377 //if (!( gKeepTrackOfCryptoThread %
> 100)) {
> 1378 // printk("8," );
> 1379 //}
> 1380 /*
> 1381 * Nothing more to be processed. Sleep
> until we're
> 1382 * woken because there are more ops to
> process.
> 1383 * This happens either by submission or
> by a driver
> 1384 * becoming unblocked and notifying us
> through
> 1385 * crypto_unblock. Note that when we
> wakeup we
> 1386 * start processing each queue again
> from the
> 1387 * front. It's not clear that it's
> important to
> 1388 * preserve this ordering since ops may
> finish
> 1389 * out of order if dispatched to
> different devices
> 1390 * and some become blocked while others
> do not.
> 1391 */
> 1392 //printk("1 : sleeping \n");
> 1393 dprintk("%s - sleeping\n",
> __FUNCTION__);
> 1394 CRYPTO_Q_UNLOCK();
> 1395
> 1414
> wait_event_interruptible(cryptoproc_wait,
> 1415 cryptoproc == (pid_t) -1
> ||
> 1416 !list_empty(&crp_q) ||
> 1417 !list_empty(&crp_kq));
> 1418 ...
>
>
>
> >From the crypto driver routine I call crypto_unblock and believe the
> purpose of this is to wake up the crypto thread. In the first place this
> thread didn't go to sleep. Am I missing something? How this scheme is
> supposed to work? Didn't anyone run into ERESTART condition and was able
> to get out this condition gracefully? This ERESTART problem will show up
> more when the injected traffic to the VPN testbed is very high. For
> instance, in the lab I was blasting 100% of line rate (gigabit) with
> packet size of 1024B.
>
> Please shed some light.
>
> Ahsan.
> _______________________________________________
> Dev mailing list
> Dev at openswan.org
> http://lists.openswan.org/mailman/listinfo/dev
--
David McCullough, david_mccullough at securecomputing.com, Ph:+61 734352815
Secure Computing - SnapGear http://www.uCdot.org http://www.cyberguard.com
-------------- next part --------------
Index: modules/ocf/crypto.c
===================================================================
RCS file: /cvs/sw/new-wave/modules/ocf/crypto.c,v
retrieving revision 1.25
retrieving revision 1.26
diff -u -r1.25 -r1.26
--- modules/ocf/crypto.c 10 May 2006 00:09:03 -0000 1.25
+++ modules/ocf/crypto.c 11 May 2006 10:52:14 -0000 1.26
@@ -616,7 +616,7 @@
unsigned long q_flags;
dprintk("%s()\n", __FUNCTION__);
- CRYPTO_Q_LOCK();
+ CRYPTO_Q_LOCK(); //DAVIDM should this be a driver lock
cap = crypto_checkdriver(driverid);
if (cap != NULL) {
needwakeup = 0;
@@ -633,7 +633,7 @@
err = 0;
} else
err = EINVAL;
- CRYPTO_Q_UNLOCK();
+ CRYPTO_Q_UNLOCK(); //DAVIDM should this be a driver lock
return err;
}
@@ -674,6 +674,7 @@
* driver unless the driver is currently blocked.
*/
if (cap && !cap->cc_qblocked) {
+ crypto_drivers[hid].cc_qblocked = 1;
CRYPTO_Q_UNLOCK();
result = crypto_invoke(crp, 0);
CRYPTO_Q_LOCK();
@@ -687,11 +688,11 @@
* order is preserved but this can place them
* behind batch'd ops.
*/
- crypto_drivers[hid].cc_qblocked = 1;
list_add_tail(&crp->crp_list, &crp_q);
cryptostats.cs_blocks++;
result = 0;
- }
+ } else
+ crypto_drivers[hid].cc_qblocked = 0;
} else {
/*
* The driver is blocked, just queue the op until
@@ -738,6 +739,7 @@
CRYPTO_Q_LOCK();
cap = crypto_checkdriver(krp->krp_hid);
if (cap && !cap->cc_kqblocked) {
+ crypto_drivers[krp->krp_hid].cc_kqblocked = 1;
CRYPTO_Q_UNLOCK();
result = crypto_kinvoke(krp, 0);
CRYPTO_Q_LOCK();
@@ -751,10 +753,10 @@
* at the front. This should be ok; putting
* it at the end does not work.
*/
- crypto_drivers[krp->krp_hid].cc_kqblocked = 1;
list_add_tail(&krp->krp_list, &crp_kq);
cryptostats.cs_kblocks++;
- }
+ } else
+ crypto_drivers[krp->krp_hid].cc_kqblocked = 0;
} else {
/*
* The driver is blocked, just queue the op until
@@ -1103,6 +1105,7 @@
}
if (submit != NULL) {
list_del(&submit->crp_list);
+ crypto_drivers[CRYPTO_SESID2HID(submit->crp_sid)].cc_qblocked = 1;
CRYPTO_Q_UNLOCK();
result = crypto_invoke(submit, hint);
CRYPTO_Q_LOCK();
@@ -1117,10 +1120,10 @@
* it at the end does not work.
*/
/* XXX validate sid again? */
- crypto_drivers[CRYPTO_SESID2HID(submit->crp_sid)].cc_qblocked = 1;
list_add(&submit->crp_list, &crp_q);
cryptostats.cs_blocks++;
- }
+ } else
+ crypto_drivers[CRYPTO_SESID2HID(submit->crp_sid)].cc_qblocked=0;
}
/* As above, but for key ops */
@@ -1139,6 +1142,7 @@
}
if (krp != NULL) {
list_del(&krp->krp_list);
+ crypto_drivers[krp->krp_hid].cc_kqblocked = 1;
CRYPTO_Q_UNLOCK();
result = crypto_kinvoke(krp, 0);
CRYPTO_Q_LOCK();
@@ -1153,10 +1157,10 @@
* it at the end does not work.
*/
/* XXX validate sid again? */
- crypto_drivers[krp->krp_hid].cc_kqblocked = 1;
list_add(&krp->krp_list, &crp_kq);
cryptostats.cs_kblocks++;
- }
+ } else
+ crypto_drivers[krp->krp_hid].cc_kqblocked = 0;
}
if (submit == NULL && krp == NULL) {
More information about the Dev
mailing list