[OpenAFS-devel] Discarding tokens -- is this good?

[Derrick J Brashear]

On Sun, 12 Nov 2006, Robert Banz wrote:

>> VICETOKENDEAD does not mean that the client's tokens are expired.
>> It means that the token's associated with the RX connection are expired.

Correct.

> According to the code in the client, all three of these states lead to the 
> same result -- getting UTokensBad set in user->states, with the only 
> difference in handling being that VICETOKENDEAD & RXKADEXPIRED printing the 
> "...expired" error message, and  any other RXK error gets it's error code 
> printed.  Check out afs/afs_analyze.c...

Yup.

> The intended behavior that you describe above makes sense, however, the 
> current way the error handling is coded doesn't follow.  My only "fear" in 
> fixing this a lack of understanding of some of the ramifications.  While it 
> looks relatively simple to ask for a retry of the call, and initiate a new RX 
> connection, there doesn't seem to currently be a way to put a limit on how 
> many times something would be retried.  I think that would require adding a

Well, consider we already return shouldRetry after flushing the tokens, so 
we already retry once. Here, we would reconnect and try again once, and 
then we couldn't possibly get back the same error: either we establish 
again with new tokens which are valid, or we have no (or bad) tokens and 
don't establish an authenticated connection; Either way, we get a 
different error next time (the brand new connection will not immediately 
expire), which is already a known quantity.

Index: afs_analyze.c
===================================================================
RCS file: /cvs/openafs/src/afs/afs_analyze.c,v
retrieving revision 1.22
diff -u -r1.22 afs_analyze.c
--- afs_analyze.c       27 Aug 2003 21:43:16 -0000      1.22
+++ afs_analyze.c       21 Nov 2006 03:18:28 -0000
@@ -636,7 +636,11 @@

         tu = afs_FindUser(areq->uid, tsp->cell->cellNum, READ_LOCK);
         if (tu) {
-           if ((acode == VICETOKENDEAD) || (acode == RXKADEXPIRED))
+           if (acode == VICETOKENDEAD) {
+               aconn->forceConnectFS = 1;      /* don't check until new tokens set */
+               shouldRetry = 1;        /* Try again (as root). */
+           }
+           else if (acode == RXKADEXPIRED)
                 afs_warnuser
                     ("afs: Tokens for user of AFS id %d for cell %s have expired\n",
                      tu->vid, aconn->srvr->server->cell->cellName);
@@ -647,7 +651,11 @@
             afs_PutUser(tu, READ_LOCK);
         } else {
             /* The else case shouldn't be possible and should probably be replaced by a panic? */
-           if ((acode == VICETOKENDEAD) || (acode == RXKADEXPIRED))
+           if (acode == VICETOKENDEAD) {
+               aconn->forceConnectFS = 1;      /* don't check until new tokens set */
+               shouldRetry = 1;        /* Try again (as root). */
+           }
+           else if (acode == RXKADEXPIRED)
                 afs_warnuser
                     ("afs: Tokens for user %d for cell %s have expired\n",
                      areq->uid, aconn->srvr->server->cell->cellName);